From 52d7eed192367bb0d1f5fe8654cca3026ab0423e Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 7 Jun 2021 17:02:52 +0200 Subject: [PATCH 1/4] Introduce IgnoredTypesConfigurer SPI to enable defining per-module ignores --- benchmark/benchmark.gradle | 2 + .../IgnoredTypesMatcherBenchmark.java | 51 +++ .../extension/ignore/IgnoredTypesBuilder.java | 106 +++++ .../ignore/IgnoredTypesConfigurer.java | 26 ++ .../javaagent/spi/IgnoreMatcherProvider.java | 4 + .../javaagent/tooling/AgentInstaller.java | 21 +- ...ditionalLibraryIgnoredTypesConfigurer.java | 250 ++++++++++++ .../javaagent/tooling/ignore/IgnoreAllow.java | 11 + .../ignore/IgnoredTypesBuilderImpl.java | 55 +++ .../tooling/ignore/IgnoredTypesMatcher.java | 25 ++ .../javaagent/tooling/ignore/trie/Trie.java | 33 ++ .../tooling/ignore/trie/TrieImpl.java | 109 +++++ .../AdditionalLibraryIgnoresMatcher.java | 373 ------------------ .../tooling/matcher/GlobalIgnoresMatcher.java | 39 +- ...AdditionalLibraryIgnoresMatcherTest.groovy | 85 ---- .../javaagent/tooling/ignore/TrieTest.java | 43 ++ testing/agent-exporter/agent-exporter.gradle | 1 + .../testing/bytebuddy/TestAgentListener.java | 13 +- 18 files changed, 750 insertions(+), 497 deletions(-) create mode 100644 benchmark/src/jmh/java/io/opentelemetry/benchmark/IgnoredTypesMatcherBenchmark.java create mode 100644 javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java create mode 100644 javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesConfigurer.java create mode 100644 javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/AdditionalLibraryIgnoredTypesConfigurer.java create mode 100644 javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoreAllow.java create mode 100644 javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesBuilderImpl.java create mode 100644 javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesMatcher.java create mode 100644 javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/trie/Trie.java create mode 100644 javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/trie/TrieImpl.java delete mode 100644 javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcher.java delete mode 100644 javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcherTest.groovy create mode 100644 javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/ignore/TrieTest.java diff --git a/benchmark/benchmark.gradle b/benchmark/benchmark.gradle index beef2f25638f..1991220a9dd7 100644 --- a/benchmark/benchmark.gradle +++ b/benchmark/benchmark.gradle @@ -12,6 +12,8 @@ dependencies { jmh "net.bytebuddy:byte-buddy-agent" jmh project(':instrumentation-api') + jmh project(':javaagent-tooling') + jmh project(':javaagent-extension-api') jmh "com.github.ben-manes.caffeine:caffeine" diff --git a/benchmark/src/jmh/java/io/opentelemetry/benchmark/IgnoredTypesMatcherBenchmark.java b/benchmark/src/jmh/java/io/opentelemetry/benchmark/IgnoredTypesMatcherBenchmark.java new file mode 100644 index 000000000000..e6cf93d87605 --- /dev/null +++ b/benchmark/src/jmh/java/io/opentelemetry/benchmark/IgnoredTypesMatcherBenchmark.java @@ -0,0 +1,51 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.benchmark; + +import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.javaagent.tooling.ignore.AdditionalLibraryIgnoredTypesConfigurer; +import io.opentelemetry.javaagent.tooling.ignore.IgnoredTypesBuilderImpl; +import java.util.concurrent.TimeUnit; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Warmup; + +@Fork(1) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 10, time = 1) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +@BenchmarkMode(Mode.Throughput) +public class IgnoredTypesMatcherBenchmark { + + private static final TypeDescription springType = + new TypeDescription.Latent("org.springframework.test.SomeClass", 0, null); + private static final TypeDescription testAppType = + new TypeDescription.Latent("com.example.myapp.Main", 0, null); + + private static final ElementMatcher ignoredTypesMatcher; + + static { + IgnoredTypesBuilderImpl builder = new IgnoredTypesBuilderImpl(); + new AdditionalLibraryIgnoredTypesConfigurer().configure(Config.get(), builder); + ignoredTypesMatcher = builder.buildIgnoredTypesMatcher(); + } + + @Benchmark + public boolean springType() { + return ignoredTypesMatcher.matches(springType); + } + + @Benchmark + public boolean appType() { + return ignoredTypesMatcher.matches(testAppType); + } +} diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java new file mode 100644 index 000000000000..f7a336187498 --- /dev/null +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java @@ -0,0 +1,106 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.extension.ignore; + +/** + * This interface defines different ways to ignore/allow instrumenting classes or packages. + * + *

This interface should not be implemented by the javaagent extension developer - the javaagent + * will provide the implementation of all transformations described here. + */ +public interface IgnoredTypesBuilder { + + /** + * Ignore the class or package specified by {@code classNameOrPrefix} and exclude it from being + * instrumented. Calling this will overwrite any previous settings for passed prefix. + * + *

{@code classNameOrPrefix} can be the full class name (ex. {@code com.example.MyClass}), + * package name (ex. {@code com.example.mypackage.}), or outer class name (ex {@code + * com.example.OuterClass$}) + * + * @return {@code this} + */ + IgnoredTypesBuilder ignoreClass(String classNameOrPrefix); + + /** + * Allow the class or package specified by {@code classNameOrPrefix} to be instrumented. Calling + * this will overwrite any previous settings for passed prefix. + * + *

{@code classNameOrPrefix} can be the full class name (ex. {@code com.example.MyClass}), + * package name (ex. {@code com.example.mypackage.}), or outer class name (ex {@code + * com.example.OuterClass$}) + * + * @return {@code this} + */ + IgnoredTypesBuilder allowClass(String classNameOrPrefix); + + /** + * Ignore the class loader specified by {@code classNameOrPrefix} and exclude it from being + * instrumented. Calling this will overwrite any previous settings for passed prefix. + * + *

{@code classNameOrPrefix} can be the full class name (ex. {@code com.example.MyClass}), + * package name (ex. {@code com.example.mypackage.}), or outer class name (ex {@code + * com.example.OuterClass$}) + * + * @return {@code this} + */ + IgnoredTypesBuilder ignoreClassLoader(String classNameOrPrefix); + + /** + * Allow the class loader specified by {@code classNameOrPrefix} to be instrumented. Calling this + * will overwrite any previous settings for passed prefix. + * + *

{@code classNameOrPrefix} can be the full class name (ex. {@code com.example.MyClass}), + * package name (ex. {@code com.example.mypackage.}), or outer class name (ex {@code + * com.example.OuterClass$}) + * + * @return {@code this} + */ + IgnoredTypesBuilder allowClassLoader(String classNameOrPrefix); + + /** + * Ignore the Java concurrent task class specified by {@code classNameOrPrefix} and exclude it + * from being instrumented. Concurrent task classes implement or extend one of the following + * classes: + * + *

+ * + * Calling this will overwrite any previous settings for passed prefix. + * + *

{@code classNameOrPrefix} can be the full class name (ex. {@code com.example.MyClass}), + * package name (ex. {@code com.example.mypackage.}), or outer class name (ex {@code + * com.example.OuterClass$}) + * + * @return {@code this} + */ + IgnoredTypesBuilder ignoreTaskClass(String classNameOrPrefix); + + /** + * Allow the Java concurrent task class specified by {@code classNameOrPrefix} to be instrumented. + * Concurrent task classes implement or extend one of the following classes: + * + *

+ * + * Calling this will overwrite any previous settings for passed prefix. + * + *

{@code classNameOrPrefix} can be the full class name (ex. {@code com.example.MyClass}), + * package name (ex. {@code com.example.mypackage.}), or outer class name (ex {@code + * com.example.OuterClass$}) + * + * @return {@code this} + */ + IgnoredTypesBuilder allowTaskClass(String classNameOrPrefix); +} diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesConfigurer.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesConfigurer.java new file mode 100644 index 000000000000..788b262c990b --- /dev/null +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesConfigurer.java @@ -0,0 +1,26 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.extension.ignore; + +import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.javaagent.extension.Ordered; + +/** + * An {@link IgnoredTypesConfigurer} can be used to override built-in instrumentation restrictions: + * ignore some classes and exclude them from being instrumented, or explicitly allow them to be + * instrumented if the agent ignored them by default. + * + *

This is a service provider interface that requires implementations to be registered in a + * provider-configuration file stored in the {@code META-INF/services} resource directory. + */ +public interface IgnoredTypesConfigurer extends Ordered { + + /** + * Configure the passed {@code builder} and define which classes should be ignored when + * instrumenting. + */ + void configure(Config config, IgnoredTypesBuilder builder); +} diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/spi/IgnoreMatcherProvider.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/spi/IgnoreMatcherProvider.java index 1e137fcb342c..04a853eff624 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/spi/IgnoreMatcherProvider.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/spi/IgnoreMatcherProvider.java @@ -16,7 +16,11 @@ * *

