From 0ac1eed9198ed3ff6a9da538a7e34be4807120c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Thu, 6 Apr 2023 07:47:01 +0000 Subject: [PATCH] Remove breadcrumbs from transaction to avoid duplication (#1366) --- CHANGELOG.md | 1 + dart/lib/src/sentry_client.dart | 45 ++++++++++++++++++------------- dart/test/sentry_client_test.dart | 33 +++++++++++++++++++---- 3 files changed, 55 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c80aca64c3..bece573012 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Fix Dart web builds breaking due to `dart:io` imports when using `SentryIsolate` or `SentryIsolateExtension` ([#1371](https://github.com/getsentry/sentry-dart/pull/1371)) - When using `SentryIsolate` or `SentryIsolateExtension`, import `sentry_io.dart`. +- Remove breadcrumbs from transaction to avoid duplication ([#1366](https://github.com/getsentry/sentry-dart/pull/1366)) ## 7.4.0 diff --git a/dart/lib/src/sentry_client.dart b/dart/lib/src/sentry_client.dart index 44489f42a6..bd3ccc585d 100644 --- a/dart/lib/src/sentry_client.dart +++ b/dart/lib/src/sentry_client.dart @@ -108,18 +108,7 @@ class SentryClient { return _sentryId; } - if (_options.platformChecker.platform.isAndroid && - _options.enableScopeSync) { - /* - We do this to avoid duplicate breadcrumbs on Android as sentry-android applies the breadcrumbs - from the native scope onto every envelope sent through it. This scope will contain the breadcrumbs - sent through the scope sync feature. This causes duplicate breadcrumbs. - We then remove the breadcrumbs in all cases but if it is handled == false, - this is a signal that the app would crash and android would lose the breadcrumbs by the time the app is restarted to read - the envelope. - */ - preparedEvent = _eventWithRemovedBreadcrumbsIfHandled(preparedEvent); - } + preparedEvent = _eventWithoutBreadcrumbsIfNeeded(preparedEvent); var attachments = List.from(scope?.attachments ?? []); var screenshot = hint.screenshot; @@ -329,6 +318,8 @@ class SentryClient { return _sentryId; } + preparedTransaction = _eventWithoutBreadcrumbsIfNeeded(preparedTransaction); + final attachments = scope?.attachments .where((element) => element.addToTransactions) .toList(); @@ -462,18 +453,34 @@ class SentryClient { _options.recorder.recordLostEvent(reason, category); } - SentryEvent _eventWithRemovedBreadcrumbsIfHandled(SentryEvent event) { + T _eventWithoutBreadcrumbsIfNeeded(T event) { + if (_shouldRemoveBreadcrumbs(event)) { + return event.copyWith(breadcrumbs: []) as T; + } else { + return event; + } + } + + /// We do this to avoid duplicate breadcrumbs on Android as sentry-android applies the breadcrumbs + /// from the native scope onto every envelope sent through it. This scope will contain the breadcrumbs + /// sent through the scope sync feature. This causes duplicate breadcrumbs. + /// We then remove the breadcrumbs in all cases but if it is handled == false, + /// this is a signal that the app would crash and android would lose the breadcrumbs by the time the app is restarted to read + /// the envelope. + bool _shouldRemoveBreadcrumbs(SentryEvent event) { + final isAndroid = _options.platformChecker.platform.isAndroid; + final enableScopeSync = _options.enableScopeSync; + + if (!isAndroid || !enableScopeSync) { + return false; + } + final mechanisms = (event.exceptions ?? []).map((e) => e.mechanism).whereType(); final hasNoMechanism = mechanisms.isEmpty; final hasOnlyHandledMechanism = mechanisms.every((e) => (e.handled ?? true)); - - if (hasNoMechanism || hasOnlyHandledMechanism) { - return event.copyWith(breadcrumbs: []); - } else { - return event; - } + return hasNoMechanism || hasOnlyHandledMechanism; } Future _attachClientReportsAndSend(SentryEnvelope envelope) { diff --git a/dart/test/sentry_client_test.dart b/dart/test/sentry_client_test.dart index 3839f28ffb..4062f59b98 100644 --- a/dart/test/sentry_client_test.dart +++ b/dart/test/sentry_client_test.dart @@ -1154,7 +1154,28 @@ void main() { fixture = Fixture(); }); - test('Clears breadcrumbs on Android if mechanism.handled is true', + test('Clears breadcrumbs on Android for transaction', () async { + fixture.options.enableScopeSync = true; + fixture.options.platformChecker = + MockPlatformChecker(platform: MockPlatform.android()); + + final client = fixture.getSut(); + final transaction = SentryTransaction( + fixture.tracer, + breadcrumbs: [ + Breadcrumb(), + ], + ); + await client.captureTransaction(transaction); + + final capturedEnvelope = (fixture.transport).envelopes.first; + final capturedTransaction = + await transactionFromEnvelope(capturedEnvelope); + + expect((capturedTransaction['breadcrumbs'] ?? []).isEmpty, true); + }); + + test('Clears breadcrumbs on Android if mechanism.handled is true for event', () async { fixture.options.enableScopeSync = true; fixture.options.platformChecker = @@ -1181,7 +1202,7 @@ void main() { expect((capturedEvent.breadcrumbs ?? []).isEmpty, true); }); - test('Clears breadcrumbs on Android if mechanism.handled is null', + test('Clears breadcrumbs on Android if mechanism.handled is null for event', () async { fixture.options.enableScopeSync = true; fixture.options.platformChecker = @@ -1205,7 +1226,8 @@ void main() { expect((capturedEvent.breadcrumbs ?? []).isEmpty, true); }); - test('Clears breadcrumbs on Android if theres no mechanism', () async { + test('Clears breadcrumbs on Android if theres no mechanism for event', + () async { fixture.options.enableScopeSync = true; fixture.options.platformChecker = MockPlatformChecker(platform: MockPlatform.android()); @@ -1227,7 +1249,8 @@ void main() { expect((capturedEvent.breadcrumbs ?? []).isEmpty, true); }); - test('Does not clear breadcrumbs on Android if mechanism.handled is false', + test( + 'Does not clear breadcrumbs on Android if mechanism.handled is false for event', () async { fixture.options.enableScopeSync = true; fixture.options.platformChecker = @@ -1255,7 +1278,7 @@ void main() { }); test( - 'Does not clear breadcrumbs on Android if any mechanism.handled is false', + 'Does not clear breadcrumbs on Android if any mechanism.handled is false for event', () async { fixture.options.enableScopeSync = true; fixture.options.platformChecker =