Zero-valued metrics are lost during json serialization/de-serialization

[zyxue]

Describe the bug

To reproduce

>>> import json
>>> import seldon_core.proto.prediction_pb2
>>> from google.protobuf.json_format import MessageToJson
>>> msg = seldon_core.proto.prediction_pb2.Metric(
...     key="my_metric",
...     type="GAUGE",
...     value=0,
... )
>>> json.loads(MessageToJson(msg))
{'key': 'my_metric', 'type': 'GAUGE'}

As seen, value 0 is lost.

I feel this is essentially what's done at https://github.com/SeldonIO/seldon-core/blob/e3aea47667af500ddbbcf9b4297dacbaf36d0a20/python/seldon_core/utils.py#L137-L138.

It leads to error like

  File "/usr/local/lib/python3.8/site-packages/seldon_core/metrics.py", line 146, in update
    worker_data[key] = {"value": metrics["value"], "tags": tags}
KeyError: 'value'

for code at https://github.com/SeldonIO/seldon-core/blob/master/python/seldon_core/metrics.py#L146-L147

Expected behavior

Zero should be preserved by duration serialization/de-serialization.

I'm not sure why serialization/deserialization is needed here. If it's really needed, why not use protobuf serialization/deserialization directly instead of json?

>>> msg2 = seldon_core.proto.prediction_pb2.Metric.FromString(msg.SerializeToString())
>>> msg2
key: "my_metric"
type: GAUGE
>>> msg2.value
0.0

[ukclivecox]

It true that Proto's don't preserve zero valued fields. @RafalSkolasinski don't we have separate paths for Proto requests and REST requests now?

[RafalSkolasinski]

@zyxue Ander what circumstances the problem occurs? As in: is your model build for REST / gRPC and which method are you calling over API that registers the wrong metrics?

[zyxue]

My model is for gRPC, and I'm using the prediction_raw method following advice on https://github.com/SeldonIO/seldon-core/issues/2240#issuecomment-668462396

Our seldon version is 1.0.2

[RafalSkolasinski]

Could you share version of wrapper you used (or is it master?) and example output of your predict_raw?

[zyxue]

if you implement a predict_raw that outputs a zero-value metric, it would cause the except of

  File "/usr/local/lib/python3.8/site-packages/seldon_core/metrics.py", line 146, in update
    worker_data[key] = {"value": metrics["value"], "tags": tags}
KeyError: 'value'

Something like

Class MyPredictor:
    def predict_raw(
        self, seldon_request: seldon_core.proto.prediction_pb2.SeldonMessage
    ):

        request = MyRequest.FromString(seldon_request.binData)

        response = predict(request)

        seldon_response = seldon_core.proto.prediction_pb2.SeldonMessage(
            binData=response.SerializeToString(),
            meta=seldon_core.proto.prediction_pb2.Meta(
                metrics=[
                    seldon_core.proto.prediction_pb2.Metric(
                        type="GAUGE",
                        key="my_metric",
                        value=0,
                    )
                ]
            ),
        )

        return seldon_response

[duongnt]

@cliveseldon @axsaucedo wondering what should we do about this issue?

[axsaucedo]

Would you be able to try again and share if issue still persists with latest version? Also you can try using MLServer instead as we'll be moving to use that as our v2 python server