From 8a20eba3a6d14e73ea46ac14bd80d33970d0b1bd Mon Sep 17 00:00:00 2001 From: tsushanth <78000697+tsushanth@users.noreply.github.com> Date: Mon, 6 Jul 2026 07:29:56 -0700 Subject: [PATCH 1/3] fix(otel): only set Scope.Transaction in CreateRootSpan when null Rewrite test to exercise CreateRootSpan directly via ScopeManager, use correct types, and remove noisy inline comments per review feedback. --- .../SentrySpanProcessor.cs | 10 +++++++- .../SentrySpanProcessorTests.cs | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index cc341c73df..f9f8985e55 100644 --- a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs +++ b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs @@ -172,7 +172,15 @@ private void CreateRootSpan(Activity data) tracer.Contexts.Trace.Origin = OpenTelemetryOrigin; tracer.StartTimestamp = data.StartTimeUtc; } - _hub.ConfigureScope(static (scope, transaction) => scope.Transaction = transaction, transaction); + // Only set Scope.Transaction when null to avoid clobbering an existing transaction + // when two root OTel activities overlap. See #5314. + _hub.ConfigureScope(static (scope, transaction) => + { + if (scope.Transaction is null) + { + scope.Transaction = transaction; + } + }, transaction); transaction.SetFused(data); _map[data.SpanId] = transaction; } diff --git a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs index a986a4779f..a5192e4fbc 100644 --- a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs +++ b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs @@ -1012,4 +1012,29 @@ public void ParseOtelSpanDescription_HttpClient() description.Should().Be("POST https://example.com/foo"); source.Should().Be(TransactionNameSource.Custom); } + + [Fact] + public void OnStart_WithExistingTransactionOnScope_DoesNotOverwriteExistingTransaction() + { + // Arrange + _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; + _fixture.Options.TracesSampleRate = 1.0; + _fixture.ScopeManager = Substitute.For(); + var scope = new Scope(); + var clientScope = new KeyValuePair(scope, _fixture.Client); + _fixture.ScopeManager.GetCurrent().Returns(clientScope); + var sut = _fixture.GetSut(); + + using var firstActivity = Tracer.StartActivity("Existing"); + sut.OnStart(firstActivity!); + var existingTransaction = scope.Transaction; + + // Act: second root activity starts while Existing is still on the scope + using var secondActivity = Tracer.StartActivity("NewRootActivity"); + sut.OnStart(secondActivity!); + + // Assert + scope.Transaction.Should().BeSameAs(existingTransaction, + "CreateRootSpan should not overwrite an already-set Scope.Transaction"); + } } From c9f0b847b20c7147b8f6733f5c45c28b14bc0c0d Mon Sep 17 00:00:00 2001 From: tsushanth <78000697+tsushanth@users.noreply.github.com> Date: Mon, 6 Jul 2026 13:34:06 -0700 Subject: [PATCH 2/3] fix(otel): use ??= operator and fix test to exercise the actual guard --- src/Sentry.OpenTelemetry/SentrySpanProcessor.cs | 7 +------ .../SentrySpanProcessorTests.cs | 14 +++++--------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index f9f8985e55..6c9113aca2 100644 --- a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs +++ b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs @@ -172,14 +172,9 @@ private void CreateRootSpan(Activity data) tracer.Contexts.Trace.Origin = OpenTelemetryOrigin; tracer.StartTimestamp = data.StartTimeUtc; } - // Only set Scope.Transaction when null to avoid clobbering an existing transaction - // when two root OTel activities overlap. See #5314. _hub.ConfigureScope(static (scope, transaction) => { - if (scope.Transaction is null) - { - scope.Transaction = transaction; - } + scope.Transaction ??= transaction; }, transaction); transaction.SetFused(data); _map[data.SpanId] = transaction; diff --git a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs index a5192e4fbc..93a8d22db7 100644 --- a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs +++ b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs @@ -1016,25 +1016,21 @@ public void ParseOtelSpanDescription_HttpClient() [Fact] public void OnStart_WithExistingTransactionOnScope_DoesNotOverwriteExistingTransaction() { - // Arrange _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; _fixture.Options.TracesSampleRate = 1.0; - _fixture.ScopeManager = Substitute.For(); - var scope = new Scope(); - var clientScope = new KeyValuePair(scope, _fixture.Client); - _fixture.ScopeManager.GetCurrent().Returns(clientScope); var sut = _fixture.GetSut(); using var firstActivity = Tracer.StartActivity("Existing"); sut.OnStart(firstActivity!); - var existingTransaction = scope.Transaction; + Assert.True(sut._map.TryGetValue(firstActivity.SpanId, out var firstTransaction)); - // Act: second root activity starts while Existing is still on the scope using var secondActivity = Tracer.StartActivity("NewRootActivity"); sut.OnStart(secondActivity!); - // Assert - scope.Transaction.Should().BeSameAs(existingTransaction, + object scopeTransaction = null; + _fixture.Hub.ConfigureScope(scope => scopeTransaction = scope.Transaction); + + scopeTransaction.Should().BeSameAs(firstTransaction, "CreateRootSpan should not overwrite an already-set Scope.Transaction"); } } From 49584fde5e34feae8d38d81024eb732cb63e2877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20P=C3=B6lz?= <38893694+Flash0ver@users.noreply.github.com> Date: Wed, 8 Jul 2026 18:07:09 +0200 Subject: [PATCH 3/3] test: fix test to actually impact the code change --- .../SentrySpanProcessorTests.cs | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs index 93a8d22db7..1c3adbbea5 100644 --- a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs +++ b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs @@ -1016,21 +1016,31 @@ public void ParseOtelSpanDescription_HttpClient() [Fact] public void OnStart_WithExistingTransactionOnScope_DoesNotOverwriteExistingTransaction() { + // Arrange _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; - _fixture.Options.TracesSampleRate = 1.0; var sut = _fixture.GetSut(); - using var firstActivity = Tracer.StartActivity("Existing"); - sut.OnStart(firstActivity!); - Assert.True(sut._map.TryGetValue(firstActivity.SpanId, out var firstTransaction)); + var parentContext = new TransactionContext("Program", "Main") + { + Instrumenter = Instrumenter.OpenTelemetry, + }; + var parent = _fixture.Hub.StartTransaction(parentContext); + _fixture.Hub.ConfigureScope(scope => scope.Transaction = parent); + + using var data = Tracer.StartActivity("TestActivity"); + + // Act + sut.OnStart(data!); - using var secondActivity = Tracer.StartActivity("NewRootActivity"); - sut.OnStart(secondActivity!); + // Assert + data.ParentSpanId.Should().Be(default(ActivitySpanId), $"{nameof(data)} should be a new root Activity, not a child Activity"); + Assert.True(sut._map.TryGetValue(data.SpanId, out var span)); + span.Should().BeOfType($"{nameof(data)} should be a new root Transaction, not a child Span"); object scopeTransaction = null; _fixture.Hub.ConfigureScope(scope => scopeTransaction = scope.Transaction); - scopeTransaction.Should().BeSameAs(firstTransaction, + scopeTransaction.Should().BeSameAs(parent, "CreateRootSpan should not overwrite an already-set Scope.Transaction"); } }