Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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 @@ -5,21 +5,14 @@

package io.opentelemetry.instrumentation.api.incubator.config.internal;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.incubator.config.ConfigProvider;
import io.opentelemetry.api.incubator.config.InstrumentationConfigUtil;
import io.opentelemetry.instrumentation.api.incubator.log.LoggingContextConstants;
import io.opentelemetry.instrumentation.api.incubator.semconv.net.PeerServiceResolver;
import io.opentelemetry.instrumentation.api.internal.HttpConstants;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.function.Supplier;
import javax.annotation.Nullable;

/**
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
Expand All @@ -43,74 +36,6 @@ public final class CommonConfig {
private final String loggingSpanIdKey;
private final String loggingTraceFlagsKey;

interface ValueProvider<T> {
@Nullable
T get(ConfigProvider configProvider);
}

public CommonConfig(InstrumentationConfig config) {
peerServiceResolver =
PeerServiceResolver.create(
getFromConfigProviderOrFallback(
config,
InstrumentationConfigUtil::peerServiceMapping,
emptyMap(),
() ->
config.getMap("otel.instrumentation.common.peer-service-mapping", emptyMap())));

clientRequestHeaders =
getFromConfigProviderOrFallback(
config,
InstrumentationConfigUtil::httpClientRequestCapturedHeaders,
emptyList(),
() -> config.getList("otel.instrumentation.http.client.capture-request-headers"));
clientResponseHeaders =
getFromConfigProviderOrFallback(
config,
InstrumentationConfigUtil::httpClientResponseCapturedHeaders,
emptyList(),
() -> config.getList("otel.instrumentation.http.client.capture-response-headers"));
serverRequestHeaders =
getFromConfigProviderOrFallback(
config,
InstrumentationConfigUtil::httpServerRequestCapturedHeaders,
emptyList(),
() -> config.getList("otel.instrumentation.http.server.capture-request-headers"));
serverResponseHeaders =
getFromConfigProviderOrFallback(
config,
InstrumentationConfigUtil::httpServerResponseCapturedHeaders,
emptyList(),
() -> config.getList("otel.instrumentation.http.server.capture-response-headers"));
knownHttpRequestMethods =
new HashSet<>(
config.getList(
"otel.instrumentation.http.known-methods",
new ArrayList<>(HttpConstants.KNOWN_METHODS)));
statementSanitizationEnabled =
config.getBoolean("otel.instrumentation.common.db-statement-sanitizer.enabled", true);
sqlCommenterEnabled =
config.getBoolean(
"otel.instrumentation.common.experimental.db-sqlcommenter.enabled", false);
emitExperimentalHttpClientTelemetry =
config.getBoolean("otel.instrumentation.http.client.emit-experimental-telemetry", false);
redactQueryParameters =
config.getBoolean(
"otel.instrumentation.http.client.experimental.redact-query-parameters", true);
emitExperimentalHttpServerTelemetry =
config.getBoolean("otel.instrumentation.http.server.emit-experimental-telemetry", false);
enduserConfig = new EnduserConfig(config);
loggingTraceIdKey =
config.getString(
"otel.instrumentation.common.logging.trace-id", LoggingContextConstants.TRACE_ID);
loggingSpanIdKey =
config.getString(
"otel.instrumentation.common.logging.span-id", LoggingContextConstants.SPAN_ID);
loggingTraceFlagsKey =
config.getString(
"otel.instrumentation.common.logging.trace-flags", LoggingContextConstants.TRACE_FLAGS);
}

public CommonConfig(OpenTelemetry openTelemetry) {
ExtendedDeclarativeConfigProperties generalConfig =
DeclarativeConfigUtil.getGeneralInstrumentationConfig(openTelemetry);
Expand Down Expand Up @@ -231,18 +156,4 @@ public String getSpanIdKey() {
public String getTraceFlagsKey() {
return loggingTraceFlagsKey;
}

private static <T> T getFromConfigProviderOrFallback(
InstrumentationConfig config,
ValueProvider<T> getFromConfigProvider,
T defaultValue,
Supplier<T> fallback) {
ConfigProvider configProvider = config.getConfigProvider();
if (configProvider != null) {
T value = getFromConfigProvider.get(configProvider);
return value != null ? value : defaultValue;
}
// fallback doesn't return null, so we can safely call it
return fallback.get();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,6 @@ public class EnduserConfig {
private final boolean roleEnabled;
private final boolean scopeEnabled;

EnduserConfig(InstrumentationConfig instrumentationConfig) {
Objects.requireNonNull(instrumentationConfig, "instrumentationConfig must not be null");

/*
* Capturing enduser.* attributes is disabled by default, because of this requirement in the specification:
*
* Given the sensitive nature of this information, SDKs and exporters SHOULD drop these attributes by default and then provide a configuration parameter to turn on retention for use cases where the information is required and would not violate any policies or regulations.
*
* https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/attributes.md#general-identity-attributes
*/
this.idEnabled =
instrumentationConfig.getBoolean("otel.instrumentation.common.enduser.id.enabled", false);
this.roleEnabled =
instrumentationConfig.getBoolean("otel.instrumentation.common.enduser.role.enabled", false);
this.scopeEnabled =
instrumentationConfig.getBoolean(
"otel.instrumentation.common.enduser.scope.enabled", false);
}

