Merge pull request #2457 from DataDog/xgouchet/RUM-7549/trace_coroutines

RUM-7549 Handle trace with coroutines
DataDog · Dec 17, 2024 · b6df7de · b6df7de
2 parents 9f3d758 + 340a1d1
commit b6df7de
Show file tree

Hide file tree

Showing 14 changed files with 229 additions and 52 deletions.
diff --git a/features/dd-sdk-android-trace/api/dd-sdk-android-trace.api b/features/dd-sdk-android-trace/api/dd-sdk-android-trace.api
@@ -988,7 +988,7 @@ public class com/datadog/opentracing/DDSpanContext : io/opentracing/SpanContext
 	public static final field ORIGIN_KEY Ljava/lang/String;
 	public static final field PRIORITY_SAMPLING_KEY Ljava/lang/String;
 	public static final field SAMPLE_RATE_KEY Ljava/lang/String;
-	public fun <init> (Ljava/math/BigInteger;Ljava/math/BigInteger;Ljava/math/BigInteger;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/String;Ljava/util/Map;ZLjava/lang/String;Ljava/util/Map;Lcom/datadog/opentracing/PendingTrace;Lcom/datadog/opentracing/DDTracer;Ljava/util/Map;)V
+	public fun <init> (Ljava/math/BigInteger;Ljava/math/BigInteger;Ljava/math/BigInteger;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/String;Ljava/util/Map;ZLjava/lang/String;Ljava/util/Map;Lcom/datadog/opentracing/PendingTrace;Lcom/datadog/opentracing/DDTracer;Ljava/util/Map;Lcom/datadog/android/api/InternalLogger;)V
 	public fun baggageItems ()Ljava/lang/Iterable;
 	public fun getBaggageItem (Ljava/lang/String;)Ljava/lang/String;
 	public fun getBaggageItems ()Ljava/util/Map;
@@ -1071,6 +1071,7 @@ public class com/datadog/opentracing/DDTracer$DDSpanBuilder : io/opentracing/Tra
 	public fun startActive (Z)Lio/opentracing/Scope;
 	public fun startManual ()Lio/opentracing/Span;
 	public fun withErrorFlag ()Lcom/datadog/opentracing/DDTracer$DDSpanBuilder;
+	public fun withInternalLogger (Lcom/datadog/android/api/InternalLogger;)Lcom/datadog/opentracing/DDTracer$DDSpanBuilder;
 	public fun withLogHandler (Lcom/datadog/opentracing/LogHandler;)Lcom/datadog/opentracing/DDTracer$DDSpanBuilder;
 	public fun withOrigin (Ljava/lang/String;)Lcom/datadog/opentracing/DDTracer$DDSpanBuilder;
 	public fun withResourceName (Ljava/lang/String;)Lcom/datadog/opentracing/DDTracer$DDSpanBuilder;

diff --git a/features/dd-sdk-android-trace/src/main/java/com/datadog/opentracing/DDSpan.java b/features/dd-sdk-android-trace/src/main/java/com/datadog/opentracing/DDSpan.java
@@ -6,6 +6,7 @@
 
 package com.datadog.opentracing;
 
+import com.datadog.android.api.InternalLogger;
 import com.datadog.legacy.trace.api.DDTags;
 import com.datadog.legacy.trace.api.interceptor.MutableSpan;
 import com.datadog.legacy.trace.api.sampling.PrioritySampling;
