Skip to content
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.

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

Filtered OTel activities should not be sent to Sentry #3890

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
- .NET on iOS: Add experimental EnableAppHangTrackingV2 configuration flag to the options binding SDK ([#3877](https://github.com/getsentry/sentry-dotnet/pull/3877))
- Added `SentryOptions.DisableSentryHttpMessageHandler`. Useful if you're using `OpenTelemetry.Instrumentation.Http` and ending up with duplicate spans. ([#3879](https://github.com/getsentry/sentry-dotnet/pull/3879))

### Fixes

- OTel activities that are marked as not recorded are no longer sent to Sentry ([#3890](https://github.com/getsentry/sentry-dotnet/pull/3890))

## 5.0.1

### Fixes
Expand Down
8 changes: 8 additions & 0 deletions src/Sentry.OpenTelemetry/SentrySpanProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,14 @@ public override void OnEnd(Activity data)
return;
}

// Skip any activities that are not recorded.
if (data is { Recorded: false, IsAllDataRequested: false })
jamescrosswell marked this conversation as resolved.
Show resolved Hide resolved
{
_options?.DiagnosticLogger?.LogDebug($"Ignoring unrecorded Activity {data.SpanId}.");
_map.TryRemove(data.SpanId, out _);
return;
}

// Make a dictionary of the attributes (aka "tags") for faster lookup when used throughout the processor.
var attributes = data.TagObjects.ToDict();

Expand Down
82 changes: 82 additions & 0 deletions test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,88 @@ public void OnEnd_FinishesTransaction()
}
}

[Fact]
public void OnEnd_FilteredTransaction_DoesNotFinishTransaction()
{
// Arrange
_fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
var sut = _fixture.GetSut();

var parent = Tracer.StartActivity("transaction")!;
sut.OnStart(parent);

var data = Tracer.StartActivity("test operation", kind: ActivityKind.Internal)!;
data.DisplayName = "test display name";
sut.OnStart(data);

FilterActivity(parent);

sut._map.TryGetValue(parent.SpanId, out var span);

// Act
sut.OnEnd(data);
sut.OnEnd(parent);

// Assert
if (span is not TransactionTracer transaction)
{
Assert.Fail("Span is not a transaction tracer");
return;
}

using (new AssertionScope())
{
transaction.EndTimestamp.Should().BeNull();
transaction.Status.Should().BeNull();
}
}

[Fact]
public void OnEnd_FilteredSpan_RemovesSpan()
{
// Arrange
_fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
var sut = _fixture.GetSut();

var parent = Tracer.StartActivity("transaction")!;
sut.OnStart(parent);

var data = Tracer.StartActivity("test operation", kind: ActivityKind.Internal)!;
data.DisplayName = "test display name";
sut.OnStart(data);

FilterActivity(data);

sut._map.TryGetValue(parent.SpanId, out var parentSpan);
sut._map.TryGetValue(data.SpanId, out var childSpan);

// Act
sut.OnEnd(data);
sut.OnEnd(parent);

// Assert
if (parentSpan is not TransactionTracer transaction)
{
Assert.Fail("parentSpan is not a transaction tracer");
return;
}
if (childSpan is not SpanTracer span)
{
Assert.Fail("span is not a span tracer");
return;
}

using (new AssertionScope())
{
span.EndTimestamp.Should().BeNull();
span.Status.Should().BeNull();

transaction.EndTimestamp.Should().NotBeNull();
transaction.Status.Should().Be(SpanStatus.Ok);
transaction.Spans.Should().BeEmpty();
}
}

[Theory]
[InlineData(OtelSemanticConventions.AttributeUrlFull)]
[InlineData(OtelSemanticConventions.AttributeHttpUrl)]
Expand Down
Loading