Skip to content

[Spring starter] Make sdk bean overridable and cache env calls #15132

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[Spring starter] Make sdk bean overridable and cache env calls #15132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d975b9
add bean override and cache for spring environment calls
jaydeluca Oct 24, 2025
39757e1
use helper methods
jaydeluca Oct 24, 2025
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
1 change: 1 addition & 0 deletions ...io/opentelemetry/instrumentation/spring/autoconfigure/OpenTelemetryAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public ResourceProvider otelDistroVersionResourceProvider() {
}

@Bean
@ConditionalOnMissingBean
public AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetrySdk(
Environment env,
OtlpExporterProperties otlpExporterProperties,
Expand Down
178 changes: 129 additions & 49 deletions ...etry/instrumentation/spring/autoconfigure/internal/properties/SpringConfigProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import javax.annotation.Nullable;
import org.springframework.core.env.Environment;
import org.springframework.expression.ExpressionParser;
Expand All @@ -34,6 +37,23 @@ public class SpringConfigProperties implements ConfigProperties {
private final ConfigProperties customizedListProperties;
private final Map<String, List<String>> listPropertyValues;

private final ConcurrentHashMap<String, Optional<String>> cachedStringValues =
new ConcurrentHashMap<>();
private final ConcurrentHashMap<String, Optional<Boolean>> cachedBooleanValues =
new ConcurrentHashMap<>();
private final ConcurrentHashMap<String, Optional<Integer>> cachedIntValues =
new ConcurrentHashMap<>();
private final ConcurrentHashMap<String, Optional<Long>> cachedLongValues =
new ConcurrentHashMap<>();
private final ConcurrentHashMap<String, Optional<Double>> cachedDoubleValues =
new ConcurrentHashMap<>();
private final ConcurrentHashMap<String, Optional<List<String>>> cachedListValues =
new ConcurrentHashMap<>();
private final ConcurrentHashMap<String, Optional<Duration>> cachedDurationValues =
new ConcurrentHashMap<>();
private final ConcurrentHashMap<String, Optional<Map<String, String>>> cachedMapValues =
new ConcurrentHashMap<>();

static final String DISABLED_KEY = "otel.java.disabled.resource.providers";
static final String ENABLED_KEY = "otel.java.enabled.resource.providers";

Expand Down Expand Up @@ -148,89 +168,155 @@ public static ConfigProperties create(
fallback);
}

@Nullable
private static <T> T getCachedValue(
ConcurrentHashMap<String, Optional<T>> cache,
String name,
Function<String, T> valueFunction) {
return cache
.computeIfAbsent(name, key -> Optional.ofNullable(valueFunction.apply(key)))
.orElse(null);
}

@Nullable
@Override
public String getString(String name) {
String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(name);
String value = environment.getProperty(normalizedName, String.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative approach that could be considered would be to wrap the environment into a caching PropertyResolver. I presume that the sdk deliberately rereads these properties so they could be updated with opamp in the future. Idk whether this is something we should be concerned with.

if (value == null && normalizedName.equals("otel.exporter.otlp.protocol")) {
// SDK autoconfigure module defaults to `grpc`, but this module aligns with recommendation
// in specification to default to `http/protobuf`
return OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF;
}
return or(value, otelSdkProperties.getString(name));
return getCachedValue(
cachedStringValues,
name,
key -> {
String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(key);
String value = environment.getProperty(normalizedName, String.class);
if (value == null && normalizedName.equals("otel.exporter.otlp.protocol")) {
// SDK autoconfigure module defaults to `grpc`, but this module aligns with
// recommendation in specification to default to `http/protobuf`
return OtlpConfigUtil.PROTOCOL_HTTP_PROTOBUF;
}
return or(value, otelSdkProperties.getString(key));
});
}

@Nullable
@Override
public Boolean getBoolean(String name) {
return or(
environment.getProperty(ConfigUtil.normalizeEnvironmentVariableKey(name), Boolean.class),
otelSdkProperties.getBoolean(name));
return getCachedValue(
cachedBooleanValues,
name,
key ->
or(
environment.getProperty(
ConfigUtil.normalizeEnvironmentVariableKey(key), Boolean.class),
otelSdkProperties.getBoolean(key)));
}

@Nullable
@Override
public Integer getInt(String name) {
return or(
environment.getProperty(ConfigUtil.normalizeEnvironmentVariableKey(name), Integer.class),
otelSdkProperties.getInt(name));
return getCachedValue(
cachedIntValues,
name,
key ->
or(
environment.getProperty(
ConfigUtil.normalizeEnvironmentVariableKey(key), Integer.class),
otelSdkProperties.getInt(key)));
}

@Nullable
@Override
public Long getLong(String name) {
return or(
environment.getProperty(ConfigUtil.normalizeEnvironmentVariableKey(name), Long.class),
otelSdkProperties.getLong(name));
return getCachedValue(
cachedLongValues,
name,
key ->
or(
environment.getProperty(
ConfigUtil.normalizeEnvironmentVariableKey(key), Long.class),
otelSdkProperties.getLong(key)));
}

@Nullable
@Override
public Double getDouble(String name) {
return or(
environment.getProperty(ConfigUtil.normalizeEnvironmentVariableKey(name), Double.class),
otelSdkProperties.getDouble(name));
return getCachedValue(
cachedDoubleValues,
name,
key ->
or(
environment.getProperty(
ConfigUtil.normalizeEnvironmentVariableKey(key), Double.class),
otelSdkProperties.getDouble(key)));
}

@SuppressWarnings("unchecked")
@Override
public List<String> getList(String name) {
return getCachedValue(
cachedListValues,
name,
key -> {
String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(key);

String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(name);

List<String> list = listPropertyValues.get(normalizedName);
if (list != null) {
List<String> c = customizedListProperties.getList(name);
if (!c.isEmpty()) {
return c;
}
if (!list.isEmpty()) {
return list;
}
}
List<String> list = listPropertyValues.get(normalizedName);
if (list != null) {
List<String> c = customizedListProperties.getList(key);
if (!c.isEmpty()) {
return c;
}
if (!list.isEmpty()) {
return list;
}
}

return or(environment.getProperty(normalizedName, List.class), otelSdkProperties.getList(name));
List<String> envValue =
(List<String>) environment.getProperty(normalizedName, List.class);
return or(envValue, otelSdkProperties.getList(key));
});
}

@Nullable
@Override
public Duration getDuration(String name) {
String value = getString(name);
if (value == null) {
return otelSdkProperties.getDuration(name);
}
return DefaultConfigProperties.createFromMap(Collections.singletonMap(name, value))
.getDuration(name);
return getCachedValue(
cachedDurationValues,
name,
key -> {
String value = getString(key);
if (value == null) {
return otelSdkProperties.getDuration(key);
}
return DefaultConfigProperties.createFromMap(Collections.singletonMap(key, value))
.getDuration(key);
});
}

@SuppressWarnings("unchecked")
@Override
public Map<String, String> getMap(String name) {
Map<String, String> otelSdkMap = otelSdkProperties.getMap(name);
return getCachedValue(
cachedMapValues,
name,
key -> {
Map<String, String> otelSdkMap = otelSdkProperties.getMap(key);

String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(key);
// maps from config properties are not supported by Environment, so we have to fake it
Map<String, String> specialMap = getSpecialMapProperty(normalizedName, otelSdkMap);
if (specialMap != null) {
return specialMap;
}

String value = environment.getProperty(normalizedName);
if (value == null) {
return otelSdkMap;
}
return (Map<String, String>) parser.parseExpression(value).getValue();
});
}

String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(name);
// maps from config properties are not supported by Environment, so we have to fake it
@Nullable
private Map<String, String> getSpecialMapProperty(
String normalizedName, Map<String, String> otelSdkMap) {
switch (normalizedName) {
case "otel.resource.attributes":
return mergeWithOtel(resourceProperties.getAttributes(), otelSdkMap);
Expand All @@ -243,14 +329,8 @@ public Map<String, String> getMap(String name) {
case "otel.exporter.otlp.traces.headers":
return mergeWithOtel(otlpExporterProperties.getTraces().getHeaders(), otelSdkMap);
default:
break;
}

String value = environment.getProperty(normalizedName);
if (value == null) {
return otelSdkMap;
return null;
}
return (Map<String, String>) parser.parseExpression(value).getValue();
}

/**
Expand Down
22 changes: 22 additions & 0 deletions ...pentelemetry/instrumentation/spring/autoconfigure/OpenTelemetryAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.exporter.otlp.internal.OtlpSpanExporterProvider;
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
import io.opentelemetry.sdk.autoconfigure.spi.internal.AutoConfigureListener;
import io.opentelemetry.sdk.trace.export.SpanExporter;
Expand Down Expand Up @@ -58,6 +59,27 @@ void customOpenTelemetry() {
.hasBean("otelProperties"));
}

private static class CustomAutoConfiguredSdkConfiguration {
@Bean
public AutoConfiguredOpenTelemetrySdk customAutoConfiguredSdk() {
return AutoConfiguredOpenTelemetrySdk.builder().build();
}
}

@Test
@DisplayName(
"when Application Context contains AutoConfiguredOpenTelemetrySdk bean should NOT use default")
void customAutoConfiguredOpenTelemetrySdk() {
this.contextRunner
.withUserConfiguration(CustomAutoConfiguredSdkConfiguration.class)
.withConfiguration(AutoConfigurations.of(OpenTelemetryAutoConfiguration.class))
.run(
context ->
assertThat(context)
.hasBean("customAutoConfiguredSdk")
.doesNotHaveBean("autoConfiguredOpenTelemetrySdk"));
}

@Test
@DisplayName(
"when Application Context DOES NOT contain OpenTelemetry bean should initialize openTelemetry")
Expand Down
70 changes: 69 additions & 1 deletion .../instrumentation/spring/autoconfigure/internal/properties/SpringConfigPropertiesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@

package io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties;

import static java.util.Collections.emptyMap;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.entry;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.instrumentation.spring.autoconfigure.OpenTelemetryAutoConfiguration;
Expand All @@ -17,6 +22,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
import java.util.stream.Stream;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -162,8 +168,70 @@ void shouldInitializeAttributesByMap() {
});
}

public static Stream<Arguments> propertyCachingTestCases() {
return Stream.of(
// property, typeClass, assertion
Arguments.of(
"otel.service.name=test-service",
String.class,
(Consumer<SpringConfigProperties>)
config ->
assertThat(config.getString("otel.service.name")).isEqualTo("test-service")),
Arguments.of(
"otel.exporter.otlp.enabled=true",
Boolean.class,
(Consumer<SpringConfigProperties>)
config -> assertThat(config.getBoolean("otel.exporter.otlp.enabled")).isTrue()),
Arguments.of(
"otel.metric.export.interval=10",
Integer.class,
(Consumer<SpringConfigProperties>)
config -> assertThat(config.getInt("otel.metric.export.interval")).isEqualTo(10)),
Arguments.of(
"otel.bsp.schedule.delay=5000",
Long.class,
(Consumer<SpringConfigProperties>)
config -> assertThat(config.getLong("otel.bsp.schedule.delay")).isEqualTo(5000L)),
Arguments.of(
"otel.traces.sampler.arg=0.5",
Double.class,
(Consumer<SpringConfigProperties>)
config -> assertThat(config.getDouble("otel.traces.sampler.arg")).isEqualTo(0.5)));
}

@ParameterizedTest
@MethodSource("propertyCachingTestCases")
@DisplayName("should cache property lookups and call Environment.getProperty() only once")
void propertyCaching(
String property, Class<?> typeClass, Consumer<SpringConfigProperties> assertion) {
String propertyName = property.split("=")[0];

this.contextRunner
.withPropertyValues(property)
.run(
context -> {
Environment realEnvironment = context.getBean("environment", Environment.class);
Environment spyEnvironment = spy(realEnvironment);

SpringConfigProperties config =
new SpringConfigProperties(
spyEnvironment,
new SpelExpressionParser(),
context.getBean(OtlpExporterProperties.class),
context.getBean(OtelResourceProperties.class),
context.getBean(OtelSpringProperties.class),
DefaultConfigProperties.createFromMap(emptyMap()));

for (int i = 0; i < 100; i++) {
assertion.accept(config);
}

verify(spyEnvironment, times(1)).getProperty(eq(propertyName), eq(typeClass));
});
}

private static ConfigProperties getConfig(AssertableApplicationContext context) {
return getConfig(context, Collections.emptyMap());
return getConfig(context, emptyMap());
}

private static SpringConfigProperties getConfig(
Expand Down
Loading