@@ -57,14 +58,17 @@ public class DDSpan implements Span, MutableSpan {
   /** Implementation detail. Stores the weak reference to this span. Used by TraceCollection. */
   volatile WeakReference<DDSpan> ref;
 
+  /** The internal logger to report warnings/errors in the span lifecycle. */
+  private final InternalLogger internalLogger;
+
   /**
    * Spans should be constructed using the builder, not by calling the constructor directly.
    *
    * @param timestampMicro if greater than zero, use this time instead of the current time
    * @param context the context used for the span
    */
   DDSpan(final long timestampMicro, final DDSpanContext context) {
-    this(timestampMicro, context, new DefaultLogHandler());
+    this(timestampMicro, context, new DefaultLogHandler(), InternalLogger.Companion.getUNBOUND());
   }
 
   /**
@@ -73,10 +77,17 @@ public class DDSpan implements Span, MutableSpan {
    * @param timestampMicro if greater than zero, use this time instead of the current time
    * @param context the context used for the span
    * @param logHandler as the handler where to delegate the log actions
+   * @param internalLogger as the internal logger to report mishaps in the span lifecycle
    */
-  DDSpan(final long timestampMicro, final DDSpanContext context, final LogHandler logHandler) {
+  DDSpan(
+    final long timestampMicro,
+    final DDSpanContext context,
+    final LogHandler logHandler,
+    final InternalLogger internalLogger
+  ) {
     this.context = context;
     this.logHandler = logHandler;
+    this.internalLogger = internalLogger;
 
     if (timestampMicro <= 0L) {
       // record the start time
@@ -100,6 +111,15 @@ private void finishAndAddToTrace(final long durationNano) {
     if (this.durationNano.compareAndSet(0, Math.max(1, durationNano))) {
       context.getTrace().addSpan(this);
     } else {
+      internalLogger.log(
+        InternalLogger.Level.WARN,
+        InternalLogger.Target.USER,
+        () -> "Span " + getOperationName() + " finished but duration already set; " +
+            "dropped spanId:" + getSpanId() + " traceid:" + getTraceId(),
+        null,
+        false,
+        new HashMap<>()
+      );
     }
   }
 

diff --git a/features/dd-sdk-android-trace/src/main/java/com/datadog/opentracing/DDSpanContext.java b/features/dd-sdk-android-trace/src/main/java/com/datadog/opentracing/DDSpanContext.java
@@ -6,11 +6,13 @@
 
 package com.datadog.opentracing;
 
+import com.datadog.android.api.InternalLogger;
 import com.datadog.opentracing.decorators.AbstractDecorator;
 import com.datadog.legacy.trace.api.DDTags;
 import com.datadog.legacy.trace.api.sampling.PrioritySampling;
 import java.math.BigInteger;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
@@ -78,6 +80,8 @@ public class DDSpanContext implements io.opentracing.SpanContext {
 
   private final Map<String, String> serviceNameMappings;
 
+  private final InternalLogger internalLogger;
+
   public DDSpanContext(
       final BigInteger traceId,
       final BigInteger spanId,
@@ -93,7 +97,9 @@ public DDSpanContext(
       final Map<String, Object> tags,
       final PendingTrace trace,
       final DDTracer tracer,
-      final Map<String, String> serviceNameMappings) {
+      final Map<String, String> serviceNameMappings,
+      final InternalLogger internalLogger
+      ) {
 
     assert tracer != null;
     assert trace != null;
@@ -134,6 +140,7 @@ public DDSpanContext(
     }
     this.tags.put(DDTags.THREAD_NAME, threadName);
     this.tags.put(DDTags.THREAD_ID, threadId);
+    this.internalLogger = internalLogger;
   }
 
   public BigInteger getTraceId() {
@@ -213,6 +220,14 @@ public void setSpanType(final String spanType) {
   /** @return if sampling priority was set by this method invocation */
   public boolean setSamplingPriority(final int newPriority) {
     if (newPriority == PrioritySampling.UNSET) {
+      internalLogger.log(
+          InternalLogger.Level.WARN,
+          InternalLogger.Target.USER,
+          () -> "Can't set sampling priority to unset",
+          null,
+          false,
+          new HashMap<>()
+      );
       return false;
     }
 
@@ -263,6 +278,14 @@ public boolean lockSamplingPriority() {
     // sync with setSamplingPriority
     synchronized (this) {
       if (getMetrics().get(PRIORITY_SAMPLING_KEY) == null) {
+        internalLogger.log(
+          InternalLogger.Level.INFO,
+          InternalLogger.Target.USER,
+          () -> "Sampling priority unset, can't lock it",
+          null,
+            false,
+            new HashMap<>()
+        );
       } else if (samplingPriorityLocked == false) {
         samplingPriorityLocked = true;
       }

diff --git a/features/dd-sdk-android-trace/src/main/java/com/datadog/opentracing/DDTracer.java b/features/dd-sdk-android-trace/src/main/java/com/datadog/opentracing/DDTracer.java
@@ -8,6 +8,7 @@
 
 import android.os.StrictMode;
 
+import com.datadog.android.api.InternalLogger;
 import com.datadog.opentracing.decorators.AbstractDecorator;
 import com.datadog.opentracing.decorators.DDDecoratorsFactory;
 import com.datadog.opentracing.jfr.DDNoopScopeEventFactory;
@@ -452,6 +453,7 @@ public class DDSpanBuilder implements SpanBuilder {
         private String spanType;
         private boolean ignoreScope = false;
         private LogHandler logHandler = new DefaultLogHandler();
+        private InternalLogger internalLogger = InternalLogger.Companion.getUNBOUND();
 
         public DDSpanBuilder(final String operationName, final ScopeManager scopeManager) {
             this.operationName = operationName;
@@ -465,7 +467,7 @@ public SpanBuilder ignoreActiveSpan() {
         }
 
         private Span startSpan() {
-            return new DDSpan(timestampMicro, buildSpanContext(), logHandler);
+            return new DDSpan(timestampMicro, buildSpanContext(), logHandler, internalLogger);
         }
 
         @Override
@@ -547,6 +549,13 @@ public DDSpanBuilder withLogHandler(final LogHandler logHandler) {
             return this;
         }
 
+        public DDSpanBuilder withInternalLogger(final InternalLogger internalLogger) {
+            if (internalLogger != null) {
+                this.internalLogger = internalLogger;
+            }
+            return this;
+        }
+
         @Override
         public DDSpanBuilder asChildOf(final Span span) {
             return asChildOf(span == null ? null : span.context());
@@ -679,7 +688,7 @@ private DDSpanContext buildSpanContext() {
 
                 tags.putAll(localRootSpanTags);
 
-                parentTrace = new PendingTrace(DDTracer.this, traceId);
+                parentTrace = new PendingTrace(DDTracer.this, traceId, internalLogger);
             }
 
             if (serviceName == null) {
@@ -705,7 +714,8 @@ private DDSpanContext buildSpanContext() {
                             tags,
                             parentTrace,
                             DDTracer.this,
-                            serviceNameMappings);
+                            serviceNameMappings,
+                            internalLogger);
 
             // Apply Decorators to handle any tags that may have been set via the builder.
             for (final Map.Entry<String, Object> tag : tags.entrySet()) {