From dd0451c955aeda884e07f726c03cd1fb4a532fcb Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Mon, 17 Feb 2025 18:00:44 +1300 Subject: [PATCH 1/2] WIP --- src/Sentry/Internal/Hub.cs | 67 +++++++++-------- src/Sentry/Internal/NoOpSpan.cs | 20 ++--- src/Sentry/Internal/NoOpTransaction.cs | 6 +- src/Sentry/Internal/UnsampledTransaction.cs | 83 +++++++++++++++++++++ 4 files changed, 132 insertions(+), 44 deletions(-) create mode 100644 src/Sentry/Internal/UnsampledTransaction.cs diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index b90f4ca603..b8dea26b61 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -127,8 +127,6 @@ internal ITransactionTracer StartTransaction( IReadOnlyDictionary customSamplingContext, DynamicSamplingContext? dynamicSamplingContext) { - var transaction = new TransactionTracer(this, context); - // If the hub is disabled, we will always sample out. In other words, starting a transaction // after disposing the hub will result in that transaction not being sent to Sentry. // Additionally, we will always sample out if tracing is explicitly disabled. @@ -136,43 +134,48 @@ internal ITransactionTracer StartTransaction( // that may have been already set (i.e.: from a sentry-trace header). if (!IsEnabled) { - transaction.IsSampled = false; - transaction.SampleRate = 0.0; + return NoOpTransaction.Instance; } - else - { - // Except when tracing is disabled, TracesSampler runs regardless of whether a decision - // has already been made, as it can be used to override it. - if (_options.TracesSampler is { } tracesSampler) - { - var samplingContext = new TransactionSamplingContext( - context, - customSamplingContext); - if (tracesSampler(samplingContext) is { } sampleRate) - { - transaction.IsSampled = _randomValuesFactory.NextBool(sampleRate); - transaction.SampleRate = sampleRate; - } - } + double? sampleRate = null; - // Random sampling runs only if the sampling decision hasn't been made already. - if (transaction.IsSampled == null) - { - var sampleRate = _options.TracesSampleRate ?? 0.0; - transaction.IsSampled = _randomValuesFactory.NextBool(sampleRate); - transaction.SampleRate = sampleRate; - } + // Except when tracing is disabled, TracesSampler runs regardless of whether a decision + // has already been made, as it can be used to override it. + if (_options.TracesSampler is { } tracesSampler) + { + var samplingContext = new TransactionSamplingContext( + context, + customSamplingContext); - if (transaction.IsSampled is true && - _options.TransactionProfilerFactory is { } profilerFactory && - _randomValuesFactory.NextBool(_options.ProfilesSampleRate ?? 0.0)) + if (tracesSampler(samplingContext) is { } samplerSampleRate) { - // TODO cancellation token based on Hub being closed? - transaction.TransactionProfiler = profilerFactory.Start(transaction, CancellationToken.None); + sampleRate = samplerSampleRate; } } + // If the sampling decision isn't made by a trace sampler then fallback to Random sampling + sampleRate ??= _options.TracesSampleRate ?? 0.0; + + var isSampled = _randomValuesFactory.NextBool(sampleRate.Value); + if (!isSampled) + { + // var unsampledTransaction = new UnsampledTransaction(this, context); + // return unsampledTransaction; + return new UnsampledTransaction(this, context); + } + + var transaction = new TransactionTracer(this, context) + { + IsSampled = true, + SampleRate = sampleRate + }; + if (_options.TransactionProfilerFactory is { } profilerFactory && + _randomValuesFactory.NextBool(_options.ProfilesSampleRate ?? 0.0)) + { + // TODO cancellation token based on Hub being closed? + transaction.TransactionProfiler = profilerFactory.Start(transaction, CancellationToken.None); + } + // Use the provided DSC, or create one based on this transaction. // DSC creation must be done AFTER the sampling decision has been made. transaction.DynamicSamplingContext = @@ -213,6 +216,8 @@ public SentryTraceHeader GetTraceHeader() public BaggageHeader GetBaggage() { var span = GetSpan(); + // TODO: Things like the SampleRand won't get propagated unless we get them from a DSC + // ... so we'd need to get these from an UnsampledTransaction as well as a TransactionTracer if (span?.GetTransaction() is TransactionTracer { DynamicSamplingContext: { IsEmpty: false } dsc }) { return dsc.ToBaggageHeader(); diff --git a/src/Sentry/Internal/NoOpSpan.cs b/src/Sentry/Internal/NoOpSpan.cs index f6e49e5a2c..dbd87492fa 100644 --- a/src/Sentry/Internal/NoOpSpan.cs +++ b/src/Sentry/Internal/NoOpSpan.cs @@ -13,10 +13,10 @@ protected NoOpSpan() { } - public SpanId SpanId => SpanId.Empty; + public virtual SpanId SpanId => SpanId.Empty; public SpanId? ParentSpanId => SpanId.Empty; - public SentryId TraceId => SentryId.Empty; - public bool? IsSampled => default; + public virtual SentryId TraceId => SentryId.Empty; + public virtual bool? IsSampled => default; public IReadOnlyDictionary Tags => ImmutableDictionary.Empty; public IReadOnlyDictionary Extra => ImmutableDictionary.Empty; public IReadOnlyDictionary Data => ImmutableDictionary.Empty; @@ -24,7 +24,7 @@ protected NoOpSpan() public DateTimeOffset? EndTimestamp => default; public bool IsFinished => default; - public string Operation + public virtual string Operation { get => string.Empty; set { } @@ -42,21 +42,21 @@ public SpanStatus? Status set { } } - public ISpan StartChild(string operation) => this; + public virtual ISpan StartChild(string operation) => this; - public void Finish() + public virtual void Finish() { } - public void Finish(SpanStatus status) + public virtual void Finish(SpanStatus status) { } - public void Finish(Exception exception, SpanStatus status) + public virtual void Finish(Exception exception, SpanStatus status) { } - public void Finish(Exception exception) + public virtual void Finish(Exception exception) { } @@ -76,7 +76,7 @@ public void SetData(string key, object? value) { } - public SentryTraceHeader GetTraceHeader() => SentryTraceHeader.Empty; + public virtual SentryTraceHeader GetTraceHeader() => SentryTraceHeader.Empty; public IReadOnlyDictionary Measurements => ImmutableDictionary.Empty; diff --git a/src/Sentry/Internal/NoOpTransaction.cs b/src/Sentry/Internal/NoOpTransaction.cs index 9709bcc7d4..ebbcb38d71 100644 --- a/src/Sentry/Internal/NoOpTransaction.cs +++ b/src/Sentry/Internal/NoOpTransaction.cs @@ -7,13 +7,13 @@ internal class NoOpTransaction : NoOpSpan, ITransactionTracer { public new static ITransactionTracer Instance { get; } = new NoOpTransaction(); - private NoOpTransaction() + protected NoOpTransaction() { } public SdkVersion Sdk => SdkVersion.Instance; - public string Name + public virtual string Name { get => string.Empty; set { } @@ -87,7 +87,7 @@ public IReadOnlyList Fingerprint set { } } - public IReadOnlyCollection Spans => ImmutableList.Empty; + public virtual IReadOnlyCollection Spans => ImmutableList.Empty; public IReadOnlyCollection Breadcrumbs => ImmutableList.Empty; diff --git a/src/Sentry/Internal/UnsampledTransaction.cs b/src/Sentry/Internal/UnsampledTransaction.cs new file mode 100644 index 0000000000..b1c946248d --- /dev/null +++ b/src/Sentry/Internal/UnsampledTransaction.cs @@ -0,0 +1,83 @@ +using Sentry.Extensibility; + +namespace Sentry.Internal; + +/// +/// We know already, when starting a transaction, whether it's going to be sampled or not. When it's not sampled, we can +/// avoid lots of unecessary processing. The only thing we need to track is the number of spans that would have been +/// created (the client reports detailing discarded events includes this detail). +/// +internal sealed class UnsampledTransaction : NoOpTransaction +{ + // Although it's a little bit wasteful to create separate individual class instances here when all we're going to + // report to sentry is the span count (in the client report), SDK users may refer to things like + // `ITransaction.Spans.Count`, so we create an actual collection + private readonly ConcurrentBag _spans = []; + private readonly IHub _hub; + private readonly ITransactionContext _context; + private readonly SentryOptions? _options; + + public UnsampledTransaction(IHub hub, ITransactionContext context) + { + _hub = hub; + _options = _hub.GetSentryOptions(); + _options?.LogDebug("Starting unsampled transaction"); + _context = context; + } + + internal DynamicSamplingContext? DynamicSamplingContext { get; set; } + + public override IReadOnlyCollection Spans => _spans; + + public override SpanId SpanId => _context.SpanId; + public override SentryId TraceId => _context.TraceId; + public override bool? IsSampled => false; + + public override string Name + { + get => _context.Name; + set { } + } + + public override string Operation + { + get => _context.Operation; + set { } + } + + public override void Finish() + { + _options?.LogDebug("Finishing unsampled transaction"); + + // Clear the transaction from the scope + _hub.ConfigureScope(scope => scope.ResetTransaction(this)); + + // Record the discarded events + var spanCount = Spans.Count + 1; // 1 for each span + 1 for the transaction itself + _options?.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.SampleRate, DataCategory.Transaction); + _options?.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.SampleRate, DataCategory.Span, spanCount); + + _options?.LogDebug("Finished unsampled transaction"); + } + + public override void Finish(SpanStatus status) => Finish(); + + public override void Finish(Exception exception, SpanStatus status) => Finish(); + + public override void Finish(Exception exception) => Finish(); + + /// + public override SentryTraceHeader GetTraceHeader() => new(TraceId, SpanId, IsSampled); + + public override ISpan StartChild(string operation) + { + var span = new UnsampledSpan(this); + _spans.Add(span); + return span; + } + + private class UnsampledSpan(UnsampledTransaction transaction) : NoOpSpan + { + public override ISpan StartChild(string operation) => transaction.StartChild(operation); + } +} From 4b51b6e215e9655aa4709883031279ffcb0f06dd Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Mon, 17 Feb 2025 05:16:27 +0000 Subject: [PATCH 2/2] Format code --- src/Sentry/Internal/UnsampledTransaction.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry/Internal/UnsampledTransaction.cs b/src/Sentry/Internal/UnsampledTransaction.cs index b1c946248d..a65d45fe15 100644 --- a/src/Sentry/Internal/UnsampledTransaction.cs +++ b/src/Sentry/Internal/UnsampledTransaction.cs @@ -76,7 +76,7 @@ public override ISpan StartChild(string operation) return span; } - private class UnsampledSpan(UnsampledTransaction transaction) : NoOpSpan + private class UnsampledSpan(UnsampledTransaction transaction) : NoOpSpan { public override ISpan StartChild(string operation) => transaction.StartChild(operation); }