Skip to content

Commit

Permalink
Fixed: processing spans after SDK is closed (OTel and ASP.NET Core) (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
jamescrosswell authored Nov 4, 2024
1 parent 8037991 commit 3bcab16
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 2 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- When using OTel and ASP.NET Core the SDK could try to process OTel spans after the SDK had been closed ([#3726](https://github.com/getsentry/sentry-dotnet/pull/3726))

## 4.12.2

### Features
Expand Down
16 changes: 16 additions & 0 deletions src/Sentry.OpenTelemetry/SentrySpanProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ internal SentrySpanProcessor(IHub hub, IEnumerable<IOpenTelemetryEnricher>? enri
/// <inheritdoc />
public override void OnStart(Activity data)
{
if (!_hub.IsEnabled)
{
// This would be unusual... it might happen if the SDK is closed while the processor is still running and
// we receive new telemetry. In this case, we can't log anything because our logger is disabled, so we just
// swallow it
return;
}

if (data.ParentSpanId != default && _map.TryGetValue(data.ParentSpanId, out var mappedParent))
{
// Explicit ParentSpanId of another activity that we have already mapped
Expand Down Expand Up @@ -165,6 +173,14 @@ private void CreateRootSpan(Activity data)
/// <inheritdoc />
public override void OnEnd(Activity data)
{
if (!_hub.IsEnabled)
{
// This would be unusual... it might happen if the SDK is closed while the processor is still running and
// we receive new telemetry. In this case, we can't log anything because our logger is disabled, so we just
// swallow it
return;
}

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

Expand Down
52 changes: 50 additions & 2 deletions test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ public Fixture()

public Hub GetHub() => Hub ??= new Hub(Options, Client, SessionManager, Clock, ScopeManager);

public SentrySpanProcessor GetSut()
public SentrySpanProcessor GetSut(IHub hub = null)
{
return new SentrySpanProcessor(GetHub(), Enrichers);
return new SentrySpanProcessor(hub ?? GetHub(), Enrichers);
}
}

Expand Down Expand Up @@ -488,6 +488,54 @@ public void OnEnd_IsSentryRequest_DoesNotFinishTransaction(string urlKey)
transaction.IsSentryRequest.Should().BeTrue();
}

[Fact]
public void OnStart_DisabledHub_DoesNothing()
{
// Arrange
_fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
SentryClientExtensions.SentryOptionsForTestingOnly = _fixture.Options;
var hub = Substitute.For<IHub>();
hub.IsEnabled.Returns(false);
var sut = _fixture.GetSut(hub);

var data = Tracer.StartActivity()!;

// Act
sut.OnStart(data);

// Assert
sut._map.Should().BeEmpty();
hub.Received(0).GetSpan();
hub.Received(0).StartTransaction(
Arg.Any<ITransactionContext>(),
Arg.Any<IReadOnlyDictionary<string, object>>()
);
}

[Fact]
public void OnEnd_DisabledHub_DoesNothing()
{
// Arrange
_fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
SentryClientExtensions.SentryOptionsForTestingOnly = _fixture.Options;
var hub = Substitute.For<IHub>();
hub.IsEnabled.Returns(true, false);
var sut = _fixture.GetSut(hub);

var data = Tracer.StartActivity()!;

// Act
sut.OnEnd(data);

// Assert
sut._map.Should().BeEmpty();
hub.Received(0).GetSpan();
hub.Received(0).StartTransaction(
Arg.Any<ITransactionContext>(),
Arg.Any<IReadOnlyDictionary<string, object>>()
);
}

private static void FilterActivity(Activity activity)
{
// Simulates filtering an activity - see https://github.com/getsentry/sentry-dotnet/pull/3198
Expand Down

0 comments on commit 3bcab16

Please sign in to comment.