Client reports now include dropped spans (#3463)

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Features
+
+- Client reports now include dropped spans ([#3463](https://github.com/getsentry/sentry-dotnet/pull/3463))
+
 ## 4.8.1
 
 ### Fixes

src/Sentry/Http/HttpTransportBase.cs

@@ -100,14 +100,23 @@ private void ProcessEnvelopeItem(DateTimeOffset now, EnvelopeItem item, List<Env
 
         if (isRateLimited)
         {
-            _options.ClientReportRecorder
-                .RecordDiscardedEvent(DiscardReason.RateLimitBackoff, item.DataCategory);
+            var reason = DiscardReason.RateLimitBackoff;
+            _options.ClientReportRecorder.RecordDiscardedEvent(reason, item.DataCategory);
 
             _options.LogDebug(
                 "{0}: Envelope item of type {1} was discarded because it's rate-limited.",
                 _typeName,
                 item.TryGetType());
 
+            if (item.DataCategory.Equals(DataCategory.Transaction))
+            {
+                if (item.Payload is JsonSerializable { Source: SentryTransaction transaction })
+                {
+                    // Span count + 1 (transaction/root span)
+                    _options.ClientReportRecorder.RecordDiscardedEvent(reason, DataCategory.Span, transaction.Spans.Count + 1);
+                }
+            }
+
             // Check if session update with init=true
             if (item.Payload is JsonSerializable
                 {

src/Sentry/Internal/ClientReportRecorder.cs

@@ -18,7 +18,7 @@ public ClientReportRecorder(SentryOptions options, ISystemClock? clock = default
         _clock = clock ?? SystemClock.Clock;
     }
 
-    public void RecordDiscardedEvent(DiscardReason reason, DataCategory category)
+    public void RecordDiscardedEvent(DiscardReason reason, DataCategory category, int quantity = 1)
     {
         // Don't count discarded events if we're not going to be sending them.
         if (!_options.SendClientReports)
@@ -27,7 +27,7 @@ public void RecordDiscardedEvent(DiscardReason reason, DataCategory category)
         }
 
         // Increment the counter for the discarded event.
-        _discardedEvents.Increment(reason.WithCategory(category));
+        _discardedEvents.Add(reason.WithCategory(category), quantity);
     }
 
     public ClientReport? GenerateClientReport()

src/Sentry/Internal/DataCategory.cs

@@ -9,6 +9,7 @@ namespace Sentry.Internal;
     public static DataCategory Internal = new("internal");
     public static DataCategory Security = new("security");
     public static DataCategory Session = new("session");
+    public static DataCategory Span = new("span");
     public static DataCategory Transaction = new("transaction");
     public static DataCategory Profile = new("profile");
 

src/Sentry/Internal/Extensions/ClientReportExtensions.cs

@@ -9,6 +9,14 @@ public static void RecordDiscardedEvents(this IClientReportRecorder recorder, Di
         foreach (var item in envelope.Items)
         {
             recorder.RecordDiscardedEvent(reason, item.DataCategory);
+            if (item.DataCategory.Equals(DataCategory.Transaction))
+            {
+                if (item.Payload is JsonSerializable { Source: SentryTransaction transaction })
+                {
+                    // Span count + 1 (transaction/root span)
+                    recorder.RecordDiscardedEvent(reason, DataCategory.Span, transaction.Spans.Count + 1);
+                }
+            }
         }
     }
 }

src/Sentry/Internal/IClientReportRecorder.cs

@@ -6,8 +6,9 @@ internal interface IClientReportRecorder
     /// Records one count of a discarded event, with the given <paramref name="reason"/> and <paramref name="category"/>.
     /// </summary>
     /// <param name="reason">The reason for the event being discarded.</param>
-    /// <param name="category">The data category of the event being discarded.</param>
-    void RecordDiscardedEvent(DiscardReason reason, DataCategory category);
+    /// <param name="category">The data category of the event being discardedd.</param>
+    /// <param name="quantity">The number of items discarded (defaults to 1)</param>
+    void RecordDiscardedEvent(DiscardReason reason, DataCategory category, int quantity = 1);
 
     /// <summary>
     /// Generates a <see cref="ClientReport"/> containing counts of discarded events that have been recorded.

src/Sentry/Protocol/Envelopes/EnvelopeItem.cs

@@ -15,6 +15,7 @@ public sealed class EnvelopeItem : ISerializable, IDisposable
     internal const string TypeValueEvent = "event";
     internal const string TypeValueUserReport = "user_report";
     internal const string TypeValueTransaction = "transaction";
+    internal const string TypeValueSpan = "span";
     internal const string TypeValueSession = "session";
     internal const string TypeValueCheckIn = "check_in";
     internal const string TypeValueAttachment = "attachment";
@@ -43,6 +44,7 @@ public sealed class EnvelopeItem : ISerializable, IDisposable
 
         // These ones are equivalent
         TypeValueTransaction => DataCategory.Transaction,
+        TypeValueSpan => DataCategory.Span,
         TypeValueSession => DataCategory.Session,
         TypeValueAttachment => DataCategory.Attachment,
         TypeValueProfile => DataCategory.Profile,

src/Sentry/SentryClient.cs

@@ -126,9 +126,11 @@ public void CaptureTransaction(SentryTransaction transaction, Scope? scope, Sent
         // Sampling decision MUST have been made at this point
         Debug.Assert(transaction.IsSampled is not null, "Attempt to capture transaction without sampling decision.");
 
+        var spanCount = transaction.Spans.Count + 1; // 1 for each span + 1 for the transaction itself
         if (transaction.IsSampled is false)
         {
             _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.SampleRate, DataCategory.Transaction);
+            _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.SampleRate, DataCategory.Span, spanCount);
             _options.LogDebug("Transaction dropped by sampling.");
             return;
         }
@@ -151,6 +153,7 @@ public void CaptureTransaction(SentryTransaction transaction, Scope? scope, Sent
             if (processedTransaction == null) // Rejected transaction
             {
                 _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.EventProcessor, DataCategory.Transaction);
+                _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.EventProcessor, DataCategory.Span, spanCount);
                 _options.LogInfo("Event dropped by processor {0}", processor.GetType().Name);
                 return;
             }
@@ -160,6 +163,7 @@ public void CaptureTransaction(SentryTransaction transaction, Scope? scope, Sent
         if (processedTransaction is null) // Rejected transaction
         {
             _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Transaction);
+            _options.ClientReportRecorder.RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Span, spanCount);
             _options.LogInfo("Transaction dropped by BeforeSendTransaction callback.");
             return;
         }

test/Sentry.Tests/Internals/Extensions/ClientReportExtensionsTests.cs

@@ -0,0 +1,26 @@
+namespace Sentry.Tests.Internals.Extensions;
+
+public class ClientReportExtensionsTests
+{
+    [Fact]
+    public void EnvelopeContainsTransaction_ReportsDroppedSpans()
+    {
+        // Arrange
+        var recorder = Substitute.For<IClientReportRecorder>();
+        var hub = Substitute.For<IHub>();
+        var tracer = new TransactionTracer(hub, "name", "op");
+        var span1 = (SpanTracer)tracer.StartChild(null, tracer.SpanId, "span1");
+        tracer.StartChild(null, span1.SpanId, "span2");
+        tracer.StartChild(null, tracer.SpanId, "span3");
+        var transaction = new SentryTransaction(tracer);
+        var envelope = Envelope.FromTransaction(transaction);
+
+        // Act
+        recorder.RecordDiscardedEvents(DiscardReason.EventProcessor, envelope);
+
+        // Assert
+        recorder.Received(1).RecordDiscardedEvent(DiscardReason.EventProcessor, DataCategory.Transaction, 1);
+        // 1 for each span + 1 for the transaction root span
+        recorder.Received(1).RecordDiscardedEvent(DiscardReason.EventProcessor, DataCategory.Span, transaction.Spans.Count + 1);
+    }
+}

test/Sentry.Tests/Internals/Http/HttpTransportTests.cs

@@ -793,7 +793,7 @@ public async Task CreateRequest_Content_IncludesEvent()
     }
 
     [Fact]
-    public void ProcessEnvelope_ShouldNotAttachClientReportWhenOptionDisabled()
+    public void ProcessEnvelope_SendClientReportsDisabled_ShouldNotAttachClientReport()
     {
         var options = new SentryOptions
         {
@@ -813,4 +813,37 @@ public void ProcessEnvelope_ShouldNotAttachClientReportWhenOptionDisabled()
         Assert.Single(processedEnvelope.Items);
         Assert.Equal("event", processedEnvelope.Items[0].TryGetType());
     }
+
+    [Fact]
+    public void ProcessEnvelope_SendClientReportsEnabled_ShouldReportTransactionsAndSpans()
+    {
+        // Arrange
+        var options = new SentryOptions
+        {
+            SendClientReports = true,
+            ClientReportRecorder = Substitute.For<IClientReportRecorder>()
+        };
+
+        var httpTransport = Substitute.For<HttpTransportBase>(options, null, null);
+        var transactionCategory = new RateLimitCategory(EnvelopeItem.TypeValueTransaction);
+        httpTransport.CategoryLimitResets[transactionCategory] = DateTimeOffset.UtcNow.AddMonths(1);
+
+        var hub = Substitute.For<IHub>();
+        var tracer = new TransactionTracer(hub, "name", "op");
+        var span1 = (SpanTracer)tracer.StartChild(null, tracer.SpanId, "span1");
+        tracer.StartChild(null, span1.SpanId, "span2");
+        tracer.StartChild(null, tracer.SpanId, "span3");
+        var transaction = new SentryTransaction(tracer);
+        var envelope = Envelope.FromTransaction(transaction);
+
+        // Act
+        var processedEnvelope = httpTransport.ProcessEnvelope(envelope);
+
+        // Assert
+        processedEnvelope.Items.Should().BeEmpty();
+        options.ClientReportRecorder.Received(1).RecordDiscardedEvent(DiscardReason.RateLimitBackoff, DataCategory.Transaction, 1);
+        // 1 for each span + 1 for the transaction root span
+        var expectedDiscardedSpanCount = transaction.Spans.Count + 1;
+        options.ClientReportRecorder.Received(1).RecordDiscardedEvent(DiscardReason.RateLimitBackoff, DataCategory.Span, expectedDiscardedSpanCount);
+    }
 }

test/Sentry.Tests/SentryClientTests.cs

@@ -1,3 +1,4 @@
+using NSubstitute.ReceivedExtensions;
 using Sentry.Internal.Http;
 using BackgroundWorker = Sentry.Internal.BackgroundWorker;
 
@@ -862,18 +863,23 @@ public void CaptureTransaction_SampledOut_Dropped()
         // Arrange
         var client = _fixture.GetSut();
 
+        var hub = Substitute.For<IHub>();
+        var transaction = new TransactionTracer(hub, "test name", "test operation");
+        transaction.StartChild("span1");
+        transaction.StartChild("span2");
+        transaction.IsSampled = false; // <-- *** Sampled out ***
+        transaction.EndTimestamp = DateTimeOffset.Now;
+
         // Act
-        client.CaptureTransaction(new SentryTransaction(
-            "test name",
-            "test operation"
-        )
-        {
-            IsSampled = false,
-            EndTimestamp = DateTimeOffset.Now // finished
-        });
+        client.CaptureTransaction(new SentryTransaction(transaction));
 
         // Assert
         _ = client.Worker.DidNotReceive().EnqueueEnvelope(Arg.Any<Envelope>());
+
+        var expectedReason = DiscardReason.SampleRate;
+        var expectedSpanCount = transaction.Spans.Count + 1; // 1 for each span + one for the root transaction / span
+        _fixture.ClientReportRecorder.Received(1).RecordDiscardedEvent(expectedReason, DataCategory.Transaction);
+        _fixture.ClientReportRecorder.Received(1).RecordDiscardedEvent(expectedReason, DataCategory.Span, expectedSpanCount);
     }
 
     [Fact]
@@ -1147,6 +1153,33 @@ public void CaptureTransaction_TransactionProcessor_ReceivesScopeAttachments()
         hint.Attachments.Should().Contain(attachments);
     }
 
+
+    [Fact]
+    public void CaptureTransaction_TransactionProcessorRejectsEvent_RecordDiscardedEvent()
+    {
+        // Arrange
+        var processor = Substitute.For<ISentryTransactionProcessorWithHint>();
+        processor.Process(Arg.Any<SentryTransaction>(), Arg.Any<SentryHint>()).Returns((SentryTransaction)null);
+        _fixture.SentryOptions.AddTransactionProcessor(processor);
+
+        var hub = Substitute.For<IHub>();
+        var transaction = new TransactionTracer(hub, "test name", "test operation");
+        transaction.StartChild("span1");
+        transaction.StartChild("span2");
+        transaction.IsSampled = true;
+        transaction.EndTimestamp = DateTimeOffset.Now; // finished
+
+        // Act
+        _fixture.GetSut().CaptureTransaction(new SentryTransaction(transaction));
+
+        // Assert
+        var reason = DiscardReason.EventProcessor;
+        _fixture.ClientReportRecorder.Received(1).RecordDiscardedEvent(reason, DataCategory.Transaction);
+        // 1 for each span + one for the root transaction / span
+        var expectedDroppedSpanCount = transaction.Spans.Count + 1;
+        _fixture.ClientReportRecorder.Received(1).RecordDiscardedEvent(reason, DataCategory.Span, expectedDroppedSpanCount);
+    }
+
     [Fact]
     public void CaptureTransaction_BeforeSendTransaction_GetsHint()
     {
@@ -1246,17 +1279,24 @@ public void CaptureTransaction_BeforeSendTransaction_SamplingNull_DropsEvent()
     [Fact]
     public void CaptureTransaction_BeforeSendTransaction_RejectEvent_RecordsDiscard()
     {
+        // Arrange
         _fixture.SentryOptions.SetBeforeSendTransaction((_, _) => null);
 
         var sut = _fixture.GetSut();
-        sut.CaptureTransaction(new SentryTransaction("test name", "test operation")
-        {
-            IsSampled = true,
-            EndTimestamp = DateTimeOffset.Now // finished
-        });
+        var hub = Substitute.For<IHub>();
+        var transaction = new TransactionTracer(hub, "test name", "test operation");
+        transaction.StartChild("span1");
+        transaction.StartChild("span2");
+        transaction.IsSampled = true;
+        transaction.EndTimestamp = DateTimeOffset.Now; // finished
 
-        _fixture.ClientReportRecorder.Received(1)
-            .RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Transaction);
+        // Act
+        sut.CaptureTransaction(new SentryTransaction(transaction));
+
+        _fixture.ClientReportRecorder.Received(1).RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Transaction);
+        // 1 for each span + one for the root transaction / span
+        var expectedDroppedSpanCount = transaction.Spans.Count + 1;
+        _fixture.ClientReportRecorder.Received(1).RecordDiscardedEvent(DiscardReason.BeforeSend, DataCategory.Span, expectedDroppedSpanCount);
     }
 
     [Fact]

test/Sentry.Tests/TransactionProcessorTests.Discard.verified.txt

@@ -58,6 +58,7 @@
         Payload: {
           Source: {
             DiscardedEvents: {
+              { Reason = "event_processor", Category = "span" }: 1,
               { Reason = "event_processor", Category = "transaction" }: 1
             }
           }