Skip to content

Conversation

@trask
Copy link
Member

@trask trask commented Dec 19, 2025

No description provided.

private static void maybeEnableLoggingExporter(
SdkTracerProviderBuilder builder, ConfigProperties config) {
if (AgentConfig.isDebugModeEnabled(config)) {
if (config.getBoolean("otel.javaagent.debug", false)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoConfigurationCustomizerProvider implementations are part of the (non-declarative configuration) SDK autoconfiguration, and so will continue to use ConfigProperties

Comment on lines 12 to 26
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);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@trask trask marked this pull request as ready for review December 20, 2025 00:50
@trask trask requested a review from a team as a code owner December 20, 2025 00:50
ExtendedDeclarativeConfigProperties config =
DeclarativeConfigUtil.get(GlobalOpenTelemetry.get());
for (String name : instrumentationNames) {
String normalizedName = name.replace('-', '_');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renaming usages to use _ is a later PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's right.
I meant that you could also rename the instrumentation to apache_httpclient - not sure that's really better though

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

.addOverride("otel.instrumentation.common.default-enabled", defaultEnabled(configProvider))
// these properties are used to initialize the SDK before the configuration file
// is loaded for consistency, we pass them to the bridge, so that they can be read
// later with the same value from the {@link DeclarativeConfigPropertiesBridge}
.addOverride("otel.javaagent.debug", earlyConfig.getBoolean("otel.javaagent.debug", false))
.addOverride("otel.javaagent.logging", earlyConfig.getString("otel.javaagent.logging"))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, I've reverted the portion of this PR related to otel.javaagent.debug

@trask trask changed the title Move AgentConfig usages to declarative configuration API Move one AgentConfig usage to declarative configuration API Dec 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants