From 16da1b0869f468b6360afdfb93abc2cee5743269 Mon Sep 17 00:00:00 2001 From: robsunday Date: Tue, 10 Jun 2025 19:56:08 +0200 Subject: [PATCH 1/7] Vendor specific instrumentation config options handling --- .../jmx/JmxMetricInsightInstaller.java | 3 +- .../oshi/OshiMetricsInstaller.java | 3 +- .../java8/JarAnalyzerInstaller.java | 4 +- .../javaagent/extension/AgentListener.java | 25 ------------ .../extension/ConfigPropertiesUtil.java | 39 +++++++++++++++++++ .../DeclarativeConfigPropertiesBridge.java | 24 ++++++++---- ...DeclarativeConfigPropertiesBridgeTest.java | 9 ++++- .../javaagent/tooling/AgentInstaller.java | 3 +- 8 files changed, 72 insertions(+), 38 deletions(-) create mode 100644 javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java diff --git a/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java b/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java index 2c2d989ba9f7..c639cac53f05 100644 --- a/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java +++ b/instrumentation/jmx-metrics/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/jmx/JmxMetricInsightInstaller.java @@ -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; @@ -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 = diff --git a/instrumentation/oshi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/OshiMetricsInstaller.java b/instrumentation/oshi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/OshiMetricsInstaller.java index 923f95dea061..edca2dfdb8dc 100644 --- a/instrumentation/oshi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/OshiMetricsInstaller.java +++ b/instrumentation/oshi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/oshi/OshiMetricsInstaller.java @@ -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; @@ -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)) { diff --git a/instrumentation/runtime-telemetry/runtime-telemetry-java8/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/java8/JarAnalyzerInstaller.java b/instrumentation/runtime-telemetry/runtime-telemetry-java8/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/java8/JarAnalyzerInstaller.java index f16b00cb2903..8ee541662bc9 100644 --- a/instrumentation/runtime-telemetry/runtime-telemetry-java8/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/java8/JarAnalyzerInstaller.java +++ b/instrumentation/runtime-telemetry/runtime-telemetry-java8/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/java8/JarAnalyzerInstaller.java @@ -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; @@ -19,7 +19,7 @@ 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); diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java index 8b5578104dba..4887be6d3051 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java @@ -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; @@ -30,25 +26,4 @@ public interface AgentListener extends Ordered { */ void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk); - /** Resolve {@link ConfigProperties} from the {@code autoConfiguredOpenTelemetrySdk}. */ - 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"); - } } diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java new file mode 100644 index 000000000000..1b269be07aa0 --- /dev/null +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java @@ -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"); + } +} diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java index 03bc9737c030..499c13d93632 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java @@ -52,9 +52,12 @@ final class DeclarativeConfigPropertiesBridge implements ConfigProperties { // The node at .instrumentation.java private final DeclarativeConfigProperties instrumentationJavaNode; + // The node at .instrumentation.java.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 } @Nullable @@ -129,17 +132,19 @@ public Map getMap(String propertyName) { @Nullable private T getPropertyValue( - String property, BiFunction extractor) { - if (!property.startsWith(OTEL_INSTRUMENTATION_PREFIX)) { - return null; + String propertyName, BiFunction 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 getPropertyValue(DeclarativeConfigProperties target, String propertyName, BiFunction 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()); @@ -148,4 +153,9 @@ private T getPropertyValue( String lastPart = segments[segments.length - 1]; return extractor.apply(target, lastPart); } + + private T getVendorPropertyValue(String propertyName, BiFunction extractor) { + return getPropertyValue(vendorNode, propertyName, extractor); + } + } diff --git a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java index 24df92885e11..8ffc67b6714c 100644 --- a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java +++ b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java @@ -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; @@ -132,5 +136,8 @@ 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(); } } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java index 5a08597845f1..6bcc6e34cbe0 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java @@ -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; @@ -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); From c707f0ea8ba888fcdc4415d8ba9f267dd18fd16f Mon Sep 17 00:00:00 2001 From: robsunday Date: Tue, 10 Jun 2025 20:32:29 +0200 Subject: [PATCH 2/7] spotless --- .../runtimemetrics/java8/JarAnalyzerInstaller.java | 3 ++- .../javaagent/extension/AgentListener.java | 1 - .../DeclarativeConfigPropertiesBridge.java | 13 +++++++++---- .../DeclarativeConfigPropertiesBridgeTest.java | 3 ++- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/instrumentation/runtime-telemetry/runtime-telemetry-java8/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/java8/JarAnalyzerInstaller.java b/instrumentation/runtime-telemetry/runtime-telemetry-java8/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/java8/JarAnalyzerInstaller.java index 8ee541662bc9..51ac4a7f1bb3 100644 --- a/instrumentation/runtime-telemetry/runtime-telemetry-java8/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/java8/JarAnalyzerInstaller.java +++ b/instrumentation/runtime-telemetry/runtime-telemetry-java8/javaagent/src/main/java/io/opentelemetry/instrumentation/javaagent/runtimemetrics/java8/JarAnalyzerInstaller.java @@ -19,7 +19,8 @@ public class JarAnalyzerInstaller implements BeforeAgentListener { @Override public void beforeAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) { - ConfigProperties config = ConfigPropertiesUtil.resolveConfigProperties(autoConfiguredOpenTelemetrySdk); + ConfigProperties config = + ConfigPropertiesUtil.resolveConfigProperties(autoConfiguredOpenTelemetrySdk); boolean enabled = config.getBoolean("otel.instrumentation.runtime-telemetry.package-emitter.enabled", false); diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java index 4887be6d3051..4cbcbfb8970b 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/AgentListener.java @@ -25,5 +25,4 @@ public interface AgentListener extends Ordered { * on an {@link Instrumentation}. */ void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk); - } diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java index 499c13d93632..703a4152067d 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java @@ -57,7 +57,9 @@ final class DeclarativeConfigPropertiesBridge implements ConfigProperties { DeclarativeConfigPropertiesBridge(DeclarativeConfigProperties instrumentationNode) { instrumentationJavaNode = instrumentationNode.getStructured("java", empty()); - vendorNode = instrumentationJavaNode.getStructured("vendor", empty()); // vendor could go to instrumentation.general instead + vendorNode = + instrumentationJavaNode.getStructured( + "vendor", empty()); // vendor could go to instrumentation.general instead } @Nullable @@ -140,7 +142,10 @@ private T getPropertyValue( return getPropertyValue(instrumentationJavaNode, suffix, extractor); } - private static T getPropertyValue(DeclarativeConfigProperties target, String propertyName, BiFunction extractor) { + private static T getPropertyValue( + DeclarativeConfigProperties target, + String propertyName, + BiFunction extractor) { String[] segments = propertyName.split("\\."); if (segments.length == 0) { return null; @@ -154,8 +159,8 @@ private static T getPropertyValue(DeclarativeConfigProperties target, String return extractor.apply(target, lastPart); } - private T getVendorPropertyValue(String propertyName, BiFunction extractor) { + private T getVendorPropertyValue( + String propertyName, BiFunction extractor) { return getPropertyValue(vendorNode, propertyName, extractor); } - } diff --git a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java index 8ffc67b6714c..bd47a193d90f 100644 --- a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java +++ b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java @@ -137,7 +137,8 @@ void getProperties() { 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.*) + // 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(); } } From 73b17975fe4aae29dc538c7e00df55511fa88d80 Mon Sep 17 00:00:00 2001 From: robsunday Date: Tue, 17 Jun 2025 16:58:39 +0200 Subject: [PATCH 3/7] Custom properties are no longer under "vendor" node. --- .../DeclarativeConfigPropertiesBridge.java | 54 ++++++++++++------- ...DeclarativeConfigPropertiesBridgeTest.java | 10 ++-- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java index 703a4152067d..40bc5c7f9209 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java @@ -52,14 +52,9 @@ final class DeclarativeConfigPropertiesBridge implements ConfigProperties { // The node at .instrumentation.java private final DeclarativeConfigProperties instrumentationJavaNode; - // The node at .instrumentation.java.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 } @Nullable @@ -134,22 +129,19 @@ public Map getMap(String propertyName) { @Nullable private T getPropertyValue( - String propertyName, BiFunction extractor) { - if (!propertyName.startsWith(OTEL_INSTRUMENTATION_PREFIX)) { - return getVendorPropertyValue(propertyName, extractor); + String property, BiFunction extractor) { + T value = getOtelPropertyValue(property, extractor); + if (value == null) { + value = getVendorPropertyValue(property, extractor); } - String suffix = propertyName.substring(OTEL_INSTRUMENTATION_PREFIX.length()); - return getPropertyValue(instrumentationJavaNode, suffix, extractor); + return value; } - private static T getPropertyValue( - DeclarativeConfigProperties target, - String propertyName, - BiFunction extractor) { - String[] segments = propertyName.split("\\."); - if (segments.length == 0) { - return null; - } + @Nullable + private T getPropertyValue( + String[] segments, BiFunction extractor) { + + DeclarativeConfigProperties target = instrumentationJavaNode; if (segments.length > 1) { for (int i = 0; i < segments.length - 1; i++) { target = target.getStructured(segments[i], empty()); @@ -159,8 +151,30 @@ private static T getPropertyValue( return extractor.apply(target, lastPart); } + @Nullable + private T getOtelPropertyValue( + String property, BiFunction extractor) { + if (!property.startsWith(OTEL_INSTRUMENTATION_PREFIX)) { + return null; + } + 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("\\."); + if (segments.length == 0) { + return null; + } + + return getPropertyValue(segments, extractor); + } + + @Nullable private T getVendorPropertyValue( - String propertyName, BiFunction extractor) { - return getPropertyValue(vendorNode, propertyName, extractor); + String property, BiFunction extractor) { + String[] segments = property.split("\\."); + if (segments.length == 0) { + return null; + } + + return getPropertyValue(segments, extractor); } } diff --git a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java index bd47a193d90f..e7ef30d740e5 100644 --- a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java +++ b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java @@ -45,10 +45,9 @@ class DeclarativeConfigPropertiesBridgeTest { + " string_key1: value1\n" + " string_key2: value2\n" + " bool_key: true\n" - + " vendor:\n" - + " acme:\n" - + " full_name:\n" - + " preserved: true"; + + " acme:\n" + + " full_name:\n" + + " preserved: true"; private ConfigProperties bridge; private ConfigProperties emptyBridge; @@ -140,5 +139,8 @@ void getProperties() { // 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(); + // Example of property name collision: + assertThat(bridge.getString("acme.full_name.preserved")) + .isEqualTo(bridge.getString("otel.instrumentation.acme.full_name.preserved")); } } From 506425b050a3323fadd8a12f309ff717b899d387 Mon Sep 17 00:00:00 2001 From: robsunday Date: Tue, 17 Jun 2025 17:05:55 +0200 Subject: [PATCH 4/7] UT update --- .../extension/DeclarativeConfigPropertiesBridgeTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java index e7ef30d740e5..1c98246a8f59 100644 --- a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java +++ b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java @@ -140,7 +140,7 @@ void getProperties() { // as for otel.instrumentation.*) assertThat(bridge.getBoolean("acme.full_name.preserved")).isTrue(); // Example of property name collision: - assertThat(bridge.getString("acme.full_name.preserved")) - .isEqualTo(bridge.getString("otel.instrumentation.acme.full_name.preserved")); + assertThat(bridge.getBoolean("acme.full_name.preserved")) + .isEqualTo(bridge.getBoolean("otel.instrumentation.acme.full_name.preserved")); } } From 8e03ae6376f8694cad77dc74505383344eae13b7 Mon Sep 17 00:00:00 2001 From: robsunday Date: Fri, 20 Jun 2025 09:41:00 +0200 Subject: [PATCH 5/7] Code review changes. UT added --- .../extension/ConfigPropertiesUtil.java | 7 +- .../DeclarativeConfigPropertiesBridge.java | 47 +++------- .../extension/ConfigPropertiesUtilTest.java | 87 +++++++++++++++++++ 3 files changed, 103 insertions(+), 38 deletions(-) create mode 100644 javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtilTest.java diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java index 1b269be07aa0..2e164e71ffb5 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java @@ -11,7 +11,6 @@ import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; -// TODO: Add unit test public class ConfigPropertiesUtil { private ConfigPropertiesUtil() {} @@ -28,9 +27,11 @@ public static ConfigProperties resolveConfigProperties( if (configProvider != null) { DeclarativeConfigProperties instrumentationConfig = configProvider.getInstrumentationConfig(); - if (instrumentationConfig != null) { - return new DeclarativeConfigPropertiesBridge(instrumentationConfig); + if (instrumentationConfig == null) { + instrumentationConfig = DeclarativeConfigProperties.empty(); } + + return new DeclarativeConfigPropertiesBridge(instrumentationConfig); } // Should never happen throw new IllegalStateException( diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java index 40bc5c7f9209..01ad41ef222f 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java @@ -130,17 +130,20 @@ public Map getMap(String propertyName) { @Nullable private T getPropertyValue( String property, BiFunction extractor) { - T value = getOtelPropertyValue(property, extractor); - if (value == null) { - value = getVendorPropertyValue(property, extractor); + if (instrumentationJavaNode == null) { + return null; } - return value; - } - @Nullable - private T getPropertyValue( - String[] segments, BiFunction extractor) { + if (property.startsWith(OTEL_INSTRUMENTATION_PREFIX)) { + property = property.substring(OTEL_INSTRUMENTATION_PREFIX.length()); + } + // Split the remainder of the property on "." + String[] segments = property.split("\\."); + if (segments.length == 0) { + return null; + } + // Extract the value by walking to the N-1 entry DeclarativeConfigProperties target = instrumentationJavaNode; if (segments.length > 1) { for (int i = 0; i < segments.length - 1; i++) { @@ -148,33 +151,7 @@ private T getPropertyValue( } } String lastPart = segments[segments.length - 1]; - return extractor.apply(target, lastPart); - } - @Nullable - private T getOtelPropertyValue( - String property, BiFunction extractor) { - if (!property.startsWith(OTEL_INSTRUMENTATION_PREFIX)) { - return null; - } - 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("\\."); - if (segments.length == 0) { - return null; - } - - return getPropertyValue(segments, extractor); - } - - @Nullable - private T getVendorPropertyValue( - String property, BiFunction extractor) { - String[] segments = property.split("\\."); - if (segments.length == 0) { - return null; - } - - return getPropertyValue(segments, extractor); + return extractor.apply(target, lastPart); } } diff --git a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtilTest.java b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtilTest.java new file mode 100644 index 000000000000..4894b44f6b86 --- /dev/null +++ b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtilTest.java @@ -0,0 +1,87 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.extension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +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 org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +@SuppressWarnings("DoNotMockAutoValue") +class ConfigPropertiesUtilTest { + @Test + void shouldUseConfigPropertiesForAutoConfiguration() { + ConfigProperties configPropertiesMock = mock(ConfigProperties.class); + AutoConfiguredOpenTelemetrySdk sdkMock = mock(AutoConfiguredOpenTelemetrySdk.class); + try (MockedStatic autoConfigureUtilMock = + Mockito.mockStatic(AutoConfigureUtil.class)) { + autoConfigureUtilMock + .when(() -> AutoConfigureUtil.getConfig(sdkMock)) + .thenReturn(configPropertiesMock); + + ConfigProperties configProperties = ConfigPropertiesUtil.resolveConfigProperties(sdkMock); + + assertThat(configProperties).isSameAs(configPropertiesMock); + } + } + + @Test + void shouldUseConfigProviderForDeclarativeConfiguration() { + String propertyName = "testProperty"; + String expectedValue = "the value"; + DeclarativeConfigProperties javaNodeMock = mock(DeclarativeConfigProperties.class); + when(javaNodeMock.getString(propertyName)).thenReturn(expectedValue); + + DeclarativeConfigProperties instrumentationConfigMock = mock(DeclarativeConfigProperties.class); + when(instrumentationConfigMock.getStructured(eq("java"), any())).thenReturn(javaNodeMock); + + ConfigProvider configProviderMock = mock(ConfigProvider.class); + when(configProviderMock.getInstrumentationConfig()).thenReturn(instrumentationConfigMock); + + AutoConfiguredOpenTelemetrySdk sdkMock = mock(AutoConfiguredOpenTelemetrySdk.class); + + try (MockedStatic autoConfigureUtilMock = + Mockito.mockStatic(AutoConfigureUtil.class)) { + autoConfigureUtilMock.when(() -> AutoConfigureUtil.getConfig(sdkMock)).thenReturn(null); + autoConfigureUtilMock + .when(() -> AutoConfigureUtil.getConfigProvider(sdkMock)) + .thenReturn(configProviderMock); + + ConfigProperties configProperties = ConfigPropertiesUtil.resolveConfigProperties(sdkMock); + + assertThat(configProperties.getString(propertyName)).isEqualTo(expectedValue); + } + } + + @Test + void shouldUseConfigProviderForDeclarativeConfiguration_noInstrumentationConfig() { + AutoConfiguredOpenTelemetrySdk sdkMock = mock(AutoConfiguredOpenTelemetrySdk.class); + ConfigProvider configProviderMock = mock(ConfigProvider.class); + when(configProviderMock.getInstrumentationConfig()).thenReturn(null); + + try (MockedStatic autoConfigureUtilMock = + Mockito.mockStatic(AutoConfigureUtil.class)) { + autoConfigureUtilMock.when(() -> AutoConfigureUtil.getConfig(sdkMock)).thenReturn(null); + autoConfigureUtilMock + .when(() -> AutoConfigureUtil.getConfigProvider(sdkMock)) + .thenReturn(configProviderMock); + + ConfigProperties configProperties = ConfigPropertiesUtil.resolveConfigProperties(sdkMock); + + assertThat(configProperties.getString("testProperty")).isEqualTo(null); + } + } +} From c01877c6ac629166b7b2386110fd4fd5f883f455 Mon Sep 17 00:00:00 2001 From: robsunday Date: Fri, 20 Jun 2025 11:30:16 +0200 Subject: [PATCH 6/7] Removed assertion in UT that was used to demonstrate name collision --- .../extension/DeclarativeConfigPropertiesBridgeTest.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java index 1c98246a8f59..cd4f032ed187 100644 --- a/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java +++ b/javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridgeTest.java @@ -139,8 +139,5 @@ void getProperties() { // 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(); - // Example of property name collision: - assertThat(bridge.getBoolean("acme.full_name.preserved")) - .isEqualTo(bridge.getBoolean("otel.instrumentation.acme.full_name.preserved")); } } From 73757168792f5c47ee55176d594f3bd4abfa602d Mon Sep 17 00:00:00 2001 From: robsunday Date: Tue, 8 Jul 2025 13:48:46 +0200 Subject: [PATCH 7/7] Additional changes for mockito --- javaagent-extension-api/build.gradle.kts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/javaagent-extension-api/build.gradle.kts b/javaagent-extension-api/build.gradle.kts index 96852314dcb8..144a3a459bf3 100644 --- a/javaagent-extension-api/build.gradle.kts +++ b/javaagent-extension-api/build.gradle.kts @@ -22,3 +22,8 @@ dependencies { // Used by byte-buddy but not brought in as a transitive dependency. compileOnly("com.google.code.findbugs:annotations") } + +// Needed by mockito +configurations.testRuntimeClasspath { + exclude(group = "net.bytebuddy", module = "byte-buddy-dep") +}