fix: InvalidCastException in SentrySpanProcessor (#4245)

jamescrosswell · web-flow · commit ebac4e390ea8 · 2025-06-04T21:54:05.000+12:00
Resolves #4235: - #4235
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 
 ### Fixes
 
+- InvalidCastException in SentrySpanProcessor when using the Sentry.OpenTelemetry integration ([#4245](https://github.com/getsentry/sentry-dotnet/pull/4245))
 - Fix InApp Exclude for frames without Module by checking against frame's Package ([#4236](https://github.com/getsentry/sentry-dotnet/pull/4236))
 
 ## 5.9.0
diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs
@@ -133,12 +133,15 @@ private void CreateChildSpan(Activity data, ISpan parentSpan, SpanId? parentSpan
             Instrumenter = Instrumenter.OpenTelemetry
         };
 
-        var span = (SpanTracer)parentSpan.StartChild(context);
-        span.Origin = OpenTelemetryOrigin;
-        span.StartTimestamp = data.StartTimeUtc;
-        // Used to filter out spans that are not recorded when finishing a transaction.
-        span.SetFused(data);
-        span.IsFiltered = () => span.GetFused<Activity>() is { IsAllDataRequested: false, Recorded: false };
+        var span = parentSpan.StartChild(context);
+        if (span is SpanTracer spanTracer)
+        {
+            spanTracer.Origin = OpenTelemetryOrigin;
+            spanTracer.StartTimestamp = data.StartTimeUtc;
+            // Used to filter out spans that are not recorded when finishing a transaction.
+            spanTracer.SetFused(data);
+            spanTracer.IsFiltered = () => spanTracer.GetFused<Activity>() is { IsAllDataRequested: false, Recorded: false };
+        }
         _map[data.SpanId] = span;
     }
 
@@ -161,17 +164,19 @@ private void CreateRootSpan(Activity data)
 
         var baggageHeader = data.Baggage.AsBaggageHeader();
         var dynamicSamplingContext = baggageHeader.CreateDynamicSamplingContext(_replaySession);
-        var transaction = (TransactionTracer)_hub.StartTransaction(
+        var transaction = _hub.StartTransaction(
             transactionContext, new Dictionary<string, object?>(), dynamicSamplingContext
         );
-        transaction.Contexts.Trace.Origin = OpenTelemetryOrigin;
-        transaction.StartTimestamp = data.StartTimeUtc;
+        if (transaction is TransactionTracer tracer)
+        {
+            tracer.Contexts.Trace.Origin = OpenTelemetryOrigin;
+            tracer.StartTimestamp = data.StartTimeUtc;
+        }
         _hub.ConfigureScope(scope => scope.Transaction = transaction);
         transaction.SetFused(data);
         _map[data.SpanId] = transaction;
     }
 
-
     /// <inheritdoc />
     public override void OnEnd(Activity data)
     {
@@ -236,15 +241,15 @@ public override void OnEnd(Activity data)
             // Transactions set otel attributes (and resource attributes) as context.
             transaction.Contexts["otel"] = GetOtelContext(attributes);
         }
-        else
+        else if (span is SpanTracer spanTracer)
         {
             // Use the end timestamp from the activity data.
-            ((SpanTracer)span).EndTimestamp = data.StartTimeUtc + data.Duration;
+            spanTracer.EndTimestamp = data.StartTimeUtc + data.Duration;
 
             // Spans set otel attributes in extras (passed to Sentry as "data" on the span).
             // Resource attributes do not need to be set, as they would be identical as those set on the transaction.
-            span.SetExtras(attributes);
-            span.SetExtra("otel.kind", data.Kind);
+            spanTracer.SetExtras(attributes);
+            spanTracer.SetExtra("otel.kind", data.Kind);
         }
 
         // In ASP.NET Core the middleware finishes up (and the scope gets popped) before the activity is ended.  So we
diff --git a/src/Sentry/ISpan.cs b/src/Sentry/ISpan.cs
@@ -70,15 +70,21 @@ public static ISpan StartChild(this ISpan span, string operation, string? descri
 
     internal static ISpan StartChild(this ISpan span, SpanContext context)
     {
-        var transaction = span.GetTransaction() as TransactionTracer;
-        if (transaction?.StartChild(context.SpanId, span.SpanId, context.Operation, context.Instrumenter)
-            is not SpanTracer childSpan)
+        var transaction = span.GetTransaction();
+        if (transaction is TransactionTracer transactionTracer)
         {
-            return NoOpSpan.Instance;
+            var child = transactionTracer.StartChild(context.SpanId, span.SpanId, context.Operation, context.Instrumenter);
+            if (child is SpanTracer childTracer)
+            {
+                childTracer.Description = context.Description;
+                return childTracer;
+            }
         }
-
-        childSpan.Description = context.Description;
-        return childSpan;
+        if (transaction is UnsampledTransaction unsampledTransaction)
+        {
+            return unsampledTransaction.StartChild(context.Operation, context.SpanId);
+        }
+        return NoOpSpan.Instance;
     }
 
     /// <summary>
@@ -88,7 +94,8 @@ public static ITransactionTracer GetTransaction(this ISpan span) =>
         span switch
         {
             ITransactionTracer transaction => transaction,
-            SpanTracer tracer => tracer.Transaction,
+            UnsampledSpan unsampledSpan => unsampledSpan.Transaction,
+            SpanTracer spanTracer => spanTracer.Transaction,
             _ => throw new ArgumentOutOfRangeException(nameof(span), span, null)
         };
 
diff --git a/src/Sentry/Internal/UnsampledSpan.cs b/src/Sentry/Internal/UnsampledSpan.cs
@@ -0,0 +1,9 @@
+namespace Sentry.Internal;
+
+internal sealed class UnsampledSpan(UnsampledTransaction transaction, SpanId? spanId = null) : NoOpSpan
+{
+    public override bool? IsSampled => false;
+    public override SpanId SpanId { get; } = spanId ?? SpanId.Empty;
+    internal UnsampledTransaction Transaction => transaction;
+    public override ISpan StartChild(string operation) => transaction.StartChild(operation);
+}
diff --git a/src/Sentry/Internal/UnsampledTransaction.cs b/src/Sentry/Internal/UnsampledTransaction.cs
@@ -94,10 +94,10 @@ public override ISpan StartChild(string operation)
         return span;
     }
 
-    private class UnsampledSpan(UnsampledTransaction transaction) : NoOpSpan
+    public ISpan StartChild(string operation, SpanId spanId)
     {
-        public override ISpan StartChild(string operation) => transaction.StartChild(operation);
-
-        public override bool? IsSampled => false;
+        var span = new UnsampledSpan(this, spanId);
+        _spans.Add(span);
+        return span;
     }
 }
diff --git a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs
@@ -146,10 +146,11 @@ public void OnStart_Transaction_With_DynamicSamplingContext()
     }
 
     [Fact]
