Revert "Make inclusion of metrics in SeldonMessage configurable in 1.…

…1" (#1624)

* Revert "make testing more robust: wait_for_rollout will repeat get_deployment_names if that is empty"

This reverts commit fc10d48.

* Revert "partially revert 8afb6f3 to read env var only once"

This reverts commit 5f5f520.

* Revert "add note in docs"

This reverts commit aa72cbd.

* Revert "improve hadling and testing of INCLUDE_METRICS_IN_CLIENT_RESPONSE env variable"

This reverts commit c330b23.

* Revert "fixes from code review"

This reverts commit 03ba1b3.

* Revert "add tests for custom metrics returned to client"

This reverts commit 258dd55.

* Revert "simplify metrics test code"

This reverts commit 6987fe2.

* Revert "skip metrics also in raw methods"

This reverts commit be6969c.

* Revert "WIP: skip metrics in meta returned to client"

This reverts commit 9d32de7.

doc/source/python/python_component.md

@@ -119,14 +119,12 @@ class ModelWithMetrics(object):
 
     def metrics(self):
         return [
-            {"type": "COUNTER", "key": "mycounter", "value": 1}, # a counter which will increase by the given value
-            {"type": "GAUGE", "key": "mygauge", "value": 100},   # a gauge which will be set to given value
-            {"type": "TIMER", "key": "mytimer", "value": 20.2},  # a timer which will add sum and count metrics - assumed millisecs
-        ]
-```
-
-Note: prior to Seldon Core 1.1 custom metrics have always been returned to client. From SC 1.1 you can control this behaviour setting `INCLUDE_METRICS_IN_CLIENT_RESPONSE` environmental variable to either `true` or `false`. Despite value of this environmental variable custom metrics will always be exposed to Prometheus.
+            {"type":"COUNTER","key":"mycounter","value":1}, # a counter which will increase by the given value
+            {"type":"GAUGE","key":"mygauge","value":100}, # a gauge which will be set to given value
+            {"type":"TIMER","key":"mytimer","value":20.2}, # a timer which will add sum and count metrics - assumed millisecs
+            ]
 
+```
 
 ## Returning Tags
 

python/seldon_core/seldon_methods.py

