Move one AgentConfig usage to declarative configuration API#15698
Move one AgentConfig usage to declarative configuration API#15698trask merged 3 commits intoopen-telemetry:mainfrom
Conversation
| private static void maybeEnableLoggingExporter( | ||
| SdkTracerProviderBuilder builder, ConfigProperties config) { | ||
| if (AgentConfig.isDebugModeEnabled(config)) { | ||
| if (config.getBoolean("otel.javaagent.debug", false)) { |
There was a problem hiding this comment.
AutoConfigurationCustomizerProvider implementations are part of the (non-declarative configuration) SDK autoconfiguration, and so will continue to use ConfigProperties
| public static boolean isInstrumentationEnabled( | ||
| ConfigProperties config, Iterable<String> instrumentationNames, boolean defaultEnabled) { | ||
| for (String name : instrumentationNames) { | ||
| String propertyName = "otel.instrumentation." + name + ".enabled"; | ||
| Boolean enabled = config.getBoolean(propertyName); | ||
| if (enabled != null) { | ||
| return enabled; | ||
| } | ||
| } | ||
| return defaultEnabled; | ||
| } | ||
|
|
||
| public static boolean isDebugModeEnabled(ConfigProperties config) { | ||
| return config.getBoolean("otel.javaagent.debug", false); | ||
| } |
There was a problem hiding this comment.
both of these methods ended up only being called from a single place, so moved them (and their tests) and deleted this file (and AgentConfigTest)
| ExtendedDeclarativeConfigProperties config = | ||
| DeclarativeConfigUtil.get(GlobalOpenTelemetry.get()); | ||
| for (String name : instrumentationNames) { | ||
| String normalizedName = name.replace('-', '_'); |
There was a problem hiding this comment.
renaming usages to use _ is a later PR?
There was a problem hiding this comment.
I think it's needed here?
otherwise you'd need to use
instrumentation/development:
java:
apache-httpclient:
enabled: false
instead of
instrumentation/development:
java:
apache_httpclient:
enabled: false
There was a problem hiding this comment.
that's right.
I meant that you could also rename the instrumentation to apache_httpclient - not sure that's really better though
There was a problem hiding this comment.
I kind of like that, it would be nice for the "instrumentation name" to match what's in declarative config yaml
| } | ||
|
|
||
| private static boolean isDebugModeEnabled() { | ||
| return DeclarativeConfigUtil.get(GlobalOpenTelemetry.get()) |
There was a problem hiding this comment.
this is problematic because some usages of the debug flag only use system properties - which would be ignored here.
I added these overrides to address that:
There was a problem hiding this comment.
good point, I've reverted the portion of this PR related to otel.javaagent.debug
No description provided.