PrometheusHttpServer drops metrics with same name and different type #5078
Conversation
| @@ -47,7 +47,7 @@ class SerializerTest { | |||
| .setVersion("version") | |||
| .setAttributes(Attributes.of(stringKey("ks"), "vs")) | |||
| .build(), | |||
| "instrument.name", | |||
| "monotonic.cumulative.double.sum", | |||
There was a problem hiding this comment.
All the metrics in this test used the same name, which produce name collisions with the new rules. I've changed the metric names to be a dot.delimited.version of the names of the fields they're assigned to.
| Map<String, List<MetricData>> metricsByName = new LinkedHashMap<>(); | ||
| Set<InstrumentationScopeInfo> scopes = new LinkedHashSet<>(); | ||
| // Iterate through metrics, filtering and grouping by headerName | ||
| for (MetricData metric : metrics) { |
There was a problem hiding this comment.
A for loop is more appropriate because we now need to track the unique scopes we've seen and metric name conflicts, which we return to the caller for logging purposes.
Codecov ReportBase: 91.06% // Head: 91.06% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #5078 +/- ##
=========================================
Coverage 91.06% 91.06%
+ Complexity 4888 4887 -1
=========================================
Files 553 553
Lines 14453 14465 +12
Branches 1381 1387 +6
=========================================
+ Hits 13161 13173 +12
Misses 895 895
Partials 397 397
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| LOGGER.log( | ||
| Level.WARNING, | ||
| "Metric conflict(s) detected. Multiple metrics with same name but different type: " | ||
| + conflictHeaderNames.stream().collect(joining(",", "[", "]"))); |
There was a problem hiding this comment.
Goal is to log each name conflict that is detected once to avoid spammy logs.
...ters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServer.java
Outdated
Show resolved
Hide resolved
...ters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServer.java
Outdated
Show resolved
Hide resolved
...ters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServer.java
Show resolved
Hide resolved
exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Serializer.java
Outdated
Show resolved
Hide resolved
exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Serializer.java
Outdated
Show resolved
Hide resolved
Resolves #5014.