add thread details for declarative config#15209
add thread details for declarative config#15209zeitlinger wants to merge 17 commits intoopen-telemetry:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for configuring thread details (thread ID and thread name) as span attributes through declarative configuration in the instrumentation API. When enabled via configuration, an AttributesExtractor automatically adds thread information to spans created by instrumenters.
- Adds
ThreadDetailsAttributesExtractortoInstrumenterBuilderto capture thread ID and name - Integrates with declarative configuration under
instrumentation/java/thread_details/enabled - Adds comprehensive test coverage for both enabled and disabled states
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
InstrumenterBuilder.java |
Implements thread details extraction logic and configuration parsing |
AddThreadDetailsTest.java |
Tests thread details functionality with parameterized enabled/disabled scenarios |
build.gradle.kts |
Adds test dependency for declarative configuration support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6fb9796 to
29883b5
Compare
trask
left a comment
There was a problem hiding this comment.
can you update the PR description with a summary of the relationship between this and the existing thread details span processor? and mention it's off by default when using declarative config (and we can consider making it off by default in 3.0 for others)
acdc794 to
dd2b211
Compare
created #15334 |
|
@trask please check again |
There was a problem hiding this comment.
@jaydeluca can you take a look also?
let's also figure out where to document this, ideally in same place as other declarative config "common" attributes
|
@trask please have another look |
fff4d30 to
8c8a96c
Compare
|
@trask please have another look |
46e9ee9 to
aaea68d
Compare
|
@trask please have a look |
…rTest Signed-off-by: Gregor Zeitlinger <[email protected]>
Java8RuntimeMetricsProvider and Java17RuntimeMetricsProvider are not @configuration classes and should not be registered as Spring auto-configurations. Their presence causes Spring AOT processing to fail with FileNotFoundException when the classes are not on the classpath. Signed-off-by: Gregor Zeitlinger <[email protected]>
…nfig registries Signed-off-by: Gregor Zeitlinger <[email protected]>
Signed-off-by: Gregor Zeitlinger <[email protected]>
The Spring Boot starter's ThreadDetailsInstrumenterCustomizerProvider reads from the declarative config path spring_starter > thread_details > enabled, but the ConfigPropertiesBackedDeclarativeConfigProperties bridge only handles paths starting with "java." or explicitly listed in SPECIAL_MAPPINGS. This caused the property otel.instrumentation.common.thread-details.enabled to have no effect in non-declarative-config mode, breaking smoke tests. Signed-off-by: Gregor Zeitlinger <[email protected]>
…ExtendedOpenTelemetry path Replace SPI-based ThreadDetailsInstrumenterCustomizerProvider with ThreadDetailsAutoConfiguration that reads config directly from Spring Environment, branching on declarative config vs properties-based like EarlyConfig. Programmatically registers the customizer provider - no static mutable fields needed. Signed-off-by: Gregor Zeitlinger <[email protected]>
…ry to fix timing The previous ThreadDetailsAutoConfiguration ran too late — Spring creates beans on-demand via DI, so instrumenters were already built before the flag was set. Moving it into the openTelemetry() bean methods guarantees correct ordering. Signed-off-by: Gregor Zeitlinger <[email protected]>
c975258 to
753691a
Compare
|
@trask good to merge? |
|
hey @zeitlinger, the above review is from AI (based on proposal in #17889) |
Signed-off-by: Gregor Zeitlinger <[email protected]>
…thread-details-provider2 # Conflicts: # javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/AgentDistributionConfig.java
- configureDeclarativeConfig now takes ConfigProvider and reads java.spring_starter.thread_details.enabled from the instrumentation config. The call site in OpenTelemetryAutoConfiguration passes SpringConfigProvider, which coerces Spring's flattened String values back to Boolean; the previous attempts via SdkConfigProvider/YamlDeclarativeConfigProperties silently dropped the flag because Spring's application.yaml pipeline stores every scalar as a String and YamlDeclarativeConfigProperties refuses to coerce. - configureProperties keeps reading otel.instrumentation.common.thread-details.enabled from Environment for the properties-based path; no legacy fallback in the declarative path. - Add declarative-config smoke test assertion: enable thread_details in the testDeclarativeConfig application.yaml and assert thread.id / thread.name appear on the server span. - Simplify segments[0] == "agent" guard in ConfigPropertiesBackedDeclarativeConfigProperties (drop dead segments.length check, work off translatedPath).
|
@zeitlinger I just sent zeitlinger#13 |
Signed-off-by: Gregor Zeitlinger <[email protected]>
|
I put up an alternative follow-up at zeitlinger#14. The main reason I didn’t take the config relocation from #13 is that we had already decided in the declarative config SIG to keep this setting under the distro-controlled nodes, not under So #14 keeps the existing spring starter declarative destination
I did not take the |
good point, I'll do a final pass after you merge those updates 👍 |
Keep declarative thread details distro-scoped
|
@copilot resolve the merge conflicts in this pull request |
…thread-details-provider2 # Conflicts: # smoke-tests-otel-starter/spring-boot-2/src/testDeclarativeConfig/java/io/opentelemetry/spring/smoketest/OtelSpringStarterSmokeTest.java
…/github.com/trask/opentelemetry-java-instrumentation into declarative-config-thread-details-provider2 # Conflicts: # declarative-config-bridge/src/main/java/io/opentelemetry/instrumentation/config/bridge/ConfigPropertiesBackedDeclarativeConfigProperties.java # smoke-tests-otel-starter/spring-boot-2/src/testDeclarativeConfig/java/io/opentelemetry/spring/smoketest/OtelSpringStarterSmokeTest.java
|
sorry, more thoughts on this. I think it's a breaking change and we should hide it behind the v3 preview flag. and/or maybe we should do both, add the thread attributes in the instrumenter, but also add them as a fallback in the span processor in order to support manual instrumentation done outside of the instrumenter |
It should not be breaking - but maybe I miss something. Or do you mean to support #16459 - which is releated? |
I was thinking this could break Java agent users who currently get |
Part of #14087
Alternative for #14564
Summary
Moves thread details (
thread.id,thread.name) from theAddThreadDetailsSpanProcessorto anInstrumenterCustomizer-based approach usingThreadDetailsAttributesExtractor. This means thread attributes are added viaAttributesExtractorat instrumentation build time rather than viaSpanProcessorat span start time.The existing
AddThreadDetailsSpanProcessoris still used for non-declarative-config agent mode (where it remains on by default viaotel.javaagent.add-thread-details=true). The newInstrumenterCustomizerpath is used for declarative config, where it is off by default — users must explicitly enable it:thread_details_enabled: trueunder thejava_agentdistribution node (default: false)thread_details.enabled: trueunderspring_starterinstrumentation config (default: false)otel.javaagent.add-thread-details(default: true, unchanged)Making it off by default in 3.0 for non-declarative-config users is tracked in #15334.
Approach
ThreadDetailsAttributesExtractorininstrumentation-api-incubatorcapturesthread.idandthread.nameon span startInstrumenterCustomizerProviderimplementations (one for agent, one for spring starter) register the extractor when enabledAgentDistributionConfig.isThreadDetailsEnabled()ExtendedOpenTelemetry.getInstrumentationConfig("spring_starter")instrumentation.common) per SIG feedback — this is controlled by agent/starter, not individual instrumentationsAddThreadDetailsSpanProcessorregistration is removed fromAgentTracerProviderConfigurer(the processor class itself is retained for non-DC use)