-    public void OnStart_WithParentSpanId_StartsChildSpan()
+    public void OnStart_SampledWithParentSpanId_StartsChildSpan()
     {
         // Arrange
         _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
+        _fixture.Options.TracesSampleRate = 1.0;
         var sut = _fixture.GetSut();
 
         using var parent = Tracer.StartActivity("Parent");
@@ -164,16 +165,18 @@ public void OnStart_WithParentSpanId_StartsChildSpan()
         Assert.True(sut._map.TryGetValue(data.SpanId, out var span));
         using (new AssertionScope())
         {
-            span.Should().BeOfType<SpanTracer>();
+            span.IsSampled.Should().Be(true);
             span.SpanId.Should().Be(data.SpanId.AsSentrySpanId());
+
             if (span is not SpanTracer spanTracer)
             {
                 Assert.Fail("Span is not a span tracer");
                 return;
             }
+
+            span.SpanId.Should().Be(data.SpanId.AsSentrySpanId());
             using (new AssertionScope())
             {
-                spanTracer.SpanId.Should().Be(data.SpanId.AsSentrySpanId());
                 spanTracer.ParentSpanId.Should().Be(data.ParentSpanId.AsSentrySpanId());
                 spanTracer.TraceId.Should().Be(data.TraceId.AsSentryId());
                 spanTracer.Operation.Should().Be(data.OperationName);
@@ -184,6 +187,32 @@ public void OnStart_WithParentSpanId_StartsChildSpan()
         }
     }
 
+    [Fact]
+    public void OnStart_NotSampledWithParentSpanId_StartsChildSpan()
+    {
+        // Arrange
+        _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
+        _fixture.Options.TracesSampleRate = 0.0;
+        var sut = _fixture.GetSut();
+
+        using var parent = Tracer.StartActivity("Parent");
+        sut.OnStart(parent);
+
+        using var data = Tracer.StartActivity("TestActivity");
+
+        // Act
+        sut.OnStart(data!);
+
+        // Assert
+        Assert.True(sut._map.TryGetValue(data.SpanId, out var span));
+        using (new AssertionScope())
+        {
+            span.IsSampled.Should().Be(false);
+            span.SpanId.Should().Be(data.SpanId.AsSentrySpanId());
+            span.Should().BeOfType<UnsampledSpan>();
+        }
+    }
+
     [Fact]
     public void OnStart_WithSentryParentSpanId_StartsChildSpan()
     {
@@ -257,10 +286,11 @@ public void StartSpan_UsingSentryTracing_StartsChildSpan()
     }
 
     [Fact]
-    public void OnStart_WithoutParentSpanId_StartsNewTransaction()
+    public void OnStart_SampledWithoutParentSpanId_StartsNewTransaction()
     {
         // Arrange
         _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
+        _fixture.Options.TracesSampleRate = 1.0;
         _fixture.ScopeManager = Substitute.For<IInternalScopeManager>();
         var scope = new Scope();
         var clientScope = new KeyValuePair<Scope, ISentryClient>(scope, _fixture.Client);
@@ -279,6 +309,7 @@ public void OnStart_WithoutParentSpanId_StartsNewTransaction()
             Assert.Fail("Span is not a transaction tracer");
             return;
         }
+
         using (new AssertionScope())
         {
             transaction.SpanId.Should().Be(data.SpanId.AsSentrySpanId());
@@ -293,7 +324,39 @@ public void OnStart_WithoutParentSpanId_StartsNewTransaction()
     }
 
     [Fact]
-    public void OnEnd_FinishesSpan()
+    public void OnStart_NotSampledWithoutParentSpanId_StartsNewTransaction()
+    {
+        // Arrange
+        _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
+        _fixture.Options.TracesSampleRate = 0.0;
+        _fixture.ScopeManager = Substitute.For<IInternalScopeManager>();
+        var scope = new Scope();
+        var clientScope = new KeyValuePair<Scope, ISentryClient>(scope, _fixture.Client);
+        _fixture.ScopeManager.GetCurrent().Returns(clientScope);
+        var sut = _fixture.GetSut();
+
+        var data = Tracer.StartActivity("test op");
+
+        // Act
+        sut.OnStart(data!);
+
+        // Assert
+        Assert.True(sut._map.TryGetValue(data.SpanId, out var span));
+        if (span is not UnsampledTransaction transaction)
+        {
+            Assert.Fail("Span is not an unsampled transaction");
+            return;
+        }
+
+        using (new AssertionScope())
+        {
+            transaction.SpanId.Should().Be(data.SpanId.AsSentrySpanId());
+            transaction.TraceId.Should().Be(data.TraceId.AsSentryId());
+        }
+    }
+
+    [Fact]
+    public void OnEnd_Sampled_Span_FinishesSpan()
     {
         // Arrange
         _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
@@ -338,6 +401,34 @@ public void OnEnd_FinishesSpan()
         }
     }
 