This is a service provider interface that requires implementations to be registered in {@code * META-INF/services} folder. Only a single implementation of this SPI can be provided. + * + * @deprecated Please use {@link io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer} + * instead. */ +@Deprecated public interface IgnoreMatcherProvider { /** 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 1f9b2cb642ce..c39132ab673a 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 @@ -17,12 +17,14 @@ import io.opentelemetry.javaagent.bootstrap.AgentClassLoader; import io.opentelemetry.javaagent.extension.AgentExtension; import io.opentelemetry.javaagent.extension.AgentListener; +import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.instrumentation.api.internal.BootstrapPackagePrefixesHolder; import io.opentelemetry.javaagent.spi.BootstrapPackagesProvider; import io.opentelemetry.javaagent.spi.IgnoreMatcherProvider; import io.opentelemetry.javaagent.tooling.config.ConfigInitializer; import io.opentelemetry.javaagent.tooling.context.FieldBackedProvider; +import io.opentelemetry.javaagent.tooling.ignore.IgnoredTypesBuilderImpl; import io.opentelemetry.javaagent.tooling.matcher.GlobalClassloaderIgnoresMatcher; import java.lang.instrument.Instrumentation; import java.lang.reflect.Proxy; @@ -62,12 +64,6 @@ public class AgentInstaller { private static final String FORCE_SYNCHRONOUS_AGENT_LISTENERS_CONFIG = "otel.javaagent.experimental.force-synchronous-agent-listeners"; - // We set this system property when running the agent with unit tests to allow verifying that we - // don't ignore libraries that we actually attempt to instrument. It means either the list is - // wrong or a type matcher is. - private static final String ADDITIONAL_LIBRARY_IGNORES_ENABLED = - "otel.javaagent.testing.additional-library-ignores.enabled"; - private static final Map> CLASS_LOAD_CALLBACKS = new HashMap<>(); private static volatile Instrumentation instrumentation; @@ -140,9 +136,7 @@ public static ResettableClassFileTransformer installBytebuddyAgent( ignoredAgentBuilder = ignoredAgentBuilder.or( - globalIgnoresMatcher( - config.getBooleanProperty(ADDITIONAL_LIBRARY_IGNORES_ENABLED, true), - ignoreMatcherProvider)); + globalIgnoresMatcher(ignoreMatcherProvider, loadIgnoredTypesMatcher(config))); ignoredAgentBuilder = ignoredAgentBuilder.or(matchesConfiguredExcludes()); @@ -180,6 +174,14 @@ public static ResettableClassFileTransformer installBytebuddyAgent( return resettableClassFileTransformer; } + private static ElementMatcher loadIgnoredTypesMatcher(Config config) { + IgnoredTypesBuilderImpl ignoredTypesBuilder = new IgnoredTypesBuilderImpl(); + for (IgnoredTypesConfigurer configurer : loadOrdered(IgnoredTypesConfigurer.class)) { + configurer.configure(config, ignoredTypesBuilder); + } + return ignoredTypesBuilder.buildIgnoredTypesMatcher(); + } + private static void runBeforeAgentListeners( Iterable agentListeners, Config config) { for (AgentListener agentListener : agentListeners) { @@ -248,6 +250,7 @@ private static void addByteBuddyRawSetting() { } } + // TODO: rewrite to an IgnoredTypesConfigurer private static ElementMatcher.Junction matchesConfiguredExcludes() { List excludedClasses = Config.get().getListProperty(EXCLUDED_CLASSES_CONFIG); ElementMatcher.Junction matcher = none(); diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/AdditionalLibraryIgnoredTypesConfigurer.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/AdditionalLibraryIgnoredTypesConfigurer.java new file mode 100644 index 000000000000..78a36a6a7897 --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/AdditionalLibraryIgnoredTypesConfigurer.java @@ -0,0 +1,250 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.ignore; + +import com.google.auto.service.AutoService; +import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesBuilder; +import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesConfigurer; +import io.opentelemetry.javaagent.tooling.matcher.GlobalIgnoresMatcher; + +/** + * Additional global ignore settings that are used to reduce number of classes we try to apply + * expensive matchers to. + * + *

This is separated from {@link GlobalIgnoresMatcher} to allow for better testing. The idea is + * that we should be able to remove this matcher from the agent and all tests should still pass. + * Moreover, no classes matched by this matcher should be modified during test run. + */ +@AutoService(IgnoredTypesConfigurer.class) +public class AdditionalLibraryIgnoredTypesConfigurer implements IgnoredTypesConfigurer { + + // We set this system property when running the agent with unit tests to allow verifying that we + // don't ignore libraries that we actually attempt to instrument. It means either the list is + // wrong or a type matcher is. + private static final String ADDITIONAL_LIBRARY_IGNORES_ENABLED = + "otel.javaagent.testing.additional-library-ignores.enabled"; + + @Override + public void configure(Config config, IgnoredTypesBuilder builder) { + if (!config.getBooleanProperty(ADDITIONAL_LIBRARY_IGNORES_ENABLED, true)) { + return; + } + + builder + .ignoreClass("com.beust.jcommander.") + .ignoreClass("com.fasterxml.classmate.") + .ignoreClass("com.github.mustachejava.") + .ignoreClass("com.jayway.jsonpath.") + .ignoreClass("com.lightbend.lagom.") + .ignoreClass("javax.el.") + .ignoreClass("org.apache.lucene.") + .ignoreClass("org.apache.tartarus.") + .ignoreClass("org.json.simple.") + .ignoreClass("org.yaml.snakeyaml."); + + builder.ignoreClass("net.sf.cglib.").allowClass("net.sf.cglib.core.internal.LoadingCache$2"); + + builder + .ignoreClass("org.springframework.aop.") + .ignoreClass("org.springframework.cache.") + .ignoreClass("org.springframework.dao.") + .ignoreClass("org.springframework.ejb.") + .ignoreClass("org.springframework.expression.") + .ignoreClass("org.springframework.format.") + .ignoreClass("org.springframework.jca.") + .ignoreClass("org.springframework.jdbc.") + .ignoreClass("org.springframework.jmx.") + .ignoreClass("org.springframework.jndi.") + .ignoreClass("org.springframework.lang.") + .ignoreClass("org.springframework.messaging.") + .ignoreClass("org.springframework.objenesis.") + .ignoreClass("org.springframework.orm.") + .ignoreClass("org.springframework.remoting.") + .ignoreClass("org.springframework.scripting.") + .ignoreClass("org.springframework.stereotype.") + .ignoreClass("org.springframework.transaction.") + .ignoreClass("org.springframework.ui.") + .ignoreClass("org.springframework.validation."); + + builder + .ignoreClass("org.springframework.data.") + .allowClass("org.springframework.data.repository.core.support.RepositoryFactorySupport") + .allowClass("org.springframework.data.convert.ClassGeneratingEntityInstantiator$") + .allowClass("org.springframework.data.jpa.repository.config.InspectionClassLoader"); + + builder + .ignoreClass("org.springframework.amqp.") + .allowClass("org.springframework.amqp.rabbit.connection."); + + builder + .ignoreClass("org.springframework.beans.") + .allowClass("org.springframework.beans.factory.support.DisposableBeanAdapter") + .allowClass("org.springframework.beans.factory.groovy.GroovyBeanDefinitionReader$"); + + builder + .ignoreClass("org.springframework.boot.") + .allowClass("org.springframework.boot.context.web.") + .allowClass("org.springframework.boot.logging.logback.") + .allowClass("org.springframework.boot.web.filter.") + .allowClass("org.springframework.boot.web.servlet.") + .allowClass("org.springframework.boot.autoconfigure.BackgroundPreinitializer$") + .allowClass("org.springframework.boot.autoconfigure.condition.OnClassCondition$") + .allowClass("org.springframework.boot.web.embedded.netty.NettyWebServer$") + .allowClass( + "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainer$") + .allowClass( + "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedWebappClassLoader") + .allowClass("org.springframework.boot.context.embedded.EmbeddedWebApplicationContext") + .allowClass( + "org.springframework.boot.context.embedded.AnnotationConfigEmbeddedWebApplicationContext") + // spring boot 2 classes + .allowClass( + "org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext") + .allowClass( + "org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext") + .allowClass("org.springframework.boot.web.embedded.tomcat.TomcatWebServer$") + .allowClass("org.springframework.boot.web.embedded.tomcat.TomcatEmbeddedWebappClassLoader") + .allowClass("org.springframework.boot.web.servlet.DelegatingFilterProxyRegistrationBean$") + .allowClass("org.springframework.boot.StartupInfoLogger$"); + + builder + .ignoreClass("org.springframework.cglib.") + // This class contains nested Callable instance that we'd happily not touch, but + // unfortunately our field injection code is not flexible enough to realize that, so instead + // we instrument this Callable to make tests happy. + .allowClass("org.springframework.cglib.core.internal.LoadingCache$"); + + builder + .ignoreClass("org.springframework.context.") + // More runnables to deal with + .allowClass("org.springframework.context.support.AbstractApplicationContext$") + .allowClass("org.springframework.context.support.ContextTypeMatchClassLoader"); + + builder + .ignoreClass("org.springframework.core.") + .allowClass("org.springframework.core.task.") + .allowClass("org.springframework.core.DecoratingClassLoader") + .allowClass("org.springframework.core.OverridingClassLoader") + .allowClass("org.springframework.core.ReactiveAdapterRegistry$EmptyCompletableFuture"); + + builder + .ignoreClass("org.springframework.instrument.") + .allowClass("org.springframework.instrument.classloading.SimpleThrowawayClassLoader") + .allowClass("org.springframework.instrument.classloading.ShadowingClassLoader"); + + builder + .ignoreClass("org.springframework.http.") + // There are some Mono implementation that get instrumented + .allowClass("org.springframework.http.server.reactive."); + + builder + .ignoreClass("org.springframework.jms.") + .ignoreClass("org.springframework.jms.listener.") + .ignoreClass( + "org.springframework.jms.config.JmsListenerEndpointRegistry$AggregatingCallback"); + + builder + .ignoreClass("org.springframework.util.") + .allowClass("org.springframework.util.concurrent."); + + builder + .ignoreClass("org.springframework.web.") + .allowClass("org.springframework.web.servlet.") + .allowClass("org.springframework.web.filter.") + .allowClass("org.springframework.web.multipart.") + .allowClass("org.springframework.web.reactive.") + .allowClass("org.springframework.web.context.request.async.") + .allowClass( + "org.springframework.web.context.support.AbstractRefreshableWebApplicationContext") + .allowClass("org.springframework.web.context.support.GenericWebApplicationContext") + .allowClass("org.springframework.web.context.support.XmlWebApplicationContext"); + + // xml-apis, xerces, xalan, but not xml web-services + builder + .ignoreClass("javax.xml.") + .allowClass("javax.xml.ws.") + .ignoreClass("org.apache.bcel.") + .ignoreClass("org.apache.html.") + .ignoreClass("org.apache.regexp.") + .ignoreClass("org.apache.wml.") + .ignoreClass("org.apache.xalan.") + .ignoreClass("org.apache.xerces.") + .ignoreClass("org.apache.xml.") + .ignoreClass("org.apache.xpath.") + .ignoreClass("org.xml."); + + builder + .ignoreClass("ch.qos.logback.") + // We instrument this Runnable + .allowClass("ch.qos.logback.core.AsyncAppenderBase$Worker") + // Allow instrumenting loggers & events + .allowClass("ch.qos.logback.classic.Logger") + .allowClass("ch.qos.logback.classic.spi.LoggingEvent") + .allowClass("ch.qos.logback.classic.spi.LoggingEventVO"); + + builder + .ignoreClass("com.codahale.metrics.") + // We instrument servlets + .allowClass("com.codahale.metrics.servlets."); + + builder + .ignoreClass("com.couchbase.client.deps.") + // Couchbase library includes some packaged dependencies, unfortunately some of them are + // instrumented by executors instrumentation + .allowClass("com.couchbase.client.deps.io.netty.") + .allowClass("com.couchbase.client.deps.org.LatencyUtils.") + .allowClass("com.couchbase.client.deps.com.lmax.disruptor."); + + builder + .ignoreClass("com.google.cloud.") + .ignoreClass("com.google.instrumentation.") + .ignoreClass("com.google.j2objc.") + .ignoreClass("com.google.gson.") + .ignoreClass("com.google.logging.") + .ignoreClass("com.google.longrunning.") + .ignoreClass("com.google.protobuf.") + .ignoreClass("com.google.rpc.") + .ignoreClass("com.google.thirdparty.") + .ignoreClass("com.google.type."); + + builder + .ignoreClass("com.google.common.") + .allowClass("com.google.common.util.concurrent.") + .allowClass("com.google.common.base.internal.Finalizer"); + + builder + .ignoreClass("com.google.inject.") + // We instrument Runnable there + .allowClass("com.google.inject.internal.AbstractBindingProcessor$") + .allowClass("com.google.inject.internal.BytecodeGen$") + .allowClass("com.google.inject.internal.cglib.core.internal.$LoadingCache$"); + + builder.ignoreClass("com.google.api.").allowClass("com.google.api.client.http.HttpRequest"); + + builder + .ignoreClass("org.h2.") + .allowClass("org.h2.Driver") + .allowClass("org.h2.jdbc.") + .allowClass("org.h2.jdbcx.") + // Some runnables that get instrumented + .allowClass("org.h2.util.Task") + .allowClass("org.h2.store.FileLock") + .allowClass("org.h2.engine.DatabaseCloser") + .allowClass("org.h2.engine.OnExitDatabaseCloser"); + + builder + .ignoreClass("com.carrotsearch.hppc.") + .allowClass("com.carrotsearch.hppc.HashOrderMixing$"); + + builder + .ignoreClass("com.fasterxml.jackson.") + .allowClass("com.fasterxml.jackson.module.afterburner.util.MyClassLoader"); + + // kotlin, note we do not ignore kotlinx because we instrument coroutines code + builder.ignoreClass("kotlin.").allowClass("kotlin.coroutines.jvm.internal.DebugProbesKt"); + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoreAllow.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoreAllow.java new file mode 100644 index 000000000000..509c8b75de81 --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoreAllow.java @@ -0,0 +1,11 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.ignore; + +enum IgnoreAllow { + IGNORE, + ALLOW +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesBuilderImpl.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesBuilderImpl.java new file mode 100644 index 000000000000..8e9c2380d4b7 --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesBuilderImpl.java @@ -0,0 +1,55 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.ignore; + +import io.opentelemetry.javaagent.extension.ignore.IgnoredTypesBuilder; +import io.opentelemetry.javaagent.tooling.ignore.trie.Trie; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +public class IgnoredTypesBuilderImpl implements IgnoredTypesBuilder { + private final Trie.Builder ignoreMatcherTrie = Trie.newBuilder(); + + @Override + public IgnoredTypesBuilder ignoreClass(String className) { + ignoreMatcherTrie.put(className, IgnoreAllow.IGNORE); + return this; + } + + @Override + public IgnoredTypesBuilder allowClass(String className) { + ignoreMatcherTrie.put(className, IgnoreAllow.ALLOW); + return this; + } + + @Override + public IgnoredTypesBuilder ignoreClassLoader(String classNameOrPrefix) { + // TODO: collect classloader classes into a separate trie + return this; + } + + @Override + public IgnoredTypesBuilder allowClassLoader(String classNameOrPrefix) { + // TODO: collect classloader classes into a separate trie + return this; + } + + @Override + public IgnoredTypesBuilder ignoreTaskClass(String className) { + // TODO: collect task classes into a separate trie + return this; + } + + @Override + public IgnoredTypesBuilder allowTaskClass(String className) { + // TODO: collect task classes into a separate trie + return this; + } + + public ElementMatcher buildIgnoredTypesMatcher() { + return new IgnoredTypesMatcher(ignoreMatcherTrie.build()); + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesMatcher.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesMatcher.java new file mode 100644 index 000000000000..f9e929e3807f --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesMatcher.java @@ -0,0 +1,25 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.ignore; + +import io.opentelemetry.javaagent.tooling.ignore.trie.Trie; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +class IgnoredTypesMatcher extends ElementMatcher.Junction.AbstractBase { + private final Trie ignoredTypes; + + IgnoredTypesMatcher(Trie ignoredTypes) { + this.ignoredTypes = ignoredTypes; + } + + @Override + public boolean matches(TypeDescription target) { + IgnoreAllow ignored = ignoredTypes.getOrNull(target.getActualName()); + // ALLOW or null (default) mean that the type should not be ignored + return ignored == IgnoreAllow.IGNORE; + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/trie/Trie.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/trie/Trie.java new file mode 100644 index 000000000000..87830900aa0b --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/trie/Trie.java @@ -0,0 +1,33 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.ignore.trie; + +import org.checkerframework.checker.nullness.qual.Nullable; + +/** A prefix tree that maps from the longest matching prefix to a value {@code V}. */ +public interface Trie { + + /** Start building a trie. */ + static Builder newBuilder() { + return new TrieImpl.BuilderImpl<>(); + } + + /** + * Returns the value associated with the longest matched prefix, or null if there wasn't a match. + * For example: for a trie containing an {@code ("abc", 10)} entry {@code trie.getOrNull("abcd")} + * will return {@code 10}. + */ + @Nullable + V getOrNull(CharSequence str); + + interface Builder { + + /** Associate {@code value} with the string {@code str}. */ + Builder put(CharSequence str, V value); + + Trie build(); + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/trie/TrieImpl.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/trie/TrieImpl.java new file mode 100644 index 000000000000..bba637ded045 --- /dev/null +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/trie/TrieImpl.java @@ -0,0 +1,109 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.ignore.trie; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import org.checkerframework.checker.nullness.qual.Nullable; + +final class TrieImpl implements Trie { + + private final Node root; + + private TrieImpl(Node root) { + this.root = root; + } + + @Override + public V getOrNull(CharSequence str) { + Node node = root; + V lastMatchedValue = null; + + for (int i = 0; i < str.length(); ++i) { + char c = str.charAt(i); + Node next = node.getNext(c); + if (next == null) { + return lastMatchedValue; + } + node = next; + // next node matched, use its value if it's defined + lastMatchedValue = next.value != null ? next.value : lastMatchedValue; + } + + return lastMatchedValue; + } + + static final class Node { + final char[] chars; + final Node[] children; + final V value; + + Node(char[] chars, Node[] children, V value) { + this.chars = chars; + this.children = children; + this.value = value; + } + + @Nullable + Node getNext(char c) { + int index = Arrays.binarySearch(chars, c); + if (index < 0) { + return null; + } + return children[index]; + } + } + + static final class BuilderImpl implements Builder { + + private final NodeBuilder root = new NodeBuilder<>(); + + @Override + public Builder put(CharSequence str, V value) { + put(root, str, 0, value); + return this; + } + + private void put(NodeBuilder node, CharSequence str, int i, V value) { + if (str.length() == i) { + node.value = value; + return; + } + char c = str.charAt(i); + NodeBuilder next = node.children.computeIfAbsent(c, k -> new NodeBuilder<>()); + put(next, str, i + 1, value); + } + + @Override + public Trie build() { + return new TrieImpl<>(root.build()); + } + } + + static final class NodeBuilder { + final Map> children = new HashMap<>(); + V value; + + Node build() { + int size = children.size(); + char[] chars = new char[size]; + Node[] nodes = new Node[size]; + + int i = 0; + Iterator>> it = + this.children.entrySet().stream().sorted(Map.Entry.comparingByKey()).iterator(); + while (it.hasNext()) { + Map.Entry> e = it.next(); + chars[i] = e.getKey(); + nodes[i++] = e.getValue().build(); + } + + return new Node<>(chars, nodes, value); + } + } +} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcher.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcher.java deleted file mode 100644 index 6e6bc9d0d5d7..000000000000 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcher.java +++ /dev/null @@ -1,373 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.matcher; - -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -/** - * Additional global matchers that are used to reduce number of classes we try to apply expensive - * matchers to. - * - *

This is separated from {@link GlobalIgnoresMatcher} to allow for better testing. The idea is - * that we should be able to remove this matcher from the agent and all tests should still pass. - * Moreover, no classes matched by this matcher should be modified during test run. - * - *

TODO: refactor/optimize this class (spring boot approach as a possible solution) - */ -public class AdditionalLibraryIgnoresMatcher - extends ElementMatcher.Junction.AbstractBase { - - public static Junction additionalLibraryIgnoresMatcher() { - return new AdditionalLibraryIgnoresMatcher(); - } - - /** - * Be very careful about the types of matchers used in this section as they are called on every - * class load, so they must be fast. Generally speaking try to only use name matchers as they - * don't have to load additional info. - */ - @Override - public boolean matches(TypeDescription target) { - String name = target.getActualName(); - - if (name.startsWith("com.beust.jcommander.") - || name.startsWith("com.fasterxml.classmate.") - || name.startsWith("com.github.mustachejava.") - || name.startsWith("com.jayway.jsonpath.") - || name.startsWith("com.lightbend.lagom.") - || name.startsWith("javax.el.") - || name.startsWith("org.apache.lucene.") - || name.startsWith("org.apache.tartarus.") - || name.startsWith("org.json.simple.") - || name.startsWith("org.yaml.snakeyaml.")) { - return true; - } - - if (name.startsWith("net.sf.cglib.")) { - return !name.equals("net.sf.cglib.core.internal.LoadingCache$2"); - } - - if (name.startsWith("org.springframework.")) { - if (name.startsWith("org.springframework.aop.") - || name.startsWith("org.springframework.cache.") - || name.startsWith("org.springframework.dao.") - || name.startsWith("org.springframework.ejb.") - || name.startsWith("org.springframework.expression.") - || name.startsWith("org.springframework.format.") - || name.startsWith("org.springframework.jca.") - || name.startsWith("org.springframework.jdbc.") - || name.startsWith("org.springframework.jmx.") - || name.startsWith("org.springframework.jndi.") - || name.startsWith("org.springframework.lang.") - || name.startsWith("org.springframework.messaging.") - || name.startsWith("org.springframework.objenesis.") - || name.startsWith("org.springframework.orm.") - || name.startsWith("org.springframework.remoting.") - || name.startsWith("org.springframework.scripting.") - || name.startsWith("org.springframework.stereotype.") - || name.startsWith("org.springframework.transaction.") - || name.startsWith("org.springframework.ui.") - || name.startsWith("org.springframework.validation.")) { - return true; - } - - if (name.startsWith("org.springframework.data.")) { - if (name.equals("org.springframework.data.repository.core.support.RepositoryFactorySupport") - || name.startsWith( - "org.springframework.data.convert.ClassGeneratingEntityInstantiator$") - || name.equals( - "org.springframework.data.jpa.repository.config.InspectionClassLoader")) { - return false; - } - return true; - } - - if (name.startsWith("org.springframework.amqp.")) { - if (name.startsWith("org.springframework.amqp.rabbit.connection.")) { - return false; - } - return true; - } - - if (name.startsWith("org.springframework.beans.")) { - if (name.equals("org.springframework.beans.factory.support.DisposableBeanAdapter") - || name.startsWith( - "org.springframework.beans.factory.groovy.GroovyBeanDefinitionReader$")) { - return false; - } - return true; - } - - if (name.startsWith("org.springframework.boot.")) { - return !instrumentedSpringBootClasses(name) - && !name.startsWith("org.springframework.boot.context.web.") - && !name.startsWith("org.springframework.boot.logging.logback.") - && !name.startsWith("org.springframework.boot.web.filter.") - && !name.startsWith("org.springframework.boot.web.servlet."); - } - - if (name.startsWith("org.springframework.cglib.")) { - // This class contains nested Callable instance that we'd happily not touch, but - // unfortunately our field injection code is not flexible enough to realize that, so instead - // we instrument this Callable to make tests happy. - if (name.startsWith("org.springframework.cglib.core.internal.LoadingCache$")) { - return false; - } - return true; - } - - if (name.startsWith("org.springframework.context.")) { - // More runnables to deal with - if (name.startsWith("org.springframework.context.support.AbstractApplicationContext$") - || name.equals("org.springframework.context.support.ContextTypeMatchClassLoader")) { - return false; - } - return true; - } - - if (name.startsWith("org.springframework.core.")) { - if (name.startsWith("org.springframework.core.task.") - || name.equals("org.springframework.core.DecoratingClassLoader") - || name.equals("org.springframework.core.OverridingClassLoader") - || name.equals( - "org.springframework.core.ReactiveAdapterRegistry$EmptyCompletableFuture")) { - return false; - } - return true; - } - - if (name.startsWith("org.springframework.instrument.")) { - if (name.equals("org.springframework.instrument.classloading.SimpleThrowawayClassLoader") - || name.equals("org.springframework.instrument.classloading.ShadowingClassLoader")) { - return false; - } - return true; - } - - if (name.startsWith("org.springframework.http.")) { - // There are some Mono implementation that get instrumented - if (name.startsWith("org.springframework.http.server.reactive.")) { - return false; - } - return true; - } - - if (name.startsWith("org.springframework.jms.")) { - if (name.startsWith("org.springframework.jms.listener.") - || name.equals( - "org.springframework.jms.config.JmsListenerEndpointRegistry$AggregatingCallback")) { - return false; - } - return true; - } - - if (name.startsWith("org.springframework.util.")) { - if (name.startsWith("org.springframework.util.concurrent.")) { - return false; - } - return true; - } - - if (name.startsWith("org.springframework.web.")) { - if (name.startsWith("org.springframework.web.servlet.") - || name.startsWith("org.springframework.web.filter.") - || name.startsWith("org.springframework.web.multipart.") - || name.startsWith("org.springframework.web.reactive.") - || name.startsWith("org.springframework.web.context.request.async.") - || name.equals( - "org.springframework.web.context.support.AbstractRefreshableWebApplicationContext") - || name.equals("org.springframework.web.context.support.GenericWebApplicationContext") - || name.equals("org.springframework.web.context.support.XmlWebApplicationContext")) { - return false; - } - return true; - } - - return false; - } - - // xml-apis, xerces, xalan, but not xml web-services - if ((name.startsWith("javax.xml.") && !name.startsWith("javax.xml.ws.")) - || name.startsWith("org.apache.bcel.") - || name.startsWith("org.apache.html.") - || name.startsWith("org.apache.regexp.") - || name.startsWith("org.apache.wml.") - || name.startsWith("org.apache.xalan.") - || name.startsWith("org.apache.xerces.") - || name.startsWith("org.apache.xml.") - || name.startsWith("org.apache.xpath.") - || name.startsWith("org.xml.")) { - return true; - } - - if (name.startsWith("ch.qos.logback.")) { - // We instrument this Runnable - if (name.equals("ch.qos.logback.core.AsyncAppenderBase$Worker")) { - return false; - } - // Allow instrumenting loggers & events - if (name.equals("ch.qos.logback.classic.Logger") - || name.equals("ch.qos.logback.classic.spi.LoggingEvent") - || name.equals("ch.qos.logback.classic.spi.LoggingEventVO")) { - return false; - } - return true; - } - - if (name.startsWith("com.codahale.metrics.")) { - // We instrument servlets - if (name.startsWith("com.codahale.metrics.servlets.")) { - return false; - } - return true; - } - - if (name.startsWith("com.couchbase.client.deps.")) { - // Couchbase library includes some packaged dependencies, unfortunately some of them are - // instrumented by executors instrumentation - if (name.startsWith("com.couchbase.client.deps.io.netty.") - || name.startsWith("com.couchbase.client.deps.org.LatencyUtils.") - || name.startsWith("com.couchbase.client.deps.com.lmax.disruptor.")) { - return false; - } - return true; - } - - if (name.startsWith("com.google.cloud.") - || name.startsWith("com.google.instrumentation.") - || name.startsWith("com.google.j2objc.") - || name.startsWith("com.google.gson.") - || name.startsWith("com.google.logging.") - || name.startsWith("com.google.longrunning.") - || name.startsWith("com.google.protobuf.") - || name.startsWith("com.google.rpc.") - || name.startsWith("com.google.thirdparty.") - || name.startsWith("com.google.type.")) { - return true; - } - if (name.startsWith("com.google.common.")) { - if (name.startsWith("com.google.common.util.concurrent.") - || name.equals("com.google.common.base.internal.Finalizer")) { - return false; - } - return true; - } - if (name.startsWith("com.google.inject.")) { - // We instrument Runnable there - if (name.startsWith("com.google.inject.internal.AbstractBindingProcessor$") - || name.startsWith("com.google.inject.internal.BytecodeGen$") - || name.startsWith("com.google.inject.internal.cglib.core.internal.$LoadingCache$")) { - return false; - } - // We instrument Callable there - if (name.startsWith("com.google.inject.internal.cglib.core.internal.$LoadingCache$")) { - return false; - } - return true; - } - if (name.startsWith("com.google.api.")) { - if (name.startsWith("com.google.api.client.http.HttpRequest")) { - return false; - } - return true; - } - - if (name.startsWith("org.h2.")) { - if (name.equals("org.h2.Driver") - || name.startsWith("org.h2.jdbc.") - || name.startsWith("org.h2.jdbcx.") - // Some runnables that get instrumented - || name.equals("org.h2.util.Task") - || name.equals("org.h2.store.FileLock") - || name.equals("org.h2.engine.DatabaseCloser") - || name.equals("org.h2.engine.OnExitDatabaseCloser")) { - return false; - } - return true; - } - - if (name.startsWith("com.carrotsearch.hppc.")) { - if (name.startsWith("com.carrotsearch.hppc.HashOrderMixing$")) { - return false; - } - return true; - } - - if (name.startsWith("com.fasterxml.jackson.")) { - if (name.equals("com.fasterxml.jackson.module.afterburner.util.MyClassLoader")) { - return false; - } - return true; - } - - // kotlin, note we do not ignore kotlinx because we instrument coroutins code - if (name.startsWith("kotlin.")) { - if (name.equals("kotlin.coroutines.jvm.internal.DebugProbesKt")) { - return false; - } - return true; - } - - return false; - } - - private static final Set INSTRUMENTED_SPRING_BOOT_CLASSES; - - static { - Set instrumented = new HashSet<>(); - instrumented.add("org.springframework.boot.autoconfigure.BackgroundPreinitializer$"); - instrumented.add("org.springframework.boot.autoconfigure.condition.OnClassCondition$"); - instrumented.add("org.springframework.boot.web.embedded.netty.NettyWebServer$"); - instrumented.add( - "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainer$"); - instrumented.add( - "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedWebappClassLoader"); - instrumented.add("org.springframework.boot.context.embedded.EmbeddedWebApplicationContext"); - instrumented.add( - "org.springframework.boot.context.embedded.AnnotationConfigEmbeddedWebApplicationContext"); - // spring boot 2 classes - instrumented.add( - "org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext"); - instrumented.add( - "org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext"); - instrumented.add("org.springframework.boot.web.embedded.tomcat.TomcatWebServer$"); - instrumented.add( - "org.springframework.boot.web.embedded.tomcat.TomcatEmbeddedWebappClassLoader"); - instrumented.add("org.springframework.boot.web.servlet.DelegatingFilterProxyRegistrationBean$"); - instrumented.add("org.springframework.boot.StartupInfoLogger$"); - INSTRUMENTED_SPRING_BOOT_CLASSES = Collections.unmodifiableSet(instrumented); - } - - private static String outerClassName(final String name) { - int separator = name.indexOf('$'); - return (separator == -1 ? name : name.substring(0, separator + 1)); - } - - private static boolean instrumentedSpringBootClasses(final String name) { - String outerName = outerClassName(name); - return INSTRUMENTED_SPRING_BOOT_CLASSES.contains(outerName); - } - - @Override - public String toString() { - return "additionalLibraryIgnoresMatcher()"; - } - - @Override - public boolean equals(Object obj) { - // all instances are the same - return obj instanceof AdditionalLibraryIgnoresMatcher; - } - - @Override - public int hashCode() { - return 17; - } -} diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/GlobalIgnoresMatcher.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/GlobalIgnoresMatcher.java index d7bfa38f13bc..cb2d2d15d1a4 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/GlobalIgnoresMatcher.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/GlobalIgnoresMatcher.java @@ -20,31 +20,27 @@ * Ignore classes that are unsafe or pointless to transform. 'System' level classes like jvm * classes or groovy classes, other tracers, debuggers, etc. * - * - *

*/ +// TODO: rewrite to an IgnoredTypesConfigurer public class GlobalIgnoresMatcher extends ElementMatcher.Junction.AbstractBase { private static final Pattern COM_MCHANGE_PROXY = Pattern.compile("com\\.mchange\\.v2\\.c3p0\\..*Proxy"); public static ElementMatcher.Junction globalIgnoresMatcher( - boolean additionalLibraryMatcher, IgnoreMatcherProvider ignoreMatcherProviders) { - return new GlobalIgnoresMatcher(additionalLibraryMatcher, ignoreMatcherProviders); + IgnoreMatcherProvider ignoreMatcherProviders, + ElementMatcher ignoredTypeMatcher) { + return new GlobalIgnoresMatcher(ignoreMatcherProviders, ignoredTypeMatcher); } - private final ElementMatcher additionalLibraryIgnoreMatcher = - AdditionalLibraryIgnoresMatcher.additionalLibraryIgnoresMatcher(); - private final boolean additionalLibraryMatcher; private final IgnoreMatcherProvider ignoreMatcherProvider; + private final ElementMatcher ignoredTypeMatcher; private GlobalIgnoresMatcher( - boolean additionalLibraryMatcher, IgnoreMatcherProvider ignoreMatcherProvider) { - this.additionalLibraryMatcher = additionalLibraryMatcher; + IgnoreMatcherProvider ignoreMatcherProvider, + ElementMatcher ignoredTypeMatcher) { this.ignoreMatcherProvider = ignoreMatcherProvider; + this.ignoredTypeMatcher = ignoredTypeMatcher; } /** @@ -207,32 +203,21 @@ public boolean matches(TypeDescription target) { return true; } - if (additionalLibraryMatcher && additionalLibraryIgnoreMatcher.matches(target)) { - return true; - } - - return false; + return ignoredTypeMatcher.matches(target); } @Override public String toString() { - return "globalIgnoresMatcher(" + additionalLibraryIgnoreMatcher.toString() + ")"; + return "globalIgnoresMatcher()"; } @Override public boolean equals(Object obj) { - if (obj == this) { - return true; - } - if (!(obj instanceof GlobalIgnoresMatcher)) { - return false; - } - GlobalIgnoresMatcher other = (GlobalIgnoresMatcher) obj; - return additionalLibraryIgnoreMatcher.equals(other.additionalLibraryIgnoreMatcher); + return obj instanceof GlobalIgnoresMatcher; } @Override public int hashCode() { - return additionalLibraryIgnoreMatcher.hashCode(); + return 17; } } diff --git a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcherTest.groovy b/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcherTest.groovy deleted file mode 100644 index 12d05636a739..000000000000 --- a/javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcherTest.groovy +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.tooling.matcher - - -import net.bytebuddy.description.type.TypeDescription -import spock.lang.Specification - -class AdditionalLibraryIgnoresMatcherTest extends Specification { - - def underTest = new AdditionalLibraryIgnoresMatcher() - - def "spring boot - match not instrumented class"() { - - setup: - def type = Mock(TypeDescription) - type.getActualName() >> typeName - - when: - def matches = underTest.matches(type) - - then: - matches == true - - where: - typeName << ["org.springframework.boot.NotInstrumentedClass", - "org.springframework.boot.embedded.tomcat.NotInstrumentedClass"] - } - - def "spring boot - don't match instrumented class"() { - - setup: - def type = Mock(TypeDescription) - type.getActualName() >> typeName - - when: - def matches = underTest.matches(type) - - then: - matches == false - - where: - typeName << ["org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext", - "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedWebappClassLoader"] - } - - def "spring boot - don't match instrumented inner class"() { - - setup: - def type = Mock(TypeDescription) - type.getActualName() >> typeName - - when: - def matches = underTest.matches(type) - - then: - matches == false - - where: - typeName << ["org.springframework.boot.autoconfigure.BackgroundPreinitializer\$InnerClass1", - "org.springframework.boot.autoconfigure.condition.OnClassCondition\$ConditionMatch"] - } - - def "logback - don't match logger and logging events"() { - setup: - def type = Mock(TypeDescription) - type.getActualName() >> typeName - - when: - def matches = underTest.matches(type) - - then: - !matches - - where: - typeName << [ - "ch.qos.logback.classic.Logger", - "ch.qos.logback.classic.spi.LoggingEvent", - "ch.qos.logback.classic.spi.LoggingEventVO" - ] - } -} diff --git a/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/ignore/TrieTest.java b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/ignore/TrieTest.java new file mode 100644 index 000000000000..b0964e0128aa --- /dev/null +++ b/javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/ignore/TrieTest.java @@ -0,0 +1,43 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.tooling.ignore; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import io.opentelemetry.javaagent.tooling.ignore.trie.Trie; +import org.junit.jupiter.api.Test; + +class TrieTest { + @Test + void shouldMatchExactString() { + Trie trie = + Trie.newBuilder().put("abc", 0).put("abcd", 10).put("abcde", 20).build(); + + assertNull(trie.getOrNull("ab")); + assertEquals(0, trie.getOrNull("abc")); + assertEquals(10, trie.getOrNull("abcd")); + assertEquals(20, trie.getOrNull("abcde")); + } + + @Test + void shouldReturnLastMatchedValue() { + Trie trie = + Trie.newBuilder().put("abc", 0).put("abcde", 10).put("abcdfgh", 20).build(); + + assertNull(trie.getOrNull("ababababa")); + assertEquals(0, trie.getOrNull("abcd")); + assertEquals(10, trie.getOrNull("abcdefgh")); + assertEquals(20, trie.getOrNull("abcdfghjkl")); + } + + @Test + void shouldOverwritePreviousValue() { + Trie trie = Trie.newBuilder().put("abc", 0).put("abc", 12).build(); + + assertEquals(12, trie.getOrNull("abc")); + } +} diff --git a/testing/agent-exporter/agent-exporter.gradle b/testing/agent-exporter/agent-exporter.gradle index bdb4dd276c80..3bb1ed971e80 100644 --- a/testing/agent-exporter/agent-exporter.gradle +++ b/testing/agent-exporter/agent-exporter.gradle @@ -13,6 +13,7 @@ dependencies { annotationProcessor "com.google.auto.service:auto-service" compileOnly "com.google.auto.service:auto-service" + implementation project(':instrumentation-api') implementation project(':javaagent-extension-api') implementation project(':javaagent-tooling') implementation "io.opentelemetry:opentelemetry-proto" diff --git a/testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/bytebuddy/TestAgentListener.java b/testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/bytebuddy/TestAgentListener.java index 813aae6deb85..1a3478039b33 100644 --- a/testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/bytebuddy/TestAgentListener.java +++ b/testing/agent-exporter/src/main/java/io/opentelemetry/javaagent/testing/bytebuddy/TestAgentListener.java @@ -5,7 +5,9 @@ package io.opentelemetry.javaagent.testing.bytebuddy; -import io.opentelemetry.javaagent.tooling.matcher.AdditionalLibraryIgnoresMatcher; +import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.javaagent.tooling.ignore.AdditionalLibraryIgnoredTypesConfigurer; +import io.opentelemetry.javaagent.tooling.ignore.IgnoredTypesBuilderImpl; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -26,8 +28,13 @@ public class TestAgentListener implements AgentBuilder.Listener { private static final Logger logger = LoggerFactory.getLogger(TestAgentListener.class); - private static final ElementMatcher.Junction GLOBAL_LIBRARIES_IGNORES_MATCHER = - AdditionalLibraryIgnoresMatcher.additionalLibraryIgnoresMatcher(); + private static final ElementMatcher GLOBAL_LIBRARIES_IGNORES_MATCHER; + + static { + IgnoredTypesBuilderImpl builder = new IgnoredTypesBuilderImpl(); + new AdditionalLibraryIgnoredTypesConfigurer().configure(Config.get(), builder); + GLOBAL_LIBRARIES_IGNORES_MATCHER = builder.buildIgnoredTypesMatcher(); + } public static void reset() { INSTANCE.transformedClassesNames.clear(); From 1f4f91bf4b1cedf5d0ba389b57dc9db760c6c887 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 8 Jun 2021 08:28:49 +0200 Subject: [PATCH 2/4] code review comments --- .../extension/ignore/IgnoredTypesBuilder.java | 11 +++++++---- .../tooling/ignore/IgnoredTypesBuilderImpl.java | 8 ++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java index f7a336187498..7dfb3e2d058c 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java @@ -27,7 +27,8 @@ public interface IgnoredTypesBuilder { /** * Allow the class or package specified by {@code classNameOrPrefix} to be instrumented. Calling - * this will overwrite any previous settings for passed prefix. + * this will overwrite any previous settings for passed prefix; in particular, calling this method + * will override any previous {@link #ignoreClass(String)} setting. * *

{@code classNameOrPrefix} can be the full class name (ex. {@code com.example.MyClass}), * package name (ex. {@code com.example.mypackage.}), or outer class name (ex {@code @@ -51,7 +52,8 @@ public interface IgnoredTypesBuilder { /** * Allow the class loader specified by {@code classNameOrPrefix} to be instrumented. Calling this - * will overwrite any previous settings for passed prefix. + * will overwrite any previous settings for passed prefix; in particular, calling this method will + * override any previous {@link #ignoreClassLoader(String)} setting. * *

{@code classNameOrPrefix} can be the full class name (ex. {@code com.example.MyClass}), * package name (ex. {@code com.example.mypackage.}), or outer class name (ex {@code @@ -73,7 +75,7 @@ public interface IgnoredTypesBuilder { *

  • {@link java.util.concurrent.Future} * * - * Calling this will overwrite any previous settings for passed prefix. + *

    Calling this will overwrite any previous settings for passed prefix. * *

    {@code classNameOrPrefix} can be the full class name (ex. {@code com.example.MyClass}), * package name (ex. {@code com.example.mypackage.}), or outer class name (ex {@code @@ -94,7 +96,8 @@ public interface IgnoredTypesBuilder { *

  • {@link java.util.concurrent.Future} * * - * Calling this will overwrite any previous settings for passed prefix. + *

    Calling this will will overwrite any previous settings for passed prefix; in particular, + * calling this method will override any previous {@link #ignoreTaskClass(String)} setting. * *

    {@code classNameOrPrefix} can be the full class name (ex. {@code com.example.MyClass}), * package name (ex. {@code com.example.mypackage.}), or outer class name (ex {@code diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesBuilderImpl.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesBuilderImpl.java index 8e9c2380d4b7..74639ffdbe0e 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesBuilderImpl.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/IgnoredTypesBuilderImpl.java @@ -28,25 +28,25 @@ public IgnoredTypesBuilder allowClass(String className) { @Override public IgnoredTypesBuilder ignoreClassLoader(String classNameOrPrefix) { // TODO: collect classloader classes into a separate trie - return this; + throw new UnsupportedOperationException("not implemented yet"); } @Override public IgnoredTypesBuilder allowClassLoader(String classNameOrPrefix) { // TODO: collect classloader classes into a separate trie - return this; + throw new UnsupportedOperationException("not implemented yet"); } @Override public IgnoredTypesBuilder ignoreTaskClass(String className) { // TODO: collect task classes into a separate trie - return this; + throw new UnsupportedOperationException("not implemented yet"); } @Override public IgnoredTypesBuilder allowTaskClass(String className) { // TODO: collect task classes into a separate trie - return this; + throw new UnsupportedOperationException("not implemented yet"); } public ElementMatcher buildIgnoredTypesMatcher() { From 13a2cd0009f35cb7c920e18a2212a9ebbe94188c Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Thu, 10 Jun 2021 10:29:23 +0200 Subject: [PATCH 3/4] code review comments --- .../opentelemetry/benchmark/IgnoredTypesMatcherBenchmark.java | 2 +- .../javaagent/extension/ignore/IgnoredTypesBuilder.java | 2 +- .../javaagent/extension/ignore/IgnoredTypesConfigurer.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/benchmark/src/jmh/java/io/opentelemetry/benchmark/IgnoredTypesMatcherBenchmark.java b/benchmark/src/jmh/java/io/opentelemetry/benchmark/IgnoredTypesMatcherBenchmark.java index e6cf93d87605..45da257a95f4 100644 --- a/benchmark/src/jmh/java/io/opentelemetry/benchmark/IgnoredTypesMatcherBenchmark.java +++ b/benchmark/src/jmh/java/io/opentelemetry/benchmark/IgnoredTypesMatcherBenchmark.java @@ -23,7 +23,7 @@ @Warmup(iterations = 3, time = 1) @Measurement(iterations = 10, time = 1) @OutputTimeUnit(TimeUnit.MICROSECONDS) -@BenchmarkMode(Mode.Throughput) +@BenchmarkMode(Mode.AverageTime) public class IgnoredTypesMatcherBenchmark { private static final TypeDescription springType = diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java index 7dfb3e2d058c..ba33f6f39014 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesBuilder.java @@ -9,7 +9,7 @@ * This interface defines different ways to ignore/allow instrumenting classes or packages. * *

    This interface should not be implemented by the javaagent extension developer - the javaagent - * will provide the implementation of all transformations described here. + * will provide the implementation. */ public interface IgnoredTypesBuilder { diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesConfigurer.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesConfigurer.java index 788b262c990b..35972ea80a66 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesConfigurer.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ignore/IgnoredTypesConfigurer.java @@ -9,7 +9,7 @@ import io.opentelemetry.javaagent.extension.Ordered; /** - * An {@link IgnoredTypesConfigurer} can be used to override built-in instrumentation restrictions: + * An {@link IgnoredTypesConfigurer} can be used to augment built-in instrumentation restrictions: * ignore some classes and exclude them from being instrumented, or explicitly allow them to be * instrumented if the agent ignored them by default. * From 4ee8d8e75feab1bedf36a0283106e1919e6ec419 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Fri, 11 Jun 2021 09:13:12 +0200 Subject: [PATCH 4/4] spring-webflux latestDepTest fix --- .../ignore/AdditionalLibraryIgnoredTypesConfigurer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/AdditionalLibraryIgnoredTypesConfigurer.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/AdditionalLibraryIgnoredTypesConfigurer.java index 78a36a6a7897..9d2f12c40889 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/AdditionalLibraryIgnoredTypesConfigurer.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/ignore/AdditionalLibraryIgnoredTypesConfigurer.java @@ -109,7 +109,8 @@ public void configure(Config config, IgnoredTypesBuilder builder) { .allowClass("org.springframework.boot.web.embedded.tomcat.TomcatWebServer$") .allowClass("org.springframework.boot.web.embedded.tomcat.TomcatEmbeddedWebappClassLoader") .allowClass("org.springframework.boot.web.servlet.DelegatingFilterProxyRegistrationBean$") - .allowClass("org.springframework.boot.StartupInfoLogger$"); + .allowClass("org.springframework.boot.StartupInfoLogger$") + .allowClass("org.springframework.boot.SpringApplicationShutdownHook"); builder .ignoreClass("org.springframework.cglib.")