Skip to content

Commit

Permalink
Refactor Java concurrency instrumentations (#7101)
Browse files Browse the repository at this point in the history
* feat(concurrent): Refactor instrumentations per feature
* chore(concurrent): Move Java classes
* feat(concurrency): Refactor test suites
* feat(concurrency): Unify instrumentation names
* feat(concurrency): Introduce instrumentation names constant class
  • Loading branch information
PerfectSlayer committed Jul 9, 2024
1 parent 5a09529 commit 9e7aa67
Show file tree
Hide file tree
Showing 40 changed files with 128 additions and 39 deletions.
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

0 comments on commit 9e7aa67

Please sign in to comment.