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

Refactor Java concurrency instrumentations #7101

Merged
merged 5 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
import static net.bytebuddy.matcher.ElementMatchers.isTypeInitializer;

Expand All @@ -27,7 +28,7 @@ public final class AsyncPropagatingDisableInstrumentation extends InstrumenterMo
implements Instrumenter.CanShortcutTypeMatching {

public AsyncPropagatingDisableInstrumentation() {
super("java_concurrent");
super(EXECUTOR_INSTRUMENTATION_NAME);
}

private static final ElementMatcher.Junction<TypeDescription> RX_WORKERS =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package datadog.trace.instrumentation.java.concurrent;

/** Defines common instrumentation names. */
public final class ConcurrentInstrumentationNames {
public static final String EXECUTOR_INSTRUMENTATION_NAME = "java_concurrent";
public static final String FORK_JOIN_POOL_INSTRUMENTATION_NAME = "fjp";
public static final String RUNNABLE_INSTRUMENTATION_NAME = "runnable";
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package datadog.trace.instrumentation.java.concurrent;

import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
Expand All @@ -11,7 +13,7 @@
public class TaskUnwrappingInstrumentation extends InstrumenterModule.Profiling
implements Instrumenter.ForKnownTypes, Instrumenter.HasTypeAdvice {
public TaskUnwrappingInstrumentation() {
super("java_concurrent", "task-unwrapping");
super(EXECUTOR_INSTRUMENTATION_NAME, "task-unwrapping");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.exclude;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.not;
Expand All @@ -25,7 +26,7 @@
public final class WrapRunnableAsNewTaskInstrumentation extends InstrumenterModule.Tracing
implements Instrumenter.ForBootstrap, Instrumenter.ForKnownTypes {
public WrapRunnableAsNewTaskInstrumentation() {
super("java_concurrent", "new-task-for");
super(EXECUTOR_INSTRUMENTATION_NAME, "new-task-for");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.executor;

import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;

import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
Expand All @@ -20,13 +21,11 @@ public abstract class AbstractExecutorInstrumentation extends InstrumenterModule

private static final Logger log = LoggerFactory.getLogger(AbstractExecutorInstrumentation.class);

public static final String EXEC_NAME = "java_concurrent";

/** To apply to all executors, use override setting below. */
private final boolean TRACE_ALL_EXECUTORS = InstrumenterConfig.get().isTraceExecutorsAll();

public AbstractExecutorInstrumentation(final String... additionalNames) {
super(EXEC_NAME, additionalNames);
super(EXECUTOR_INSTRUMENTATION_NAME, additionalNames);

if (TRACE_ALL_EXECUTORS) {
log.warn("Tracing all executors enabled. This is not a recommended setting.");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.executor;

import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.executor;

import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
Expand All @@ -14,7 +15,7 @@
public final class NonStandardExecutorInstrumentation extends AbstractExecutorInstrumentation {

public NonStandardExecutorInstrumentation() {
super(EXEC_NAME + ".other");
super(EXECUTOR_INSTRUMENTATION_NAME + ".other");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.executor;

import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.nameEndsWith;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.cancelTask;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;

Expand All @@ -29,7 +30,7 @@ public class RejectedExecutionHandlerInstrumentation extends InstrumenterModule.
implements Instrumenter.ForBootstrap, Instrumenter.CanShortcutTypeMatching {

public RejectedExecutionHandlerInstrumentation() {
super("java_concurrent", "rejected-execution-handler");
super(EXECUTOR_INSTRUMENTATION_NAME, "rejected-execution-handler");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.executor;

import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE_FUTURE;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.exclude;
import static datadog.trace.instrumentation.java.concurrent.AbstractExecutorInstrumentation.EXEC_NAME;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
Expand Down Expand Up @@ -76,7 +76,7 @@ public final class ThreadPoolExecutorInstrumentation extends InstrumenterModule.
namedOneOf("org.elasticsearch.common.util.concurrent.EsThreadPoolExecutor")));

public ThreadPoolExecutorInstrumentation() {
super(EXEC_NAME);
super(EXECUTOR_INSTRUMENTATION_NAME);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.forkjoin;

import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.cancelTask;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.capture;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.FORK_JOIN_TASK;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.exclude;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.FORK_JOIN_POOL_INSTRUMENTATION_NAME;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;

Expand All @@ -26,7 +28,7 @@ public class JavaForkJoinPoolInstrumentation extends InstrumenterModule.Tracing
implements Instrumenter.ForBootstrap, Instrumenter.ForSingleType {

public JavaForkJoinPoolInstrumentation() {
super("java_concurrent", "fjp");
super(EXECUTOR_INSTRUMENTATION_NAME, FORK_JOIN_POOL_INSTRUMENTATION_NAME);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.forkjoin;

import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.declaresMethod;
import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass;
Expand All @@ -11,6 +11,8 @@
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.FORK_JOIN_TASK;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE_FUTURE;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.exclude;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.FORK_JOIN_POOL_INSTRUMENTATION_NAME;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;

Expand Down Expand Up @@ -41,7 +43,7 @@ public final class JavaForkJoinTaskInstrumentation extends InstrumenterModule.Tr
implements Instrumenter.ForBootstrap, Instrumenter.ForTypeHierarchy, ExcludeFilterProvider {

public JavaForkJoinTaskInstrumentation() {
super(AbstractExecutorInstrumentation.EXEC_NAME);
super(EXECUTOR_INSTRUMENTATION_NAME, FORK_JOIN_POOL_INSTRUMENTATION_NAME);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.runnable;

import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.endTaskScope;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.startTaskScope;
import static datadog.trace.instrumentation.java.concurrent.AbstractExecutorInstrumentation.EXEC_NAME;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;

Expand All @@ -18,11 +18,16 @@
import java.util.concurrent.ForkJoinTask;
import net.bytebuddy.asm.Advice;

/**
* Instrument the Runnable implementation of {@code ConsumerTask} (JDK 9+) where the ForkJoinTask
* parent class is already instrumented by {@link
* datadog.trace.instrumentation.java.concurrent.forkjoin.JavaForkJoinTaskInstrumentation}
*/
@AutoService(InstrumenterModule.class)
public class ConsumerTaskInstrumentation extends InstrumenterModule.Tracing
implements Instrumenter.ForBootstrap, Instrumenter.ForSingleType {
public ConsumerTaskInstrumentation() {
super(EXEC_NAME, "consumer-task");
super(EXECUTOR_INSTRUMENTATION_NAME, "consumer-task");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.runnable;

import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.nameEndsWith;
Expand All @@ -11,6 +11,7 @@
import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.startTaskScope;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE_FUTURE;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
Expand Down Expand Up @@ -38,7 +39,7 @@
public final class RunnableFutureInstrumentation extends InstrumenterModule.Tracing
implements Instrumenter.ForBootstrap, Instrumenter.ForTypeHierarchy, ExcludeFilterProvider {
public RunnableFutureInstrumentation() {
super("java_concurrent", "runnable-future");
super(EXECUTOR_INSTRUMENTATION_NAME, "runnable-future");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.runnable;

import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.notExcludedByName;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.RUNNABLE_INSTRUMENTATION_NAME;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.not;
Expand All @@ -29,7 +31,7 @@ public final class RunnableInstrumentation extends InstrumenterModule.Tracing
implements Instrumenter.ForBootstrap, Instrumenter.ForTypeHierarchy {

public RunnableInstrumentation() {
super(AbstractExecutorInstrumentation.EXEC_NAME, "runnable");
super(EXECUTOR_INSTRUMENTATION_NAME, RUNNABLE_INSTRUMENTATION_NAME);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.timer;

import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.cancelTask;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils.capture;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.exclude;
import static datadog.trace.bootstrap.instrumentation.java.concurrent.QueueTimerHelper.startQueuingTimer;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.RUNNABLE_INSTRUMENTATION_NAME;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.isPrivate;
Expand All @@ -27,7 +29,7 @@
public class JavaTimerInstrumentation extends InstrumenterModule.Tracing
implements Instrumenter.ForBootstrap, Instrumenter.ForSingleType {
public JavaTimerInstrumentation() {
super("java_timer", "java_concurrent", "runnable");
super("java_timer", EXECUTOR_INSTRUMENTATION_NAME, RUNNABLE_INSTRUMENTATION_NAME);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package datadog.trace.instrumentation.java.concurrent;
package datadog.trace.instrumentation.java.concurrent.timer;

import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass;
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.EXECUTOR_INSTRUMENTATION_NAME;
import static datadog.trace.instrumentation.java.concurrent.ConcurrentInstrumentationNames.RUNNABLE_INSTRUMENTATION_NAME;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
Expand All @@ -12,6 +14,7 @@
import datadog.trace.bootstrap.InstrumentationContext;
import datadog.trace.bootstrap.instrumentation.java.concurrent.AdviceUtils;
import datadog.trace.bootstrap.instrumentation.java.concurrent.State;
import datadog.trace.instrumentation.java.concurrent.runnable.RunnableInstrumentation;
import java.util.Map;
import java.util.TimerTask;
import net.bytebuddy.asm.Advice;
Expand All @@ -29,7 +32,7 @@ public final class TimerTaskInstrumentation extends InstrumenterModule.Tracing
implements Instrumenter.ForBootstrap, Instrumenter.ForTypeHierarchy {

public TimerTaskInstrumentation() {
super("java_timer", AbstractExecutorInstrumentation.EXEC_NAME, "runnable");
super("java_timer", EXECUTOR_INSTRUMENTATION_NAME, RUNNABLE_INSTRUMENTATION_NAME);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import io.netty.channel.nio.NioEventLoopGroup
import io.netty.channel.oio.OioEventLoopGroup
import io.netty.util.concurrent.DefaultEventExecutor
import org.apache.tomcat.util.threads.TaskQueue
import runnable.Descendant
import spock.lang.Shared

import java.util.concurrent.ExecutorService
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
package executor

import com.google.common.util.concurrent.MoreExecutors
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.Trace
import datadog.trace.bootstrap.instrumentation.api.Tags
import datadog.trace.bootstrap.instrumentation.java.concurrent.RunnableWrapper
import datadog.trace.core.DDSpan
import forkjoin.PeriodicTask
import org.apache.tomcat.util.threads.TaskQueue
import runnable.ComparableAsyncChild
import runnable.JavaAsyncChild
import spock.lang.Shared

import java.lang.reflect.InvocationTargetException
Expand Down Expand Up @@ -63,8 +68,8 @@ abstract class ExecutorInstrumentationTest extends AgentTestRunner {
void configurePreAgent() {
super.configurePreAgent()

injectSysConfig("dd.trace.executors", "CustomThreadPoolExecutor")
injectSysConfig("trace.thread-pool-executors.exclude", "ExecutorInstrumentationTest\$ToBeIgnoredExecutor")
injectSysConfig("dd.trace.executors", CustomThreadPoolExecutor.name)
injectSysConfig("trace.thread-pool-executors.exclude", ToBeIgnoredExecutor.name)
}

def "#poolName '#name' propagates"() {
Expand Down Expand Up @@ -482,12 +487,6 @@ abstract class ExecutorInstrumentationTest extends AgentTestRunner {

poolName = poolImpl.class.simpleName
}

static class ToBeIgnoredExecutor extends ThreadPoolExecutor {
ToBeIgnoredExecutor() {
super(1, 1, 0, TimeUnit.MILLISECONDS, new ArrayBlockingQueue<Runnable>(10))
}
}
}

class ExecutorInstrumentationForkedTest extends ExecutorInstrumentationTest {
Expand Down
Loading
Loading