mlbiscoc commented on code in PR #4907:
URL: https://github.com/apache/solr/pull/4907#discussion_r4063699385
##########
solr/core/src/test/org/apache/solr/metrics/JvmMetricsTest.java:
##########
@@ -138,6 +142,32 @@ public void testSystemMemoryMetrics() {
metricNames.contains("jvm_system_memory_bytes"));
}
+ @Test
+ public void testNoDuplicateJvmMemoryMetrics() {
+ // Guards against the java8/java17 RuntimeMetrics split emitting the same
series twice (each
+ // scope reporting e.g. jvm.memory.committed with identical labels), which
is what motivated
+ // migrating to the unified opentelemetry-runtime-telemetry module.
+ FilterablePrometheusMetricReader reader =
+ solrTestRule
+ .getJetty()
+ .getCoreContainer()
+ .getMetricManager()
+ .getPrometheusMetricReader("solr.jvm");
+ MetricSnapshots snapshots = reader.collect();
+
+ for (MetricSnapshot snapshot : snapshots) {
+ Set<Labels> seen = new HashSet<>();
+ for (DataPointSnapshot dataPoint : snapshot.getDataPoints()) {
+ assertTrue(
+ "Duplicate series for metric "
+ + snapshot.getMetadata().getPrometheusName()
+ + " with labels "
+ + dataPoint.getLabels(),
+ seen.add(dataPoint.getLabels()));
+ }
Review Comment:
I just ran the new test you had on my main and it still passes which didn't
seem right. For TDD it should have failed proving this bug exists. I took a
look and also `otel_scope_version` matters here.
The java17 scope adds `otel_scope_version` that java8 doesn't have, so even
with `otel_scope_name` excluded, the two rows still look different.
To really fix this group by metric name, then check how many distinct
otel_scope_name values appear for that name. If it's more than one, that metric
has two sources.
Can you fix the test and make sure it fails on main then passes on this
branch after the fix? After that, I am good with the rest of the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
