Repository: pinpoint-apm/pinpoint
Issue: Dubbo consumer interceptor does not propagate disable sampling flag to remote nodes
Labels: bug
module:plugin

#### Description
`DubboConsumerInterceptor` checks for the current trace using `TraceContext.currentTraceObject()`. This API returns the current trace object only if the current trace should be sampled. It returns null when no trace object exists **or** when the current trace shouldn't be sampled. So even if a disabled trace object exists, it doesn't propagate this information over RPC.
We should instead use `TraceContext.currentRawTraceObject()` and check for sampling with the returned trace object.

issue found from #2989 

Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/plugins/dubbo/src/main/java/com/navercorp/pinpoint/plugin/dubbo/interceptor/DubboConsumerInterceptor.java b/plugins/dubbo/src/main/java/com/navercorp/pinpoint/plugin/dubbo/interceptor/DubboConsumerInterceptor.java
index 70847b4ccd92..37508a894ddc 100644
--- a/plugins/dubbo/src/main/java/com/navercorp/pinpoint/plugin/dubbo/interceptor/DubboConsumerInterceptor.java
+++ b/plugins/dubbo/src/main/java/com/navercorp/pinpoint/plugin/dubbo/interceptor/DubboConsumerInterceptor.java
@@ -22,7 +22,12 @@ public DubboConsumerInterceptor(TraceContext traceContext, MethodDescriptor desc
 
     @Override
     public void before(Object target, Object arg0) {
-        Trace trace = this.getTrace(target);
+        // Ignore monitor service
+        if (isMonitorService(target)) {
+            return;
+        }
+
+        Trace trace = traceContext.currentRawTraceObject();
         if (trace == null) {
             return;
         }
@@ -58,7 +63,12 @@ public void before(Object target, Object arg0) {
 
     @Override
     public void after(Object target, Object arg0, Object result, Throwable throwable) {
-        Trace trace = this.getTrace(target);
+        // Ignore monitor service
+        if (isMonitorService(target)) {
+            return;
+        }
+
+        Trace trace = traceContext.currentTraceObject();
         if (trace == null) {
             return;
         }
@@ -87,14 +97,11 @@ public void after(Object target, Object arg0, Object result, Throwable throwable
         }
     }
 
-    private Trace getTrace(Object target) {
-        Invoker invoker = (Invoker) target;
-        // Ignore monitor service.
-        if (DubboConstants.MONITOR_SERVICE_FQCN.equals(invoker.getInterface().getName())) {
-            return null;
+    private boolean isMonitorService(Object target) {
+        if (target instanceof Invoker) {
+            Invoker invoker = (Invoker) target;
+            return DubboConstants.MONITOR_SERVICE_FQCN.equals(invoker.getInterface().getName());
         }
-
-        return traceContext.currentRawTraceObject();
+        return false;
     }
-
 }
diff --git a/plugins/dubbo/src/main/java/com/navercorp/pinpoint/plugin/dubbo/interceptor/DubboProviderInterceptor.java b/plugins/dubbo/src/main/java/com/navercorp/pinpoint/plugin/dubbo/interceptor/DubboProviderInterceptor.java
index 422d19901f16..b1d2d57bfa2d 100644
--- a/plugins/dubbo/src/main/java/com/navercorp/pinpoint/plugin/dubbo/interceptor/DubboProviderInterceptor.java
+++ b/plugins/dubbo/src/main/java/com/navercorp/pinpoint/plugin/dubbo/interceptor/DubboProviderInterceptor.java
@@ -37,13 +37,10 @@ protected Trace createTrace(Object target, Object[] args) {
         String transactionId = invocation.getAttachment(DubboConstants.META_TRANSACTION_ID);
 
         // If there's no trasanction id, a new trasaction begins here.
+        // FIXME There seems to be cases where the invoke method is called after a span is already created.
+        // We'll have to check if a trace object already exists and create a span event instead of a span in that case.
         if (transactionId == null) {
-            Trace trace = traceContext.currentTraceObject();
-            if (trace == null) {
-                return traceContext.newTraceObject();
-            } else {
-                return trace;
-            }
+            return traceContext.newTraceObject();
         }
 
         // otherwise, continue tracing with given data.


Test patch: