Micrometer: [Prometheus] Notify the user when trying to report duplicate metrics

[ofirnk]

Some libraries happen to report the same metric twice.
Although this use case is not clear (to me, see https://github.com/spring-projects/spring-boot/issues/6404) - it happens.

As for spring-boot POV this is OK , it's clearly not a valid report from Prometheus perspective.

What I suggest is to prevent it while gathering the metrics before the report, notify the user of the error, and prevent the continuation of the report (probably by an exception)

Code reference:
Verify uniqueness of MetricFamilySamples.Sample here:
https://github.com/micrometer-metrics/micrometer/blob/5fbaa762afd71b5091e16e165f008ba1e1fe0c18/implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheus/MicrometerCollector.java#L88
and throw an exception if this isn't a valid report (i.e with duplicate metrics)

[jkschneider]

I'm not clear how this is possible in Micrometer. You can have two different pieces of code that separate attempt to register the same Counter for example, and Micrometer ensures there is only one. Effectively these two pieces of code would be separately incrementing on the same counter and the counter would report the sum of both of their increments.

So the Prometheus exposition is guaranteed to not be broken.

It may be semantically incorrect for unrelated pieces of code to be accumulating to the same metric name, but I don't see how we can do anything about that.

Any ideas?

[ofirnk]

I'm totally for doing something about it, and hope there will be a better idea that the naive one:
recognize when such an event happens and throw an illegalstate exception (it can be a configuration from my POV).

So suggested change is to modify the stream I've mentioned before to have groupBy() and map values back to either their single value or to throw an exception before collect(toList())

(sorry for not writing clearer impl, commented from my phone)

[jkschneider]

I guess what I was trying to say was that there is no possibility for duplication at this point in the code. Micrometer has already guaranteed name+tag uniqueness at the registry level.

Accidental usage of the same set of name and tags for completely separate purposes in your codebase is not a condition we can detect programmatically. It's a semantic concern.

[mweirauch]

I can only second Jons statements. How shall we hinder two independant pieces of code from registering the same metric? If these pieces of code do by chance register the same metric, there will never be two metrics registered within the MeterRegistries. (If my naive memory serves me right, that possibility was actually the case in the old Actuator framework, but this isn't transferable to Micrometer.)

So the proposed sanitization/grouping in the MicrometerCollector has no effect.

The only thing I could think of is hindering registration of an already existing metric, but this will for sure break everything with its current get-or-create-style. (Correct me if I am wrong Jon.)

[jkschneider]

I'm going to close this for now. Feel free to continue the discussion @ofirnk if you have any further questions.