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

Define dedicated file configuration SPI ComponentProvider #6457

Merged
merged 10 commits into from
Jun 3, 2024
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.exporter.prometheus.internal;

import io.opentelemetry.exporter.prometheus.PrometheusHttpServer;
import io.opentelemetry.exporter.prometheus.PrometheusHttpServerBuilder;
import io.opentelemetry.sdk.autoconfigure.spi.internal.ComponentProvider;
import io.opentelemetry.sdk.autoconfigure.spi.internal.StructuredConfigProperties;
import io.opentelemetry.sdk.metrics.export.MetricReader;

/**
* File configuration SPI implementation for {@link PrometheusHttpServer}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
public class PrometheusComponentProvider implements ComponentProvider<MetricReader> {
Copy link
Member Author

Choose a reason for hiding this comment

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

A demonstration ComponentProvider. Notice how config properties are accessed without dot notation since the YAML node is presented directly as the StructuredConfigProperties.


@Override
public Class<MetricReader> getType() {
return MetricReader.class;
}

@Override
public String getName() {
return "prometheus";
}

@Override
public MetricReader create(StructuredConfigProperties config) {
PrometheusHttpServerBuilder prometheusBuilder = PrometheusHttpServer.builder();

Integer port = config.getInt("port");
if (port != null) {
prometheusBuilder.setPort(port);
}
String host = config.getString("host");
if (host != null) {
prometheusBuilder.setHost(host);
}

return prometheusBuilder.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
io.opentelemetry.exporter.prometheus.internal.PrometheusComponentProvider
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.trace.export.SpanExporter;

/**
* Provides configured instances of SDK extension components. {@link ComponentProvider} allows SDK
* extension components which are not part of the core SDK to be referenced in file based
* configuration.
*
* @param <T> the type of the SDK extension component. See {@link #getType()}.
*/
// TODO (jack-berg): list the specific types which are supported in file configuration
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
public interface ComponentProvider<T> {

/**
* The type of SDK extension component. For example, if providing instances of a custom span
* exporter, the type would be {@link SpanExporter}.
*/
Class<T> getType();

/**
* The name of the exporter, to be referenced in configuration files. For example, if providing
* instances of a custom span exporter for the "acme" protocol, the name might be "acme".
*
* <p>This name MUST not be the same as any other component provider name which returns components
* of the same {@link #getType() type}.
*/
String getName();

/**
* Configure an instance of the SDK extension component according to the {@code config}.
*
* @param config the configuration provided where the component is referenced in a configuration
* file.
* @return an instance the SDK extension component
*/
// TODO (jack-berg): consider dynamic configuration use case before stabilizing in case that
// affects any API decisions
T create(StructuredConfigProperties config);
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

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

import static io.opentelemetry.api.internal.ConfigUtil.defaultIfNull;

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

/**
* An interface for accessing structured configuration data.
*
* <p>An instance of {@link StructuredConfigProperties} is equivalent to a <a
* href="https://yaml.org/spec/1.2.2/#3211-nodes">YAML mapping node</a>. It has accessors for
* reading scalar properties, {@link #getStructured(String)} for reading children which are
* themselves mappings, and {@link #getStructuredList(String)} for reading children which are
* sequences of mappings.
*/
public interface StructuredConfigProperties {

/**
* Returns a {@link String} configuration property.
*
* @return null if the property has not been configured
* @throws ConfigurationException if the property is not a valid scalar string
*/
@Nullable
String getString(String name);

/**
* Returns a {@link String} configuration property.
*
* @return a {@link String} configuration property or {@code defaultValue} if a property with
* {@code name} has not been configured
* @throws ConfigurationException if the property is not a valid scalar string
*/
default String getString(String name, String defaultValue) {
return defaultIfNull(getString(name), defaultValue);
}

/**
* Returns a {@link Boolean} configuration property. Implementations should use the same rules as
* {@link Boolean#parseBoolean(String)} for handling the values.
*
* @return null if the property has not been configured
* @throws ConfigurationException if the property is not a valid scalar boolean
*/
@Nullable
Boolean getBoolean(String name);

/**
* Returns a {@link Boolean} configuration property.
*
* @return a {@link Boolean} configuration property or {@code defaultValue} if a property with
* {@code name} has not been configured
* @throws ConfigurationException if the property is not a valid scalar boolean
*/
default boolean getBoolean(String name, boolean defaultValue) {
return defaultIfNull(getBoolean(name), defaultValue);
}

/**
* Returns a {@link Integer} configuration property.
*
* <p>If the underlying config property is {@link Long}, it is converted to {@link Integer} with
* {@link Long#intValue()} which may result in loss of precision.
*
* @return null if the property has not been configured
* @throws ConfigurationException if the property is not a valid scalar integer
*/
@Nullable
Integer getInt(String name);

/**
* Returns a {@link Integer} configuration property.
*
* <p>If the underlying config property is {@link Long}, it is converted to {@link Integer} with
* {@link Long#intValue()} which may result in loss of precision.
*
* @return a {@link Integer} configuration property or {@code defaultValue} if a property with
* {@code name} has not been configured
* @throws ConfigurationException if the property is not a valid scalar integer
*/
default int getInt(String name, int defaultValue) {
return defaultIfNull(getInt(name), defaultValue);
}

/**
* Returns a {@link Long} configuration property.
*
* @return null if the property has not been configured
* @throws ConfigurationException if the property is not a valid scalar long
*/
@Nullable
Long getLong(String name);

/**
* Returns a {@link Long} configuration property.
*
* @return a {@link Long} configuration property or {@code defaultValue} if a property with {@code
* name} has not been configured
* @throws ConfigurationException if the property is not a valid scalar long
*/
default long getLong(String name, long defaultValue) {
return defaultIfNull(getLong(name), defaultValue);
}

/**
* Returns a {@link Double} configuration property.
*
* @return null if the property has not been configured
* @throws ConfigurationException if the property is not a valid scalar double
*/
@Nullable
Double getDouble(String name);

/**
* Returns a {@link Double} configuration property.
*
* @return a {@link Double} configuration property or {@code defaultValue} if a property with
* {@code name} has not been configured
* @throws ConfigurationException if the property is not a valid scalar double
*/
default double getDouble(String name, double defaultValue) {
return defaultIfNull(getDouble(name), defaultValue);
}

/**
* Returns a {@link List} configuration property. Empty values and values which do not map to the
* {@code scalarType} will be removed.
*
* @param name the property name
* @param scalarType the scalar type, one of {@link String}, {@link Boolean}, {@link Long} or
* {@link Double}
* @return a {@link List} configuration property, or null if the property has not been configured
* @throws ConfigurationException if the property is not a valid sequence of scalars, or if {@code
* scalarType} is not supported
*/
@Nullable
<T> List<T> getScalarList(String name, Class<T> scalarType);

/**
* Returns a {@link List} configuration property. Entries which are not strings are converted to
* their string representation.
*
* @see ConfigProperties#getList(String name)
* @return a {@link List} configuration property or {@code defaultValue} if a property with {@code
* name} has not been configured
* @throws ConfigurationException if the property is not a valid sequence of scalars
*/
default <T> List<T> getScalarList(String name, Class<T> scalarType, List<T> defaultValue) {
return defaultIfNull(getScalarList(name, scalarType), defaultValue);
}

/**
* Returns a {@link StructuredConfigProperties} configuration property.
*
* @return a map-valued configuration property, or {@code null} if {@code name} has not been
* configured
* @throws ConfigurationException if the property is not a mapping
*/
@Nullable
StructuredConfigProperties getStructured(String name);

/**
* Returns a list of {@link StructuredConfigProperties} configuration property.
*
* @return a list of map-valued configuration property, or {@code null} if {@code name} has not
* been configured
* @throws ConfigurationException if the property is not a sequence of mappings
*/
@Nullable
List<StructuredConfigProperties> getStructuredList(String name);
Copy link
Member Author

Choose a reason for hiding this comment

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

getStructured and getStructuredList are the key bits which allow reading complex data.


/**
* Returns a set of all configuration property keys.
*
* @return the configuration property keys
*/
Set<String> getPropertyKeys();
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.internal.StructuredConfigProperties;
import io.opentelemetry.sdk.resources.Resource;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;

/**
Expand Down Expand Up @@ -43,8 +45,12 @@ public static AutoConfiguredOpenTelemetrySdkBuilder builder() {
}

static AutoConfiguredOpenTelemetrySdk create(
OpenTelemetrySdk sdk, Resource resource, ConfigProperties config) {
return new AutoValue_AutoConfiguredOpenTelemetrySdk(sdk, resource, config);
OpenTelemetrySdk sdk,
Resource resource,
@Nullable ConfigProperties config,
@Nullable StructuredConfigProperties structuredConfigProperties) {
return new AutoValue_AutoConfiguredOpenTelemetrySdk(
sdk, resource, config, structuredConfigProperties);
}

/**
Expand All @@ -60,8 +66,23 @@ static AutoConfiguredOpenTelemetrySdk create(
/** Returns the {@link Resource} that was auto-configured. */
abstract Resource getResource();

/** Returns the {@link ConfigProperties} used for auto-configuration. */
/**
* Returns the {@link ConfigProperties} used for auto-configuration, or {@code null} if file
* configuration was used.
*
* @see #getStructuredConfig()
*/
@Nullable
abstract ConfigProperties getConfig();

/**
* Returns the {@link StructuredConfigProperties} used for auto-configuration, or {@code null} if
* file configuration was not used.
*
* @see #getConfig()
*/
@Nullable
abstract StructuredConfigProperties getStructuredConfig();
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 new method allows callers to determine if autoconfiguration was driven by file configuration or by env vars / system properties. Either this or getConfig will return null. The other will be non-null.

If file configuration is used, getStructuredConfig represents the full YAML config file.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is kinda the only unfortunate downside of this approach that I can find thus far. It's a small drag that users of the autoconfigured sdk now have to be aware or otherwise check which config was used to get the data they need.

It feels like there is room to flatten the structured config into ConfigProperties and also to objectify the ConfigProperties into a structured thing...but we can have that conversation another time.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a small drag that users of the autoconfigured sdk now have to be aware or otherwise check which config was used to get the data they need.

I would characterize it slightly differently: Providers of custom components like exporters and samplers need to be aware that somebody can use file based config or flat system property / env var based config, and provide different SPI implementations for each. This is frustrating, but seems to be the inevitable result of otel getting as far as it did without a proper configuration story. As I mentioned in the PR description, I started with the opposite approach and tried to have a single unified ConfigProperties for both configuration routes. I've concluded while this is simpler from an API standpoint, its more confusing and brittle for users, since even though we can do some gymnastics to make ConfigProperties work for both cases, the data presented to SPI implementers will be different (i.e. the property names and structure and semantics will be different for flat env vars than in structured config). So there appears to be no getting around confronting the user with this difference. Might as well make it as explicit and obvious as possible.


AutoConfiguredOpenTelemetrySdk() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.opentelemetry.sdk.autoconfigure.spi.ConfigurationException;
import io.opentelemetry.sdk.autoconfigure.spi.internal.AutoConfigureListener;
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.internal.StructuredConfigProperties;
import io.opentelemetry.sdk.logs.LogRecordProcessor;
import io.opentelemetry.sdk.logs.SdkLoggerProvider;
import io.opentelemetry.sdk.logs.SdkLoggerProviderBuilder;
Expand Down Expand Up @@ -505,7 +506,7 @@ public AutoConfiguredOpenTelemetrySdk build() {
maybeSetAsGlobal(openTelemetrySdk);
callAutoConfigureListeners(spiHelper, openTelemetrySdk);

return AutoConfiguredOpenTelemetrySdk.create(openTelemetrySdk, resource, config);
return AutoConfiguredOpenTelemetrySdk.create(openTelemetrySdk, resource, config, null);
} catch (RuntimeException e) {
logger.info(
"Error encountered during autoconfiguration. Closing partially configured components.");
Expand Down Expand Up @@ -546,11 +547,21 @@ private static AutoConfiguredOpenTelemetrySdk maybeConfigureFromFile(ConfigPrope
try {
Class<?> configurationFactory =
Class.forName("io.opentelemetry.sdk.extension.incubator.fileconfig.FileConfiguration");
Method parseAndCreate = configurationFactory.getMethod("parseAndCreate", InputStream.class);
OpenTelemetrySdk sdk = (OpenTelemetrySdk) parseAndCreate.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 create = configurationFactory.getMethod("create", openTelemetryConfiguration);
OpenTelemetrySdk sdk = (OpenTelemetrySdk) create.invoke(null, model);
Method toConfigProperties =
configurationFactory.getMethod("toConfigProperties", openTelemetryConfiguration);
StructuredConfigProperties structuredConfigProperties =
(StructuredConfigProperties) 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(), null, structuredConfigProperties);
} catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException e) {
throw new ConfigurationException(
"Error configuring from file. Is opentelemetry-sdk-extension-incubator on the classpath?",
Expand Down
Loading
Loading