Avoid conflicts in Micrometer description mapping #5452
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background: How PrometheusMeterRegistry Maps Descriptions
The following shows an example of metrics in Prometheus format as produced by Micrometer's
PrometheusMeterRegistry:If you look closely at the
HELPtext, you will see a slight mismatch: The description is specific tolevel="error"and not to other attribute values.The reason is that in Micrometer you can define descriptions per set of attributes. It is not very common to have different descriptions per set of attributes, but there are a few real-world examples like Micrometer's built-in
LogbackMetricshttps://github.com/micrometer-metrics/micrometer/blob/fc9c4eed843f02f00cc6f3c9621aed5d4797e29d/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/logging/LogbackMetrics.java#L139-L169
As Prometheus supports only one description per metric, Micrometer's
PrometheusMeterRegistrymaintains acollectorMapthat caches the description per metric name, so metrics with the same name will get the same description. As a result, all metrics namedlogback_events_totalwill get the first description that was registered, which happens to be the description forlevel="error"in the example above.https://github.com/fstab/micrometer/blob/31488cc4ca1232f6599f9479ddb6b8445ece3d06/implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheus/PrometheusMeterRegistry.java#L482-L502
This PR: Make OpenTelemetryMeterRegistry use The Same Strategy as PrometheusMeterRegistry
The
OpenTelemetryMeterRegistryhas the same mismatch with Micrometer, as OTel metrics also don't support different descriptions for different attribute values. However, the current implementation does not handle this, and eventually the SDK throws aDuplicateMetricStorageExceptionwhen Micrometer'sLogbackMetricsregister metrics with the same name but different descriptions:https://github.com/open-telemetry/opentelemetry-java/blob/b19157ed98ffc81d5083752631b04e4074ea71f6/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/MetricStorageRegistry.java#L61-L67
This will happen whenever you use the
OpenTelemetryMeterRegistryin a Spring application and Spring's Micrometer auto-detectsLogback.This PR introduces a
descriptionsCacheto mimic the behavior of the Micrometer -> Prometheus mapping. We cache the first description seen for each metric name and re-use it for all attributes.This will result in descriptions that don't fit to the attributes (as in the example above). However, in practice this slight mismatch in the description has never been reported as an issue by Prometheus users (to my knowledge), and I think it is better than throwing a
DuplicateMetricStorageExceptionwhen Spring detectsLogback.