@@ -9,7 +9,6 @@
     extract_feedback_request_parts,
 )
 from seldon_core.user_model import (
-    INCLUDE_METRICS_IN_CLIENT_RESPONSE,
     client_predict,
     client_aggregate,
     client_route,
@@ -30,27 +29,6 @@
 logger = logging.getLogger(__name__)
 
 
-def handle_raw_custom_metrics(
-    msg: Union[prediction_pb2.SeldonMessage, Dict],
-    seldon_metrics: SeldonMetrics,
-    is_proto: bool,
-):
-    """
-    Update SeldonMetrics object with custom metrics from raw methods.
-    If INCLUDE_METRICS_IN_CLIENT_RESPONSE environmental variable is set to "true"
-    metrics will be dropped from msg.
-    """
-    if is_proto:
-        metrics = seldon_message_to_json(msg.meta).get("metrics", [])
-        if metrics and not INCLUDE_METRICS_IN_CLIENT_RESPONSE:
-            del msg.meta.metrics[:]
-    else:
-        metrics = msg.get("meta", {}).get("metrics", [])
-        if metrics and not INCLUDE_METRICS_IN_CLIENT_RESPONSE:
-            del msg["meta"]["metrics"]
-    seldon_metrics.update(metrics)
-
-
 def predict(
     user_model: Any,
     request: Union[prediction_pb2.SeldonMessage, List, Dict],
@@ -81,7 +59,11 @@ def predict(
         if hasattr(user_model, "predict_raw"):
             try:
                 response = user_model.predict_raw(request)
-                handle_raw_custom_metrics(response, seldon_metrics, is_proto)
+                if is_proto:
+                    metrics = seldon_message_to_json(response.meta).get("metrics", [])
+                else:
+                    metrics = response.get("meta", {}).get("metrics", [])
+                seldon_metrics.update(metrics)
                 return response
             except SeldonNotImplementedError:
                 pass
@@ -92,7 +74,9 @@ def predict(
                 user_model, features, datadef.names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model, seldon_metrics)
+            metrics = client_custom_metrics(user_model)
+            if seldon_metrics is not None:
+                seldon_metrics.update(metrics)
 
             return construct_response(
                 user_model, False, request, client_response, meta, metrics
@@ -104,7 +88,9 @@ def predict(
                 user_model, features, class_names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model, seldon_metrics)
+            metrics = client_custom_metrics(user_model)
+            if seldon_metrics is not None:
+                seldon_metrics.update(metrics)
 
             return construct_response_json(
                 user_model, False, request, client_response, meta, metrics
@@ -195,7 +181,11 @@ def transform_input(
         if hasattr(user_model, "transform_input_raw"):
             try:
                 response = user_model.transform_input_raw(request)
-                handle_raw_custom_metrics(response, seldon_metrics, is_proto)
+                if is_proto:
+                    metrics = seldon_message_to_json(response.meta).get("metrics", [])
+                else:
+                    metrics = response.get("meta", {}).get("metrics", [])
+                seldon_metrics.update(metrics)
                 return response
             except SeldonNotImplementedError:
                 pass
@@ -206,7 +196,9 @@ def transform_input(
                 user_model, features, datadef.names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model, seldon_metrics)
+            metrics = client_custom_metrics(user_model)
+            if seldon_metrics is not None:
+                seldon_metrics.update(metrics)
 
             return construct_response(
                 user_model, False, request, client_response, meta, metrics
@@ -218,7 +210,9 @@ def transform_input(
                 user_model, features, class_names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model, seldon_metrics)
+            metrics = client_custom_metrics(user_model)
+            if seldon_metrics is not None:
+                seldon_metrics.update(metrics)
 
             return construct_response_json(
                 user_model, False, request, client_response, meta, metrics
@@ -260,7 +254,11 @@ def transform_output(
         if hasattr(user_model, "transform_output_raw"):
             try:
                 response = user_model.transform_output_raw(request)
-                handle_raw_custom_metrics(response, seldon_metrics, is_proto)
+                if is_proto:
+                    metrics = seldon_message_to_json(response.meta).get("metrics", [])
+                else:
+                    metrics = response.get("meta", {}).get("metrics", [])
+                seldon_metrics.update(metrics)
                 return response
             except SeldonNotImplementedError:
                 pass
@@ -271,7 +269,9 @@ def transform_output(
                 user_model, features, datadef.names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model, seldon_metrics)
+            metrics = client_custom_metrics(user_model)
+            if seldon_metrics is not None:
+                seldon_metrics.update(metrics)
 
             return construct_response(
                 user_model, False, request, client_response, meta, metrics
@@ -283,7 +283,9 @@ def transform_output(
                 user_model, features, class_names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model, seldon_metrics)
+            metrics = client_custom_metrics(user_model)
+            if seldon_metrics is not None:
+                seldon_metrics.update(metrics)
 
             return construct_response_json(
                 user_model, False, request, client_response, meta, metrics
@@ -303,8 +305,6 @@ def route(
        A Seldon user model
     request
        A SelodonMessage proto
-    seldon_metrics
-        A SeldonMetrics instance
     Returns
     -------
 
@@ -321,7 +321,11 @@ def route(
         if hasattr(user_model, "route_raw"):
             try:
                 response = user_model.route_raw(request)
-                handle_raw_custom_metrics(response, seldon_metrics, is_proto)
+                if is_proto:
+                    metrics = seldon_message_to_json(response.meta).get("metrics", [])
+                else:
+                    metrics = response.get("meta", {}).get("metrics", [])
+                seldon_metrics.update(metrics)
                 return response
             except SeldonNotImplementedError:
                 pass
@@ -337,7 +341,9 @@ def route(
                 )
             client_response_arr = np.array([[client_response]])
 
-            metrics = client_custom_metrics(user_model, seldon_metrics)
+            metrics = client_custom_metrics(user_model)
+            if seldon_metrics is not None:
+                seldon_metrics.update(metrics)
 
             return construct_response(
                 user_model, False, request, client_response_arr, None, metrics
@@ -352,7 +358,9 @@ def route(
                 )
             client_response_arr = np.array([[client_response]])
 
-            metrics = client_custom_metrics(user_model, seldon_metrics)
+            metrics = client_custom_metrics(user_model)
+            if seldon_metrics is not None:
+                seldon_metrics.update(metrics)
 
             return construct_response_json(
                 user_model, False, request, client_response_arr, None, metrics
@@ -373,8 +381,6 @@ def aggregate(
        A Seldon user model
     request
        SeldonMessage proto
-    seldon_metrics
-        A SeldonMetrics instance
 
     Returns
     -------
@@ -409,7 +415,11 @@ def merge_metrics(meta_list, custom_metrics):
         if hasattr(user_model, "aggregate_raw"):
             try:
                 response = user_model.aggregate_raw(request)
-                handle_raw_custom_metrics(response, seldon_metrics, is_proto)
+                if is_proto:
+                    metrics = seldon_message_to_json(response.meta).get("metrics", [])
+                else:
+                    metrics = response.get("meta", {}).get("metrics", [])
+                seldon_metrics.update(metrics)
                 return response
             except SeldonNotImplementedError:
                 pass
@@ -427,7 +437,9 @@ def merge_metrics(meta_list, custom_metrics):
 
             client_response = client_aggregate(user_model, features_list, names_list)
 
-            metrics = client_custom_metrics(user_model, seldon_metrics)
+            metrics = client_custom_metrics(user_model)
+            if seldon_metrics is not None:
+                seldon_metrics.update(metrics)
 
             return construct_response(
                 user_model,
@@ -462,7 +474,9 @@ def merge_metrics(meta_list, custom_metrics):
 
             client_response = client_aggregate(user_model, features_list, names_list)
 
-            metrics = client_custom_metrics(user_model, seldon_metrics)
+            metrics = client_custom_metrics(user_model)
+            if seldon_metrics is not None:
+                seldon_metrics.update(metrics)
 
             return construct_response_json(
                 user_model,
@@ -484,9 +498,6 @@ def health_status(
     ----------
     user_model
        User defined class instance
-    seldon_metrics
-        A SeldonMetrics instance
-
     Returns
     -------
       Health check output
@@ -499,7 +510,9 @@ def health_status(
             pass
 
     client_response = client_health_status(user_model)
-    metrics = client_custom_metrics(user_model, seldon_metrics)
+    metrics = client_custom_metrics(user_model)
+    if seldon_metrics is not None:
+        seldon_metrics.update(metrics)
 
     return construct_response_json(
         user_model, False, {}, client_response, None, metrics

python/seldon_core/user_model.py

@@ -1,22 +1,15 @@
 from seldon_core.metrics import validate_metrics
 from seldon_core.flask_utils import SeldonMicroserviceException
 import json
-from typing import Dict, List, Union, Iterable
+from typing import Dict, List, Union, Iterable, Callable, Optional
 import numpy as np
 from seldon_core.proto import prediction_pb2
-from seldon_core.metrics import SeldonMetrics
 import inspect
 import logging
-import os
 
 logger = logging.getLogger(__name__)
 
 
-INCLUDE_METRICS_IN_CLIENT_RESPONSE = (
-    os.environ.get("INCLUDE_METRICS_IN_CLIENT_RESPONSE", "true").lower() == "true"
-)
-
-
 class SeldonNotImplementedError(SeldonMicroserviceException):
     status_code = 400
 
@@ -285,21 +278,14 @@ def client_transform_output(
     return features
 
 
-def client_custom_metrics(
-    user_model: SeldonComponent, seldon_metrics: SeldonMetrics
-) -> List[Dict]:
+def client_custom_metrics(user_model: SeldonComponent) -> List[Dict]:
     """
-    Get custom metrics for client and update SeldonMetrics.
-
-    This function will return empty list if INCLUDE_METRICS_IN_CLIENT_RESPONSE environmental
-    variable is NOT set to "true" or "True".
+    Get custom metrics
 
     Parameters
     ----------
     user_model
        A Seldon user model
-    seldon_metrics
-        A SeldonMetrics instance
 
     Returns
     -------
@@ -316,11 +302,7 @@ def client_custom_metrics(
                     reason="MICROSERVICE_BAD_METRIC",
                 )
 
-            seldon_metrics.update(metrics)
-            if INCLUDE_METRICS_IN_CLIENT_RESPONSE:
-                return metrics
-            else:
-                return []
+            return metrics
         except SeldonNotImplementedError:
             pass
     logger.debug("custom_metrics is not implemented")

python/tests/conftest.py

@@ -1,19 +1,3 @@
-import pytest
 import logging
 
-import seldon_core
-
-
 logging.basicConfig(level=logging.DEBUG)
-
-
-@pytest.fixture(params=[True, False])
-def client_gets_metrics(monkeypatch, request):
-    value = request.param
-    monkeypatch.setattr(
-        seldon_core.user_model, "INCLUDE_METRICS_IN_CLIENT_RESPONSE", value
-    )
-    monkeypatch.setattr(
-        seldon_core.seldon_methods, "INCLUDE_METRICS_IN_CLIENT_RESPONSE", value
-    )
-    return value