diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcUtils.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcUtils.java index 453731ca11..9631966f19 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcUtils.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcUtils.java @@ -7,6 +7,7 @@ package io.opentelemetry.javaagent.instrumentation.jdbc; import static io.opentelemetry.javaagent.instrumentation.api.db.QueryNormalizationConfig.isQueryNormalizationEnabled; +import io.opentelemetry.javaagent.instrumentation.api.BoundedCache; import io.opentelemetry.javaagent.instrumentation.api.db.SqlSanitizer; import io.opentelemetry.javaagent.instrumentation.api.db.SqlStatementInfo; import java.lang.reflect.Field; @@ -22,6 +23,8 @@ public abstract class JdbcUtils { private static final boolean NORMALIZATION_ENABLED = isQueryNormalizationEnabled("jdbc"); private static Field c3poField = null; + private static final BoundedCache sqlToStatementInfoCache = + BoundedCache.build(1000); /** Returns the unwrapped connection or null if exception was thrown. */ public static Connection connectionFromStatement(Statement statement) { @@ -71,6 +74,11 @@ public abstract class JdbcUtils { if (!NORMALIZATION_ENABLED) { return new SqlStatementInfo(sql, null, null); } - return SqlSanitizer.sanitize(sql); + return sqlToStatementInfoCache.get( + sql, + k -> { + log.trace("SQL statement cache miss"); + return SqlSanitizer.sanitize(sql); + }); } } diff --git a/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/BoundedCache.java b/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/BoundedCache.java new file mode 100644 index 0000000000..e2b18b4faa --- /dev/null +++ b/javaagent-api/src/main/java/io/opentelemetry/javaagent/instrumentation/api/BoundedCache.java @@ -0,0 +1,54 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.api; + +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +/** An LRU cache that has a fixed maximum size. */ +public interface BoundedCache { + + V get(K key, Function mappingFunction); + + static BoundedCache build(long maxSize) { + return Provider.get().build(maxSize); + } + + interface Builder { + BoundedCache build(long maxSize); + } + + class Provider { + /* + The default implementation just delegates to the lookup function and should not normally be used. + It will be replaced at startup by the AgentInstaller. + */ + private static final Builder NEVER_ACTUALLY_CACHES = + new Builder() { + @Override + public BoundedCache build(long maxSize) { + return (key, mappingFunction) -> mappingFunction.apply(key); + } + }; + private static final AtomicReference builderRef = + new AtomicReference<>(NEVER_ACTUALLY_CACHES); + + private Provider() {} + + public static boolean registerIfAbsent(Builder builder) { + return builderRef.compareAndSet(NEVER_ACTUALLY_CACHES, builder); + } + + // Method exists for testing only + static void reset() { + builderRef.set(NEVER_ACTUALLY_CACHES); + } + + public static Builder get() { + return builderRef.get(); + } + } +} diff --git a/javaagent-api/src/test/java/io/opentelemetry/javaagent/instrumentation/api/BoundedCacheTest.java b/javaagent-api/src/test/java/io/opentelemetry/javaagent/instrumentation/api/BoundedCacheTest.java new file mode 100644 index 0000000000..21e8a7bc72 --- /dev/null +++ b/javaagent-api/src/test/java/io/opentelemetry/javaagent/instrumentation/api/BoundedCacheTest.java @@ -0,0 +1,104 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.api; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; + +class BoundedCacheTest { + private AtomicInteger cacheMisses; + + @BeforeEach + void setup() { + cacheMisses = new AtomicInteger(0); + } + + @AfterEach + void reset() { + BoundedCache.Provider.reset(); + } + + String mockLookupFunction(String s) { + cacheMisses.incrementAndGet(); + return s.toUpperCase(); + } + + @Test + @Order(1) + void testCanUseBeforeRegister() { + BoundedCache cache = BoundedCache.build(3); + String result1 = cache.get("foo", this::mockLookupFunction); + String result2 = cache.get("bAr", this::mockLookupFunction); + assertThat(result1).isEqualTo("FOO"); + assertThat(result2).isEqualTo("BAR"); + assertThat(cacheMisses.get()).isEqualTo(2); + } + + @Test + @Order(2) + void testRegisterUsesInstance() { + Map map = new HashMap<>(); + BoundedCache.Builder builder = buildMapBackedBuilder(map); + BoundedCache.Provider.registerIfAbsent(builder); + BoundedCache cache = BoundedCache.build(3); + String result1 = cache.get("foo", this::mockLookupFunction); + String result2 = cache.get("fOo", this::mockLookupFunction); + String result3 = cache.get("foo", this::mockLookupFunction); + assertThat(result1).isEqualTo("FOO"); + assertThat(result2).isEqualTo("FOO"); + assertThat(result3).isEqualTo("FOO"); + assertThat(cacheMisses.get()).isEqualTo(2); // once for "foo" once for "fOo" + assertThat(map.size()).isEqualTo(2); + assertThat(map).containsKey("foo"); + assertThat(map).containsKey("fOo"); + } + + @Test + @Order(3) + void testRegisterMultipleFails() { + Map map = new HashMap<>(); + BoundedCache.Builder builder = buildMapBackedBuilder(map); + assertThat(BoundedCache.Provider.registerIfAbsent(builder)).isTrue(); + assertThat(BoundedCache.Provider.registerIfAbsent(builder)).isFalse(); + } + + @NotNull + private BoundedCache.Builder buildMapBackedBuilder(Map map) { + return new BoundedCache.Builder() { + @Override + public BoundedCache build(long maxSize) { + return new MapBackedCache((Map) map); + } + }; + } + + private static class MapBackedCache implements BoundedCache { + private final Map map; + + public MapBackedCache(Map map) { + this.map = map; + } + + @Override + public V get(K key, Function mappingFunction) { + V v = map.get(key); + if (v == null) { + v = mappingFunction.apply(key); + map.put(key, v); + } + return v; + } + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java index 37bf538649..acadb5b4df 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java @@ -69,6 +69,8 @@ public class AgentInstaller { BootstrapPackagePrefixesHolder.setBoostrapPackagePrefixes(loadBootstrapPackagePrefixes()); // WeakMap is used by other classes below, so we need to register the provider first. AgentTooling.registerWeakMapProvider(); + // Instrumentation can use a bounded cache, so register here. + AgentTooling.registerBoundedCacheProvider(); // this needs to be done as early as possible - before the first Config.get() call ConfigInitializer.initialize(); } diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentTooling.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentTooling.java index 7584975e4e..b9d67c4a10 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentTooling.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentTooling.java @@ -5,8 +5,11 @@ package io.opentelemetry.javaagent.tooling; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import io.opentelemetry.javaagent.bootstrap.WeakCache; import io.opentelemetry.javaagent.bootstrap.WeakCache.Provider; +import io.opentelemetry.javaagent.instrumentation.api.BoundedCache; import io.opentelemetry.javaagent.instrumentation.api.WeakMap; import io.opentelemetry.javaagent.tooling.bytebuddy.AgentCachingPoolStrategy; import io.opentelemetry.javaagent.tooling.bytebuddy.AgentLocationStrategy; @@ -33,6 +36,21 @@ public class AgentTooling { } } + /** + * Instances of BoundCache are backed by a guava instance that lives in the agent classloader and + * is bridged to user/instrumentation classloader through the BoundedCache.Provider interface. + */ + static void registerBoundedCacheProvider() { + BoundedCache.Provider.registerIfAbsent( + new BoundedCache.Builder() { + @Override + public BoundedCache build(long maxSize) { + Cache cache = CacheBuilder.newBuilder().maximumSize(maxSize).build(); + return new GuavaBoundedCache<>(cache); + } + }); + } + private static Provider loadWeakCacheProvider() { Iterator providers = ServiceLoader.load(Provider.class, AgentInstaller.class.getClassLoader()).iterator(); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/GuavaBoundedCache.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/GuavaBoundedCache.java new file mode 100644 index 0000000000..f43c486e40 --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/GuavaBoundedCache.java @@ -0,0 +1,29 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling; + +import com.google.common.cache.Cache; +import io.opentelemetry.javaagent.instrumentation.api.BoundedCache; +import java.util.concurrent.ExecutionException; +import java.util.function.Function; + +class GuavaBoundedCache implements BoundedCache { + + private final Cache delegate; + + public GuavaBoundedCache(Cache delegate) { + this.delegate = delegate; + } + + @Override + public V get(K key, Function mappingFunction) { + try { + return delegate.get(key, () -> mappingFunction.apply(key)); + } catch (ExecutionException e) { + throw new IllegalStateException("Unexpected cache exception", e); + } + } +} diff --git a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/GuavaBoundedCacheTest.groovy b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/GuavaBoundedCacheTest.groovy new file mode 100644 index 0000000000..c11152a24f --- /dev/null +++ b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/GuavaBoundedCacheTest.groovy @@ -0,0 +1,30 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling + +import com.google.common.cache.CacheBuilder +import spock.lang.Specification + +class GuavaBoundedCacheTest extends Specification { + + def "test cache"() { + + when: + + def delegate = CacheBuilder.newBuilder().maximumSize(3).build() + def cache = new GuavaBoundedCache<>(delegate) + def fn = { x -> x.toUpperCase() } + + then: + + cache.get("foo", fn) == "FOO" + cache.get("bar", fn) == "BAR" + cache.get("baz", fn) == "BAZ" + cache.get("fizz", fn) == "FIZZ" + cache.get("buzz", fn) == "BUZZ" + delegate.size() == 3 + } +}