Validation of [null] slice for []int should not pass

[lahsivjar]

With empty slices the validation will fail. Example:

{"metadata": {"user": {"username": "logged-in-user", "id": "axb123hg", "email": "[email protected]"}, "labels": {"tag0": null, "tag1": "one", "tag2": 2}, "process": {"ppid": null, "pid": 1234, "argv": null, "title": null}, "system": null, "service": {"name": "testme-12a3", "node": {"configured_name": "node-1"},"language": {"version": null, "name":"ecmascript"}, "agent": {"version": "3.14.0", "name": "elastic-node", "activation_method": "some_activation_method"}, "environment": null, "framework": null,"version": null, "runtime": null}}}
{"metricset": { "samples": { "latency_distribution": { "type": "histogram", "unit": "s", "counts": [], "values": [] } }, "timestamp": 1496170421368000 }}

But with null values in the slices validation will not fail. Example:

{"metadata": {"user": {"username": "logged-in-user", "id": "axb123hg", "email": "[email protected]"}, "labels": {"tag0": null, "tag1": "one", "tag2": 2}, "process": {"ppid": null, "pid": 1234, "argv": null, "title": null}, "system": null, "service": {"name": "testme-12a3", "node": {"configured_name": "node-1"},"language": {"version": null, "name":"ecmascript"}, "agent": {"version": "3.14.0", "name": "elastic-node", "activation_method": "some_activation_method"}, "environment": null, "framework": null,"version": null, "runtime": null}}}
{"metricset": { "samples": { "latency_distribution": { "type": "histogram", "unit": "s", "counts": [null], "values": [null] } }, "timestamp": 1496170421368000 }}

I think both should have the same behavior.

[raultorrecilla]

Hi @lahsivjar , @simitt explained to me a little bit the context of this.
As I understood this is part of the shared library and it will be a breaking change for agents which might be fine for apm server as we can change it as well, but it will cause issues on MIS.

Do you think this is still worth it? Shall we close it? In any case, this is too late for the 9.0 as it was initially planned.

[lahsivjar]

@raultorrecilla I have completely paged this out of memory and I don't recall what will be the impact of having essentially empty histograms indexed as metrics. If the impact is not noteworthy then I am okay to close it.

[kruskall]

I wonder if we should revisit nil handling and be more forgiving (ignore the value instead of failing)

[simitt]

I wonder if we should revisit nil handling and be more forgiving (ignore the value instead of failing)

+1 on implementing this in a non-breaking way (relaxing the check), which also takes pressure out of the timelines. Allowing nil values and filtering them out sounds like a good solution with low risk.

[raultorrecilla]

removing this from 9.0 candidate