Set instrumentation version on emitted Meter scopes#18866
Draft
trask wants to merge 1 commit into
Draft
Conversation
9f285bb to
c0fb2aa
Compare
Test runner TelemetryDataUtil.assertScopeVersion only walked SpanData, so metric-only modules whose emitted scope name does not match the gradle module name slipped through CI. Broaden the assertion to also walk MetricData (gated by the existing startsWith("test") carve-out) and hook it into InstrumentationTestRunner.waitAndAssertMetrics(...) so the same protection covers meters.
Fix the in-tree metric-only sites that the broadened assertion catches:
* failsafe-3.0, dropwizard-metrics-4.0, iceberg-1.8, micrometer-1.5: scope already matches module name; switch getMeter(...) to meterBuilder(...) with EmbeddedInstrumentationProperties version lookup so the version actually flows through.
* oshi-5.0: scope was a pre-rename literal. The library module is alpha so flip its emitted scope directly to the module-name-aligned io.opentelemetry.oshi-5.0 (the existing version-properties lookup just works once the names match). The javaagent is stable so apply the CxfSingletons pattern: emit the legacy io.opentelemetry.oshi scope by default to preserve dashboards/filters, switch to the new scope under AgentCommonConfig.isV3Preview(). Add SystemMetrics/ProcessMetrics.registerObservers(Meter) overloads so the javaagent can inject a meter with the legacy scope while reusing the library observer-registration code.
* tomcat-jdbc-8.5: javaagent-only, no library. Apply the CxfSingletons pattern: legacy io.opentelemetry.tomcat-jdbc scope by default, new io.opentelemetry.tomcat-jdbc-8.5 under v3-preview, always look up version under the new module name. Add a DbConnectionPoolMetrics.create(Meter, String) overload so tomcat-jdbc can build its own meter and inject it.
* jmx-metrics: pre-existing scope mismatch (emits io.opentelemetry.jmx, module is jmx-metrics). Add a versionLookupName parameter to MetricRegistrar and pass io.opentelemetry.jmx-metrics from JmxMetricInsight so the emitted scope finally has a version.
c0fb2aa to
1096454
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
TelemetryDataUtil.assertScopeVersiononly walkedSpanData, so metric-only modules whose emitted scope name did not match the gradle module name slipped through CI. This is what allowed the recenttomcat-jdbc-8.5andoshi-5.0module renames (#18838, #18854) to ship while still emitting the pre-rename scope names \u2014 the docs telemetry parser noticed in #18860, but the agent's own test runner did not.Changes
assertScopeVersionto also walkMetricData(gated by the existingstartsWith("test")carve-out) and hook it intoInstrumentationTestRunner.waitAndAssertMetrics(...)so the same protection covers meters.failsafe-3.0,dropwizard-metrics-4.0,iceberg-1.8,micrometer-1.5: scope already matches the module name; switchgetMeter(...)tometerBuilder(...)withEmbeddedInstrumentationPropertiesversion lookup so the version actually flows through.oshi-5.0: scope was a pre-rename literal. The library module is alpha so flip its emitted scope directly to the module-name-alignedio.opentelemetry.oshi-5.0(the existing version-properties lookup just works once the names match). The javaagent is stable so apply theCxfSingletonspattern: emit the legacyio.opentelemetry.oshiscope by default to preserve dashboards/filters, switch to the new scope underAgentCommonConfig.get().isV3Preview(). AddSystemMetrics/ProcessMetrics.registerObservers(Meter)overloads so the javaagent can inject a meter with the legacy scope while reusing the library observer-registration code.tomcat-jdbc-8.5: javaagent-only, no library. Apply the CxfSingletons pattern: legacyio.opentelemetry.tomcat-jdbcscope by default, newio.opentelemetry.tomcat-jdbc-8.5under v3-preview, always look up version under the new module name. Add aDbConnectionPoolMetrics.create(Meter, String)overload so tomcat-jdbc can build its own meter and inject it.jmx-metrics: pre-existing scope mismatch (emitsio.opentelemetry.jmx, module isjmx-metrics). Add aversionLookupNameparameter toMetricRegistrarand passio.opentelemetry.jmx-metricsfromJmxMetricInsightso the emitted scope finally has a version.Does not include the docs allow-list overrides from #18860 \u2014 once this lands the
tomcat-jdbc-8.5entry can be dropped.