-
Notifications
You must be signed in to change notification settings - Fork 1k
One configuration API to rule them all #15599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
trask
wants to merge
73
commits into
open-telemetry:main
Choose a base branch
from
trask:declarative-configuration-bridge
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
One configuration API to rule them all #15599
trask
wants to merge
73
commits into
open-telemetry:main
from
trask:declarative-configuration-bridge
+3,445
−3,359
Conversation
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
47b3ed3 to
ff7085e
Compare
Member
Author
|
|
622372c to
37ee27b
Compare
…onstructor The DeclarativeConfigUtil.getList().map(HashSet::new).orElse() pattern produces Optional<HashSet<String>>, but HttpConstants.KNOWN_METHODS is a Set<String>. Wrapping it in 'new HashSet<>()' fixes the type mismatch. Fixes compilation errors in: - AgentServletInstrumenterBuilder - Servlet2SpanNameExtractor - HttpUrlConnectionSingletons - ElasticsearchRestJavaagentInstrumenterFactory - HttpSpanDecorator (camel-2.20) - AwsLambdaEventsInstrumenterFactory
1. rxjava-3.0: Fixed import - changed package from v3 to v3_0 TracingAssembly is in io.opentelemetry.instrumentation.rxjava.v3_0 not v3 2. aws-lambda-events: Updated createInstrumenter calls - Removed third parameter (HttpConstants.KNOWN_METHODS) - Method signature changed to only accept (OpenTelemetry, String) - The known methods are now retrieved internally by the factory - Removed unused HttpConstants imports from both v2_2 and v3_11
eaa45fd to
d64f9c7
Compare
Use 'java' path instead of 'general' path for emit_experimental_telemetry configuration in NettyServerSingletons to match the client configuration and properly enable HTTP/2 protocol upgrade events on server spans.
- Fix RuntimeConfigProperties to use EmptyConfigProperties.INSTANCE instead of new - Make deprecated configure() method default in IgnoredTypesConfigurer interface - Migrate IgnoredTypesConfigurer implementations to use DeclarativeConfigUtil with GlobalOpenTelemetry - Add @SuppressWarnings with explanatory comments for deprecated API usage in fallback paths - Fix spotless formatting in OpenTelemetryAutoConfiguration
8af1359 to
9c67113
Compare
The issue was that AgentTracerProviderConfigurer.maybeEnableLoggingExporter() was calling GlobalOpenTelemetry.get() during SDK configuration, which caused GlobalOpenTelemetry to auto-initialize before the agent could call GlobalOpenTelemetry.set(). Changed to use the ConfigProperties parameter that is already available, reading otel.javaagent.debug directly from config instead of using DeclarativeConfigUtil with GlobalOpenTelemetry.get().
The ApplicationLoggingInstrumentationModule.defaultEnabled() method was using GlobalOpenTelemetry.get() to check if the application logger is enabled, but this happens very early during agent startup before GlobalOpenTelemetry is set up. At that point, GlobalOpenTelemetry.get() returns a noop instance that doesn't have the config, causing the instrumentation to incorrectly apply to classloaders where ApplicationLoggerBridge.set() was never called. Reverted to using ConfigProperties which is available earlier during agent initialization and correctly checks the otel.javaagent.logging property.
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.
Now that we have a real configuration API, let's consolidate!
This PR shows end-to-end that we can delete all of the other configuration APIs and bridges.
ConfigPropertiesstill needs to live, but only during SDK autoconfiguration.Then we can bridge
ConfigPropertiesand use the declarative configuration API everywhere else.This PR shows the final result.
Planning to break this PR into smaller pieces: