diff --git a/CHANGELOG.md b/CHANGELOG.md index eb7a3f3697..d75f2f2db3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - `Hints` now accept attachments provided as a file path via `AddAttachment` method ([#2585](https://github.com/getsentry/sentry-dotnet/pull/2585)) +### Fixes + +- Resolved an isse where the SDK would throw an exception while attempting to set the DynamicSamplingContext but the context exists already. ([#2592](https://github.com/getsentry/sentry-dotnet/pull/2592)) + ### Dependencies - Bump CLI from v2.20.5 to v2.20.6 ([#2590](https://github.com/getsentry/sentry-dotnet/pull/2590)) diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index aec1a01567..e49db790fa 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -216,8 +216,7 @@ public BaggageHeader GetBaggage() } var propagationContext = ScopeManager.GetCurrent().Key.PropagationContext; - propagationContext.DynamicSamplingContext ??= propagationContext.CreateDynamicSamplingContext(_options); - return propagationContext.DynamicSamplingContext.ToBaggageHeader(); + return propagationContext.GetOrCreateDynamicSamplingContext(_options).ToBaggageHeader(); } public TransactionContext ContinueTrace( @@ -352,9 +351,7 @@ private void ApplyTraceContextToEvent(SentryEvent evt, SentryPropagationContext evt.Contexts.Trace.TraceId = propagationContext.TraceId; evt.Contexts.Trace.SpanId = propagationContext.SpanId; evt.Contexts.Trace.ParentSpanId = propagationContext.ParentSpanId; - - propagationContext.DynamicSamplingContext ??= propagationContext.CreateDynamicSamplingContext(_options); - evt.DynamicSamplingContext = propagationContext.DynamicSamplingContext; + evt.DynamicSamplingContext = propagationContext.GetOrCreateDynamicSamplingContext(_options); } public SentryId CaptureEvent(SentryEvent evt, Action configureScope) => diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index 94c52db9ae..f89b9db5f5 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -242,7 +242,7 @@ public Scope(SentryOptions? options) internal Scope(SentryOptions? options, SentryPropagationContext? propagationContext) { Options = options ?? new SentryOptions(); - PropagationContext = propagationContext ?? new SentryPropagationContext(); + PropagationContext = new SentryPropagationContext(propagationContext); } // For testing. Should explicitly require SentryOptions. diff --git a/src/Sentry/SentryPropagationContext.cs b/src/Sentry/SentryPropagationContext.cs index ac6a8803a0..19e9bd30d3 100644 --- a/src/Sentry/SentryPropagationContext.cs +++ b/src/Sentry/SentryPropagationContext.cs @@ -8,21 +8,17 @@ internal class SentryPropagationContext public SpanId SpanId { get; } public SpanId? ParentSpanId { get; } - private DynamicSamplingContext? _dynamicSamplingContext; - public DynamicSamplingContext? DynamicSamplingContext + internal DynamicSamplingContext? _dynamicSamplingContext; + + public DynamicSamplingContext GetOrCreateDynamicSamplingContext(SentryOptions options) { - get => _dynamicSamplingContext; - set + if (_dynamicSamplingContext is null) { - if (_dynamicSamplingContext is null) - { - _dynamicSamplingContext = value; - } - else - { - throw new Exception("Attempted to set the DynamicSamplingContext but the context exists already."); - } + options.LogDebug("Creating the Dynamic Sampling Context from the Propagation Context"); + _dynamicSamplingContext = this.CreateDynamicSamplingContext(options); } + + return _dynamicSamplingContext; } internal SentryPropagationContext( @@ -33,7 +29,7 @@ internal class SentryPropagationContext TraceId = traceId; SpanId = SpanId.Create(); ParentSpanId = parentSpanId; - DynamicSamplingContext = dynamicSamplingContext; + _dynamicSamplingContext = dynamicSamplingContext; } public SentryPropagationContext() @@ -42,6 +38,15 @@ public SentryPropagationContext() SpanId = SpanId.Create(); } + public SentryPropagationContext(SentryPropagationContext? other) + { + TraceId = other?.TraceId ?? SentryId.Create(); + SpanId = other?.SpanId ?? SpanId.Create(); + ParentSpanId = other?.ParentSpanId; + + _dynamicSamplingContext = other?._dynamicSamplingContext; + } + public static SentryPropagationContext CreateFromHeaders(IDiagnosticLogger? logger, SentryTraceHeader? traceHeader, BaggageHeader? baggageHeader) { logger?.LogDebug("Creating a propagation context from headers."); diff --git a/test/Sentry.Tests/ScopeTests.cs b/test/Sentry.Tests/ScopeTests.cs index a55efc3ecf..dbd8e9308d 100644 --- a/test/Sentry.Tests/ScopeTests.cs +++ b/test/Sentry.Tests/ScopeTests.cs @@ -98,7 +98,9 @@ public void Clone_NewScope_IncludesPropagationContext() var clone = sut.Clone(); - Assert.Same(propagationContext, clone.PropagationContext); + Assert.NotSame(propagationContext, clone); // Sanity check that it really is a clone + Assert.Equal(propagationContext.TraceId, clone.PropagationContext.TraceId); + Assert.Equal(propagationContext.SpanId, clone.PropagationContext.SpanId); } [Fact] diff --git a/test/Sentry.Tests/SentryPropagationContextTests.cs b/test/Sentry.Tests/SentryPropagationContextTests.cs index 1ba3383e1a..88353203e4 100644 --- a/test/Sentry.Tests/SentryPropagationContextTests.cs +++ b/test/Sentry.Tests/SentryPropagationContextTests.cs @@ -2,6 +2,64 @@ namespace Sentry.Tests; public class SentryPropagationContextTests { + [Fact] + public void CopyConstructor_CreatesCopy() + { + var original = new SentryPropagationContext(); + original.GetOrCreateDynamicSamplingContext(new SentryOptions {Dsn = ValidDsn}); + + var copy = new SentryPropagationContext(original); + + Assert.Equal(original.TraceId, copy.TraceId); + Assert.Equal(original.SpanId, copy.SpanId); + Assert.Equal(original._dynamicSamplingContext, copy._dynamicSamplingContext); + } + + [Fact] + public void GetOrCreateDynamicSamplingContext_DynamicSamplingContextIsNull_CreatesDynamicSamplingContext() + { + var options = new SentryOptions { Dsn = ValidDsn }; + var propagationContext = new SentryPropagationContext(); + + Assert.Null(propagationContext._dynamicSamplingContext); // Sanity check + _ = propagationContext.GetOrCreateDynamicSamplingContext(options); + + Assert.NotNull(propagationContext._dynamicSamplingContext); + } + + [Fact] + public void GetOrCreateDynamicSamplingContext_DynamicSamplingContextIsNotNull_ReturnsSameDynamicSamplingContext() + { + var options = new SentryOptions { Dsn = ValidDsn }; + var propagationContext = new SentryPropagationContext(); + var firstDynamicSamplingContext = propagationContext.GetOrCreateDynamicSamplingContext(options); + + var secondDynamicSamplingContext = propagationContext.GetOrCreateDynamicSamplingContext(options); + + Assert.Same(firstDynamicSamplingContext, secondDynamicSamplingContext); + } + + [Fact] + public void CreateFromHeaders_HeadersNull_CreatesPropagationContextWithTraceAndSpanId() + { + var propagationContext = SentryPropagationContext.CreateFromHeaders(null, null, null); + + Assert.NotEqual(propagationContext.TraceId, SentryId.Empty); + Assert.NotEqual(propagationContext.SpanId, SpanId.Empty); + } + + [Fact] + public void CreateFromHeaders_TraceHeaderNotNull_CreatesPropagationContextFromTraceHeader() + { + var traceHeader = new SentryTraceHeader(SentryId.Create(), SpanId.Create(), null); + + var propagationContext = SentryPropagationContext.CreateFromHeaders(null, traceHeader, null); + + Assert.Equal(traceHeader.TraceId, propagationContext.TraceId); + Assert.NotEqual(traceHeader.SpanId, propagationContext.SpanId); // Sanity check + Assert.Equal(traceHeader.SpanId, propagationContext.ParentSpanId); + } + [Fact] public void CreateFromHeaders_TraceHeaderNullButBaggageExists_CreatesPropagationContextWithoutDynamicSamplingContext() { @@ -15,7 +73,7 @@ public void CreateFromHeaders_TraceHeaderNullButBaggageExists_CreatesPropagation var propagationContext = SentryPropagationContext.CreateFromHeaders(null, null, baggageHeader); - Assert.Null(propagationContext.DynamicSamplingContext); + Assert.Null(propagationContext._dynamicSamplingContext); } [Fact] @@ -32,7 +90,6 @@ public void CreateFromHeaders_BaggageHeaderNotNull_CreatesPropagationContextWith var propagationContext = SentryPropagationContext.CreateFromHeaders(null, traceHeader, baggageHeader); - Assert.NotNull(propagationContext.DynamicSamplingContext); - Assert.Equal(4, propagationContext.DynamicSamplingContext.Items.Count); + Assert.Equal(4, propagationContext.GetOrCreateDynamicSamplingContext(new SentryOptions()).Items.Count); } }