Record dropped spans in client reports (#2154)

* Record dropped spans

* Changelog

* Naming

* Update CHANGELOG.md

* Send dropped event as well for rate limit and network error

* Update

* Dart analyze

* Fix test

* Improve comments

* improvements

* Apply same logic of beforeSend to event processor

* Fix test

* Formatting

* Comments

* Rename mock

Author: buenaflor

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Features
 
+- Record dropped spans in client reports ([#2154](https://github.com/getsentry/sentry-dart/pull/2154))
 - Add memory usage to contexts ([#2133](https://github.com/getsentry/sentry-dart/pull/2133))
   - Only for Linux/Windows applications, as iOS/Android/macOS use native SDKs
 

dart/lib/src/client_reports/client_report_recorder.dart

@@ -13,11 +13,11 @@ class ClientReportRecorder {
   final ClockProvider _clock;
   final Map<_QuantityKey, int> _quantities = {};
 
-  void recordLostEvent(
-      final DiscardReason reason, final DataCategory category) {
+  void recordLostEvent(final DiscardReason reason, final DataCategory category,
+      {int count = 1}) {
     final key = _QuantityKey(reason, category);
     var current = _quantities[key] ?? 0;
-    _quantities[key] = current + 1;
+    _quantities[key] = current + count;
   }
 
   ClientReport? flush() {

dart/lib/src/client_reports/discarded_event.dart

@@ -54,6 +54,8 @@ extension _DataCategoryExtension on DataCategory {
         return 'session';
       case DataCategory.transaction:
         return 'transaction';
+      case DataCategory.span:
+        return 'span';
       case DataCategory.attachment:
         return 'attachment';
       case DataCategory.security:

dart/lib/src/client_reports/noop_client_report_recorder.dart

@@ -15,5 +15,6 @@ class NoOpClientReportRecorder implements ClientReportRecorder {
   }
 
   @override
-  void recordLostEvent(DiscardReason reason, DataCategory category) {}
+  void recordLostEvent(DiscardReason reason, DataCategory category,
+      {int count = 1}) {}
 }

dart/lib/src/hub.dart

@@ -542,6 +542,11 @@ class Hub {
           DiscardReason.sampleRate,
           DataCategory.transaction,
         );
+        _options.recorder.recordLostEvent(
+          DiscardReason.sampleRate,
+          DataCategory.span,
+          count: transaction.spans.length + 1,
+        );
         _options.logger(
           SentryLevel.warning,
           'Transaction ${transaction.eventId} was dropped due to sampling decision.',

dart/lib/src/sentry_client.dart

@@ -84,7 +84,8 @@ class SentryClient {
     Hint? hint,
   }) async {
     if (_sampleRate()) {
-      _recordLostEvent(event, DiscardReason.sampleRate);
+      _options.recorder
+          .recordLostEvent(DiscardReason.sampleRate, _getCategory(event));
       _options.logger(
         SentryLevel.debug,
         'Event ${event.eventId.toString()} was dropped due to sampling decision.',
@@ -403,7 +404,9 @@ class SentryClient {
     SentryEvent event,
     Hint hint,
   ) async {
-    SentryEvent? eventOrTransaction = event;
+    SentryEvent? processedEvent = event;
+    final spanCountBeforeCallback =
+        event is SentryTransaction ? event.spans.length : 0;
 
     final beforeSend = _options.beforeSend;
     final beforeSendTransaction = _options.beforeSendTransaction;
@@ -412,18 +415,18 @@ class SentryClient {
     try {
       if (event is SentryTransaction && beforeSendTransaction != null) {
         beforeSendName = 'beforeSendTransaction';
-        final e = beforeSendTransaction(event);
-        if (e is Future<SentryTransaction?>) {
-          eventOrTransaction = await e;
+        final callbackResult = beforeSendTransaction(event);
+        if (callbackResult is Future<SentryTransaction?>) {
+          processedEvent = await callbackResult;
         } else {
-          eventOrTransaction = e;
+          processedEvent = callbackResult;
         }
       } else if (beforeSend != null) {
-        final e = beforeSend(event, hint);
-        if (e is Future<SentryEvent?>) {
-          eventOrTransaction = await e;
+        final callbackResult = beforeSend(event, hint);
+        if (callbackResult is Future<SentryEvent?>) {
+          processedEvent = await callbackResult;
         } else {
-          eventOrTransaction = e;
+          processedEvent = callbackResult;
         }
       }
     } catch (exception, stackTrace) {
@@ -438,15 +441,30 @@ class SentryClient {
       }
     }
 
-    if (eventOrTransaction == null) {
-      _recordLostEvent(event, DiscardReason.beforeSend);
+    final discardReason = DiscardReason.beforeSend;
+    if (processedEvent == null) {
+      _options.recorder.recordLostEvent(discardReason, _getCategory(event));
+      if (event is SentryTransaction) {
+        // We dropped the whole transaction, the dropped count includes all child spans + 1 root span
+        _options.recorder.recordLostEvent(discardReason, DataCategory.span,
+            count: spanCountBeforeCallback + 1);
+      }
       _options.logger(
         SentryLevel.debug,
         '${event.runtimeType} was dropped by $beforeSendName callback',
       );
+    } else if (event is SentryTransaction &&
+        processedEvent is SentryTransaction) {
+      // If beforeSend removed only some spans we still report them as dropped
+      final spanCountAfterCallback = processedEvent.spans.length;
+      final droppedSpanCount = spanCountBeforeCallback - spanCountAfterCallback;
+      if (droppedSpanCount > 0) {
+        _options.recorder.recordLostEvent(discardReason, DataCategory.span,
+            count: droppedSpanCount);
+      }
     }
 
-    return eventOrTransaction;
+    return processedEvent;
   }
 
   Future<SentryEvent?> _runEventProcessors(
@@ -455,6 +473,9 @@ class SentryClient {
     required List<EventProcessor> eventProcessors,
   }) async {
     SentryEvent? processedEvent = event;
+    int spanCountBeforeEventProcessors =
+        event is SentryTransaction ? event.spans.length : 0;
+
     for (final processor in eventProcessors) {
       try {
         final e = processor.apply(processedEvent!, hint);
@@ -474,12 +495,29 @@ class SentryClient {
           rethrow;
         }
       }
+
+      final discardReason = DiscardReason.eventProcessor;
       if (processedEvent == null) {
-        _recordLostEvent(event, DiscardReason.eventProcessor);
+        _options.recorder.recordLostEvent(discardReason, _getCategory(event));
+        if (event is SentryTransaction) {
+          // We dropped the whole transaction, the dropped count includes all child spans + 1 root span
+          _options.recorder.recordLostEvent(discardReason, DataCategory.span,
+              count: spanCountBeforeEventProcessors + 1);
+        }
         _options.logger(SentryLevel.debug, 'Event was dropped by a processor');
-        break;
+      } else if (event is SentryTransaction &&
+          processedEvent is SentryTransaction) {
+        // If event processor removed only some spans we still report them as dropped
+        final spanCountAfterEventProcessors = processedEvent.spans.length;
+        final droppedSpanCount =
+            spanCountBeforeEventProcessors - spanCountAfterEventProcessors;
+        if (droppedSpanCount > 0) {
+          _options.recorder.recordLostEvent(discardReason, DataCategory.span,
+              count: droppedSpanCount);
+        }
       }
     }
+
     return processedEvent;
   }
 
@@ -490,14 +528,11 @@ class SentryClient {
     return false;
   }
 
-  void _recordLostEvent(SentryEvent event, DiscardReason reason) {
-    DataCategory category;
+  DataCategory _getCategory(SentryEvent event) {
     if (event is SentryTransaction) {
-      category = DataCategory.transaction;
-    } else {
-      category = DataCategory.error;
+      return DataCategory.transaction;
     }
-    _options.recorder.recordLostEvent(reason, category);
+    return DataCategory.error;
   }
 
   Future<SentryId?> _attachClientReportsAndSend(SentryEnvelope envelope) {

dart/lib/src/sentry_envelope_item.dart

@@ -12,7 +12,10 @@ import 'sentry_user_feedback.dart';
 
 /// Item holding header information and JSON encoded data.
 class SentryEnvelopeItem {
-  SentryEnvelopeItem(this.header, this.dataFactory);
+  /// The original, non-encoded object, used when direct access to the source data is needed.
+  Object? originalObject;
+
+  SentryEnvelopeItem(this.header, this.dataFactory, {this.originalObject});
 
   /// Creates a [SentryEnvelopeItem] which sends [SentryTransaction].
   factory SentryEnvelopeItem.fromTransaction(SentryTransaction transaction) {
@@ -24,7 +27,8 @@ class SentryEnvelopeItem {
       cachedItem.getDataLength,
       contentType: 'application/json',
     );
-    return SentryEnvelopeItem(header, cachedItem.getData);
+    return SentryEnvelopeItem(header, cachedItem.getData,
+        originalObject: transaction);
   }
 
   factory SentryEnvelopeItem.fromAttachment(SentryAttachment attachment) {
@@ -37,7 +41,8 @@ class SentryEnvelopeItem {
       fileName: attachment.filename,
       attachmentType: attachment.attachmentType,
     );
-    return SentryEnvelopeItem(header, cachedItem.getData);
+    return SentryEnvelopeItem(header, cachedItem.getData,
+        originalObject: attachment);
   }
 
   /// Create a [SentryEnvelopeItem] which sends [SentryUserFeedback].
@@ -50,7 +55,8 @@ class SentryEnvelopeItem {
       cachedItem.getDataLength,
       contentType: 'application/json',
     );
-    return SentryEnvelopeItem(header, cachedItem.getData);
+    return SentryEnvelopeItem(header, cachedItem.getData,
+        originalObject: feedback);
   }
 
   /// Create a [SentryEnvelopeItem] which holds the [SentryEvent] data.
@@ -59,13 +65,13 @@ class SentryEnvelopeItem {
         _CachedItem(() async => utf8JsonEncoder.convert(event.toJson()));
 
     return SentryEnvelopeItem(
-      SentryEnvelopeItemHeader(
-        SentryItemType.event,
-        cachedItem.getDataLength,
-        contentType: 'application/json',
-      ),
-      cachedItem.getData,
-    );
+        SentryEnvelopeItemHeader(
+          SentryItemType.event,
+          cachedItem.getDataLength,
+          contentType: 'application/json',
+        ),
+        cachedItem.getData,
+        originalObject: event);
   }
 
   /// Create a [SentryEnvelopeItem] which holds the [ClientReport] data.
@@ -80,6 +86,7 @@ class SentryEnvelopeItem {
         contentType: 'application/json',
       ),
       cachedItem.getData,
+      originalObject: clientReport,
     );
   }
 
@@ -102,7 +109,8 @@ class SentryEnvelopeItem {
       cachedItem.getDataLength,
       contentType: 'application/octet-stream',
     );
-    return SentryEnvelopeItem(header, cachedItem.getData);
+    return SentryEnvelopeItem(header, cachedItem.getData,
+        originalObject: buckets);
   }
 
   /// Header with info about type and length of data in bytes.

dart/lib/src/transport/data_category.dart

@@ -5,8 +5,28 @@ enum DataCategory {
   error,
   session,
   transaction,
+  span,
   attachment,
   security,
   metricBucket,
-  unknown
+  unknown;
+
+  static DataCategory fromItemType(String itemType) {
+    switch (itemType) {
+      case 'event':
+        return DataCategory.error;
+      case 'session':
+        return DataCategory.session;
+      case 'attachment':
+        return DataCategory.attachment;
+      case 'transaction':
+        return DataCategory.transaction;
+      // The envelope item type used for metrics is statsd,
+      // whereas the client report category is metric_bucket
+      case 'statsd':
+        return DataCategory.metricBucket;
+      default:
+        return DataCategory.unknown;
+    }
+  }
 }

dart/lib/src/transport/rate_limiter.dart

@@ -1,7 +1,5 @@
+import '../../sentry.dart';
 import '../transport/rate_limit_parser.dart';
-import '../sentry_options.dart';
-import '../sentry_envelope.dart';
-import '../sentry_envelope_item.dart';
 import 'rate_limit.dart';
 import 'data_category.dart';
 import '../client_reports/discard_reason.dart';
@@ -25,8 +23,17 @@ class RateLimiter {
 
         _options.recorder.recordLostEvent(
           DiscardReason.rateLimitBackoff,
-          _categoryFromItemType(item.header.type),
+          DataCategory.fromItemType(item.header.type),
         );
+
+        final originalObject = item.originalObject;
+        if (originalObject is SentryTransaction) {
+          _options.recorder.recordLostEvent(
+            DiscardReason.rateLimitBackoff,
+            DataCategory.span,
+            count: originalObject.spans.length + 1,
+          );
+        }
       }
     }
 
@@ -80,7 +87,7 @@ class RateLimiter {
   // Private
 
   bool _isRetryAfter(String itemType) {
-    final dataCategory = _categoryFromItemType(itemType);
+    final dataCategory = DataCategory.fromItemType(itemType);
     final currentDate = DateTime.fromMillisecondsSinceEpoch(
         _options.clock().millisecondsSinceEpoch);
 
@@ -106,25 +113,6 @@ class RateLimiter {
     return false;
   }
 
-  DataCategory _categoryFromItemType(String itemType) {
-    switch (itemType) {
-      case 'event':
-        return DataCategory.error;
-      case 'session':
-        return DataCategory.session;
-      case 'attachment':
-        return DataCategory.attachment;
-      case 'transaction':
-        return DataCategory.transaction;
-      // The envelope item type used for metrics is statsd,
-      // whereas the client report category is metric_bucket
-      case 'statsd':
-        return DataCategory.metricBucket;
-      default:
-        return DataCategory.unknown;
-    }
-  }
-
   void _applyRetryAfterOnlyIfLonger(DataCategory dataCategory, DateTime date) {
     final oldDate = _rateLimitedUntil[dataCategory];
 

dart/lib/src/utils/transport_utils.dart

@@ -19,8 +19,21 @@ class TransportUtils {
       }
 
       if (response.statusCode >= 400 && response.statusCode != 429) {
-        options.recorder
-            .recordLostEvent(DiscardReason.networkError, DataCategory.error);
+        for (final item in envelope.items) {
+          options.recorder.recordLostEvent(
+            DiscardReason.networkError,
+            DataCategory.fromItemType(item.header.type),
+          );
+
+          final originalObject = item.originalObject;
+          if (originalObject is SentryTransaction) {
+            options.recorder.recordLostEvent(
+              DiscardReason.networkError,
+              DataCategory.span,
+              count: originalObject.spans.length + 1,
+            );
+          }
+        }
       }
     } else {
       options.logger(