Merge pull request #4343 from DataDog/rgs/evolve-filter-api

use native threadid for attach/detach events
DataDog · Nov 30, 2022 · eb2f991 · eb2f991
2 parents 60e9e38 + 897b670
commit eb2f991
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 36 deletions.
diff --git a/...nt-profiling/profiling-async/src/main/java/com/datadog/profiling/async/AsyncProfiler.java b/...nt-profiling/profiling-async/src/main/java/com/datadog/profiling/async/AsyncProfiler.java
@@ -172,15 +172,15 @@ private static one.profiler.AsyncProfiler inferFromOsAndArch()
             arch, os));
   }
 
-  void addCurrentThread() {
-    if (asyncProfiler != null) {
-      asyncProfiler.addThread(Thread.currentThread());
+  void addThread(int tid) {
+    if (asyncProfiler != null && tid >= 0) {
+      asyncProfiler.addThread(tid);
     }
   }
 
-  void removeCurrentThread() {
-    if (asyncProfiler != null) {
-      asyncProfiler.removeThread(Thread.currentThread());
+  void removeThread(int tid) {
+    if (asyncProfiler != null && tid >= 0) {
+      asyncProfiler.removeThread(tid);
     }
   }
 
@@ -408,7 +408,7 @@ private int clamp(int min, int max, int value) {
   }
 
   public void setContext(int tid, long spanId, long rootSpanId) {
-    if (asyncProfiler != null) {
+    if (asyncProfiler != null && tid >= 0) {
       try {
         asyncProfiler.setContext(tid, spanId, rootSpanId);
       } catch (IllegalStateException e) {

diff --git a/...filing/profiling-async/src/main/java/com/datadog/profiling/async/ContextThreadFilter.java b/...filing/profiling-async/src/main/java/com/datadog/profiling/async/ContextThreadFilter.java
@@ -11,16 +11,16 @@ public class ContextThreadFilter implements ProfilingContextIntegration {
   private static final AsyncProfiler ASYNC_PROFILER = AsyncProfiler.getInstance();
 
   @Override
-  public void onAttach() {
+  public void onAttach(int tid) {
     if (AsyncProfilerConfig.isWallClockProfilerEnabled()) {
-      ASYNC_PROFILER.addCurrentThread();
+      ASYNC_PROFILER.addThread(tid);
     }
   }
 
   @Override
-  public void onDetach() {
+  public void onDetach(int tid) {
     if (AsyncProfilerConfig.isWallClockProfilerEnabled()) {
-      ASYNC_PROFILER.removeCurrentThread();
+      ASYNC_PROFILER.removeThread(tid);
     }
   }
 

diff --git a/...t/testing/src/main/groovy/datadog/trace/agent/test/TestProfilingContextIntegration.groovy b/...t/testing/src/main/groovy/datadog/trace/agent/test/TestProfilingContextIntegration.groovy
@@ -8,12 +8,12 @@ class TestProfilingContextIntegration implements ProfilingContextIntegration {
   final AtomicInteger attachments = new AtomicInteger()
   final AtomicInteger detachments = new AtomicInteger()
   @Override
-  void onAttach() {
+  void onAttach(int tid) {
     attachments.incrementAndGet()
   }
 
   @Override
-  void onDetach() {
+  void onDetach(int tid) {
     detachments.incrementAndGet()
   }
 

diff --git a/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java b/dd-trace-core/src/main/java/datadog/trace/core/scopemanager/ContinuableScopeManager.java
@@ -471,16 +471,6 @@ static final class ScopeStackThreadLocal extends ThreadLocal<ScopeStack> {
     protected ScopeStack initialValue() {
       return new ScopeStack(profilingContextIntegration);
     }
-
-    @Override
-    public void remove() {
-      detach();
-      super.remove();
-    }
-
-    private void detach() {
-      profilingContextIntegration.onDetach();
-    }
   }
 
   /**
@@ -600,13 +590,13 @@ private void onTopChanged(ContinuableScope top) {
 
     /** Notifies context thread listeners that this thread has a context now */
     private void onBecomeNonEmpty() {
-      profilingContextIntegration.onAttach();
+      profilingContextIntegration.onAttach(nativeThreadId);
     }
 
     /** Notifies context thread listeners that this thread no longer has a context */
     private void onBecomeEmpty() {
       profilingContextIntegration.setContext(nativeThreadId, 0, 0);
-      profilingContextIntegration.onDetach();
+      profilingContextIntegration.onDetach(nativeThreadId);
     }
   }
 

diff --git a/dd-trace-core/src/test/groovy/datadog/trace/core/scopemanager/ScopeManagerTest.groovy b/dd-trace-core/src/test/groovy/datadog/trace/core/scopemanager/ScopeManagerTest.groovy
@@ -468,7 +468,7 @@ class ScopeManagerTest extends DDCoreSpecification {
     1 * statsDClient.incrementCounter("scope.close.error")
     1 * rootSpanCheckpointer.onRootSpanStarted(_)
     3 * listener.setContext(_, _, _)
-    1 * listener.onAttach()
+    1 * listener.onAttach(_)
     0 * _
 
     when:
@@ -479,7 +479,7 @@ class ScopeManagerTest extends DDCoreSpecification {
     1 * rootSpanCheckpointer.onRootSpanFinished(_, _)
     _ * statsDClient.close()
     1 * listener.setContext(_, 0, 0)
-    1 * listener.onDetach()
+    1 * listener.onDetach(_)
     assertEvents([ACTIVATE, ACTIVATE, CLOSE, CLOSE])
     0 * _
 
@@ -506,7 +506,7 @@ class ScopeManagerTest extends DDCoreSpecification {
     tracer.activeScope() == firstScope
     assertEvents([ACTIVATE])
     1 * rootSpanCheckpointer.onRootSpanStarted(_)
-    1 * listener.onAttach()
+    1 * listener.onAttach(_)
     1 * listener.setContext(_, _, _)
     0 * _
 
@@ -584,7 +584,7 @@ class ScopeManagerTest extends DDCoreSpecification {
     ])
     _ * statsDClient.close()
     1 * listener.setContext(_, 0, 0)
-    1 * listener.onDetach()
+    1 * listener.onDetach(_)
     0 * _
   }
 
@@ -970,7 +970,7 @@ class ScopeManagerTest extends DDCoreSpecification {
       assert scopeManager.active() == null
     }).get()
     then: "the listener is not notified"
-    0 * listener.onAttach()
+    0 * listener.onAttach(_)
 
     when: "scopes activate on threads"
     AgentSpan span = tracer.buildSpan("foo").start()
@@ -991,7 +991,7 @@ class ScopeManagerTest extends DDCoreSpecification {
     }
 
     then: "the first activation notifies the listener"
-    numThreads * listener.onAttach()
+    numThreads * listener.onAttach(_)
     _ * _
 
     cleanup:

diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle
@@ -33,7 +33,7 @@ final class CachedData {
     testcontainers: '1.17.3',
     jmc           : "8.1.0-SNAPSHOT",
     autoservice   : "1.0-rc7",
-    asyncprofiler : "2.8.3-DD-20221128_1",
+    asyncprofiler : "2.8.3-DD-20221129",
     asm           : "9.2"
   ]
 

diff --git a/...rc/main/java/datadog/trace/bootstrap/instrumentation/api/ProfilingContextIntegration.java b/...rc/main/java/datadog/trace/bootstrap/instrumentation/api/ProfilingContextIntegration.java
@@ -2,10 +2,10 @@
 
 public interface ProfilingContextIntegration {
   /** Invoked when a trace first propagates to a thread */
-  void onAttach();
+  void onAttach(int tid);
 
   /** Invoked when a thread exits */
-  void onDetach();
+  void onDetach(int tid);
 
   void setContext(int tid, long rootSpanId, long spanId);
 
@@ -16,10 +16,10 @@ final class NoOp implements ProfilingContextIntegration {
     public static final NoOp INSTANCE = new NoOp();
 
     @Override
-    public void onAttach() {}
+    public void onAttach(int tid) {}
 
     @Override
-    public void onDetach() {}
+    public void onDetach(int tid) {}
 
     @Override
     public void setContext(int tid, long rootSpanId, long spanId) {}