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

Extended config properties #5912

Closed
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public final class DefaultConfigProperties implements ConfigProperties {
public final class DefaultConfigProperties implements ExtendedConfigProperties {

private final Map<String, String> config;

Expand Down Expand Up @@ -232,6 +232,16 @@ public Map<String, String> getMap(String name) {
Map.Entry::getKey, Map.Entry::getValue, (first, next) -> next, LinkedHashMap::new));
}

@Nullable
@Override
public ExtendedConfigProperties getConfigProperties(String name) {
Map<String, String> mapProperties = getMap(name);
if (mapProperties.isEmpty()) {
return null;
}
return DefaultConfigProperties.createFromMap(mapProperties);
}

/**
* Return a new {@link DefaultConfigProperties} by overriding the {@code previousProperties} with
* the {@code overrides}.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.autoconfigure.spi.internal;

import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import java.util.List;
import javax.annotation.Nullable;

/**
* An extended version of {@link ConfigProperties} that supports accessing complex types - nested
* maps and arrays of maps. {@link ExtendedConfigProperties} is used as a representation of a map,
* since it has (type safe) accessors for string keys.
*/
public interface ExtendedConfigProperties extends ConfigProperties {

/**
* Returns a map-valued configuration property, represented as {@link ExtendedConfigProperties}.
*
* @return a map-valued configuration property, or {@code null} if {@code name} has not been
* configured.
* @throws io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException if the property is not a
* map
*/
@Nullable
default ExtendedConfigProperties getConfigProperties(String name) {
return null;

Check warning on line 29 in sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ExtendedConfigProperties.java

View check run for this annotation

Codecov / codecov/patch

sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ExtendedConfigProperties.java#L29

Added line #L29 was not covered by tests
}

/**
* Returns a list of map-valued configuration property, represented as {@link
* ExtendedConfigProperties}.
*
* @return a list of map-valued configuration property, or {@code null} if {@code name} has not
* been configured.
* @throws io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException if the property is not a
* list of maps
*/
@Nullable
default List<ExtendedConfigProperties> getListConfigProperties(String name) {
return null;

Check warning on line 43 in sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ExtendedConfigProperties.java

View check run for this annotation

Codecov / codecov/patch

sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ExtendedConfigProperties.java#L43

Added line #L43 was not covered by tests
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,24 @@ class ConfigPropertiesTest {
void allValid() {
Map<String, String> properties = makeTestProps();

ConfigProperties config = DefaultConfigProperties.createFromMap(properties);
DefaultConfigProperties config = DefaultConfigProperties.createFromMap(properties);
assertThat(config.getString("test.string")).isEqualTo("str");
assertThat(config.getInt("test.int")).isEqualTo(10);
assertThat(config.getLong("test.long")).isEqualTo(20);
assertThat(config.getDouble("test.double")).isEqualTo(5.4);
assertThat(config.getList("test.list")).containsExactly("cat", "dog", "bear");
assertThat(config.getMap("test.map"))
.containsExactly(entry("cat", "meow"), entry("dog", "bark"), entry("bear", "growl"));
.containsExactly(
entry("cat", "meow"),
entry("dog", "bark"),
entry("bear", "growl"),
entry("number", "1"));
ExtendedConfigProperties testMapConfig = config.getConfigProperties("test.map");
assertThat(testMapConfig.getString("cat")).isEqualTo("meow");
assertThat(testMapConfig.getString("dog")).isEqualTo("bark");
assertThat(testMapConfig.getString("bear")).isEqualTo("growl");
assertThat(testMapConfig.getString("number")).isEqualTo("1");
assertThat(testMapConfig.getInt("number")).isEqualTo(1);
assertThat(config.getDuration("test.duration")).isEqualTo(Duration.ofSeconds(1));
}

Expand All @@ -48,19 +58,24 @@ void allValidUsingHyphens() {
assertThat(config.getDouble("test-double")).isEqualTo(5.4);
assertThat(config.getList("test-list")).containsExactly("cat", "dog", "bear");
assertThat(config.getMap("test-map"))
.containsExactly(entry("cat", "meow"), entry("dog", "bark"), entry("bear", "growl"));
.containsExactly(
entry("cat", "meow"),
entry("dog", "bark"),
entry("bear", "growl"),
entry("number", "1"));
assertThat(config.getDuration("test-duration")).isEqualTo(Duration.ofSeconds(1));
}

@Test
void allMissing() {
ConfigProperties config = DefaultConfigProperties.createFromMap(emptyMap());
DefaultConfigProperties config = DefaultConfigProperties.createFromMap(emptyMap());
assertThat(config.getString("test.string")).isNull();
assertThat(config.getInt("test.int")).isNull();
assertThat(config.getLong("test.long")).isNull();
assertThat(config.getDouble("test.double")).isNull();
assertThat(config.getList("test.list")).isEmpty();
assertThat(config.getMap("test.map")).isEmpty();
assertThat(config.getConfigProperties("test.map")).isNull();
assertThat(config.getDuration("test.duration")).isNull();
}

Expand All @@ -75,13 +90,14 @@ void allEmpty() {
properties.put("test.map", "");
properties.put("test.duration", "");

ConfigProperties config = DefaultConfigProperties.createFromMap(properties);
DefaultConfigProperties config = DefaultConfigProperties.createFromMap(properties);
assertThat(config.getString("test.string")).isEmpty();
assertThat(config.getInt("test.int")).isNull();
assertThat(config.getLong("test.long")).isNull();
assertThat(config.getDouble("test.double")).isNull();
assertThat(config.getList("test.list")).isEmpty();
assertThat(config.getMap("test.map")).isEmpty();
assertThat(config.getConfigProperties("test.map")).isNull();
assertThat(config.getDuration("test.duration")).isNull();
}

Expand Down Expand Up @@ -275,7 +291,7 @@ private static Map<String, String> makeTestProps() {
properties.put("test.double", "5.4");
properties.put("test.boolean", "true");
properties.put("test.list", "cat,dog,bear");
properties.put("test.map", "cat=meow,dog=bark,bear=growl,bird=");
properties.put("test.map", "cat=meow,dog=bark,bear=growl,bird=,number=1");
properties.put("test.duration", "1s");
return properties;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.internal.ExtendedConfigProperties;
import io.opentelemetry.sdk.logs.SdkLoggerProvider;
import io.opentelemetry.sdk.logs.SdkLoggerProviderBuilder;
import io.opentelemetry.sdk.logs.export.LogRecordExporter;
Expand Down Expand Up @@ -441,12 +442,20 @@ private static AutoConfiguredOpenTelemetrySdk maybeConfigureFromFile(ConfigPrope
try {
Class<?> configurationFactory =
Class.forName("io.opentelemetry.sdk.extension.incubator.fileconfig.ConfigurationFactory");
Method parseAndInterpret =
configurationFactory.getMethod("parseAndInterpret", InputStream.class);
OpenTelemetrySdk sdk = (OpenTelemetrySdk) parseAndInterpret.invoke(null, fis);
Method parse = configurationFactory.getMethod("parse", InputStream.class);
Object model = parse.invoke(null, fis);
Class<?> openTelemetryConfiguration =
Class.forName(
"io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfiguration");
Method interpret = configurationFactory.getMethod("interpret", openTelemetryConfiguration);
OpenTelemetrySdk sdk = (OpenTelemetrySdk) interpret.invoke(null, model);
Method toConfigProperties =
configurationFactory.getMethod("toConfigProperties", openTelemetryConfiguration);
ExtendedConfigProperties configProperties =
(ExtendedConfigProperties) toConfigProperties.invoke(null, model);
// Note: can't access file configuration resource without reflection so setting a dummy
// resource
return AutoConfiguredOpenTelemetrySdk.create(sdk, Resource.getDefault(), config);
return AutoConfiguredOpenTelemetrySdk.create(sdk, Resource.getDefault(), configProperties);
} catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException e) {
throw new ConfigurationException(
"Error configuring from file. Is opentelemetry-sdk-extension-incubator on the classpath?",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.internal.ExtendedConfigProperties;
import io.opentelemetry.sdk.logs.internal.SdkEventEmitterProvider;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.trace.SdkTracerProvider;
Expand All @@ -44,7 +45,7 @@

class FileConfigurationTest {

@RegisterExtension private static final CleanupExtension cleanup = new CleanupExtension();
@RegisterExtension static final CleanupExtension cleanup = new CleanupExtension();

@TempDir private Path tempDir;
private Path configFilePath;
Expand All @@ -60,7 +61,11 @@ void setup() throws IOException {
+ " processors:\n"
+ " - simple:\n"
+ " exporter:\n"
+ " console: {}\n";
+ " console: {}\n"
+ "other:\n"
+ " str_key: str_value\n"
+ " map_key:\n"
+ " str_key1: str_value1\n";
configFilePath = tempDir.resolve("otel-config.yaml");
Files.write(configFilePath, yaml.getBytes(StandardCharsets.UTF_8));
GlobalOpenTelemetry.resetForTest();
Expand Down Expand Up @@ -175,4 +180,27 @@ void configFile_Error(@TempDir Path tempDir) throws IOException {
.isInstanceOf(ConfigurationException.class)
.hasMessage("Unrecognized span exporter(s): [foo]");
}

@Test
void configFile_ExtendedConfigProperties() {
ConfigProperties config =
DefaultConfigProperties.createFromMap(
Collections.singletonMap("OTEL_CONFIG_FILE", configFilePath.toString()));
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to provide a way to deactivate OTEL_CONFIG_FILE config loading if the properties supplier is being used directly in the Autoconfiguration.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is unrelated to this PR. Its already possible to deactivate OTEL_CONFIG_FILE via property suppliers. See the source code for how OTEL_CONFIG_FILE is loaded. Its accessed via ConfigProperties.getString("otel.config.file") using a ConfigProperties instance that is resolved using all the property suppliers and customizers typical in autoconfigure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it is unrelated with the PR but is of the upmost importance and should be clarified. From what I can see in the code, if the file exists, the programatic interface to create the SDK is not available anymore. This is a big problem.


AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk =
AutoConfiguredOpenTelemetrySdk.builder().setConfig(config).setResultAsGlobal().build();
OpenTelemetrySdk openTelemetrySdk = autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk();
cleanup.addCloseable(openTelemetrySdk);

// getConfig() should return ExtendedConfigProperties generic representation of the config file
ConfigProperties config1 = autoConfiguredOpenTelemetrySdk.getConfig();
assertThat(config1).isInstanceOf(ExtendedConfigProperties.class);
ExtendedConfigProperties extendedConfigProps = (ExtendedConfigProperties) config1;
ExtendedConfigProperties otherProps = extendedConfigProps.getConfigProperties("other");
assertThat(otherProps).isNotNull();
assertThat(otherProps.getString("str_key")).isEqualTo("str_value");
ExtendedConfigProperties otherMapKeyProps = otherProps.getConfigProperties("map_key");
assertThat(otherMapKeyProps).isNotNull();
assertThat(otherMapKeyProps.getString("str_key1")).isEqualTo("str_value1");
}
}
Loading