+    [Fact]
+    public void OnEnd_Unsampled_Span_DoesNotThrow()
+    {
+        // Arrange
+        _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
+        _fixture.Options.TracesSampleRate = 0.0;
+        var sut = _fixture.GetSut();
+
+        var parent = Tracer.StartActivity(name: "transaction")!;
+        sut.OnStart(parent);
+
+        Dictionary<string, object> tags = [];
+        var data = Tracer.StartActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!;
+        data.DisplayName = "test display name";
+        sut.OnStart(data);
+
+        sut._map.TryGetValue(data.SpanId, out var span);
+
+        // Act
+        sut.OnEnd(data);
+
+        // Assert
+        span.Should().BeOfType<UnsampledSpan>();
+
+        // There's nothing else to assert here, as long as calling OnEnd does not throw an exception,
+        // UnsampledSpan.Finish() is basically a no-op.
+    }
+
     [Fact]
     public void OnEnd_Transaction_RestoresSavedScope()
     {
@@ -418,7 +509,7 @@ public void OnEnd_SpansEnriched()
     }
 
     [Fact]
-    public void OnEnd_FinishesTransaction()
+    public void OnEnd_Sampled_FinishesTransaction()
     {
         // Arrange
         _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
@@ -460,6 +551,33 @@ public void OnEnd_FinishesTransaction()
         }
     }
 
+    [Fact]
+    public void OnEnd_NotSampled_FinishesTransaction()
+    {
+        // Arrange
+        _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
+        _fixture.Options.TracesSampleRate = 0.0;
+        var sut = _fixture.GetSut();
+
+        Dictionary<string, object> tags = [];
+        var data = Tracer.StartActivity(name: "test operation", kind: ActivityKind.Internal, parentContext: default, tags)!;
+        data.DisplayName = "test display name";
+        sut.OnStart(data);
+
+        sut._map.TryGetValue(data.SpanId, out var span);
+
+        // Act
+        sut.OnEnd(data);
+
+        // Assert
+        if (span is not UnsampledTransaction transaction)
+        {
+            Assert.Fail("Span is not an unsampled transaction");
+            return;
+        }
+        transaction.IsFinished.Should().BeTrue();
+    }
+
     [Fact]
     public void OnEnd_FilteredTransaction_DoesNotFinishTransaction()
     {

Original file line number	Diff line number	Diff line change
`@@ -94,10 +94,10 @@ public override ISpan StartChild(string operation)`
`94`	`94`	`return span;`
`95`	`95`	`}`
`96`	`96`
`97`		`- private class UnsampledSpan(UnsampledTransaction transaction) : NoOpSpan`
	`97`	`+ public ISpan StartChild(string operation, SpanId spanId)`
`98`	`98`	`{`
`99`		`- public override ISpan StartChild(string operation) => transaction.StartChild(operation);`
`100`		`-`
`101`		`- public override bool? IsSampled => false;`
	`99`	`+ var span = new UnsampledSpan(this, spanId);`
	`100`	`+ _spans.Add(span);`
	`101`	`+ return span;`
`102`	`102`	`}`
`103`	`103`	`}`