From 08f34d00b3e395851eb95698d3692082bd33d69d Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Wed, 30 Sep 2020 00:02:16 +0200 Subject: [PATCH] Allow vendor distribution to provide default configuration (#1243) --- .../javaagent/spi/config/PropertySource.java | 34 +++++++ .../tooling/config/ConfigBuilder.java | 8 +- .../tooling/config/ConfigInitializer.java | 17 +++- .../tooling/config/ConfigBuilderTest.groovy | 91 +++++++++++++------ 4 files changed, 121 insertions(+), 29 deletions(-) create mode 100644 javaagent-spi/src/main/java/io/opentelemetry/javaagent/spi/config/PropertySource.java diff --git a/javaagent-spi/src/main/java/io/opentelemetry/javaagent/spi/config/PropertySource.java b/javaagent-spi/src/main/java/io/opentelemetry/javaagent/spi/config/PropertySource.java new file mode 100644 index 0000000000..fc62e02fa1 --- /dev/null +++ b/javaagent-spi/src/main/java/io/opentelemetry/javaagent/spi/config/PropertySource.java @@ -0,0 +1,34 @@ +/* + * Copyright The OpenTelemetry Authors + * + * 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 io.opentelemetry.javaagent.spi.config; + +import java.util.Map; + +/** + * A service provider that allows to override default OTel agent configuration. Properties returned + * by implementations of this interface will be used after the following methods fail to find a + * non-empty property value: system properties, environment variables, properties configuration + * file. + */ +public interface PropertySource { + /** + * @return all properties whose default values are overridden by this property source. Key of the + * map is the propertyName (same as system property name, e.g. {@code otel.exporter}), value + * is the property value. + */ + Map getProperties(); +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigBuilder.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigBuilder.java index 6e9a8cc8cc..a853d560c5 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigBuilder.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigBuilder.java @@ -78,9 +78,13 @@ public final class ConfigBuilder return configMap; } - ConfigBuilder readPropertiesFromAllSources(Properties configurationFile) { + ConfigBuilder readPropertiesFromAllSources( + Properties spiConfiguration, Properties configurationFile) { // ordering from least to most important - return readProperties(configurationFile).readEnvironmentVariables().readSystemProperties(); + return readProperties(spiConfiguration) + .readProperties(configurationFile) + .readEnvironmentVariables() + .readSystemProperties(); } @Override diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java index 823fb9eca4..f8a410ef1b 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigInitializer.java @@ -17,11 +17,14 @@ package io.opentelemetry.javaagent.tooling.config; import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.javaagent.spi.config.PropertySource; +import io.opentelemetry.javaagent.tooling.AgentInstaller; import java.io.File; import java.io.FileNotFoundException; import java.io.FileReader; import java.io.IOException; import java.util.Properties; +import java.util.ServiceLoader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,7 +36,19 @@ public final class ConfigInitializer { public static void initialize() { Config.internalInitializeConfig( - new ConfigBuilder().readPropertiesFromAllSources(loadConfigurationFile()).build()); + new ConfigBuilder() + .readPropertiesFromAllSources(loadSpiConfiguration(), loadConfigurationFile()) + .build()); + } + + /** Retrieves all default configuration overloads using SPI and initializes Config. */ + private static Properties loadSpiConfiguration() { + Properties propertiesFromSpi = new Properties(); + for (PropertySource propertySource : + ServiceLoader.load(PropertySource.class, AgentInstaller.class.getClassLoader())) { + propertiesFromSpi.putAll(propertySource.getProperties()); + } + return propertiesFromSpi; } /** diff --git a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/config/ConfigBuilderTest.groovy b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/config/ConfigBuilderTest.groovy index dc59097681..99c5769de7 100644 --- a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/config/ConfigBuilderTest.groovy +++ b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/config/ConfigBuilderTest.groovy @@ -31,7 +31,7 @@ class ConfigBuilderTest extends AgentSpecification { def "should use defaults"() { when: def config = new ConfigBuilder() - .readPropertiesFromAllSources(new Properties()) + .readPropertiesFromAllSources(new Properties(), new Properties()) .build() then: @@ -53,66 +53,105 @@ class ConfigBuilderTest extends AgentSpecification { config.endpointPeerServiceMapping.isEmpty() } - def "should use configuration file properties (takes precedence over defaults)"() { + def "should use configuration from SPI (takes precedence over defaults)"() { given: - def configurationFile = new Properties() - configurationFile.put(ConfigBuilder.EXPORTER, "zipkin") - configurationFile.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true") - configurationFile.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs") + def spiConfiguration = new Properties() + spiConfiguration.put(ConfigBuilder.EXPORTER, "zipkin") + spiConfiguration.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true") + spiConfiguration.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs") + spiConfiguration.put(ConfigBuilder.TRACE_METHODS, "mypackage.MyClass[myMethod]") when: def config = new ConfigBuilder() - .readPropertiesFromAllSources(configurationFile) + .readPropertiesFromAllSources(spiConfiguration, new Properties()) .build() then: config.exporter == "zipkin" config.hystrixTagsEnabled config.endpointPeerServiceMapping == ["1.2.3.4": "cats", "dogs.com": "dogs"] + config.traceMethods == "mypackage.MyClass[myMethod]" } - def "should use environment variables (takes precedence over configuration file)"() { + def "should use configuration file properties (takes precedence over SPI)"() { given: - def configurationFile = new Properties() - configurationFile.put(ConfigBuilder.EXPORTER, "zipkin") - configurationFile.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true") - configurationFile.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs") + def spiConfiguration = new Properties() + spiConfiguration.put(ConfigBuilder.EXPORTER, "zipkin") + spiConfiguration.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true") + spiConfiguration.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs") + spiConfiguration.put(ConfigBuilder.TRACE_METHODS, "mypackage.MyClass[myMethod]") - environmentVariables.set("OTEL_EXPORTER", "logging") - environmentVariables.set("OTEL_ENDPOINT_PEER_SERVICE_MAPPING", "4.2.4.2=elephants.com") + def configurationFile = new Properties() + configurationFile.put(ConfigBuilder.EXPORTER, "logging") + configurationFile.put(ConfigBuilder.TRACE_METHODS, "mypackage2.MyClass2[myMethod2]") when: def config = new ConfigBuilder() - .readPropertiesFromAllSources(configurationFile) + .readPropertiesFromAllSources(spiConfiguration, configurationFile) .build() then: config.exporter == "logging" config.hystrixTagsEnabled - config.endpointPeerServiceMapping == ["4.2.4.2": "elephants.com"] + config.endpointPeerServiceMapping == ["1.2.3.4": "cats", "dogs.com": "dogs"] + config.traceMethods == "mypackage2.MyClass2[myMethod2]" } - def "should use system properties (takes precedence over environment variables)"() { + def "should use environment variables (takes precedence over configuration file)"() { given: + def spiConfiguration = new Properties() + spiConfiguration.put(ConfigBuilder.EXPORTER, "zipkin") + spiConfiguration.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true") + spiConfiguration.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs") + spiConfiguration.put(ConfigBuilder.TRACE_METHODS, "mypackage.MyClass[myMethod]") + def configurationFile = new Properties() - configurationFile.put(ConfigBuilder.EXPORTER, "zipkin") - configurationFile.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true") - configurationFile.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs") + configurationFile.put(ConfigBuilder.EXPORTER, "logging") + configurationFile.put(ConfigBuilder.TRACE_METHODS, "mypackage2.MyClass2[myMethod2]") - environmentVariables.set("OTEL_EXPORTER", "logging") + environmentVariables.set("OTEL_EXPORTER", "jaeger") environmentVariables.set("OTEL_ENDPOINT_PEER_SERVICE_MAPPING", "4.2.4.2=elephants.com") - System.setProperty(ConfigBuilder.EXPORTER, "jaeger") - when: def config = new ConfigBuilder() - .readPropertiesFromAllSources(configurationFile) + .readPropertiesFromAllSources(spiConfiguration, configurationFile) .build() then: config.exporter == "jaeger" config.hystrixTagsEnabled config.endpointPeerServiceMapping == ["4.2.4.2": "elephants.com"] + config.traceMethods == "mypackage2.MyClass2[myMethod2]" + } + + def "should use system properties (takes precedence over environment variables)"() { + given: + def spiConfiguration = new Properties() + spiConfiguration.put(ConfigBuilder.EXPORTER, "zipkin") + spiConfiguration.put(ConfigBuilder.HYSTRIX_TAGS_ENABLED, "true") + spiConfiguration.put(ConfigBuilder.ENDPOINT_PEER_SERVICE_MAPPING, "1.2.3.4=cats,dogs.com=dogs") + spiConfiguration.put(ConfigBuilder.TRACE_METHODS, "mypackage.MyClass[myMethod]") + + def configurationFile = new Properties() + configurationFile.put(ConfigBuilder.EXPORTER, "logging") + configurationFile.put(ConfigBuilder.TRACE_METHODS, "mypackage2.MyClass2[myMethod2]") + + environmentVariables.set("OTEL_EXPORTER", "jaeger") + environmentVariables.set("OTEL_ENDPOINT_PEER_SERVICE_MAPPING", "4.2.4.2=elephants.com") + + System.setProperty(ConfigBuilder.EXPORTER, "otlp") + System.setProperty(ConfigBuilder.TRACE_METHODS, "mypackage3.MyClass3[myMethod3]") + + when: + def config = new ConfigBuilder() + .readPropertiesFromAllSources(spiConfiguration, configurationFile) + .build() + + then: + config.exporter == "otlp" + config.hystrixTagsEnabled + config.endpointPeerServiceMapping == ["4.2.4.2": "elephants.com"] + config.traceMethods == "mypackage3.MyClass3[myMethod3]" } def "should use defaults in case of parsing failure"() { @@ -121,7 +160,7 @@ class ConfigBuilderTest extends AgentSpecification { when: def config = new ConfigBuilder() - .readPropertiesFromAllSources(new Properties()) + .readPropertiesFromAllSources(new Properties(), new Properties()) .build() then: @@ -139,7 +178,7 @@ class ConfigBuilderTest extends AgentSpecification { System.setProperty("otel.integration.disabled-prop.enabled", "false") def config = new ConfigBuilder() - .readPropertiesFromAllSources(new Properties()) + .readPropertiesFromAllSources(new Properties(), new Properties()) .build() expect: