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

Remove deprecated spring properties #10454

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ dependencies {
library("org.springframework.boot:spring-boot-starter-web:$springBootVersion")
library("org.springframework.boot:spring-boot-starter-webflux:$springBootVersion")

compileOnly("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi")
implementation("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure")
Copy link
Member

Choose a reason for hiding this comment

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

It should not be library("io.opentelemetry:opentelemetry-sdk-extension-autoconfigure"), as the Spring dependencies? cc @laurit who should know

Copy link
Contributor

Choose a reason for hiding this comment

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

library means compileOnly + testImplementation. Using compileOnly is important in javaagent modules, which this module is not. It is also used to support the latest dep test where we change the version for library dependencies to latest version. In my opinion here implementation is the correct choice as it is a required dependency for this module.

compileOnly("io.opentelemetry:opentelemetry-extension-annotations")
compileOnly("io.opentelemetry:opentelemetry-extension-trace-propagators")
compileOnly("io.opentelemetry.contrib:opentelemetry-aws-xray-propagator")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import io.opentelemetry.instrumentation.spring.autoconfigure.internal.MapConverter;
import io.opentelemetry.instrumentation.spring.autoconfigure.propagators.PropagationProperties;
import io.opentelemetry.instrumentation.spring.autoconfigure.resources.OtelResourceAutoConfiguration;
import io.opentelemetry.instrumentation.spring.autoconfigure.resources.OtelResourceProperties;
import io.opentelemetry.instrumentation.spring.autoconfigure.resources.SpringConfigProperties;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
Expand Down Expand Up @@ -61,6 +62,7 @@
@EnableConfigurationProperties({
SamplerProperties.class,
OtlpExporterProperties.class,
OtelResourceProperties.class,
PropagationProperties.class
})
public class OpenTelemetryAutoConfiguration {
Expand Down Expand Up @@ -103,9 +105,14 @@ static class Metric {}
ConfigProperties configProperties(
Environment env,
OtlpExporterProperties otlpExporterProperties,
OtelResourceProperties resourceProperties,
PropagationProperties propagationProperties) {
return new SpringConfigProperties(
env, new SpelExpressionParser(), otlpExporterProperties, propagationProperties);
env,
new SpelExpressionParser(),
otlpExporterProperties,
resourceProperties,
propagationProperties);
}

@Bean(destroyMethod = "") // SDK components are shutdown from the OpenTelemetry instance
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package io.opentelemetry.instrumentation.spring.autoconfigure.exporters.internal;

import java.util.Arrays;
import javax.annotation.Nullable;
import org.springframework.core.env.Environment;

/**
Expand All @@ -18,28 +17,13 @@ public final class ExporterConfigEvaluator {
private ExporterConfigEvaluator() {}

public static boolean isExporterEnabled(
Environment environment,
@Nullable String oldAllKey,
String oldKey,
String exportersKey,
String wantExporter,
boolean defaultValue) {
Environment environment, String exportersKey, String wantExporter, boolean defaultValue) {

String exporter = environment.getProperty(exportersKey);
if (exporter != null) {
return Arrays.asList(exporter.split(",")).contains(wantExporter);
}

String old = environment.getProperty(oldKey);
if (old != null) {
return "true".equals(old);
}
if (oldAllKey != null) {
String oldAll = environment.getProperty(oldAllKey);
if (oldAll != null) {
return "true".equals(oldAll);
}
}
return defaultValue;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,7 @@ static final class CustomCondition implements Condition {
@Override
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
return ExporterConfigEvaluator.isExporterEnabled(
context.getEnvironment(),
"otel.exporter.logging.enabled",
"otel.exporter.logging.metrics.enabled",
"otel.metrics.exporter",
"logging",
false);
context.getEnvironment(), "otel.metrics.exporter", "logging", false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,7 @@ static final class CustomCondition implements Condition {
@Override
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
return ExporterConfigEvaluator.isExporterEnabled(
context.getEnvironment(),
"otel.exporter.logging.enabled",
"otel.exporter.logging.traces.enabled",
"otel.traces.exporter",
"logging",
false);
context.getEnvironment(), "otel.traces.exporter", "logging", false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,7 @@ static final class CustomCondition implements Condition {
@Override
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
return ExporterConfigEvaluator.isExporterEnabled(
context.getEnvironment(),
"otel.exporter.logging.enabled",
"otel.exporter.logging.logs.enabled",
"otel.logs.exporter",
"logging",
false);
context.getEnvironment(), "otel.logs.exporter", "logging", false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,7 @@ static final class CustomCondition implements Condition {
@Override
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
return ExporterConfigEvaluator.isExporterEnabled(
context.getEnvironment(),
"otel.exporter.otlp.enabled",
"otel.exporter.otlp.logs.enabled",
"otel.logs.exporter",
"otlp",
true);
context.getEnvironment(), "otel.logs.exporter", "otlp", true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,7 @@ static final class CustomCondition implements Condition {
@Override
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
return ExporterConfigEvaluator.isExporterEnabled(
context.getEnvironment(),
"otel.exporter.otlp.enabled",
"otel.exporter.otlp.metrics.enabled",
"otel.metrics.exporter",
"otlp",
true);
context.getEnvironment(), "otel.metrics.exporter", "otlp", true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,7 @@ static final class CustomCondition implements Condition {
@Override
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
return ExporterConfigEvaluator.isExporterEnabled(
context.getEnvironment(),
"otel.exporter.otlp.enabled",
"otel.exporter.otlp.traces.enabled",
"otel.traces.exporter",
"otlp",
true);
context.getEnvironment(), "otel.traces.exporter", "otlp", true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,7 @@ static final class CustomCondition implements Condition {
@Override
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
return ExporterConfigEvaluator.isExporterEnabled(
context.getEnvironment(),
null,
"otel.exporter.zipkin.enabled",
"otel.traces.exporter",
"zipkin",
true);
context.getEnvironment(), "otel.traces.exporter", "zipkin", true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public final class CompositeTextMapPropagatorFactory {
private static final Logger logger =
Logger.getLogger(CompositeTextMapPropagatorFactory.class.getName());

@SuppressWarnings("deprecation") // deprecated class to be updated once published in new location
static TextMapPropagator getCompositeTextMapPropagator(
BeanFactory beanFactory, List<String> types) {

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import org.springframework.beans.factory.BeanFactory;
Expand All @@ -22,12 +23,13 @@

/** Configures {@link ContextPropagators} bean for propagation. */
@Configuration
@EnableConfigurationProperties(DeprecatedPropagationProperties.class)
@EnableConfigurationProperties(PropagationProperties.class)
@AutoConfigureBefore(OpenTelemetryAutoConfiguration.class)
@ConditionalOnProperty(prefix = "otel.propagation", name = "enabled", matchIfMissing = true)
@SuppressWarnings("deprecation")
public class PropagationAutoConfiguration {

private static final List<String> DEFAULT_PROPAGATORS = Arrays.asList("tracecontext", "baggage");

@Bean
@ConditionalOnMissingBean
ContextPropagators contextPropagators(ObjectProvider<List<TextMapPropagator>> propagators) {
Expand All @@ -43,11 +45,9 @@ static class PropagatorsConfiguration {

@Bean
TextMapPropagator compositeTextMapPropagator(
BeanFactory beanFactory,
DeprecatedPropagationProperties properties,
ConfigProperties configProperties) {
BeanFactory beanFactory, ConfigProperties configProperties) {
return CompositeTextMapPropagatorFactory.getCompositeTextMapPropagator(
beanFactory, configProperties.getList("otel.propagators", properties.getType()));
beanFactory, configProperties.getList("otel.propagators", DEFAULT_PROPAGATORS));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.opentelemetry.instrumentation.resources.ProcessRuntimeResource;
import io.opentelemetry.instrumentation.resources.ProcessRuntimeResourceProvider;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
import io.opentelemetry.sdk.autoconfigure.internal.EnvironmentResourceProvider;
import io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider;
import org.springframework.boot.autoconfigure.AutoConfigureBefore;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
Expand All @@ -25,16 +26,19 @@
import org.springframework.context.annotation.Configuration;

@Configuration
@EnableConfigurationProperties({OtelSpringResourceProperties.class, OtelResourceProperties.class})
@EnableConfigurationProperties({OtelResourceProperties.class})
@AutoConfigureBefore(OpenTelemetryAutoConfiguration.class)
@ConditionalOnProperty(prefix = "otel.springboot.resource", name = "enabled", matchIfMissing = true)
public class OtelResourceAutoConfiguration {

@Bean
public ResourceProvider otelResourceProvider(
OtelSpringResourceProperties otelSpringResourceProperties,
OtelResourceProperties otelResourceProperties) {
return new SpringResourceProvider(otelSpringResourceProperties, otelResourceProperties);
public ResourceProvider otelEnvironmentResourceProvider() {
return new EnvironmentResourceProvider();
}

@Bean
public ResourceProvider otelSpringResourceProvider() {
return new SpringResourceProvider();
}

@Bean
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,19 @@ public class SpringConfigProperties implements ConfigProperties {

private final ExpressionParser parser;
private final OtlpExporterProperties otlpExporterProperties;
private final OtelResourceProperties resourceProperties;
private final PropagationProperties propagationProperties;

public SpringConfigProperties(
Environment environment,
ExpressionParser parser,
OtlpExporterProperties otlpExporterProperties,
OtelResourceProperties resourceProperties,
PropagationProperties propagationProperties) {
this.environment = environment;
this.parser = parser;
this.otlpExporterProperties = otlpExporterProperties;
this.resourceProperties = resourceProperties;
this.propagationProperties = propagationProperties;
}

Expand Down Expand Up @@ -78,6 +81,7 @@ public List<String> getList(String name) {
if (name.equals("otel.propagators")) {
return propagationProperties.getPropagators();
}

List<String> value = environment.getProperty(name, List.class);
return value == null ? Collections.emptyList() : value;
}
Expand All @@ -98,6 +102,8 @@ public Duration getDuration(String name) {
public Map<String, String> getMap(String name) {
// maps from config properties are not supported by Environment, so we have to fake it
switch (name) {
case "otel.resource.attributes":
return resourceProperties.getAttributes();
case "otel.exporter.otlp.headers":
return otlpExporterProperties.getHeaders();
case "otel.exporter.otlp.logs.headers":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,13 @@

public class SpringResourceProvider implements ResourceProvider {

private final OtelSpringResourceProperties otelSpringResourceProperties;
private final OtelResourceProperties otelResourceProperties;

public SpringResourceProvider(
OtelSpringResourceProperties otelSpringResourceProperties,
OtelResourceProperties otelResourceProperties) {
this.otelSpringResourceProperties = otelSpringResourceProperties;
this.otelResourceProperties = otelResourceProperties;
}

@Override
public Resource createResource(ConfigProperties configProperties) {
AttributesBuilder attributesBuilder = Attributes.builder();
String springApplicationName = configProperties.getString("spring.application.name");
if (springApplicationName != null) {
attributesBuilder.put(ResourceAttributes.SERVICE_NAME, springApplicationName);
}
otelSpringResourceProperties.getAttributes().forEach(attributesBuilder::put);
otelResourceProperties.getAttributes().forEach(attributesBuilder::put);
String applicationName = configProperties.getString("otel.service.name");
if (applicationName != null) {
attributesBuilder.put(ResourceAttributes.SERVICE_NAME, applicationName);
}
return Resource.create(attributesBuilder.build());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void shouldDetermineServiceNameByOtelServiceName() {
.withConfiguration(
AutoConfigurations.of(
OtelResourceAutoConfiguration.class, OpenTelemetryAutoConfiguration.class))
.withPropertyValues("otel.springboot.resource.attributes.service.name=otel-name-backend")
.withPropertyValues("otel.resource.attributes.service.name=otel-name-backend")
.run(
context -> {
Resource otelResource = context.getBean("otelResource", Resource.class);
Expand All @@ -157,9 +157,9 @@ void shouldInitializeAttributes() {
AutoConfigurations.of(
OtelResourceAutoConfiguration.class, OpenTelemetryAutoConfiguration.class))
.withPropertyValues(
"otel.springboot.resource.attributes.xyz=foo",
"otel.springboot.resource.attributes.environment=dev",
"otel.springboot.resource.attributes.service.instance.id=id-example")
"otel.resource.attributes.xyz=foo",
"otel.resource.attributes.environment=dev",
"otel.resource.attributes.service.instance.id=id-example")
.run(
context -> {
Resource otelResource = context.getBean("otelResource", Resource.class);
Expand Down
Loading
Loading