Skip to content

fix: InvalidCastException in SentrySpanProcessor #4245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jamescrosswell merged 7 commits into from
Jun 4, 2025
Merged

fix: InvalidCastException in SentrySpanProcessor #4245

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
404a0b7
fix: InvalidCastException in SentrySpanProcessor
jamescrosswell Jun 3, 2025
5b59f01
Fixed invalid cast to SpanTracer in SentrySpanProcessor
jamescrosswell Jun 3, 2025
e60f36a
Update CHANGELOG.md
jamescrosswell Jun 3, 2025
b6a6dc3
Fixed invalid casts in OnEnd activity hooks
jamescrosswell Jun 3, 2025
7c7640b
Merge branch 'main' into invalid-cast
jamescrosswell Jun 3, 2025
7c3a611
Update CHANGELOG.md
jamescrosswell Jun 3, 2025
9004730
Review feedback
jamescrosswell Jun 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fixed invalid casts in OnEnd activity hooks
  • Loading branch information
@jamescrosswell
jamescrosswell committed Jun 3, 2025
commit b6a6dc339a7960a4f3b43aead97faa3bb706eeda
4 changes: 2 additions & 2 deletions src/Sentry.OpenTelemetry/SentrySpanProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,10 @@ 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.
Expand Down
179 changes: 133 additions & 46 deletions test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,12 @@ public void OnStart_Transaction_With_DynamicSamplingContext()
}
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void OnStart_WithParentSpanId_StartsChildSpan(bool isSampled)
[Fact]
public void OnStart_SampledWithParentSpanId_StartsChildSpan()
{
// Arrange
_fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
_fixture.Options.TracesSampleRate = isSampled ? 1.0 : 0.0;
_fixture.Options.TracesSampleRate = 1.0;
var sut = _fixture.GetSut();

using var parent = Tracer.StartActivity("Parent");
Expand All @@ -167,13 +165,8 @@ public void OnStart_WithParentSpanId_StartsChildSpan(bool isSampled)
Assert.True(sut._map.TryGetValue(data.SpanId, out var span));
using (new AssertionScope())
{
span.IsSampled.Should().Be(isSampled);
span.IsSampled.Should().Be(true);
span.SpanId.Should().Be(data.SpanId.AsSentrySpanId());
if (!isSampled)
{
span.Should().BeOfType<UnsampledSpan>();
return;
}

if (span is not SpanTracer spanTracer)
{
Expand All @@ -194,6 +187,32 @@ public void OnStart_WithParentSpanId_StartsChildSpan(bool isSampled)
}
}

[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()
{
Expand Down Expand Up @@ -266,14 +285,12 @@ public void StartSpan_UsingSentryTracing_StartsChildSpan()
}
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void OnStart_WithoutParentSpanId_StartsNewTransaction(bool isSampled)
[Fact]
public void OnStart_SampledWithoutParentSpanId_StartsNewTransaction()
{
// Arrange
_fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
_fixture.Options.TracesSampleRate = isSampled ? 1.0 : 0.0;
_fixture.Options.TracesSampleRate = 1.0;
_fixture.ScopeManager = Substitute.For<IInternalScopeManager>();
var scope = new Scope();
var clientScope = new KeyValuePair<Scope, ISentryClient>(scope, _fixture.Client);
Expand All @@ -287,44 +304,59 @@ public void OnStart_WithoutParentSpanId_StartsNewTransaction(bool isSampled)

// Assert
Assert.True(sut._map.TryGetValue(data.SpanId, out var span));
if (isSampled)
if (span is not TransactionTracer transaction)
{
if (span is not TransactionTracer transaction)
{
Assert.Fail("Span is not a transaction tracer");
return;
}
Assert.Fail("Span is not a transaction tracer");
return;
}

using (new AssertionScope())
{
transaction.SpanId.Should().Be(data.SpanId.AsSentrySpanId());
transaction.ParentSpanId.Should().Be(new ActivitySpanId().AsSentrySpanId());
transaction.TraceId.Should().Be(data.TraceId.AsSentryId());
transaction.Name.Should().Be(data.DisplayName);
transaction.Operation.Should().Be(data.OperationName);
transaction.Description.Should().Be(data.DisplayName);
transaction.Status.Should().BeNull();
transaction.StartTimestamp.Should().Be(data.StartTimeUtc);
}
using (new AssertionScope())
{
transaction.SpanId.Should().Be(data.SpanId.AsSentrySpanId());
transaction.ParentSpanId.Should().Be(new ActivitySpanId().AsSentrySpanId());
transaction.TraceId.Should().Be(data.TraceId.AsSentryId());
transaction.Name.Should().Be(data.DisplayName);
transaction.Operation.Should().Be(data.OperationName);
transaction.Description.Should().Be(data.DisplayName);
transaction.Status.Should().BeNull();
transaction.StartTimestamp.Should().Be(data.StartTimeUtc);
}
else
}

[Fact]
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)
{
if (span is not UnsampledTransaction transaction)
{
Assert.Fail("Span is not an unsampled transaction");
return;
}
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());
}
using (new AssertionScope())
{
transaction.SpanId.Should().Be(data.SpanId.AsSentrySpanId());
transaction.TraceId.Should().Be(data.TraceId.AsSentryId());
}
}

[Fact]
public void OnEnd_FinishesSpan()
public void OnEnd_Sampled_Span_FinishesSpan()
{
// Arrange
_fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
Expand Down Expand Up @@ -369,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,
// UnsampleSpan.Finish() is basically a no-op.
}

[Fact]
public void OnEnd_Transaction_RestoresSavedScope()
{
Expand Down Expand Up @@ -449,7 +509,7 @@ public void OnEnd_SpansEnriched()
}

[Fact]
public void OnEnd_FinishesTransaction()
public void OnEnd_Sampled_FinishesTransaction()
{
// Arrange
_fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
Expand Down Expand Up @@ -491,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()
{
Expand Down
Loading