implement cached property resolver

jaydeluca · jaydeluca · commit 5fa3e39e3c03 · 2025-11-03T13:32:38.000-05:00
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
@@ -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.
+ *
+ * <p>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.
+ *
+ * <p>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<CacheKey, Optional<?>> 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> T getProperty(String name, Class<T> targetType) {
+    CacheKey key = new CacheKey(name, targetType);
+    // CacheKey includes targetType, ensuring type match
+    @SuppressWarnings("unchecked")
+    Optional<T> result =
+        (Optional<T>)
+            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
@@ -15,9 +15,6 @@
 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;
@@ -28,7 +25,7 @@
  * any time.
  */
 public class SpringConfigProperties implements ConfigProperties {
-  private final Environment environment;
+  private final CachedPropertyResolver cachedEnvironment;
 
   private final ExpressionParser parser;
   private final OtlpExporterProperties otlpExporterProperties;
@@ -37,23 +34,6 @@ 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";
 
@@ -64,7 +44,7 @@ public SpringConfigProperties(
       OtelResourceProperties resourceProperties,
       OtelSpringProperties otelSpringProperties,
       ConfigProperties otelSdkProperties) {
-    this.environment = environment;
+    this.cachedEnvironment = new CachedPropertyResolver(environment);
     this.parser = parser;
     this.otlpExporterProperties = otlpExporterProperties;
     this.resourceProperties = resourceProperties;
@@ -168,150 +148,103 @@ 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) {
-    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));
-        });
+    String normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(name);
+    String value = cachedEnvironment.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(name));
   }
 
   @Nullable
   @Override
   public Boolean getBoolean(String name) {
-    return getCachedValue(
-        cachedBooleanValues,
-        name,
-        key ->
-            or(
-                environment.getProperty(
-                    ConfigUtil.normalizeEnvironmentVariableKey(key), Boolean.class),
-                otelSdkProperties.getBoolean(key)));
+    return or(
+        cachedEnvironment.getProperty(
+            ConfigUtil.normalizeEnvironmentVariableKey(name), Boolean.class),
+        otelSdkProperties.getBoolean(name));
   }
 
   @Nullable
   @Override
   public Integer getInt(String name) {
-    return getCachedValue(
-        cachedIntValues,
-        name,
-        key ->
-            or(
-                environment.getProperty(
-                    ConfigUtil.normalizeEnvironmentVariableKey(key), Integer.class),
-                otelSdkProperties.getInt(key)));
+    return or(
+        cachedEnvironment.getProperty(
+            ConfigUtil.normalizeEnvironmentVariableKey(name), Integer.class),
+        otelSdkProperties.getInt(name));
   }
 
   @Nullable
   @Override
   public Long getLong(String name) {
-    return getCachedValue(
-        cachedLongValues,
-        name,
-        key ->
-            or(
-                environment.getProperty(
-                    ConfigUtil.normalizeEnvironmentVariableKey(key), Long.class),
-                otelSdkProperties.getLong(key)));
+    return or(
+        cachedEnvironment.getProperty(ConfigUtil.normalizeEnvironmentVariableKey(name), Long.class),
+        otelSdkProperties.getLong(name));
   }
 
   @Nullable
   @Override
   public Double getDouble(String name) {
-    return getCachedValue(
-        cachedDoubleValues,
-        name,
-        key ->
-            or(
-                environment.getProperty(
-                    ConfigUtil.normalizeEnvironmentVariableKey(key), Double.class),
-                otelSdkProperties.getDouble(key)));
+    return or(
+        cachedEnvironment.getProperty(
+            ConfigUtil.normalizeEnvironmentVariableKey(name), Double.class),
+        otelSdkProperties.getDouble(name));
   }
 
   @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(key);
-            if (!c.isEmpty()) {
-              return c;
-            }
-            if (!list.isEmpty()) {
-              return list;
-            }
-          }
+    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> envValue =
-              (List<String>) environment.getProperty(normalizedName, List.class);
-          return or(envValue, otelSdkProperties.getList(key));
-        });
+    List<String> envValue =
+        (List<String>) cachedEnvironment.getProperty(normalizedName, List.class);
+    return or(envValue, otelSdkProperties.getList(name));
   }
 
   @Nullable
   @Override
   public Duration getDuration(String 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);
-        });
+    String value = getString(name);
+    if (value == null) {
+      return otelSdkProperties.getDuration(name);
+    }
+    return DefaultConfigProperties.createFromMap(Collections.singletonMap(name, value))
+        .getDuration(name);
   }
 
   @SuppressWarnings("unchecked")
   @Override
   public Map<String, String> getMap(String name) {
-    return getCachedValue(
-        cachedMapValues,
-        name,
-        key -> {
-          Map<String, String> otelSdkMap = otelSdkProperties.getMap(key);
+    Map<String, String> otelSdkMap = otelSdkProperties.getMap(name);
 
-          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 normalizedName = ConfigUtil.normalizeEnvironmentVariableKey(name);
+    // 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 value = cachedEnvironment.getProperty(normalizedName);
+    if (value == null) {
+      return otelSdkMap;
+    }
+    return (Map<String, String>) parser.parseExpression(value).getValue();
   }
 
   @Nullable
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