EnduserConfig(ExtendedDeclarativeConfigProperties commonConfig) {
Objects.requireNonNull(commonConfig, "commonConfig must not be null");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ dependencies {

testImplementation("io.opentelemetry:opentelemetry-sdk")
testImplementation("io.opentelemetry:opentelemetry-sdk-testing")
testImplementation("io.opentelemetry:opentelemetry-api-incubator")
testImplementation(project(":instrumentation:resources:library"))
testImplementation("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi")
testImplementation("io.opentelemetry:opentelemetry-extension-trace-propagators")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,24 @@ class EmbeddedConfigFile {
private EmbeddedConfigFile() {}

static OpenTelemetryConfigurationModel extractModel(ConfigurableEnvironment environment) {
Map<String, String> props = extractSpringProperties(environment);
Map<String, Object> props = extractSpringProperties(environment);
return convertToOpenTelemetryConfigurationModel(props);
}

private static Map<String, String> extractSpringProperties(ConfigurableEnvironment environment) {
private static Map<String, Object> extractSpringProperties(ConfigurableEnvironment environment) {
MutablePropertySources propertySources = environment.getPropertySources();

Map<String, String> props = new HashMap<>();
Map<String, Object> props = new HashMap<>();
for (PropertySource<?> propertySource : propertySources) {
if (propertySource instanceof EnumerablePropertySource<?>) {
for (String propertyName :
((EnumerablePropertySource<?>) propertySource).getPropertyNames()) {
if (propertyName.startsWith("otel.")) {
String property = environment.getProperty(propertyName);
Object property = propertySource.getProperty(propertyName);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This change preserves boolean values across the bridge

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this needed for this PR?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah. the reason it worked before is that all of the spring boot starter config access was done via ConfigProperties which is string based anyways

and I've updated some places in this PR to now access config via Declarative Config API (which ignores properties if they're not the requested type instead of converting them)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

spring was using type coercion before - because I thought it was necessary.
I'll play around with this PR a little more to get a better picture.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I figured it out: the wrong config provider was used: trask#101

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

// Resolve ${} placeholders in String values while preserving types for others
if (property instanceof String) {
property = environment.resolvePlaceholders((String) property);
}
if (Objects.equals(property, "")) {
property = null; // spring returns empty string for yaml null
}
Expand All @@ -75,6 +79,7 @@ private static Map<String, String> extractSpringProperties(ConfigurableEnvironme
.replace("]", "")
.replace(".", "_")
.toUpperCase(Locale.ROOT);
// Use environment.getProperty() to search all property sources for the env var
String envVarValue = environment.getProperty(envVarName);
if (envVarValue != null) {
property = envVarValue;
Expand All @@ -96,7 +101,7 @@ private static Map<String, String> extractSpringProperties(ConfigurableEnvironme
}

static OpenTelemetryConfigurationModel convertToOpenTelemetryConfigurationModel(
Map<String, String> flatProps) {
Map<String, Object> flatProps) {
Map<String, Object> nested = convertFlatPropsToNested(flatProps);

return getObjectMapper().convertValue(nested, OpenTelemetryConfigurationModel.class);
Expand All @@ -112,12 +117,12 @@ static ObjectMapper getObjectMapper() {
* ["one", "two"]}}}}
*/
@SuppressWarnings("unchecked")
static Map<String, Object> convertFlatPropsToNested(Map<String, String> flatProps) {
static Map<String, Object> convertFlatPropsToNested(Map<String, Object> flatProps) {
Map<String, Object> result = new HashMap<>();

for (Map.Entry<String, String> entry : flatProps.entrySet()) {
for (Map.Entry<String, Object> entry : flatProps.entrySet()) {
String key = entry.getKey();
String value = entry.getValue();
Object value = entry.getValue();

// Split the key by dots
String[] parts = key.split("\\.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import io.opentelemetry.common.ComponentLoader;
import io.opentelemetry.instrumentation.api.incubator.config.internal.InstrumentationConfig;
import io.opentelemetry.instrumentation.api.internal.EmbeddedInstrumentationProperties;
import io.opentelemetry.instrumentation.config.bridge.ConfigPropertiesBackedConfigProvider;
import io.opentelemetry.instrumentation.config.bridge.DeclarativeConfigPropertiesBridgeBuilder;
import io.opentelemetry.instrumentation.spring.autoconfigure.internal.DeclarativeConfigDisabled;
import io.opentelemetry.instrumentation.spring.autoconfigure.internal.DeclarativeConfigEnabled;
Expand Down Expand Up @@ -125,9 +126,12 @@ public AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk(

@Bean
public OpenTelemetry openTelemetry(
AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk) {
AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk,
ConfigProperties otelProperties) {
logStart();
return autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk();
OpenTelemetrySdk openTelemetry = autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk();
ConfigProvider configProvider = ConfigPropertiesBackedConfigProvider.create(otelProperties);
return new SpringOpenTelemetrySdk(openTelemetry, configProvider);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

A test failed when I just implemented OpenTelemetry instead of OpenTelemetrySdk

  • OpenTelemetryAutoConfigurationTest.shouldInitializeSdkWhenNotDisabled

I guess it could be considered a breaking change if anyone is relying on it really being an instance of OpenTelemetrySdk, so went with OpenTelemetrySdk instead.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the test was just meant to assert that it's not the noop instance

}

@Bean
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.spring.autoconfigure;

import io.opentelemetry.api.incubator.ExtendedOpenTelemetry;
import io.opentelemetry.api.incubator.config.ConfigProvider;
import io.opentelemetry.sdk.OpenTelemetrySdk;

final class SpringOpenTelemetrySdk extends OpenTelemetrySdk implements ExtendedOpenTelemetry {

private final ConfigProvider configProvider;

SpringOpenTelemetrySdk(OpenTelemetrySdk delegate, ConfigProvider configProvider) {
super(
delegate.getSdkTracerProvider(),
delegate.getSdkMeterProvider(),
delegate.getSdkLoggerProvider(),
delegate.getPropagators());
this.configProvider = configProvider;
}

@Override
public ConfigProvider getConfigProvider() {
return configProvider;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.incubator.config.internal.InstrumentationConfig;
import io.opentelemetry.instrumentation.api.incubator.config.internal.DeclarativeConfigUtil;
import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor;
import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesGetter;
import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeSpanNameExtractor;
Expand Down Expand Up @@ -42,8 +42,7 @@ final class SpringSchedulingInstrumentationAspect {
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.spring-boot-autoconfigure";
private final Instrumenter<ClassAndMethod, Object> instrumenter;

public SpringSchedulingInstrumentationAspect(
OpenTelemetry openTelemetry, InstrumentationConfig config) {
public SpringSchedulingInstrumentationAspect(OpenTelemetry openTelemetry) {
CodeAttributesGetter<ClassAndMethod> codedAttributesGetter =
ClassAndMethod.codeAttributesGetter();
InstrumenterBuilder<ClassAndMethod, Object> builder =
Expand All @@ -52,8 +51,8 @@ public SpringSchedulingInstrumentationAspect(
INSTRUMENTATION_NAME,
CodeSpanNameExtractor.create(codedAttributesGetter))
.addAttributesExtractor(CodeAttributesExtractor.create(codedAttributesGetter));
if (config.getBoolean(
"otel.instrumentation.spring-scheduling.experimental-span-attributes", false)) {
if (DeclarativeConfigUtil.getInstrumentationConfig(openTelemetry, "spring_scheduling")
.getBoolean("experimental-span-attributes/development", false)) {
builder.addAttributesExtractor(
AttributesExtractor.constant(AttributeKey.stringKey("job.system"), "spring_scheduling"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.scheduling;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.api.incubator.config.internal.InstrumentationConfig;
import io.opentelemetry.instrumentation.spring.autoconfigure.internal.ConditionalOnEnabledInstrumentation;
import org.aspectj.lang.annotation.Aspect;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
Expand All @@ -26,7 +25,7 @@
class SpringSchedulingInstrumentationAutoConfiguration {
@Bean
SpringSchedulingInstrumentationAspect springSchedulingInstrumentationAspect(
OpenTelemetry openTelemetry, InstrumentationConfig config) {
return new SpringSchedulingInstrumentationAspect(openTelemetry, config);
OpenTelemetry openTelemetry) {
return new SpringSchedulingInstrumentationAspect(openTelemetry);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ static RestTemplate addIfNotPresent(

ClientHttpRequestInterceptor instrumentationInterceptor =
InstrumentationConfigUtil.configureClientBuilder(
config,
openTelemetry,
SpringWebTelemetry.builder(openTelemetry),
WebTelemetryUtil.getBuilderExtractor())
.build()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.webflux;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.api.incubator.config.internal.InstrumentationConfig;
import io.opentelemetry.instrumentation.spring.autoconfigure.internal.ConditionalOnEnabledInstrumentation;
import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
Expand All @@ -33,14 +32,13 @@ public SpringWebfluxInstrumentationAutoConfiguration() {}
// static to avoid "is not eligible for getting processed by all BeanPostProcessors" warning
@Bean
static WebClientBeanPostProcessor otelWebClientBeanPostProcessor(
ObjectProvider<OpenTelemetry> openTelemetryProvider,
ObjectProvider<InstrumentationConfig> configProvider) {
return new WebClientBeanPostProcessor(openTelemetryProvider, configProvider);
ObjectProvider<OpenTelemetry> openTelemetryProvider) {
return new WebClientBeanPostProcessor(openTelemetryProvider);
}

@Bean
WebFilter telemetryFilter(OpenTelemetry openTelemetry, InstrumentationConfig config) {
return WebClientBeanPostProcessor.getWebfluxServerTelemetry(openTelemetry, config)
WebFilter telemetryFilter(OpenTelemetry openTelemetry) {
return WebClientBeanPostProcessor.getWebfluxServerTelemetry(openTelemetry)
.createWebFilterAndRegisterReactorHook();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.webflux;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.api.incubator.config.internal.InstrumentationConfig;
import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.InstrumentationConfigUtil;
import io.opentelemetry.instrumentation.spring.webflux.v5_3.SpringWebfluxClientTelemetry;
import io.opentelemetry.instrumentation.spring.webflux.v5_3.SpringWebfluxServerTelemetry;
Expand All @@ -23,28 +22,22 @@
final class WebClientBeanPostProcessor implements BeanPostProcessor {

private final ObjectProvider<OpenTelemetry> openTelemetryProvider;
private final ObjectProvider<InstrumentationConfig> configProvider;

WebClientBeanPostProcessor(
ObjectProvider<OpenTelemetry> openTelemetryProvider,
ObjectProvider<InstrumentationConfig> configProvider) {
WebClientBeanPostProcessor(ObjectProvider<OpenTelemetry> openTelemetryProvider) {
this.openTelemetryProvider = openTelemetryProvider;
this.configProvider = configProvider;
}

static SpringWebfluxClientTelemetry getWebfluxClientTelemetry(
OpenTelemetry openTelemetry, InstrumentationConfig config) {
static SpringWebfluxClientTelemetry getWebfluxClientTelemetry(OpenTelemetry openTelemetry) {
return InstrumentationConfigUtil.configureClientBuilder(
config,
openTelemetry,
SpringWebfluxClientTelemetry.builder(openTelemetry),
SpringWebfluxBuilderUtil.getClientBuilderExtractor())
.build();
}

static SpringWebfluxServerTelemetry getWebfluxServerTelemetry(
OpenTelemetry openTelemetry, InstrumentationConfig config) {
static SpringWebfluxServerTelemetry getWebfluxServerTelemetry(OpenTelemetry openTelemetry) {
return InstrumentationConfigUtil.configureServerBuilder(
config,
openTelemetry,
SpringWebfluxServerTelemetry.builder(openTelemetry),
SpringWebfluxBuilderUtil.getServerBuilderExtractor())
.build();
Expand All @@ -64,7 +57,7 @@ public Object postProcessAfterInitialization(Object bean, String beanName) {

private WebClient.Builder wrapBuilder(WebClient.Builder webClientBuilder) {
SpringWebfluxClientTelemetry instrumentation =
getWebfluxClientTelemetry(openTelemetryProvider.getObject(), configProvider.getObject());
getWebfluxClientTelemetry(openTelemetryProvider.getObject());
return webClientBuilder.filters(instrumentation::addFilter);
}
}
Loading
Loading