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 2 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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,24 @@

package io.opentelemetry.sdk.extension.incubator.fileconfig;

import com.fasterxml.jackson.annotation.JsonSetter;
import com.fasterxml.jackson.annotation.Nulls;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
import io.opentelemetry.sdk.autoconfigure.spi.internal.ExtendedConfigProperties;
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfiguration;
import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.logging.Logger;
import org.snakeyaml.engine.v2.api.Load;
import org.snakeyaml.engine.v2.api.LoadSettings;

/**
* Parses YAML configuration files conforming to the schema in <a
Expand All @@ -26,25 +34,46 @@
*/
public final class ConfigurationFactory {

static final ObjectMapper MAPPER =
new ObjectMapper()
// Create empty object instances for keys which are present but have null values
.setDefaultSetterInfo(JsonSetter.Value.forValueNulls(Nulls.AS_EMPTY));

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

private ConfigurationFactory() {}

/**
* Parse the {@code inputStream} YAML to {@link OpenTelemetryConfiguration} and interpret the
* model to create {@link OpenTelemetrySdk} instance corresponding to the configuration.
*
* @param inputStream the configuration YAML
* @return the {@link OpenTelemetrySdk}
* Combines {@link #parse(InputStream)} and {@link #interpret(OpenTelemetryConfiguration)}
* operations.
*/
public static OpenTelemetrySdk parseAndInterpret(InputStream inputStream) {
OpenTelemetryConfiguration model;
try {
model = ConfigurationReader.parse(inputStream);
model = parse(inputStream);
} catch (RuntimeException e) {
throw new ConfigurationException("Unable to parse inputStream", e);
}

return interpret(model);
}

/** Parse the {@code configuration} YAML and return the {@link OpenTelemetryConfiguration}. */
public static OpenTelemetryConfiguration parse(InputStream configuration) {
LoadSettings settings = LoadSettings.builder().build();
Load yaml = new Load(settings);
Object yamlObj = yaml.loadFromInputStream(configuration);
return MAPPER.convertValue(yamlObj, OpenTelemetryConfiguration.class);
}

/**
* Interpret the {@code model} to create {@link OpenTelemetrySdk} instance corresonding to
* configuration.
*
* @param model the configuration model
* @return the {@link OpenTelemetrySdk}
*/
public static OpenTelemetrySdk interpret(OpenTelemetryConfiguration model) {
List<Closeable> closeables = new ArrayList<>();
try {
return OpenTelemetryConfigurationFactory.getInstance()
Expand All @@ -67,4 +96,18 @@ public static OpenTelemetrySdk parseAndInterpret(InputStream inputStream) {
throw new ConfigurationException("Unexpected configuration error", e);
}
}

/**
* Convert the {@code model} to a generic {@link ExtendedConfigProperties}, which can be used to
* read configuration not part of the model.
*
* @param model the configuration model
* @return a generic {@link ExtendedConfigProperties} representation of the model
*/
public static ExtendedConfigProperties toConfigProperties(OpenTelemetryConfiguration model) {
Map<String, Object> configurationMap =
ConfigurationFactory.MAPPER.convertValue(
model, new TypeReference<Map<String, Object>>() {});
return new FileConfigProperties(configurationMap);
}
}

This file was deleted.

Loading