-
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
base: main
Are you sure you want to change the base?
One configuration API to rule them all #15599
Conversation
47b3ed3 to
ff7085e
Compare
|
|
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
- 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.
|
Great work! I can rebase #15622 on top of (A) and remove the overlap - I'll come up with a new name and create a new PR |
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: