Skip to content

Vendor specific instrumentation config options handling #14016

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Jul 10, 2025
Merged

Vendor specific instrumentation config options handling #14016

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
16da1b0
Vendor specific instrumentation config options handling
robsunday Jun 10, 2025
c707f0e
spotless
robsunday Jun 10, 2025
73b1797
Custom properties are no longer under "vendor" node.
robsunday Jun 17, 2025
506425b
UT update
robsunday Jun 17, 2025
8e03ae6
Code review changes.
robsunday Jun 20, 2025
6940b8c
Merge branch 'main' into declarative-config-improvements
robsunday Jun 20, 2025
c01877c
Removed assertion in UT that was used to demonstrate name collision
robsunday Jun 20, 2025
f7be9c8
Merge branch 'main' into declarative-config-improvements
robsunday Jul 8, 2025
7375716
Additional changes for mockito
robsunday Jul 8, 2025
6d0f85a
Merge branch 'main' into declarative-config-improvements
robsunday Jul 9, 2025
f413172
Merge branch 'main' into declarative-config-improvements
robsunday Jul 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ...c/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.opentelemetry.instrumentation.jmx.engine.MetricConfiguration;
import io.opentelemetry.instrumentation.jmx.yaml.RuleParser;
import io.opentelemetry.javaagent.extension.AgentListener;
import io.opentelemetry.javaagent.extension.ConfigPropertiesUtil;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import java.io.InputStream;
Expand All @@ -28,7 +29,7 @@ public class JmxMetricInsightInstaller implements AgentListener {

@Override
public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredSdk) {
ConfigProperties config = AgentListener.resolveConfigProperties(autoConfiguredSdk);
ConfigProperties config = ConfigPropertiesUtil.resolveConfigProperties(autoConfiguredSdk);

if (config.getBoolean("otel.jmx.enabled", true)) {
JmxMetricInsight service =
Expand Down
3 changes: 2 additions & 1 deletion ...t/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/OshiMetricsInstaller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.AgentListener;
import io.opentelemetry.javaagent.extension.ConfigPropertiesUtil;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import java.lang.reflect.Method;
Expand All @@ -20,7 +21,7 @@ public class OshiMetricsInstaller implements AgentListener {

@Override
public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredSdk) {
ConfigProperties config = AgentListener.resolveConfigProperties(autoConfiguredSdk);
ConfigProperties config = ConfigPropertiesUtil.resolveConfigProperties(autoConfiguredSdk);

boolean defaultEnabled = config.getBoolean("otel.instrumentation.common.default-enabled", true);
if (!config.getBoolean("otel.instrumentation.oshi.enabled", defaultEnabled)) {
Expand Down
5 changes: 3 additions & 2 deletions ...io/opentelemetry/instrumentation/javaagent/runtimemetrics/java8/JarAnalyzerInstaller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder;
import io.opentelemetry.javaagent.extension.AgentListener;
import io.opentelemetry.javaagent.extension.ConfigPropertiesUtil;
import io.opentelemetry.javaagent.tooling.BeforeAgentListener;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
Expand All @@ -19,7 +19,8 @@ public class JarAnalyzerInstaller implements BeforeAgentListener {

@Override
public void beforeAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) {
ConfigProperties config = AgentListener.resolveConfigProperties(autoConfiguredOpenTelemetrySdk);
ConfigProperties config =
ConfigPropertiesUtil.resolveConfigProperties(autoConfiguredOpenTelemetrySdk);

boolean enabled =
config.getBoolean("otel.instrumentation.runtime-telemetry.package-emitter.enabled", false);
Expand Down
26 changes: 0 additions & 26 deletions ...agent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@

package io.opentelemetry.javaagent.extension;

import io.opentelemetry.api.incubator.config.ConfigProvider;
import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.Ordered;
import java.lang.instrument.Instrumentation;
import net.bytebuddy.agent.builder.AgentBuilder;
Expand All @@ -29,26 +25,4 @@ public interface AgentListener extends Ordered {
* on an {@link Instrumentation}.
*/
void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk);

/** Resolve {@link ConfigProperties} from the {@code autoConfiguredOpenTelemetrySdk}. */
static ConfigProperties resolveConfigProperties(
Copy link
Contributor Author

@robsunday robsunday Jun 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

[for reviewer] This was extracted to separate public class (see ConfigPropertiesUtil) so can be used also for BeforeAgentListener's

AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) {
ConfigProperties sdkConfigProperties =
AutoConfigureUtil.getConfig(autoConfiguredOpenTelemetrySdk);
if (sdkConfigProperties != null) {
return sdkConfigProperties;
}
ConfigProvider configProvider =
AutoConfigureUtil.getConfigProvider(autoConfiguredOpenTelemetrySdk);
if (configProvider != null) {
DeclarativeConfigProperties instrumentationConfig = configProvider.getInstrumentationConfig();

if (instrumentationConfig != null) {
return new DeclarativeConfigPropertiesBridge(instrumentationConfig);
}
}
// Should never happen
throw new IllegalStateException(
"AutoConfiguredOpenTelemetrySdk does not have ConfigProperties or DeclarativeConfigProperties. This is likely a programming error in opentelemetry-java");
}
}
39 changes: 39 additions & 0 deletions ...xtension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.extension;

import io.opentelemetry.api.incubator.config.ConfigProvider;
import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;

// TODO: Add unit test
public class ConfigPropertiesUtil {
private ConfigPropertiesUtil() {}

/** Resolve {@link ConfigProperties} from the {@code autoConfiguredOpenTelemetrySdk}. */
public static ConfigProperties resolveConfigProperties(
AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) {
ConfigProperties sdkConfigProperties =
AutoConfigureUtil.getConfig(autoConfiguredOpenTelemetrySdk);
if (sdkConfigProperties != null) {
return sdkConfigProperties;
}
ConfigProvider configProvider =
AutoConfigureUtil.getConfigProvider(autoConfiguredOpenTelemetrySdk);
if (configProvider != null) {
DeclarativeConfigProperties instrumentationConfig = configProvider.getInstrumentationConfig();

if (instrumentationConfig != null) {
return new DeclarativeConfigPropertiesBridge(instrumentationConfig);
}
}
// Should never happen
throw new IllegalStateException(
"AutoConfiguredOpenTelemetrySdk does not have ConfigProperties or DeclarativeConfigProperties. This is likely a programming error in opentelemetry-java");
}
}
29 changes: 22 additions & 7 deletions ...src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,14 @@ final class DeclarativeConfigPropertiesBridge implements ConfigProperties {

// The node at .instrumentation.java
private final DeclarativeConfigProperties instrumentationJavaNode;
// The node at .instrumentation.java.vendor
Copy link
Contributor Author

@robsunday robsunday Jun 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

[for reviewer] As I mentioned, we may consider moving it under 'general', so it would be .instrumentation.general.vendor'

private final DeclarativeConfigProperties vendorNode;

DeclarativeConfigPropertiesBridge(DeclarativeConfigProperties instrumentationNode) {
instrumentationJavaNode = instrumentationNode.getStructured("java", empty());
vendorNode =
instrumentationJavaNode.getStructured(
"vendor", empty()); // vendor could go to instrumentation.general instead
Copy link
Member

Choose a reason for hiding this comment

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

I don't love the idea of having a dedicated property called vendor which includes data from various vendors' config schemas. It would be better if the config could look like:

instrumentation/development:
  java:
    acme:
      server:
        name: "prime"

And then accessing ConfigProperties.getString("acme.server.name") would resolve "prime".

To accomplish this, we could introduce logic which:

  • If a property starts with otel.instrumentation., strip this prefix. I.e. otel.instrumentation.kafka.producer-propagation.enabled becomes kafka.producer-propagation.enabled.
  • Else, keep the property as is.
  • Resolve all properties starting at the .instrumentation/development.java node. For example:
    • otel.instrumentation.kafka.producer-propagation.enabled resolves to .instrumentation/development.java.kafka.producer-propagation.enabled
    • acme.server.name resolves to .instrumentation/development.java.acme.server.name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see your point.
I considered this approach as well, but decided to skip it due to the risk of name collision and kind of nonuniform approach within the same node. That's why I think having a dedicated node (vendor in our case) is not so bad idea, but definitely it is not the perfect solution.

Let's think about the other approach - do not make any prefix manipulation for otel.instrumentation.* and nest them in 1:1 yaml structure:

instrumentation/development:
  java:
    otel:
      instrumentation:
        kafka:
          producer-propagation:
            enabled: true
        micrometer:
          histogram-gauges:
            enabled: true
    acme:
      server:
        name: "prime"

This looks a bit awkward doe to repeating instrumentation, but it is repeated only once for all the properties. If this is an issue then we may consider renaming instrumentation/development: to something else, like instrumentation_config, config_properties or just config + /development for now.
This would support unified approach for standard and custom properties in yaml structure and in the code.
I know this is a big change, but still possible and worth reconsidering.
What do you think about it?

}

@Nullable
Expand Down Expand Up @@ -129,17 +134,22 @@ public Map<String, String> getMap(String propertyName) {

@Nullable
private <T> T getPropertyValue(
String property, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
if (!property.startsWith(OTEL_INSTRUMENTATION_PREFIX)) {
return null;
String propertyName, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
if (!propertyName.startsWith(OTEL_INSTRUMENTATION_PREFIX)) {
return getVendorPropertyValue(propertyName, extractor);
}
String suffix = property.substring(OTEL_INSTRUMENTATION_PREFIX.length());
// Split the remainder of the property on ".", and walk to the N-1 entry
String[] segments = suffix.split("\\.");
String suffix = propertyName.substring(OTEL_INSTRUMENTATION_PREFIX.length());
return getPropertyValue(instrumentationJavaNode, suffix, extractor);
}

private static <T> T getPropertyValue(
DeclarativeConfigProperties target,
String propertyName,
BiFunction<DeclarativeConfigProperties, String, T> extractor) {
String[] segments = propertyName.split("\\.");
if (segments.length == 0) {
return null;
}
DeclarativeConfigProperties target = instrumentationJavaNode;
if (segments.length > 1) {
for (int i = 0; i < segments.length - 1; i++) {
target = target.getStructured(segments[i], empty());
Expand All @@ -148,4 +158,9 @@ private <T> T getPropertyValue(
String lastPart = segments[segments.length - 1];
return extractor.apply(target, lastPart);
}

private <T> T getVendorPropertyValue(
String propertyName, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
return getPropertyValue(vendorNode, propertyName, extractor);
}
}
10 changes: 9 additions & 1 deletion ...test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ class DeclarativeConfigPropertiesBridgeTest {
+ " map_key:\n"
+ " string_key1: value1\n"
+ " string_key2: value2\n"
+ " bool_key: true\n";
+ " bool_key: true\n"
+ " vendor:\n"
+ " acme:\n"
+ " full_name:\n"
+ " preserved: true";

private ConfigProperties bridge;
private ConfigProperties emptyBridge;
Expand Down Expand Up @@ -132,5 +136,9 @@ void getProperties() {
.isEqualTo(Arrays.asList("value1", "value2"));
assertThat(bridge.getMap("otel.instrumentation.other-instrumentation.map_key", expectedMap))
.isEqualTo(expectedMap);

// verify vendor specific property names are preserved in unchanged form (prefix is not stripped
// as for otel.instrumentation.*)
assertThat(bridge.getBoolean("acme.full_name.preserved")).isTrue();
}
}
3 changes: 2 additions & 1 deletion javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
import io.opentelemetry.javaagent.bootstrap.internal.ConfiguredResourceAttributesHolder;
import io.opentelemetry.javaagent.extension.AgentListener;
import io.opentelemetry.javaagent.extension.ConfigPropertiesUtil;
import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer;
import io.opentelemetry.javaagent.extension.instrumentation.internal.EarlyInstrumentationModule;
import io.opentelemetry.javaagent.tooling.asyncannotationsupport.WeakRefAsyncOperationEndStrategies;
Expand Down Expand Up @@ -159,7 +160,7 @@ private static void installBytebuddyAgent(
AutoConfiguredOpenTelemetrySdk autoConfiguredSdk =
installOpenTelemetrySdk(extensionClassLoader);

ConfigProperties sdkConfig = AgentListener.resolveConfigProperties(autoConfiguredSdk);
ConfigProperties sdkConfig = ConfigPropertiesUtil.resolveConfigProperties(autoConfiguredSdk);
AgentInstrumentationConfig.internalInitializeConfig(new ConfigPropertiesBridge(sdkConfig));
copyNecessaryConfigToSystemProperties(sdkConfig);

Expand Down
Loading