diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/CachedPropertyResolver.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/CachedPropertyResolver.java new file mode 100644 index 000000000000..8fa3d948303d --- /dev/null +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/CachedPropertyResolver.java @@ -0,0 +1,97 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties; + +import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; +import javax.annotation.Nullable; +import org.springframework.core.env.Environment; + +/** + * A caching wrapper around Spring's {@link Environment} that caches property lookups to avoid + * repeated expensive operations and property source traversal. + * + *

Thread-safe for concurrent access. Cached values persist indefinitely and are assumed to be + * immutable after the first access. If properties can change at runtime, use {@link #clear()} to + * invalidate the cache. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. + */ +final class CachedPropertyResolver { + private final Environment environment; + private final ConcurrentHashMap> cache = new ConcurrentHashMap<>(); + + CachedPropertyResolver(Environment environment) { + this.environment = Objects.requireNonNull(environment, "environment"); + } + + /** + * Gets a property value from the environment, using a cache to avoid repeated lookups. + * + * @param name the property name + * @param targetType the target type to convert to + * @return the property value, or null if not found + */ + @Nullable + T getProperty(String name, Class targetType) { + CacheKey key = new CacheKey(name, targetType); + // CacheKey includes targetType, ensuring type match + @SuppressWarnings("unchecked") + Optional result = + (Optional) + cache.computeIfAbsent( + key, k -> Optional.ofNullable(environment.getProperty(name, targetType))); + return result.orElse(null); + } + + /** + * Gets a string property value from the environment. + * + * @param name the property name + * @return the property value, or null if not found + */ + @Nullable + String getProperty(String name) { + return getProperty(name, String.class); + } + + /** Clears all cached property values, forcing fresh lookups on subsequent calls. */ + void clear() { + cache.clear(); + } + + /** Cache key combining property name and target type. */ + private static final class CacheKey { + private final String name; + private final Class targetType; + private final int cachedHashCode; + + CacheKey(String name, Class targetType) { + this.name = Objects.requireNonNull(name, "name"); + this.targetType = Objects.requireNonNull(targetType, "targetType"); + this.cachedHashCode = Objects.hash(name, targetType); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof CacheKey)) { + return false; + } + CacheKey other = (CacheKey) obj; + return name.equals(other.name) && targetType.equals(other.targetType); + } + + @Override + public int hashCode() { + return cachedHashCode; + } + } +} diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/SpringConfigProperties.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/SpringConfigProperties.java index 4f0412ef023b..aae84c2f7240 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/SpringConfigProperties.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/SpringConfigProperties.java @@ -25,7 +25,7 @@ * any time. */ public class SpringConfigProperties implements ConfigProperties { - private final Environment environment; + private final CachedPropertyResolver environment; private final ExpressionParser parser; private final OtlpExporterProperties otlpExporterProperties; @@ -44,7 +44,7 @@ public SpringConfigProperties( OtelResourceProperties resourceProperties, OtelSpringProperties otelSpringProperties, ConfigProperties otelSdkProperties) { - this.environment = environment; + this.environment = new CachedPropertyResolver(environment); this.parser = parser; this.otlpExporterProperties = otlpExporterProperties; this.resourceProperties = resourceProperties; @@ -154,8 +154,8 @@ public String getString(String name) { String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(name); 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` + // 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)); @@ -196,7 +196,6 @@ public Double getDouble(String name) { @SuppressWarnings("unchecked") @Override public List getList(String name) { - String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(name); List list = listPropertyValues.get(normalizedName); @@ -210,7 +209,8 @@ public List getList(String name) { } } - return or(environment.getProperty(normalizedName, List.class), otelSdkProperties.getList(name)); + List envValue = (List) environment.getProperty(normalizedName, List.class); + return or(envValue, otelSdkProperties.getList(name)); } @Nullable @@ -231,6 +231,21 @@ public Map getMap(String name) { String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(name); // maps from config properties are not supported by Environment, so we have to fake it + Map specialMap = getSpecialMapProperty(normalizedName, otelSdkMap); + if (specialMap != null) { + return specialMap; + } + + String value = environment.getProperty(normalizedName); + if (value == null) { + return otelSdkMap; + } + return (Map) parser.parseExpression(value).getValue(); + } + + @Nullable + private Map getSpecialMapProperty( + String normalizedName, Map otelSdkMap) { switch (normalizedName) { case "otel.resource.attributes": return mergeWithOtel(resourceProperties.getAttributes(), otelSdkMap); @@ -243,14 +258,8 @@ public Map 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) parser.parseExpression(value).getValue(); } /** diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/SpringConfigPropertiesTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/SpringConfigPropertiesTest.java index 02433342b1ab..c8afd35d5aa6 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/SpringConfigPropertiesTest.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/properties/SpringConfigPropertiesTest.java @@ -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; @@ -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; @@ -162,8 +168,120 @@ void shouldInitializeAttributesByMap() { }); } + public static Stream propertyCachingTestCases() { + return Stream.of( + // property, typeClass, assertion + Arguments.of( + "otel.service.name=test-service", + String.class, + (Consumer) + config -> + assertThat(config.getString("otel.service.name")).isEqualTo("test-service")), + Arguments.of( + "otel.exporter.otlp.enabled=true", + Boolean.class, + (Consumer) + config -> assertThat(config.getBoolean("otel.exporter.otlp.enabled")).isTrue()), + Arguments.of( + "otel.metric.export.interval=10", + Integer.class, + (Consumer) + config -> assertThat(config.getInt("otel.metric.export.interval")).isEqualTo(10)), + Arguments.of( + "otel.bsp.schedule.delay=5000", + Long.class, + (Consumer) + config -> assertThat(config.getLong("otel.bsp.schedule.delay")).isEqualTo(5000L)), + Arguments.of( + "otel.traces.sampler.arg=0.5", + Double.class, + (Consumer) + config -> assertThat(config.getDouble("otel.traces.sampler.arg")).isEqualTo(0.5)), + Arguments.of( + "otel.bsp.export.timeout=30s", + String.class, + (Consumer) + config -> + assertThat(config.getDuration("otel.bsp.export.timeout")) + .isEqualByComparingTo(java.time.Duration.ofSeconds(30))), + Arguments.of( + "otel.attribute.value.length.limit=256", + List.class, + (Consumer) + config -> + assertThat(config.getList("otel.attribute.value.length.limit")) + .containsExactly("256"))); + } + + @ParameterizedTest + @MethodSource("propertyCachingTestCases") + @DisplayName("should cache property lookups and call Environment.getProperty() only once") + void propertyCaching( + String property, Class typeClass, Consumer 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)); + }); + } + + @Test + @DisplayName("should cache map property lookups and call Environment.getProperty() only once") + void mapPropertyCaching() { + this.contextRunner + .withSystemProperties( + "otel.instrumentation.common.peer-service-mapping={'host1':'serviceA','host2':'serviceB'}") + .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++) { + Map mapping = + config.getMap("otel.instrumentation.common.peer-service-mapping"); + assertThat(mapping) + .containsEntry("host1", "serviceA") + .containsEntry("host2", "serviceB"); + } + + // Map properties call getProperty(name) which delegates to getProperty(name, + // String.class) + verify(spyEnvironment, times(1)) + .getProperty( + eq("otel.instrumentation.common.peer-service-mapping"), eq(String.class)); + }); + } + private static ConfigProperties getConfig(AssertableApplicationContext context) { - return getConfig(context, Collections.emptyMap()); + return getConfig(context, emptyMap()); } private static SpringConfigProperties getConfig(