Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce IgnoredTypesConfigurer SPI to enable defining per-module ignores #3219

Merged
merged 5 commits into from
Jun 11, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions benchmark/benchmark.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
Original file line number Diff line number Diff line change
@@ -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)
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved
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<TypeDescription> 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);
}
}
Original file line number Diff line number Diff line change
@@ -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.
*
* <p>This interface should not be implemented by the javaagent extension developer - the javaagent
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved
* 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.
*
* <p>{@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.
*
* <p>{@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);
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved

/**
* Ignore the class loader specified by {@code classNameOrPrefix} and exclude it from being
* instrumented. Calling this will overwrite any previous settings for passed prefix.
*
* <p>{@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.
*
* <p>{@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:
*
* <ul>
* <li>{@link java.lang.Runnable}
* <li>{@link java.util.concurrent.Callable}
* <li>{@link java.util.concurrent.ForkJoinTask}
* <li>{@link java.util.concurrent.Future}
* </ul>
*
* Calling this will overwrite any previous settings for passed prefix.
*
* <p>{@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:
*
* <ul>
* <li>{@link java.lang.Runnable}
* <li>{@link java.util.concurrent.Callable}
* <li>{@link java.util.concurrent.ForkJoinTask}
* <li>{@link java.util.concurrent.Future}
* </ul>
*
* Calling this will overwrite any previous settings for passed prefix.
*
* <p>{@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);
}
Original file line number Diff line number Diff line change
@@ -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:
mateuszrzeszutek marked this conversation as resolved.
Show resolved Hide resolved
* ignore some classes and exclude them from being instrumented, or explicitly allow them to be
* instrumented if the agent ignored them by default.
*
* <p>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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for a separate interface instead of knob on InstrumentationModule?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought that it'd be useful to be able ignore some classes (e.g. task classes to prevent context leaking) even if you turn the instrumentation off.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm - well we could choose to respet them even if it's disabled? Not sure if it's a great idea - IIRC, dd-trace-java has the knob on InstrumentationModule so it could keep us more consistent with them, though maybe that's a difficult goal at this point

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I understand, they have something similar just for concurrent/task classes: https://github.com/DataDog/dd-trace-java/blob/master/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ExcludeFilterProvider.java
And they're doing a separate pass over Instrumenters and check with instanceof if this interface is implemented.

it could keep us more consistent with them, though maybe that's a difficult goal at this point

With all our changes to muzzle, the split to InstrumentationModule/TypeInstrumentation, and DD's custom things (like profiling) it's probably impossible 😅

Hmm - well we could choose to respet them even if it's disabled?

That's what seemed a bit confusing to me - you have the enabled flag but it only works for some InstrumentationModule methods.


/**
* Configure the passed {@code builder} and define which classes should be ignored when
* instrumenting.
*/
void configure(Config config, IgnoredTypesBuilder builder);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@
*
* <p>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 {
Comment on lines +23 to 24
Copy link
Member

Choose a reason for hiding this comment

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

always nice to remove something when adding something 😄👍


/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, List<Runnable>> CLASS_LOAD_CALLBACKS = new HashMap<>();
private static volatile Instrumentation instrumentation;

Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -180,6 +174,14 @@ public static ResettableClassFileTransformer installBytebuddyAgent(
return resettableClassFileTransformer;
}

private static ElementMatcher<TypeDescription> 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<AgentListener> agentListeners, Config config) {
for (AgentListener agentListener : agentListeners) {
Expand Down Expand Up @@ -248,6 +250,7 @@ private static void addByteBuddyRawSetting() {
}
}

// TODO: rewrite to an IgnoredTypesConfigurer
private static ElementMatcher.Junction<? super TypeDescription> matchesConfiguredExcludes() {
List<String> excludedClasses = Config.get().getListProperty(EXCLUDED_CLASSES_CONFIG);
ElementMatcher.Junction<? super TypeDescription> matcher = none();
Expand Down
Loading