[api] Fix Tracer.StartRootSpan so that it always creates a root span (#…

…4890)
open-telemetry · Sep 29, 2023 · 23b041c · 23b041c
1 parent 043272e
commit 23b041c
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 13 deletions.
diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Fixed a bug which caused `Tracer.StartRootSpan` to generate a child span if a
+  trace was running (`Activity.Current != null`).
+  ([#4890](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4890))
+
 ## 1.6.0
 
 Released 2023-Sep-05

diff --git a/src/OpenTelemetry.Api/Trace/Tracer.cs b/src/OpenTelemetry.Api/Trace/Tracer.cs
@@ -35,6 +35,14 @@ internal Tracer(ActivitySource activitySource)
         this.ActivitySource = activitySource;
     }
 
+    [Flags]
+    private enum StartSpanBehaviors
+    {
+        ActivateNewSpan = 0b1,
+        DeactivateNewSpan = 0b10,
+        NewSpanAsRoot = 0b100,
+    }
+
     /// <summary>
     /// Gets the current span from the context.
     /// </summary>
@@ -86,7 +94,7 @@ public TelemetrySpan StartRootSpan(
         IEnumerable<Link>? links = null,
         DateTimeOffset startTime = default)
     {
-        return this.StartSpanHelper(false, name, kind, default, initialAttributes, links, startTime);
+        return this.StartSpanHelper(StartSpanBehaviors.NewSpanAsRoot | StartSpanBehaviors.DeactivateNewSpan, name, kind, default, initialAttributes, links, startTime);
     }
 
     /// <summary>
@@ -132,7 +140,7 @@ public TelemetrySpan StartSpan(
         IEnumerable<Link>? links = null,
         DateTimeOffset startTime = default)
     {
-        return this.StartSpanHelper(false, name, kind, in parentContext, initialAttributes, links, startTime);
+        return this.StartSpanHelper(StartSpanBehaviors.DeactivateNewSpan, name, kind, in parentContext, initialAttributes, links, startTime);
     }
 
     /// <summary>
@@ -178,7 +186,7 @@ public TelemetrySpan StartActiveSpan(
         IEnumerable<Link>? links = null,
         DateTimeOffset startTime = default)
     {
-        return this.StartSpanHelper(true, name, kind, in parentContext, initialAttributes, links, startTime);
+        return this.StartSpanHelper(StartSpanBehaviors.ActivateNewSpan, name, kind, in parentContext, initialAttributes, links, startTime);
     }
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -197,7 +205,7 @@ private static ActivityKind ConvertToActivityKind(SpanKind kind)
 
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private TelemetrySpan StartSpanHelper(
-        bool isActiveSpan,
+        StartSpanBehaviors startSpanBehavior,
         string name,
         SpanKind kind,
         in SpanContext parentContext = default,
@@ -212,19 +220,28 @@ private TelemetrySpan StartSpanHelper(
 
         var activityKind = ConvertToActivityKind(kind);
         var activityLinks = links?.Select(l => l.ActivityLink);
-        var previousActivity = !isActiveSpan ? Activity.Current : null;
+        var previousActivity = Activity.Current;
 
-        var activity = this.ActivitySource.StartActivity(name, activityKind, parentContext.ActivityContext, initialAttributes?.Attributes ?? null, activityLinks, startTime);
-        if (activity == null)
+        if (startSpanBehavior.HasFlag(StartSpanBehaviors.NewSpanAsRoot)
+            && previousActivity != null)
         {
-            return TelemetrySpan.NoopInstance;
+            Activity.Current = null;
         }
 
-        if (!isActiveSpan)
+        try
         {
-            Activity.Current = previousActivity;
+            var activity = this.ActivitySource.StartActivity(name, activityKind, parentContext.ActivityContext, initialAttributes?.Attributes ?? null, activityLinks, startTime);
+            return activity == null
+                ? TelemetrySpan.NoopInstance
+                : new TelemetrySpan(activity);
+        }
+        finally
+        {
+            if (startSpanBehavior.HasFlag(StartSpanBehaviors.DeactivateNewSpan)
+                && Activity.Current != previousActivity)
+            {
+                Activity.Current = previousActivity;
+            }
         }
-
-        return new TelemetrySpan(activity);
     }
 }
diff --git a/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs b/test/OpenTelemetry.Api.Tests/Trace/TracerTest.cs
@@ -71,7 +71,7 @@ public void Tracer_StartRootSpan_BadArgs_NullSpanName()
         Assert.Null(span3.Activity.DisplayName);
     }
 
-    [Fact(Skip = "See https://github.com/open-telemetry/opentelemetry-dotnet/issues/2803")]
+    [Fact]
     public async Task Tracer_StartRootSpan_StartsNewTrace()
     {
         var exportedItems = new List<Activity>();