Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the order of configuration file application #6697

Merged
merged 1 commit into from
Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions javaagent-tooling/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ dependencies {

testImplementation(project(":testing-common"))
testImplementation("com.google.guava:guava")
testImplementation("org.junit-pioneer:junit-pioneer")
}

testing {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import io.opentelemetry.javaagent.bootstrap.AgentInitializer;
import io.opentelemetry.javaagent.bootstrap.AgentLogEmitterProvider;
import io.opentelemetry.javaagent.bootstrap.OpenTelemetrySdkAccess;
import io.opentelemetry.javaagent.tooling.config.ConfigurationFileLoader;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdkBuilder;
Expand All @@ -26,11 +25,9 @@ public final class OpenTelemetryInstaller {
*
* @return the {@link AutoConfiguredOpenTelemetrySdk}
*/
static AutoConfiguredOpenTelemetrySdk installOpenTelemetrySdk() {
public static AutoConfiguredOpenTelemetrySdk installOpenTelemetrySdk() {
AutoConfiguredOpenTelemetrySdkBuilder builder =
AutoConfiguredOpenTelemetrySdk.builder()
.setResultAsGlobal(true)
.addPropertiesSupplier(new ConfigurationFileLoader());
AutoConfiguredOpenTelemetrySdk.builder().setResultAsGlobal(true);

ClassLoader classLoader = AgentInitializer.getExtensionsClassLoader();
if (classLoader != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
import static java.util.Collections.emptyMap;
import static java.util.logging.Level.SEVERE;

import com.google.auto.service.AutoService;
import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil;
import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizer;
import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
Expand All @@ -17,19 +20,23 @@
import java.nio.charset.StandardCharsets;
import java.util.Map;
import java.util.Properties;
import java.util.function.Supplier;
import java.util.logging.Logger;
import java.util.stream.Collectors;

public final class ConfigurationFileLoader implements Supplier<Map<String, String>> {
@AutoService(AutoConfigurationCustomizerProvider.class)
public final class ConfigurationFileLoader implements AutoConfigurationCustomizerProvider {

private static final Logger logger = Logger.getLogger(ConfigurationFileLoader.class.getName());

static final String CONFIGURATION_FILE_PROPERTY = "otel.javaagent.configuration-file";

@Override
public Map<String, String> get() {
public void customize(AutoConfigurationCustomizer autoConfiguration) {
autoConfiguration.addPropertiesSupplier(ConfigurationFileLoader::loadConfigFile);
}

// visible for tests
static Map<String, String> loadConfigFile() {
// Reading from system property first and from env after
String configurationFilePath = ConfigPropertiesUtil.getString(CONFIGURATION_FILE_PROPERTY);
if (configurationFilePath == null) {
Expand Down Expand Up @@ -63,4 +70,10 @@ public Map<String, String> get() {
return properties.entrySet().stream()
.collect(Collectors.toMap(e -> e.getKey().toString(), e -> e.getValue().toString()));
}

@Override
public int order() {
// make sure it runs after all the user-provided customizers
return Integer.MAX_VALUE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,13 @@ class ConfigurationFileTest extends Specification {
@Shared
public File tmpDir

ConfigurationFileLoader loader = new ConfigurationFileLoader()

def "should use env property"() {
given:
def path = createFile("config", "property1=val-env")
environmentVariables.set("OTEL_JAVAAGENT_CONFIGURATION_FILE", path)

when:
def properties = loader.get()
def properties = ConfigurationFileLoader.loadConfigFile()

then:
properties.get("property1") == "val-env"
Expand All @@ -42,7 +40,7 @@ class ConfigurationFileTest extends Specification {
System.setProperty("otel.javaagent.configuration-file", path)

when:
def properties = loader.get()
def properties = ConfigurationFileLoader.loadConfigFile()

then:
properties.get("property1") == "val-sys"
Expand All @@ -57,7 +55,7 @@ class ConfigurationFileTest extends Specification {
System.setProperty("otel.javaagent.configuration-file", pathSys)

when:
def properties = loader.get()
def properties = ConfigurationFileLoader.loadConfigFile()

then:
properties.get("property1") == "val-sys"
Expand All @@ -69,15 +67,15 @@ class ConfigurationFileTest extends Specification {
environmentVariables.set("OTEL_JAVAAGENT_CONFIGURATION_FILE", "somePath")

when:
def properties = loader.get()
def properties = ConfigurationFileLoader.loadConfigFile()

then:
properties.isEmpty()
}

def "should return empty properties if property is not set"() {
when:
def properties = loader.get()
def properties = ConfigurationFileLoader.loadConfigFile()

then:
properties.isEmpty()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.tooling.config;

import static java.util.Collections.singleton;
import static java.util.Collections.singletonMap;
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.javaagent.tooling.OpenTelemetryInstaller;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizer;
import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junitpioneer.jupiter.ClearSystemProperty;
import org.junitpioneer.jupiter.SetSystemProperty;

@ClearSystemProperty(key = ConfigurationFileLoader.CONFIGURATION_FILE_PROPERTY)
class ConfigurationFileLoaderTest {

@BeforeAll
@AfterAll
static void cleanUp() {
GlobalOpenTelemetry.resetForTest();
}

// regression for https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/6696
@SetSystemProperty(key = "otel.experimental.sdk.enabled", value = "false") // don't setup the SDK
@Test
void fileConfigOverwritesUserPropertiesSupplier(@TempDir Path tempDir) throws IOException {
// given
Path configFile = tempDir.resolve("test-config.properties");
Files.write(configFile, singleton("custom.key = 42"));
System.setProperty(ConfigurationFileLoader.CONFIGURATION_FILE_PROPERTY, configFile.toString());

// when
AutoConfiguredOpenTelemetrySdk autoConfiguredSdk =
OpenTelemetryInstaller.installOpenTelemetrySdk();

// then
assertThat(autoConfiguredSdk.getConfig().getString("custom.key")).isEqualTo("42");
}

// SPI used in test
public static class UserCustomPropertiesSupplier implements AutoConfigurationCustomizerProvider {

@Override
public void customize(AutoConfigurationCustomizer autoConfiguration) {
autoConfiguration.addPropertiesSupplier(() -> singletonMap("custom.key", "123"));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
io.opentelemetry.javaagent.tooling.config.ConfigurationFileLoader
io.opentelemetry.javaagent.tooling.config.ConfigurationFileLoaderTest$UserCustomPropertiesSupplier