From dd933d4e20ee057899c79921f0f9b980425e4041 Mon Sep 17 00:00:00 2001 From: Giancarlo Buenaflor Date: Thu, 11 Jul 2024 13:04:46 +0200 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + .../client_report_recorder.dart | 6 +- .../src/client_reports/discarded_event.dart | 2 + .../noop_client_report_recorder.dart | 3 +- dart/lib/src/hub.dart | 5 + dart/lib/src/sentry_client.dart | 77 ++++++++--- dart/lib/src/sentry_envelope_item.dart | 32 +++-- dart/lib/src/transport/data_category.dart | 22 +++- dart/lib/src/transport/rate_limiter.dart | 36 ++---- dart/lib/src/utils/transport_utils.dart | 17 ++- dart/test/hub_test.dart | 15 ++- dart/test/mocks.dart | 19 +++ .../mocks/mock_client_report_recorder.dart | 10 +- dart/test/protocol/rate_limiter_test.dart | 32 ++++- dart/test/sentry_client_test.dart | 122 ++++++++++++++++-- dart/test/transport/http_transport_test.dart | 60 ++++++++- 16 files changed, 368 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88967b15dc..de711bff1d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/dart/lib/src/client_reports/client_report_recorder.dart b/dart/lib/src/client_reports/client_report_recorder.dart index d064941f84..045168f0c5 100644 --- a/dart/lib/src/client_reports/client_report_recorder.dart +++ b/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() { diff --git a/dart/lib/src/client_reports/discarded_event.dart b/dart/lib/src/client_reports/discarded_event.dart index 01caa31ca2..2d413e01a7 100644 --- a/dart/lib/src/client_reports/discarded_event.dart +++ b/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: diff --git a/dart/lib/src/client_reports/noop_client_report_recorder.dart b/dart/lib/src/client_reports/noop_client_report_recorder.dart index acd6472347..b03bd9c9ef 100644 --- a/dart/lib/src/client_reports/noop_client_report_recorder.dart +++ b/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}) {} } diff --git a/dart/lib/src/hub.dart b/dart/lib/src/hub.dart index a8e06e28ed..b69bc5056f 100644 --- a/dart/lib/src/hub.dart +++ b/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.', diff --git a/dart/lib/src/sentry_client.dart b/dart/lib/src/sentry_client.dart index 63fcbfb421..57c1bd9326 100644 --- a/dart/lib/src/sentry_client.dart +++ b/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) { - eventOrTransaction = await e; + final callbackResult = beforeSendTransaction(event); + if (callbackResult is Future) { + processedEvent = await callbackResult; } else { - eventOrTransaction = e; + processedEvent = callbackResult; } } else if (beforeSend != null) { - final e = beforeSend(event, hint); - if (e is Future) { - eventOrTransaction = await e; + final callbackResult = beforeSend(event, hint); + if (callbackResult is Future) { + 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 _runEventProcessors( @@ -455,6 +473,9 @@ class SentryClient { required List 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 _attachClientReportsAndSend(SentryEnvelope envelope) { diff --git a/dart/lib/src/sentry_envelope_item.dart b/dart/lib/src/sentry_envelope_item.dart index 019e1e0a08..b0f19cfccd 100644 --- a/dart/lib/src/sentry_envelope_item.dart +++ b/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. diff --git a/dart/lib/src/transport/data_category.dart b/dart/lib/src/transport/data_category.dart index ecdb1c9500..cbfb26ea58 100644 --- a/dart/lib/src/transport/data_category.dart +++ b/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; + } + } } diff --git a/dart/lib/src/transport/rate_limiter.dart b/dart/lib/src/transport/rate_limiter.dart index ef9b168edd..fa0f7a9018 100644 --- a/dart/lib/src/transport/rate_limiter.dart +++ b/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]; diff --git a/dart/lib/src/utils/transport_utils.dart b/dart/lib/src/utils/transport_utils.dart index fa2f20096a..399809b179 100644 --- a/dart/lib/src/utils/transport_utils.dart +++ b/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( diff --git a/dart/test/hub_test.dart b/dart/test/hub_test.dart index 8dcc622654..c53ab74e48 100644 --- a/dart/test/hub_test.dart +++ b/dart/test/hub_test.dart @@ -694,11 +694,22 @@ void main() { test('record sample rate dropping transaction', () async { final hub = fixture.getSut(sampled: false); var transaction = SentryTransaction(fixture.tracer); + fixture.tracer.startChild('child1'); + fixture.tracer.startChild('child2'); + fixture.tracer.startChild('child3'); await hub.captureTransaction(transaction); - expect(fixture.recorder.reason, DiscardReason.sampleRate); - expect(fixture.recorder.category, DataCategory.transaction); + expect(fixture.recorder.discardedEvents.length, 2); + + // we dropped the whole tracer and it has 3 span children so the span count should be 4 + // 3 children + 1 root span + final spanCount = fixture.recorder.discardedEvents + .firstWhere((element) => + element.category == DataCategory.span && + element.reason == DiscardReason.sampleRate) + .quantity; + expect(spanCount, 4); }); }); diff --git a/dart/test/mocks.dart b/dart/test/mocks.dart index 7c960c8a07..b05843be8e 100644 --- a/dart/test/mocks.dart +++ b/dart/test/mocks.dart @@ -137,6 +137,25 @@ class DropAllEventProcessor implements EventProcessor { } } +class DropSpansEventProcessor implements EventProcessor { + DropSpansEventProcessor(this.numberOfSpansToDrop); + + final int numberOfSpansToDrop; + + @override + SentryEvent? apply(SentryEvent event, Hint hint) { + if (event is SentryTransaction) { + if (numberOfSpansToDrop > event.spans.length) { + throw ArgumentError( + 'numberOfSpansToDrop must be less than the number of spans in the transaction'); + } + final droppedSpans = event.spans.take(numberOfSpansToDrop).toList(); + event.spans.removeWhere((element) => droppedSpans.contains(element)); + } + return event; + } +} + class FunctionEventProcessor implements EventProcessor { FunctionEventProcessor(this.applyFunction); diff --git a/dart/test/mocks/mock_client_report_recorder.dart b/dart/test/mocks/mock_client_report_recorder.dart index fa1af54ed8..4d8eaa5b1d 100644 --- a/dart/test/mocks/mock_client_report_recorder.dart +++ b/dart/test/mocks/mock_client_report_recorder.dart @@ -1,11 +1,11 @@ import 'package:sentry/src/client_reports/client_report_recorder.dart'; import 'package:sentry/src/client_reports/discard_reason.dart'; import 'package:sentry/src/client_reports/client_report.dart'; +import 'package:sentry/src/client_reports/discarded_event.dart'; import 'package:sentry/src/transport/data_category.dart'; class MockClientReportRecorder implements ClientReportRecorder { - DiscardReason? reason; - DataCategory? category; + List discardedEvents = []; ClientReport? clientReport; @@ -18,8 +18,8 @@ class MockClientReportRecorder implements ClientReportRecorder { } @override - void recordLostEvent(DiscardReason reason, DataCategory category) { - this.reason = reason; - this.category = category; + void recordLostEvent(DiscardReason reason, DataCategory category, + {int count = 1}) { + discardedEvents.add(DiscardedEvent(reason, category, count)); } } diff --git a/dart/test/protocol/rate_limiter_test.dart b/dart/test/protocol/rate_limiter_test.dart index 1f52a60003..b4364bceff 100644 --- a/dart/test/protocol/rate_limiter_test.dart +++ b/dart/test/protocol/rate_limiter_test.dart @@ -1,3 +1,4 @@ +import 'package:collection/collection.dart'; import 'package:sentry/sentry.dart'; import 'package:sentry/src/client_reports/discard_reason.dart'; import 'package:sentry/src/transport/data_category.dart'; @@ -205,14 +206,18 @@ void main() { final result = rateLimiter.filter(eventEnvelope); expect(result, isNull); - expect(fixture.mockRecorder.category, DataCategory.error); - expect(fixture.mockRecorder.reason, DiscardReason.rateLimitBackoff); + expect(fixture.mockRecorder.discardedEvents.first.category, + DataCategory.error); + expect(fixture.mockRecorder.discardedEvents.first.reason, + DiscardReason.rateLimitBackoff); }); test('dropping of transaction recorded', () { final rateLimiter = fixture.getSut(); final transaction = fixture.getTransaction(); + transaction.tracer.startChild('child1'); + transaction.tracer.startChild('child2'); final eventItem = SentryEnvelopeItem.fromTransaction(transaction); final eventEnvelope = SentryEnvelope( SentryEnvelopeHeader.newEventId(), @@ -225,8 +230,21 @@ void main() { final result = rateLimiter.filter(eventEnvelope); expect(result, isNull); - expect(fixture.mockRecorder.category, DataCategory.transaction); - expect(fixture.mockRecorder.reason, DiscardReason.rateLimitBackoff); + expect(fixture.mockRecorder.discardedEvents.length, 2); + + final transactionDiscardedEvent = fixture.mockRecorder.discardedEvents + .firstWhereOrNull((element) => + element.category == DataCategory.transaction && + element.reason == DiscardReason.rateLimitBackoff); + + final spanDiscardedEvent = fixture.mockRecorder.discardedEvents + .firstWhereOrNull((element) => + element.category == DataCategory.span && + element.reason == DiscardReason.rateLimitBackoff); + + expect(transactionDiscardedEvent, isNotNull); + expect(spanDiscardedEvent, isNotNull); + expect(spanDiscardedEvent!.quantity, 3); }); test('dropping of metrics recorded', () { @@ -244,8 +262,10 @@ void main() { final result = rateLimiter.filter(eventEnvelope); expect(result, isNull); - expect(fixture.mockRecorder.category, DataCategory.metricBucket); - expect(fixture.mockRecorder.reason, DiscardReason.rateLimitBackoff); + expect(fixture.mockRecorder.discardedEvents.first.category, + DataCategory.metricBucket); + expect(fixture.mockRecorder.discardedEvents.first.reason, + DiscardReason.rateLimitBackoff); }); group('apply rateLimit', () { diff --git a/dart/test/sentry_client_test.dart b/dart/test/sentry_client_test.dart index 153c1515f0..92b6792fdf 100644 --- a/dart/test/sentry_client_test.dart +++ b/dart/test/sentry_client_test.dart @@ -1440,8 +1440,108 @@ void main() { await client.captureEvent(fakeEvent); - expect(fixture.recorder.reason, DiscardReason.eventProcessor); - expect(fixture.recorder.category, DataCategory.error); + expect(fixture.recorder.discardedEvents.first.reason, + DiscardReason.eventProcessor); + expect( + fixture.recorder.discardedEvents.first.category, DataCategory.error); + }); + + test('record event processor dropping transaction', () async { + final sut = fixture.getSut(eventProcessor: DropAllEventProcessor()); + final transaction = SentryTransaction(fixture.tracer); + fixture.tracer.startChild('child1'); + fixture.tracer.startChild('child2'); + fixture.tracer.startChild('child3'); + + await sut.captureTransaction(transaction); + + expect(fixture.recorder.discardedEvents.length, 2); + + final spanCount = fixture.recorder.discardedEvents + .firstWhere((element) => + element.category == DataCategory.span && + element.reason == DiscardReason.eventProcessor) + .quantity; + expect(spanCount, 4); + }); + + test('record event processor dropping partially spans', () async { + final numberOfSpansDropped = 2; + final sut = fixture.getSut( + eventProcessor: DropSpansEventProcessor(numberOfSpansDropped)); + final transaction = SentryTransaction(fixture.tracer); + fixture.tracer.startChild('child1'); + fixture.tracer.startChild('child2'); + fixture.tracer.startChild('child3'); + + await sut.captureTransaction(transaction); + + expect(fixture.recorder.discardedEvents.length, 1); + + final spanCount = fixture.recorder.discardedEvents + .firstWhere((element) => + element.category == DataCategory.span && + element.reason == DiscardReason.eventProcessor) + .quantity; + expect(spanCount, numberOfSpansDropped); + }); + + test('beforeSendTransaction correctly records partially dropped spans', + () async { + final sut = fixture.getSut(); + final transaction = SentryTransaction(fixture.tracer); + fixture.tracer.startChild('child1'); + fixture.tracer.startChild('child2'); + fixture.tracer.startChild('child3'); + + fixture.options.beforeSendTransaction = (transaction) { + if (transaction.tracer == fixture.tracer) { + return null; + } + return transaction; + }; + + await sut.captureTransaction(transaction); + + expect(fixture.recorder.discardedEvents.length, 2); + + final spanCount = fixture.recorder.discardedEvents + .firstWhere((element) => + element.category == DataCategory.span && + element.reason == DiscardReason.beforeSend) + .quantity; + expect(spanCount, 4); + }); + + test('beforeSendTransaction correctly records partially dropped spans', + () async { + final sut = fixture.getSut(); + final transaction = SentryTransaction(fixture.tracer); + fixture.tracer.startChild('child1'); + fixture.tracer.startChild('child2'); + fixture.tracer.startChild('child3'); + + fixture.options.beforeSendTransaction = (transaction) { + if (transaction.tracer == fixture.tracer) { + transaction.spans + .removeWhere((element) => element.context.operation == 'child2'); + return transaction; + } + return transaction; + }; + + await sut.captureTransaction(transaction); + + // we didn't drop the whole transaction, we only have 1 event for the dropped spans + expect(fixture.recorder.discardedEvents.length, 1); + + // tracer has 3 span children and we dropped 1 of them + final spanCount = fixture.recorder.discardedEvents + .firstWhere((element) => + element.category == DataCategory.span && + element.reason == DiscardReason.beforeSend) + .quantity; + expect(spanCount, 1); }); test('record event processor dropping transaction', () async { @@ -1453,8 +1553,10 @@ void main() { await client.captureTransaction(transaction); - expect(fixture.recorder.reason, DiscardReason.eventProcessor); - expect(fixture.recorder.category, DataCategory.transaction); + expect(fixture.recorder.discardedEvents.first.reason, + DiscardReason.eventProcessor); + expect(fixture.recorder.discardedEvents.first.category, + DataCategory.transaction); }); test('record beforeSend dropping event', () async { @@ -1464,8 +1566,10 @@ void main() { await client.captureEvent(fakeEvent); - expect(fixture.recorder.reason, DiscardReason.beforeSend); - expect(fixture.recorder.category, DataCategory.error); + expect(fixture.recorder.discardedEvents.first.reason, + DiscardReason.beforeSend); + expect( + fixture.recorder.discardedEvents.first.category, DataCategory.error); }); test('record sample rate dropping event', () async { @@ -1475,8 +1579,10 @@ void main() { await client.captureEvent(fakeEvent); - expect(fixture.recorder.reason, DiscardReason.sampleRate); - expect(fixture.recorder.category, DataCategory.error); + expect(fixture.recorder.discardedEvents.first.reason, + DiscardReason.sampleRate); + expect( + fixture.recorder.discardedEvents.first.category, DataCategory.error); }); test('user feedback envelope contains dsn', () async { diff --git a/dart/test/transport/http_transport_test.dart b/dart/test/transport/http_transport_test.dart index 8319e21b7d..90065d89d3 100644 --- a/dart/test/transport/http_transport_test.dart +++ b/dart/test/transport/http_transport_test.dart @@ -1,5 +1,6 @@ import 'dart:convert'; +import 'package:collection/collection.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; import 'package:sentry/sentry.dart'; @@ -7,6 +8,7 @@ import 'package:sentry/src/client_reports/discard_reason.dart'; import 'package:sentry/src/sentry_envelope_header.dart'; import 'package:sentry/src/sentry_envelope_item_header.dart'; import 'package:sentry/src/sentry_item_type.dart'; +import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry/src/transport/data_category.dart'; import 'package:sentry/src/transport/http_transport.dart'; import 'package:sentry/src/transport/rate_limiter.dart'; @@ -14,6 +16,7 @@ import 'package:test/test.dart'; import '../mocks.dart'; import '../mocks/mock_client_report_recorder.dart'; +import '../mocks/mock_hub.dart'; void main() { SentryEnvelope givenEnvelope() { @@ -205,8 +208,42 @@ void main() { ); await sut.send(envelope); - expect(fixture.clientReportRecorder.reason, DiscardReason.networkError); - expect(fixture.clientReportRecorder.category, DataCategory.error); + expect(fixture.clientReportRecorder.discardedEvents.first.reason, + DiscardReason.networkError); + expect(fixture.clientReportRecorder.discardedEvents.first.category, + DataCategory.error); + }); + + test('does records lost transaction and span for error >= 400', () async { + final httpMock = MockClient((http.Request request) async { + return http.Response('{}', 400); + }); + final sut = fixture.getSut(httpMock, MockRateLimiter()); + + final transaction = fixture.getTransaction(); + transaction.tracer.startChild('child1'); + transaction.tracer.startChild('child2'); + final envelope = SentryEnvelope.fromTransaction( + transaction, + fixture.options.sdk, + dsn: fixture.options.dsn, + ); + await sut.send(envelope); + + final transactionDiscardedEvent = fixture + .clientReportRecorder.discardedEvents + .firstWhereOrNull((element) => + element.category == DataCategory.transaction && + element.reason == DiscardReason.networkError); + + final spanDiscardedEvent = fixture.clientReportRecorder.discardedEvents + .firstWhereOrNull((element) => + element.category == DataCategory.span && + element.reason == DiscardReason.networkError); + + expect(transactionDiscardedEvent, isNotNull); + expect(spanDiscardedEvent, isNotNull); + expect(spanDiscardedEvent!.quantity, 3); }); test('does not record lost event for error 429', () async { @@ -223,8 +260,7 @@ void main() { ); await sut.send(envelope); - expect(fixture.clientReportRecorder.reason, null); - expect(fixture.clientReportRecorder.category, null); + expect(fixture.clientReportRecorder.discardedEvents.isEmpty, isTrue); }); test('does record lost event for error >= 500', () async { @@ -241,8 +277,10 @@ void main() { ); await sut.send(envelope); - expect(fixture.clientReportRecorder.reason, DiscardReason.networkError); - expect(fixture.clientReportRecorder.category, DataCategory.error); + expect(fixture.clientReportRecorder.discardedEvents.first.reason, + DiscardReason.networkError); + expect(fixture.clientReportRecorder.discardedEvents.first.category, + DataCategory.error); }); }); } @@ -262,4 +300,14 @@ class Fixture { }; return HttpTransport(options, rateLimiter); } + + SentryTransaction getTransaction() { + final context = SentryTransactionContext( + 'name', + 'op', + samplingDecision: SentryTracesSamplingDecision(true), + ); + final tracer = SentryTracer(context, MockHub()); + return SentryTransaction(tracer); + } }