Revert "Revert "Make inclusion of metrics in SeldonMessage configurab…

…le in 1.1" (#1624)"

This reverts commit 1c2b511 and effectively restores original PR.

doc/source/python/python_component.md

@@ -119,13 +119,15 @@ 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
-            ]
-
+            {"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.
+
+
 ## Returning Tags
 
 If we wish to add arbitrary tags to the returned metadata you can provide a `tags` method with signature as shown below:

python/seldon_core/seldon_methods.py

@@ -9,6 +9,7 @@
     extract_feedback_request_parts,
 )
 from seldon_core.user_model import (
+    INCLUDE_METRICS_IN_CLIENT_RESPONSE,
     client_predict,
     client_aggregate,
     client_route,
@@ -29,6 +30,27 @@
 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],
@@ -59,11 +81,7 @@ def predict(
         if hasattr(user_model, "predict_raw"):
             try:
                 response = user_model.predict_raw(request)
-                if is_proto:
-                    metrics = seldon_message_to_json(response.meta).get("metrics", [])
-                else:
-                    metrics = response.get("meta", {}).get("metrics", [])
-                seldon_metrics.update(metrics)
+                handle_raw_custom_metrics(response, seldon_metrics, is_proto)
                 return response
             except SeldonNotImplementedError:
                 pass
@@ -74,9 +92,7 @@ def predict(
                 user_model, features, datadef.names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model)
-            if seldon_metrics is not None:
-                seldon_metrics.update(metrics)
+            metrics = client_custom_metrics(user_model, seldon_metrics)
 
             return construct_response(
                 user_model, False, request, client_response, meta, metrics
@@ -88,9 +104,7 @@ def predict(
                 user_model, features, class_names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model)
-            if seldon_metrics is not None:
-                seldon_metrics.update(metrics)
+            metrics = client_custom_metrics(user_model, seldon_metrics)
 
             return construct_response_json(
                 user_model, False, request, client_response, meta, metrics
@@ -181,11 +195,7 @@ def transform_input(
         if hasattr(user_model, "transform_input_raw"):
             try:
                 response = user_model.transform_input_raw(request)
-                if is_proto:
-                    metrics = seldon_message_to_json(response.meta).get("metrics", [])
-                else:
-                    metrics = response.get("meta", {}).get("metrics", [])
-                seldon_metrics.update(metrics)
+                handle_raw_custom_metrics(response, seldon_metrics, is_proto)
                 return response
             except SeldonNotImplementedError:
                 pass
@@ -196,9 +206,7 @@ def transform_input(
                 user_model, features, datadef.names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model)
-            if seldon_metrics is not None:
-                seldon_metrics.update(metrics)
+            metrics = client_custom_metrics(user_model, seldon_metrics)
 
             return construct_response(
                 user_model, False, request, client_response, meta, metrics
@@ -210,9 +218,7 @@ def transform_input(
                 user_model, features, class_names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model)
-            if seldon_metrics is not None:
-                seldon_metrics.update(metrics)
+            metrics = client_custom_metrics(user_model, seldon_metrics)
 
             return construct_response_json(
                 user_model, False, request, client_response, meta, metrics
@@ -254,11 +260,7 @@ def transform_output(
         if hasattr(user_model, "transform_output_raw"):
             try:
                 response = user_model.transform_output_raw(request)
-                if is_proto:
-                    metrics = seldon_message_to_json(response.meta).get("metrics", [])
-                else:
-                    metrics = response.get("meta", {}).get("metrics", [])
-                seldon_metrics.update(metrics)
+                handle_raw_custom_metrics(response, seldon_metrics, is_proto)
                 return response
             except SeldonNotImplementedError:
                 pass
@@ -269,9 +271,7 @@ def transform_output(
                 user_model, features, datadef.names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model)
-            if seldon_metrics is not None:
-                seldon_metrics.update(metrics)
+            metrics = client_custom_metrics(user_model, seldon_metrics)
 
             return construct_response(
                 user_model, False, request, client_response, meta, metrics
@@ -283,9 +283,7 @@ def transform_output(
                 user_model, features, class_names, meta=meta
             )
 
-            metrics = client_custom_metrics(user_model)
-            if seldon_metrics is not None:
-                seldon_metrics.update(metrics)
+            metrics = client_custom_metrics(user_model, seldon_metrics)
 
             return construct_response_json(
                 user_model, False, request, client_response, meta, metrics
@@ -305,6 +303,8 @@ def route(
        A Seldon user model
     request
        A SelodonMessage proto
+    seldon_metrics
+        A SeldonMetrics instance
     Returns
     -------
 
@@ -321,11 +321,7 @@ def route(
         if hasattr(user_model, "route_raw"):
             try:
                 response = user_model.route_raw(request)
-                if is_proto:
-                    metrics = seldon_message_to_json(response.meta).get("metrics", [])
-                else:
-                    metrics = response.get("meta", {}).get("metrics", [])
-                seldon_metrics.update(metrics)
+                handle_raw_custom_metrics(response, seldon_metrics, is_proto)
                 return response
             except SeldonNotImplementedError:
                 pass
@@ -341,9 +337,7 @@ def route(
                 )
             client_response_arr = np.array([[client_response]])
 
-            metrics = client_custom_metrics(user_model)
-            if seldon_metrics is not None:
-                seldon_metrics.update(metrics)
+            metrics = client_custom_metrics(user_model, seldon_metrics)
 
             return construct_response(
                 user_model, False, request, client_response_arr, None, metrics
@@ -358,9 +352,7 @@ def route(
                 )
             client_response_arr = np.array([[client_response]])
 
-            metrics = client_custom_metrics(user_model)
-            if seldon_metrics is not None:
-                seldon_metrics.update(metrics)
+            metrics = client_custom_metrics(user_model, seldon_metrics)
 
             return construct_response_json(
                 user_model, False, request, client_response_arr, None, metrics
@@ -381,6 +373,8 @@ def aggregate(
        A Seldon user model
     request
        SeldonMessage proto
+    seldon_metrics
+        A SeldonMetrics instance
 
     Returns
     -------
@@ -415,11 +409,7 @@ def merge_metrics(meta_list, custom_metrics):
         if hasattr(user_model, "aggregate_raw"):
             try:
                 response = user_model.aggregate_raw(request)
-                if is_proto:
-                    metrics = seldon_message_to_json(response.meta).get("metrics", [])
-                else:
-                    metrics = response.get("meta", {}).get("metrics", [])
-                seldon_metrics.update(metrics)
+                handle_raw_custom_metrics(response, seldon_metrics, is_proto)
                 return response
             except SeldonNotImplementedError:
                 pass
@@ -437,9 +427,7 @@ def merge_metrics(meta_list, custom_metrics):
 
             client_response = client_aggregate(user_model, features_list, names_list)
 
-            metrics = client_custom_metrics(user_model)
-            if seldon_metrics is not None:
-                seldon_metrics.update(metrics)
+            metrics = client_custom_metrics(user_model, seldon_metrics)
 
             return construct_response(
                 user_model,
@@ -474,9 +462,7 @@ def merge_metrics(meta_list, custom_metrics):
 
             client_response = client_aggregate(user_model, features_list, names_list)
 
-            metrics = client_custom_metrics(user_model)
-            if seldon_metrics is not None:
-                seldon_metrics.update(metrics)
+            metrics = client_custom_metrics(user_model, seldon_metrics)
 
             return construct_response_json(
                 user_model,
@@ -498,6 +484,9 @@ def health_status(
     ----------
     user_model
        User defined class instance
+    seldon_metrics
+        A SeldonMetrics instance
+
     Returns
     -------
       Health check output
@@ -510,9 +499,7 @@ def health_status(
             pass
 
     client_response = client_health_status(user_model)
-    metrics = client_custom_metrics(user_model)
-    if seldon_metrics is not None:
-        seldon_metrics.update(metrics)
+    metrics = client_custom_metrics(user_model, seldon_metrics)
 
     return construct_response_json(
         user_model, False, {}, client_response, None, metrics

python/seldon_core/user_model.py

@@ -1,15 +1,22 @@
 from seldon_core.metrics import validate_metrics
 from seldon_core.flask_utils import SeldonMicroserviceException
 import json
-from typing import Dict, List, Union, Iterable, Callable, Optional
+from typing import Dict, List, Union, Iterable
 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
 
@@ -278,14 +285,21 @@ def client_transform_output(
     return features
 
 
-def client_custom_metrics(user_model: SeldonComponent) -> List[Dict]:
+def client_custom_metrics(
+    user_model: SeldonComponent, seldon_metrics: SeldonMetrics
+) -> List[Dict]:
     """
-    Get custom metrics
+    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".
 
     Parameters
     ----------
     user_model
        A Seldon user model
+    seldon_metrics
+        A SeldonMetrics instance
 
     Returns
     -------
@@ -302,7 +316,11 @@ def client_custom_metrics(user_model: SeldonComponent) -> List[Dict]:
                     reason="MICROSERVICE_BAD_METRIC",
                 )
 
-            return metrics
+            seldon_metrics.update(metrics)
+            if INCLUDE_METRICS_IN_CLIENT_RESPONSE:
+                return metrics
+            else:
+                return []
         except SeldonNotImplementedError:
             pass
     logger.debug("custom_metrics is not implemented")

python/tests/conftest.py

@@ -1,3 +1,19 @@
+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