From 41e779435101266e7b34d7712fed130aa051445d Mon Sep 17 00:00:00 2001 From: cleverchuk Date: Tue, 21 Jul 2026 13:43:59 -0400 Subject: [PATCH 1/7] Fix flaky test --- .../JdbcInstrumentationTest.java | 70 ++++++++++--------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/instrumentation/jdbc/javaagent/src/test/java/com/solarwinds/opentelemetry/instrumentation/JdbcInstrumentationTest.java b/instrumentation/jdbc/javaagent/src/test/java/com/solarwinds/opentelemetry/instrumentation/JdbcInstrumentationTest.java index 5bea6a36..17f3f28c 100644 --- a/instrumentation/jdbc/javaagent/src/test/java/com/solarwinds/opentelemetry/instrumentation/JdbcInstrumentationTest.java +++ b/instrumentation/jdbc/javaagent/src/test/java/com/solarwinds/opentelemetry/instrumentation/JdbcInstrumentationTest.java @@ -16,22 +16,23 @@ package com.solarwinds.opentelemetry.instrumentation; -import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.awaitility.Awaitility.await; +import static org.junit.jupiter.api.Assertions.assertTrue; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; +import io.opentelemetry.sdk.trace.data.SpanData; import java.sql.Connection; import java.sql.DriverManager; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Statement; +import java.time.Duration; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.RegisterExtension; -import org.mockito.junit.jupiter.MockitoExtension; import org.testcontainers.containers.MySQLContainer; import org.testcontainers.junit.jupiter.Container; import org.testcontainers.junit.jupiter.Testcontainers; @@ -39,15 +40,19 @@ @Testcontainers @SuppressWarnings("all") -@ExtendWith(MockitoExtension.class) class JdbcInstrumentationTest { + private static final AttributeKey QUERY_TAG = AttributeKey.stringKey("sw.query_tag"); + @RegisterExtension private static final AgentInstrumentationExtension testing = AgentInstrumentationExtension.create(); + // Static so the container starts once for the whole class instead of once per test method, + // which keeps startup cheap and avoids per-test container/network timeouts. @Container - public MySQLContainer mysql = new MySQLContainer<>(DockerImageName.parse("mysql:9.2.0")); + private static final MySQLContainer mysql = + new MySQLContainer<>(DockerImageName.parse("mysql:9.2.0")); private Connection connection; @@ -57,6 +62,11 @@ void setup() throws SQLException { String.format( "%s?user=%s&password=%s", mysql.getJdbcUrl(), mysql.getUsername(), mysql.getPassword()); connection = DriverManager.getConnection(url); + + // Opening the connection issues driver setup statements that are themselves instrumented and + // emit a nondeterministic number of spans/traces. Drop them so the assertions below only see + // the trace produced by the code under test. + testing.clearData(); } @AfterEach @@ -76,20 +86,7 @@ void verifyDbContextInjectionForStatementWhenQueryThrowsAnException() { } }); - testing.waitAndAssertTraces( - trace -> - // no real checks because we don't care about these spans - trace.hasSpansSatisfyingExactly(span -> {}, span -> {}, span -> {}, span -> {}), - trace -> - trace.hasSpansSatisfyingExactly( - span -> - span.hasName("root") - .hasKind(SpanKind.INTERNAL) - .hasAttributesSatisfying( - attributes -> - assertNotNull( - attributes.get(AttributeKey.stringKey("sw.query_tag")))), - span -> {})); + assertRootSpanHasQueryTag(); } @Test @@ -104,19 +101,26 @@ void verifyDbContextInjectionForPrepareStatement() { } }); - testing.waitAndAssertTraces( - trace -> - // no real checks because we don't care about these spans - trace.hasSpansSatisfyingExactly(span -> {}, span -> {}, span -> {}, span -> {}), - trace -> - trace.hasSpansSatisfyingExactly( - span -> - span.hasName("root") - .hasKind(SpanKind.INTERNAL) - .hasAttributesSatisfying( - attributes -> - assertNotNull( - attributes.get(AttributeKey.stringKey("sw.query_tag")))), - span -> {})); + assertRootSpanHasQueryTag(); + } + + // Assert the injection outcome (the "root" span carries sw.query_tag) by polling the flat list + // of exported spans. This avoids pinning the exact number/grouping of driver-generated spans, + // which is nondeterministic across driver/server versions and timing, and only waits until the + // span we care about has been exported. + private static void assertRootSpanHasQueryTag() { + await() + .atMost(Duration.ofSeconds(20)) + .untilAsserted( + () -> + assertTrue( + testing.spans().stream().anyMatch(JdbcInstrumentationTest::isTaggedRootSpan), + "expected a 'root' INTERNAL span carrying the sw.query_tag attribute")); + } + + private static boolean isTaggedRootSpan(SpanData span) { + return "root".equals(span.getName()) + && span.getKind() == SpanKind.INTERNAL + && span.getAttributes().get(QUERY_TAG) != null; } } From f3e363f30a320945499446c7d3f6a12c9300091c Mon Sep 17 00:00:00 2001 From: cleverchuk Date: Tue, 21 Jul 2026 17:51:55 -0400 Subject: [PATCH 2/7] read distribution node for shared agent configs --- .../extensions/config/DeclarativeLoader.java | 96 +++++---- .../config/DeclarativeLoaderTest.java | 186 +++++++++++++++--- .../SharedConfigCustomizerProvider.java | 3 +- 3 files changed, 228 insertions(+), 57 deletions(-) diff --git a/custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoader.java b/custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoader.java index fdfced47..9b55672d 100644 --- a/custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoader.java +++ b/custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoader.java @@ -25,53 +25,83 @@ import com.solarwinds.joboe.logging.LoggerFactory; import com.solarwinds.opentelemetry.extensions.LoggingConfigProvider; import com.solarwinds.opentelemetry.extensions.config.parser.yaml.DeclarativeConfigParser; -import io.opentelemetry.api.incubator.ExtendedOpenTelemetry; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; -import io.opentelemetry.javaagent.tooling.BeforeAgentListener; -import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfiguration; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfigurationCustomizer; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfigurationCustomizerProvider; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.OpenTelemetryConfigurationModel; +import java.util.Arrays; +import java.util.List; -@AutoService(BeforeAgentListener.class) -public class DeclarativeLoader implements BeforeAgentListener { +@AutoService(DeclarativeConfigurationCustomizerProvider.class) +public class DeclarativeLoader implements DeclarativeConfigurationCustomizerProvider { private static final Logger logger = LoggerFactory.getLogger(); @Override - public void beforeAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) { - OpenTelemetrySdk openTelemetrySdk = autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk(); - if (!(openTelemetrySdk instanceof ExtendedOpenTelemetry)) { - // Shouldn't happen in practice, but just in case - logger.warn( - "OpenTelemetrySdk is not an instance of ExtendedOpenTelemetry. Declarative configuration will not be applied."); - return; - } + public void customize(DeclarativeConfigurationCustomizer customizer) { + customizer.addModelCustomizer( + configurationModel -> { + loadConfig(configurationModel); + return configurationModel; + }); + } - ExtendedOpenTelemetry extendedOpenTelemetry = (ExtendedOpenTelemetry) openTelemetrySdk; - DeclarativeConfigProperties solarwinds = - extendedOpenTelemetry.getInstrumentationConfig("solarwinds"); + private void loadConfig(OpenTelemetryConfigurationModel configurationModel) { + DeclarativeConfigProperties configProperties = + DeclarativeConfiguration.toConfigProperties(configurationModel); - if (solarwinds != null && !solarwinds.getPropertyKeys().isEmpty()) { - try { - ConfigContainer configContainer = new ConfigContainer(); - DeclarativeConfigParser declarativeConfigParser = - new DeclarativeConfigParser(configContainer); + // Sources are parsed in order into a single container; the first source to define a given + // key wins (see ConfigContainer#put), so distribution takes precedence over the + // instrumentation node. + List sources = + Arrays.asList( + getDistributionConfig(configProperties), getInstrumentationConfig(configProperties)); - declarativeConfigParser.parse(solarwinds); - ConfigurationLoader.processConfigs(configContainer); + ConfigContainer configContainer = new ConfigContainer(); + DeclarativeConfigParser declarativeConfigParser = new DeclarativeConfigParser(configContainer); - ConfigContainer agentConfig = configContainer.subset(ConfigGroup.AGENT); - LoggerFactory.init(LoggingConfigProvider.getLoggerConfiguration(agentConfig)); - logger.info("Loaded via declarative config"); + for (DeclarativeConfigProperties source : sources) { + if (source == null || source.getPropertyKeys().isEmpty()) { + continue; + } + try { + declarativeConfigParser.parse(source); } catch (InvalidConfigException e) { throw new RuntimeException(e); } + } - if (!JavaRuntimeVersionChecker.isJdkVersionSupported()) { - logger.warn( - String.format( - "Profiling is not supported for Java runtime version: %s . The lowest Java version supported for profiling is %s.", - System.getProperty("java.version"), JavaRuntimeVersionChecker.minVersionSupported)); - } + try { + ConfigurationLoader.processConfigs(configContainer); + ConfigContainer agentConfig = configContainer.subset(ConfigGroup.AGENT); + LoggerFactory.init(LoggingConfigProvider.getLoggerConfiguration(agentConfig)); + + logger.info("Loaded via declarative config"); + } catch (InvalidConfigException e) { + throw new RuntimeException(e); + } + + if (!JavaRuntimeVersionChecker.isJdkVersionSupported()) { + logger.warn( + String.format( + "Profiling is not supported for Java runtime version: %s . The lowest Java version supported for profiling is %s.", + System.getProperty("java.version"), JavaRuntimeVersionChecker.minVersionSupported)); } } + + private DeclarativeConfigProperties getDistributionConfig( + DeclarativeConfigProperties configProperties) { + return configProperties + .getStructured("distribution", DeclarativeConfigProperties.empty()) + .getStructured("solarwinds"); + } + + private DeclarativeConfigProperties getInstrumentationConfig( + DeclarativeConfigProperties configProperties) { + return configProperties + .getStructured("instrumentation/development", DeclarativeConfigProperties.empty()) + .getStructured("java", DeclarativeConfigProperties.empty()) + .getStructured("solarwinds"); + } } diff --git a/custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoaderTest.java b/custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoaderTest.java index 339d4899..69df41d0 100644 --- a/custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoaderTest.java +++ b/custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoaderTest.java @@ -16,20 +16,28 @@ package com.solarwinds.opentelemetry.extensions.config; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.when; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.verify; import com.solarwinds.joboe.config.ConfigManager; import com.solarwinds.joboe.config.ConfigProperty; -import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; -import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; -import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfiguration; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfigurationCustomizer; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.DistributionModel; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.DistributionPropertyModel; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.ExperimentalInstrumentationModel; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.ExperimentalLanguageSpecificInstrumentationModel; import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.ExperimentalLanguageSpecificInstrumentationPropertyModel; -import io.opentelemetry.sdk.internal.ExtendedOpenTelemetrySdk; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.OpenTelemetryConfigurationModel; +import java.util.function.Function; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -39,16 +47,16 @@ class DeclarativeLoaderTest { @InjectMocks private DeclarativeLoader tested; - @Mock private AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdkMock; + @Mock private DeclarativeConfigurationCustomizer customizerMock; - @Mock private ExtendedOpenTelemetrySdk extendedOpenTelemetrySdkMock; + @Captor + private ArgumentCaptor> + modelCustomizerCaptor; - private final ExperimentalLanguageSpecificInstrumentationPropertyModel solarwinds = - new ExperimentalLanguageSpecificInstrumentationPropertyModel() - .withAdditionalProperty( - ConfigProperty.AGENT_SERVICE_KEY.getConfigFileKey(), "token:service") - .withAdditionalProperty( - ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), "apm.collector.com"); + @BeforeEach + public void resetConfig() { + ConfigManager.reset(); + } @AfterAll public static void clearConfig() { @@ -56,16 +64,150 @@ public static void clearConfig() { } @Test - public void testBeforeAgent() { - DeclarativeConfigProperties configProperties = - DeclarativeConfiguration.toConfigProperties(solarwinds); - when(autoConfiguredOpenTelemetrySdkMock.getOpenTelemetrySdk()) - .thenReturn(extendedOpenTelemetrySdkMock); - when(extendedOpenTelemetrySdkMock.getInstrumentationConfig(eq("solarwinds"))) - .thenReturn(configProperties); - tested.beforeAgent(autoConfiguredOpenTelemetrySdkMock); + public void testCustomizeLoadsDistributionConfig() { + OpenTelemetryConfigurationModel model = + new OpenTelemetryConfigurationModel() + .withDistribution( + new DistributionModel() + .withAdditionalProperty( + "solarwinds", + new DistributionPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_SERVICE_KEY.getConfigFileKey(), + "token:service") + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), + "apm.collector.com"))); + + OpenTelemetryConfigurationModel returned = customize(model); + + assertNotNull(ConfigManager.getConfig(ConfigProperty.AGENT_COLLECTOR)); + assertNotNull(ConfigManager.getConfig(ConfigProperty.AGENT_SERVICE_KEY)); + assertEquals(model, returned); + } + + @Test + public void testCustomizeLoadsInstrumentationConfig() { + OpenTelemetryConfigurationModel model = + new OpenTelemetryConfigurationModel() + .withInstrumentationDevelopment( + new ExperimentalInstrumentationModel() + .withJava( + new ExperimentalLanguageSpecificInstrumentationModel() + .withAdditionalProperty( + "solarwinds", + new ExperimentalLanguageSpecificInstrumentationPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_SERVICE_KEY.getConfigFileKey(), + "token:service") + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), + "apm.collector.com")))); + + customize(model); assertNotNull(ConfigManager.getConfig(ConfigProperty.AGENT_COLLECTOR)); assertNotNull(ConfigManager.getConfig(ConfigProperty.AGENT_SERVICE_KEY)); } + + @Test + public void testDistributionTakesPrecedenceWhenBothNodesPresent() { + OpenTelemetryConfigurationModel model = + new OpenTelemetryConfigurationModel() + .withDistribution(distribution("distribution-token:distribution-service")) + .withInstrumentationDevelopment(instrumentation("instrumentation-token:inst-service")); + + customize(model); + + assertEquals( + "distribution-token:distribution-service", + ConfigManager.getConfig(ConfigProperty.AGENT_SERVICE_KEY)); + } + + @Test + public void testCustomizeWithEmptyModelThrows() { + // No source node is present, so both are skipped and processConfigs runs against an empty + // container, which fails because the service key is required. + OpenTelemetryConfigurationModel model = new OpenTelemetryConfigurationModel(); + + assertThrows(RuntimeException.class, () -> customize(model)); + assertNull(ConfigManager.getConfig(ConfigProperty.AGENT_SERVICE_KEY)); + } + + @Test + public void testCustomizeSkipsEmptySolarwindsNode() { + // distribution.solarwinds is present but has no properties, so it is skipped; configuration is + // still loaded from the instrumentation node. + OpenTelemetryConfigurationModel model = + new OpenTelemetryConfigurationModel() + .withDistribution( + new DistributionModel() + .withAdditionalProperty("solarwinds", new DistributionPropertyModel())) + .withInstrumentationDevelopment(instrumentation("token:service")); + + customize(model); + + assertEquals("token:service", ConfigManager.getConfig(ConfigProperty.AGENT_SERVICE_KEY)); + } + + @Test + public void testCustomizeThrowsOnUnknownKey() { + OpenTelemetryConfigurationModel model = + new OpenTelemetryConfigurationModel() + .withDistribution( + new DistributionModel() + .withAdditionalProperty( + "solarwinds", + new DistributionPropertyModel() + .withAdditionalProperty("not_a_real_config_key", "value"))); + + assertThrows(RuntimeException.class, () -> customize(model)); + } + + @Test + public void testCustomizeThrowsWhenServiceKeyMissing() { + OpenTelemetryConfigurationModel model = + new OpenTelemetryConfigurationModel() + .withDistribution( + new DistributionModel() + .withAdditionalProperty( + "solarwinds", + new DistributionPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), + "apm.collector.com"))); + + assertThrows(RuntimeException.class, () -> customize(model)); + } + + private static DistributionModel distribution(String serviceKey) { + return new DistributionModel() + .withAdditionalProperty( + "solarwinds", + new DistributionPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_SERVICE_KEY.getConfigFileKey(), serviceKey) + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), "apm.collector.com")); + } + + private static ExperimentalInstrumentationModel instrumentation(String serviceKey) { + return new ExperimentalInstrumentationModel() + .withJava( + new ExperimentalLanguageSpecificInstrumentationModel() + .withAdditionalProperty( + "solarwinds", + new ExperimentalLanguageSpecificInstrumentationPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_SERVICE_KEY.getConfigFileKey(), serviceKey) + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), + "apm.collector.com"))); + } + + private OpenTelemetryConfigurationModel customize(OpenTelemetryConfigurationModel model) { + tested.customize(customizerMock); + verify(customizerMock).addModelCustomizer(modelCustomizerCaptor.capture()); + return modelCustomizerCaptor.getValue().apply(model); + } } diff --git a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java index 98fe068d..06cf8a52 100644 --- a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java +++ b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java @@ -328,8 +328,7 @@ private DeclarativeConfigProperties getSolarwindsConfig(OpenTelemetryConfigurati return Objects.requireNonNull( configProperties - .getStructured("instrumentation/development", DeclarativeConfigProperties.empty()) - .getStructured("java", DeclarativeConfigProperties.empty()) + .getStructured("distribution", DeclarativeConfigProperties.empty()) .getStructured("solarwinds"), "Solarwinds configuration cannot be null."); } From be6d3db5db4c0ed6a232c373e84ef267890403f4 Mon Sep 17 00:00:00 2001 From: cleverchuk Date: Wed, 22 Jul 2026 00:14:36 -0400 Subject: [PATCH 3/7] add thread name to apm logs --- .../main/java/com/solarwinds/joboe/core/rpc/RpcClient.java | 2 +- .../src/main/java/com/solarwinds/joboe/logging/Logger.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/libs/core/src/main/java/com/solarwinds/joboe/core/rpc/RpcClient.java b/libs/core/src/main/java/com/solarwinds/joboe/core/rpc/RpcClient.java index 84bb9e7c..a3805613 100644 --- a/libs/core/src/main/java/com/solarwinds/joboe/core/rpc/RpcClient.java +++ b/libs/core/src/main/java/com/solarwinds/joboe/core/rpc/RpcClient.java @@ -377,7 +377,7 @@ private void resetClient(String arg) throws ClientFatalException { private void asyncInitClient() { ExecutorService executorService = Executors.newSingleThreadExecutor(DaemonThreadFactory.newInstance("init-rpc-client")); - executorService.submit(() -> initClient()); + executorService.submit(this::initClient); executorService.shutdown(); } diff --git a/libs/logging/src/main/java/com/solarwinds/joboe/logging/Logger.java b/libs/logging/src/main/java/com/solarwinds/joboe/logging/Logger.java index 96f0914f..993e53b2 100644 --- a/libs/logging/src/main/java/com/solarwinds/joboe/logging/Logger.java +++ b/libs/logging/src/main/java/com/solarwinds/joboe/logging/Logger.java @@ -180,7 +180,10 @@ private void print(Level level, String message, Throwable t) { private static String getFormattedMessage(Level level, String message) { String timestamp = DATE_FORMAT.get().format(Calendar.getInstance().getTime()); - String label = timestamp + " " + level.toString() + " " + SOLARWINDSS_TAG + " "; + String label = + String.format( + "%s %s %s [%s] ", + timestamp, level.toString(), SOLARWINDSS_TAG, Thread.currentThread().getName()); return message != null ? label + message : label; } From 4b3704048e04352fcce28eeea5ab6287ebe61d3d Mon Sep 17 00:00:00 2001 From: cleverchuk Date: Wed, 22 Jul 2026 00:23:38 -0400 Subject: [PATCH 4/7] update sdk-config.yaml --- sdk-config.yaml | 16 ++++++++++++---- smoke-tests/sdk-config.yaml | 10 ++++++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/sdk-config.yaml b/sdk-config.yaml index e391b71e..59130935 100644 --- a/sdk-config.yaml +++ b/sdk-config.yaml @@ -19,17 +19,28 @@ resource: - name: service.name value: swi-test-app +# Provide shared config for multi-langauge deployment +distribution: + solarwinds: + agent.serviceKey: "" + agent.collector: "" + agent.logging: + level: info + # Configure general attribute limits. See also tracer_provider.limits, logger_provider.limits. attribute_limits: # Configure tracer provider. tracer_provider: + processors: # Configure meter provider. meter_provider: + readers: # Configure logger provider. Remove to disable log export logger_provider: + processors: instrumentation/development: java: @@ -46,7 +57,4 @@ instrumentation/development: - javax - com.sun - sun - - sunw - agent.logging: - level: info - agent.serviceKey: "" \ No newline at end of file + - sunw \ No newline at end of file diff --git a/smoke-tests/sdk-config.yaml b/smoke-tests/sdk-config.yaml index 9f59f8dd..71684d25 100644 --- a/smoke-tests/sdk-config.yaml +++ b/smoke-tests/sdk-config.yaml @@ -21,6 +21,14 @@ resource: - name: test.app value: java-apm-smoke-test-v2 +# Provide shared config for multi-langauge deployment +distribution: + solarwinds: + agent.serviceKey: ${SW_APM_SERVICE_KEY} + agent.collector: apm.collector.na-01.st-ssp.solarwinds.com + agent.logging: + level: trace + # Configure general attribute limits. See also tracer_provider.limits, logger_provider.limits. attribute_limits: # Configure max attribute value size. @@ -130,8 +138,6 @@ instrumentation/development: - com.sun - sun - sunw - agent.logging: - level: trace agent.sqlTag: true agent.sqlTagPrepared: true agent.sqlTagDatabases: From f2c8b598d32f1eb1da6dc0f0bca4f8e6a06cf8bf Mon Sep 17 00:00:00 2001 From: cleverchuk Date: Wed, 22 Jul 2026 00:53:53 -0400 Subject: [PATCH 5/7] also still check instrumentation node --- .../config/provider/SharedConfigCustomizerProvider.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java index 06cf8a52..4bb754f7 100644 --- a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java +++ b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java @@ -329,7 +329,13 @@ private DeclarativeConfigProperties getSolarwindsConfig(OpenTelemetryConfigurati return Objects.requireNonNull( configProperties .getStructured("distribution", DeclarativeConfigProperties.empty()) - .getStructured("solarwinds"), + .getStructured( + "solarwinds", + configProperties + .getStructured( + "instrumentation/development", DeclarativeConfigProperties.empty()) + .getStructured("java", DeclarativeConfigProperties.empty()) + .getStructured("solarwinds")), "Solarwinds configuration cannot be null."); } } From 28c213ae6df787c95c83458ef371ff78915e9f88 Mon Sep 17 00:00:00 2001 From: cleverchuk Date: Wed, 22 Jul 2026 16:00:20 -0400 Subject: [PATCH 6/7] use merging --- .../extensions/config/DeclarativeLoader.java | 45 ++--- .../config/SolarwindsConfigResolver.java | 163 ++++++++++++++++++ .../SharedConfigCustomizerProvider.java | 11 +- .../SharedConfigCustomizerProviderTest.java | 163 ++++++++++++++++++ 4 files changed, 340 insertions(+), 42 deletions(-) create mode 100644 libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolver.java diff --git a/custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoader.java b/custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoader.java index 9b55672d..cd1e1048 100644 --- a/custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoader.java +++ b/custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoader.java @@ -30,8 +30,7 @@ import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfigurationCustomizer; import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfigurationCustomizerProvider; import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.OpenTelemetryConfigurationModel; -import java.util.Arrays; -import java.util.List; +import java.util.Objects; @AutoService(DeclarativeConfigurationCustomizerProvider.class) public class DeclarativeLoader implements DeclarativeConfigurationCustomizerProvider { @@ -50,26 +49,21 @@ private void loadConfig(OpenTelemetryConfigurationModel configurationModel) { DeclarativeConfigProperties configProperties = DeclarativeConfiguration.toConfigProperties(configurationModel); - // Sources are parsed in order into a single container; the first source to define a given - // key wins (see ConfigContainer#put), so distribution takes precedence over the - // instrumentation node. - List sources = - Arrays.asList( - getDistributionConfig(configProperties), getInstrumentationConfig(configProperties)); + // SolarwindsConfigResolver merges the distribution and instrumentation nodes per key + // (distribution takes precedence, instrumentation fills the rest); it returns null when + // neither node carries any properties. + DeclarativeConfigProperties solarwinds = + Objects.requireNonNull( + SolarwindsConfigResolver.resolve(configProperties), + "Solarwinds configuration cannot be null."); ConfigContainer configContainer = new ConfigContainer(); DeclarativeConfigParser declarativeConfigParser = new DeclarativeConfigParser(configContainer); - for (DeclarativeConfigProperties source : sources) { - if (source == null || source.getPropertyKeys().isEmpty()) { - continue; - } - - try { - declarativeConfigParser.parse(source); - } catch (InvalidConfigException e) { - throw new RuntimeException(e); - } + try { + declarativeConfigParser.parse(solarwinds); + } catch (InvalidConfigException e) { + throw new RuntimeException(e); } try { @@ -89,19 +83,4 @@ private void loadConfig(OpenTelemetryConfigurationModel configurationModel) { System.getProperty("java.version"), JavaRuntimeVersionChecker.minVersionSupported)); } } - - private DeclarativeConfigProperties getDistributionConfig( - DeclarativeConfigProperties configProperties) { - return configProperties - .getStructured("distribution", DeclarativeConfigProperties.empty()) - .getStructured("solarwinds"); - } - - private DeclarativeConfigProperties getInstrumentationConfig( - DeclarativeConfigProperties configProperties) { - return configProperties - .getStructured("instrumentation/development", DeclarativeConfigProperties.empty()) - .getStructured("java", DeclarativeConfigProperties.empty()) - .getStructured("solarwinds"); - } } diff --git a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolver.java b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolver.java new file mode 100644 index 00000000..d0b41b44 --- /dev/null +++ b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolver.java @@ -0,0 +1,163 @@ +/* + * © SolarWinds Worldwide, LLC. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.solarwinds.opentelemetry.extensions.config; + +import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; +import io.opentelemetry.common.ComponentLoader; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +/** + * Resolves the solarwinds agent configuration from a declarative config tree. + * + *

The configuration may live under the top-level {@code distribution.solarwinds} node (shared + * across languages in a multi-language deployment) and/or under the language-specific {@code + * instrumentation/development.java.solarwinds} node. When both are present they are merged per key: + * the distribution node takes precedence for any key it defines and the instrumentation node fills + * in the rest. A present-but-empty node is treated as absent. + * + *

This is the single source of truth for that precedence so every consumer resolves the same + * effective config for a given tree. + */ +public final class SolarwindsConfigResolver { + + private SolarwindsConfigResolver() {} + + /** + * Returns the effective solarwinds config, merging the distribution and instrumentation nodes per + * key, or {@code null} if neither node is present. + * + *

Presence — not emptiness — decides the outcome: an absent node ({@code getStructured} + * returns {@code null}) is ignored, whereas a present-but-empty node is honored. When both nodes + * are present the result is a per-key merge in which the distribution node wins for every key it + * defines and the instrumentation node supplies the rest; an empty distribution node therefore + * simply defers every key to the instrumentation node. + */ + public static DeclarativeConfigProperties resolve(DeclarativeConfigProperties configProperties) { + DeclarativeConfigProperties distribution = getDistributionConfig(configProperties); + DeclarativeConfigProperties instrumentation = getInstrumentationConfig(configProperties); + + if (distribution != null && instrumentation != null) { + return new MergedConfigProperties(distribution, instrumentation); + } + + if (distribution != null) { + return distribution; + } + + return instrumentation; + } + + /** + * Returns the top-level {@code distribution.solarwinds} node, or {@code null} if it is absent. + */ + public static DeclarativeConfigProperties getDistributionConfig( + DeclarativeConfigProperties configProperties) { + return configProperties + .getStructured("distribution", DeclarativeConfigProperties.empty()) + .getStructured("solarwinds"); + } + + /** + * Returns the {@code instrumentation/development.java.solarwinds} node, or {@code null} if it is + * absent. + */ + public static DeclarativeConfigProperties getInstrumentationConfig( + DeclarativeConfigProperties configProperties) { + return configProperties + .getStructured("instrumentation/development", DeclarativeConfigProperties.empty()) + .getStructured("java", DeclarativeConfigProperties.empty()) + .getStructured("solarwinds"); + } + + /** + * Read-only view over two solarwinds config nodes that resolves each key from the primary node + * when present and falls back to the secondary node otherwise, giving a per-key merge rather than + * an all-or-nothing choice between the nodes. + */ + private static final class MergedConfigProperties implements DeclarativeConfigProperties { + private final DeclarativeConfigProperties primary; + private final DeclarativeConfigProperties fallback; + + MergedConfigProperties( + DeclarativeConfigProperties primary, DeclarativeConfigProperties fallback) { + this.primary = primary; + this.fallback = fallback; + } + + @Override + public String getString(String name) { + String value = primary.getString(name); + return value != null ? value : fallback.getString(name); + } + + @Override + public Boolean getBoolean(String name) { + Boolean value = primary.getBoolean(name); + return value != null ? value : fallback.getBoolean(name); + } + + @Override + public Integer getInt(String name) { + Integer value = primary.getInt(name); + return value != null ? value : fallback.getInt(name); + } + + @Override + public Long getLong(String name) { + Long value = primary.getLong(name); + return value != null ? value : fallback.getLong(name); + } + + @Override + public Double getDouble(String name) { + Double value = primary.getDouble(name); + return value != null ? value : fallback.getDouble(name); + } + + @Override + public List getScalarList(String name, Class scalarType) { + List value = primary.getScalarList(name, scalarType); + return value != null ? value : fallback.getScalarList(name, scalarType); + } + + @Override + public DeclarativeConfigProperties getStructured(String name) { + DeclarativeConfigProperties value = primary.getStructured(name); + return value != null ? value : fallback.getStructured(name); + } + + @Override + public List getStructuredList(String name) { + List value = primary.getStructuredList(name); + return value != null ? value : fallback.getStructuredList(name); + } + + @Override + public Set getPropertyKeys() { + Set keys = new HashSet<>(primary.getPropertyKeys()); + keys.addAll(fallback.getPropertyKeys()); + return keys; + } + + @Override + public ComponentLoader getComponentLoader() { + return primary.getComponentLoader(); + } + } +} diff --git a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java index 4bb754f7..55f4b68c 100644 --- a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java +++ b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java @@ -24,6 +24,7 @@ import com.solarwinds.joboe.config.InvalidConfigException; import com.solarwinds.joboe.config.ProxyConfig; import com.solarwinds.joboe.config.ServiceKeyUtils; +import com.solarwinds.opentelemetry.extensions.config.SolarwindsConfigResolver; import com.solarwinds.opentelemetry.extensions.config.parser.yaml.ProxyParser; import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfiguration; @@ -327,15 +328,7 @@ private DeclarativeConfigProperties getSolarwindsConfig(OpenTelemetryConfigurati DeclarativeConfiguration.toConfigProperties(model); return Objects.requireNonNull( - configProperties - .getStructured("distribution", DeclarativeConfigProperties.empty()) - .getStructured( - "solarwinds", - configProperties - .getStructured( - "instrumentation/development", DeclarativeConfigProperties.empty()) - .getStructured("java", DeclarativeConfigProperties.empty()) - .getStructured("solarwinds")), + SolarwindsConfigResolver.resolve(configProperties), "Solarwinds configuration cannot be null."); } } diff --git a/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProviderTest.java b/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProviderTest.java index 8e80bb4a..c57dd8f8 100644 --- a/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProviderTest.java +++ b/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProviderTest.java @@ -29,6 +29,8 @@ import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.AttributeLimitsModel; import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.BatchLogRecordProcessorModel; import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.BatchSpanProcessorModel; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.DistributionModel; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.DistributionPropertyModel; import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.ExperimentalInstrumentationModel; import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.ExperimentalLanguageSpecificInstrumentationModel; import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.ExperimentalLanguageSpecificInstrumentationPropertyModel; @@ -779,4 +781,165 @@ void metricsExportDisabledWhenMeterProviderAbsent() { assertNotNull(openTelemetryConfigurationModel.getMeterProvider()); assertFalse((Boolean) ConfigManager.getConfig(ConfigProperty.AGENT_EXPORT_METRICS_ENABLED)); } + + @Test + void readsSolarwindsConfigFromDistributionNode() { + OpenTelemetryConfigurationModel openTelemetryConfigurationModel = + new OpenTelemetryConfigurationModel() + .withLoggerProvider(new LoggerProviderModel().withProcessors(Collections.emptyList())) + .withDistribution( + new DistributionModel() + .withAdditionalProperty( + "solarwinds", + new DistributionPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_SERVICE_KEY.getConfigFileKey(), + "token:service") + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), + "apm.collector.com"))); + + doNothing() + .when(declarativeConfigurationCustomizerMock) + .addModelCustomizer(functionArgumentCaptor.capture()); + + tested.customize(declarativeConfigurationCustomizerMock); + functionArgumentCaptor.getValue().apply(openTelemetryConfigurationModel); + + Map logConfigs = logExporterConfigs(openTelemetryConfigurationModel); + assertEquals("https://otel.collector.com/v1/logs", logConfigs.get("endpoint")); + assertEquals("authorization=Bearer token", logConfigs.get("headers_list")); + } + + @Test + void distributionNodeTakesPrecedenceOverInstrumentationNode() { + OpenTelemetryConfigurationModel openTelemetryConfigurationModel = + new OpenTelemetryConfigurationModel() + .withLoggerProvider(new LoggerProviderModel().withProcessors(Collections.emptyList())) + .withDistribution( + new DistributionModel() + .withAdditionalProperty( + "solarwinds", + new DistributionPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_SERVICE_KEY.getConfigFileKey(), + "token:service") + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), + "apm.distribution.com"))) + .withInstrumentationDevelopment( + new ExperimentalInstrumentationModel() + .withJava( + new ExperimentalLanguageSpecificInstrumentationModel() + .withAdditionalProperty( + "solarwinds", + new ExperimentalLanguageSpecificInstrumentationPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_SERVICE_KEY.getConfigFileKey(), + "token:service") + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), + "apm.instrumentation.com")))); + + doNothing() + .when(declarativeConfigurationCustomizerMock) + .addModelCustomizer(functionArgumentCaptor.capture()); + + tested.customize(declarativeConfigurationCustomizerMock); + functionArgumentCaptor.getValue().apply(openTelemetryConfigurationModel); + + Map logConfigs = logExporterConfigs(openTelemetryConfigurationModel); + assertEquals("https://otel.distribution.com/v1/logs", logConfigs.get("endpoint")); + } + + @Test + void emptyDistributionSolarwindsFallsBackToInstrumentationNode() { + OpenTelemetryConfigurationModel openTelemetryConfigurationModel = + new OpenTelemetryConfigurationModel() + .withLoggerProvider(new LoggerProviderModel().withProcessors(Collections.emptyList())) + .withDistribution( + new DistributionModel() + .withAdditionalProperty("solarwinds", new DistributionPropertyModel())) + .withInstrumentationDevelopment( + new ExperimentalInstrumentationModel() + .withJava( + new ExperimentalLanguageSpecificInstrumentationModel() + .withAdditionalProperty( + "solarwinds", + new ExperimentalLanguageSpecificInstrumentationPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_SERVICE_KEY.getConfigFileKey(), + "token:service") + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), + "apm.instrumentation.com")))); + + doNothing() + .when(declarativeConfigurationCustomizerMock) + .addModelCustomizer(functionArgumentCaptor.capture()); + + tested.customize(declarativeConfigurationCustomizerMock); + functionArgumentCaptor.getValue().apply(openTelemetryConfigurationModel); + + Map logConfigs = logExporterConfigs(openTelemetryConfigurationModel); + assertEquals("https://otel.instrumentation.com/v1/logs", logConfigs.get("endpoint")); + } + + @Test + void partialDistributionConfigIsFilledFromInstrumentationNode() { + // distribution supplies only the collector; the service key must still be picked up from the + // instrumentation node so the merged config is complete. + OpenTelemetryConfigurationModel openTelemetryConfigurationModel = + new OpenTelemetryConfigurationModel() + .withLoggerProvider(new LoggerProviderModel().withProcessors(Collections.emptyList())) + .withDistribution( + new DistributionModel() + .withAdditionalProperty( + "solarwinds", + new DistributionPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), + "apm.distribution.com"))) + .withInstrumentationDevelopment( + new ExperimentalInstrumentationModel() + .withJava( + new ExperimentalLanguageSpecificInstrumentationModel() + .withAdditionalProperty( + "solarwinds", + new ExperimentalLanguageSpecificInstrumentationPropertyModel() + .withAdditionalProperty( + ConfigProperty.AGENT_SERVICE_KEY.getConfigFileKey(), + "token:service") + .withAdditionalProperty( + ConfigProperty.AGENT_COLLECTOR.getConfigFileKey(), + "apm.instrumentation.com")))); + + doNothing() + .when(declarativeConfigurationCustomizerMock) + .addModelCustomizer(functionArgumentCaptor.capture()); + + tested.customize(declarativeConfigurationCustomizerMock); + functionArgumentCaptor.getValue().apply(openTelemetryConfigurationModel); + + Map logConfigs = logExporterConfigs(openTelemetryConfigurationModel); + // collector comes from distribution (wins per key)... + assertEquals("https://otel.distribution.com/v1/logs", logConfigs.get("endpoint")); + // ...and the service key is filled in from the instrumentation node. + assertEquals("authorization=Bearer token", logConfigs.get("headers_list")); + } + + private static Map logExporterConfigs( + OpenTelemetryConfigurationModel openTelemetryConfigurationModel) { + LoggerProviderModel loggerProvider = openTelemetryConfigurationModel.getLoggerProvider(); + LogRecordProcessorModel logRecordProcessorModel = loggerProvider.getProcessors().get(0); + BatchLogRecordProcessorModel batch = logRecordProcessorModel.getBatch(); + + assertNotNull(batch); + LogRecordExporterModel exporter = batch.getExporter(); + LogRecordExporterPropertyModel logExporterProperty = + exporter.getAdditionalProperties().get(LogExporterComponentProvider.COMPONENT_NAME); + + assertNotNull(logExporterProperty); + return logExporterProperty.getAdditionalProperties(); + } } From fca48d190061fa44cf1827b8cdc49138dde3067e Mon Sep 17 00:00:00 2001 From: cleverchuk Date: Wed, 22 Jul 2026 16:31:47 -0400 Subject: [PATCH 7/7] add resolver test --- .../config/DeclarativeLoaderTest.java | 6 +- .../config/SolarwindsConfigResolver.java | 3 +- .../config/SolarwindsConfigResolverTest.java | 151 ++++++++++++++++++ 3 files changed, 154 insertions(+), 6 deletions(-) create mode 100644 libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolverTest.java diff --git a/custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoaderTest.java b/custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoaderTest.java index 69df41d0..969629c0 100644 --- a/custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoaderTest.java +++ b/custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoaderTest.java @@ -126,18 +126,14 @@ public void testDistributionTakesPrecedenceWhenBothNodesPresent() { @Test public void testCustomizeWithEmptyModelThrows() { - // No source node is present, so both are skipped and processConfigs runs against an empty - // container, which fails because the service key is required. OpenTelemetryConfigurationModel model = new OpenTelemetryConfigurationModel(); - assertThrows(RuntimeException.class, () -> customize(model)); + assertThrows(NullPointerException.class, () -> customize(model)); assertNull(ConfigManager.getConfig(ConfigProperty.AGENT_SERVICE_KEY)); } @Test public void testCustomizeSkipsEmptySolarwindsNode() { - // distribution.solarwinds is present but has no properties, so it is skipped; configuration is - // still loaded from the instrumentation node. OpenTelemetryConfigurationModel model = new OpenTelemetryConfigurationModel() .withDistribution( diff --git a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolver.java b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolver.java index d0b41b44..d6626515 100644 --- a/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolver.java +++ b/libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolver.java @@ -29,7 +29,8 @@ * across languages in a multi-language deployment) and/or under the language-specific {@code * instrumentation/development.java.solarwinds} node. When both are present they are merged per key: * the distribution node takes precedence for any key it defines and the instrumentation node fills - * in the rest. A present-but-empty node is treated as absent. + * in the rest. A node counts as present when it exists in the tree, even if it carries no + * properties; only a node that is entirely absent is skipped (see {@link #resolve}). * *

This is the single source of truth for that precedence so every consumer resolves the same * effective config for a given tree. diff --git a/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolverTest.java b/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolverTest.java new file mode 100644 index 00000000..0c725278 --- /dev/null +++ b/libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/SolarwindsConfigResolverTest.java @@ -0,0 +1,151 @@ +/* + * © SolarWinds Worldwide, LLC. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.solarwinds.opentelemetry.extensions.config; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.DeclarativeConfiguration; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.DistributionModel; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.DistributionPropertyModel; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.ExperimentalInstrumentationModel; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.ExperimentalLanguageSpecificInstrumentationModel; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.ExperimentalLanguageSpecificInstrumentationPropertyModel; +import io.opentelemetry.sdk.autoconfigure.declarativeconfig.model.OpenTelemetryConfigurationModel; +import org.junit.jupiter.api.Test; + +class SolarwindsConfigResolverTest { + + @Test + void resolveReturnsNullWhenNeitherNodePresent() { + assertNull(resolve(new OpenTelemetryConfigurationModel())); + } + + @Test + void resolveReturnsDistributionWhenOnlyDistributionPresent() { + DeclarativeConfigProperties resolved = + resolve( + new OpenTelemetryConfigurationModel() + .withDistribution(distribution("service-key", "distribution.collector"))); + + assertNotNull(resolved); + assertEquals("service-key", resolved.getString("agent.serviceKey")); + assertEquals("distribution.collector", resolved.getString("agent.collector")); + } + + @Test + void resolveReturnsInstrumentationWhenOnlyInstrumentationPresent() { + DeclarativeConfigProperties resolved = + resolve( + new OpenTelemetryConfigurationModel() + .withInstrumentationDevelopment( + instrumentation("service-key", "instrumentation.collector"))); + + assertNotNull(resolved); + assertEquals("service-key", resolved.getString("agent.serviceKey")); + assertEquals("instrumentation.collector", resolved.getString("agent.collector")); + } + + @Test + void presentButEmptyDistributionNodeIsHonoredAndDefersEveryKeyToInstrumentation() { + DeclarativeConfigProperties resolved = + resolve( + new OpenTelemetryConfigurationModel() + .withDistribution( + new DistributionModel() + .withAdditionalProperty("solarwinds", new DistributionPropertyModel())) + .withInstrumentationDevelopment( + instrumentation("service-key", "instrumentation.collector"))); + + assertNotNull(resolved); + assertEquals("service-key", resolved.getString("agent.serviceKey")); + assertEquals("instrumentation.collector", resolved.getString("agent.collector")); + } + + @Test + void distributionWinsPerKeyAndInstrumentationFillsTheRest() { + DeclarativeConfigProperties resolved = + resolve( + new OpenTelemetryConfigurationModel() + .withDistribution( + new DistributionModel() + .withAdditionalProperty( + "solarwinds", + new DistributionPropertyModel() + .withAdditionalProperty("agent.collector", "distribution.collector"))) + .withInstrumentationDevelopment( + instrumentation("instrumentation-key", "instrumentation.collector"))); + + assertNotNull(resolved); + assertEquals("distribution.collector", resolved.getString("agent.collector")); + assertEquals("instrumentation-key", resolved.getString("agent.serviceKey")); + } + + @Test + void getPropertyKeysIsTheUnionOfBothNodes() { + DeclarativeConfigProperties resolved = + resolve( + new OpenTelemetryConfigurationModel() + .withDistribution( + new DistributionModel() + .withAdditionalProperty( + "solarwinds", + new DistributionPropertyModel() + .withAdditionalProperty("agent.collector", "distribution.collector"))) + .withInstrumentationDevelopment( + new ExperimentalInstrumentationModel() + .withJava( + new ExperimentalLanguageSpecificInstrumentationModel() + .withAdditionalProperty( + "solarwinds", + new ExperimentalLanguageSpecificInstrumentationPropertyModel() + .withAdditionalProperty( + "agent.serviceKey", "instrumentation-key"))))); + + assertNotNull(resolved); + assertTrue(resolved.getPropertyKeys().contains("agent.collector")); + assertTrue(resolved.getPropertyKeys().contains("agent.serviceKey")); + } + + private static DeclarativeConfigProperties resolve(OpenTelemetryConfigurationModel model) { + return SolarwindsConfigResolver.resolve(DeclarativeConfiguration.toConfigProperties(model)); + } + + private static DistributionModel distribution(String serviceKey, String collector) { + return new DistributionModel() + .withAdditionalProperty( + "solarwinds", + new DistributionPropertyModel() + .withAdditionalProperty("agent.serviceKey", serviceKey) + .withAdditionalProperty("agent.collector", collector)); + } + + private static ExperimentalInstrumentationModel instrumentation( + String serviceKey, String collector) { + return new ExperimentalInstrumentationModel() + .withJava( + new ExperimentalLanguageSpecificInstrumentationModel() + .withAdditionalProperty( + "solarwinds", + new ExperimentalLanguageSpecificInstrumentationPropertyModel() + .withAdditionalProperty("agent.serviceKey", serviceKey) + .withAdditionalProperty("agent.collector", collector))); + } +}