Prometheus server latency metrics no longer provide labels per service and method

[tyler-clark]

We recently upgraded a service from v0.14.1 to v0.18.4. In the old version there was a way to opt in to latency metrics with labels per service and method. I may be missing something, but I'm not seeing a mechanism provided that would allow for the same sort of per-endpoint latency metrics. If that is the case, is there maybe a reason those were removed that I might be missing? And if there is no reason, would anyone be opposed to adding something back in? I would be happy to submit a PR if there are no objections to the idea.

[fedefernandez]

Hi @tyler-clark, you mean adding the serviceName to this?
https://github.com/higherkindness/mu/blob/master/modules/metrics/prometheus/src/main/scala/higherkindness/mu/rpc/prometheus/PrometheusMetrics.scala#L36

Probably something we missed in the migration to the new approach. I'm ok with adding back.

In the meantime, you could create your own implementation of the PrometheusMetrics and add methodInfo.serviceName here

[tyler-clark]

Thanks @fedefernandez ! I'll probably take your suggestion to create my own implementation for the short term, and then work on getting the metrics added back to mu for a more long term solution.

Would the preference be for those latency metrics specifically to be another opt-in like they used to be?

[fedefernandez]

Great! I think including histograms as an opt-in would be the best option, but we could add a simple param to the PrometheusMetrics builder, WDYT?

[tyler-clark]

@fedefernandez That seems reasonable enough to me, but that would break binary compatibility, wouldn't it? Maybe not a huge deal as it would just mean a new minor version at this point. Thoughts?

[fedefernandez]

@tyler-clark maybe we can keep the default builder and add a new one for specifying that param. I'm ok with breaking binary compatibility and release a new minor version though but I think a different builder could be enough.