Remove duplicated breadcrumbs when syncing with iOS/macOS #1283

denrase · 2023-02-13T15:18:28Z

📜 Description

By fixing the issue that scope sync was never enabled on iOS with #1196, we introduced the issue that breadcrumbs could be duplicated because of bi-directional sync by calling addBreadcrumbs, and loading the native context with loadContexts to enrich events, which also includes the native breadcrumbs.

Create breadcrumbs with correct timestamp when syncing down to native layer through addBreadcrumbs
Remove duplicate breadcrumbs when syncing up from native layer through loadContexts

💡 Motivation and Context

Closes #1270

💚 How did you test it?

Added unit tests

📝 Checklist

I reviewed submitted code
I added tests to verify changes
No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
I updated the docs if needed
All tests passing
No breaking changes

🔮 Next steps

This approach in this PR fixes the issue, but we now have to compare all breadcrumbs with each other, which could get slow with a larger number of breadcrumbs.

Another way would be to just remove syncing breadcrumbs down to the native layer.

github-actions · 2023-02-13T15:18:42Z

Android Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	285.38 ms	344.74 ms	59.36 ms
Size	6.06 MiB	7.09 MiB	1.03 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`8fa3934`	340.64 ms	407.92 ms	67.28 ms
`fbf42af`	323.04 ms	365.09 ms	42.04 ms
`21d4150`	379.31 ms	449.23 ms	69.93 ms
`08a7b4f`	346.47 ms	403.29 ms	56.82 ms
`9811573`	309.76 ms	402.60 ms	92.84 ms
`c70e01a`	331.04 ms	401.46 ms	70.42 ms
`48e79fd`	354.22 ms	391.46 ms	37.24 ms
`3637a22`	322.59 ms	390.00 ms	67.41 ms
`4efee31`	308.92 ms	368.68 ms	59.76 ms
`6d7a391`	331.94 ms	367.04 ms	35.10 ms

App size

Revision	Plain	With Sentry	Diff
`8fa3934`	6.06 MiB	7.09 MiB	1.03 MiB
`fbf42af`	5.94 MiB	6.96 MiB	1.02 MiB
`21d4150`	5.94 MiB	6.97 MiB	1.03 MiB
`08a7b4f`	5.94 MiB	6.95 MiB	1.01 MiB
`9811573`	5.94 MiB	6.96 MiB	1.02 MiB
`c70e01a`	5.94 MiB	6.97 MiB	1.03 MiB
`48e79fd`	5.94 MiB	6.95 MiB	1.01 MiB
`3637a22`	6.06 MiB	7.09 MiB	1.03 MiB
`4efee31`	5.94 MiB	6.92 MiB	1003.76 KiB
`6d7a391`	5.94 MiB	6.95 MiB	1.01 MiB

Previous results on branch: fix/duplicate-breadcrumbs-logging

Startup times

Revision	Plain	With Sentry	Diff
`40f188e`	330.40 ms	361.53 ms	31.14 ms
`130de29`	301.22 ms	354.31 ms	53.09 ms
`8d570b0`	310.31 ms	368.27 ms	57.96 ms
`91e64ae`	316.44 ms	359.84 ms	43.40 ms
`c910142`	317.40 ms	355.68 ms	38.28 ms
`a4d5fe6`	302.13 ms	378.06 ms	75.94 ms
`a638fd5`	339.84 ms	379.14 ms	39.30 ms

App size

Revision	Plain	With Sentry	Diff
`40f188e`	6.06 MiB	7.09 MiB	1.03 MiB
`130de29`	6.06 MiB	7.09 MiB	1.03 MiB
`8d570b0`	6.06 MiB	7.09 MiB	1.03 MiB
`91e64ae`	6.06 MiB	7.09 MiB	1.03 MiB
`c910142`	6.06 MiB	7.09 MiB	1.03 MiB
`a4d5fe6`	6.06 MiB	7.09 MiB	1.03 MiB
`a638fd5`	6.06 MiB	7.09 MiB	1.03 MiB

github-actions · 2023-02-13T15:39:40Z

iOS Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1252.86 ms	1267.53 ms	14.67 ms
Size	8.10 MiB	9.08 MiB	1004.34 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`21845e2`	1279.37 ms	1298.81 ms	19.45 ms
`b47809a`	1252.61 ms	1278.57 ms	25.96 ms
`49a149b`	1296.47 ms	1320.20 ms	23.73 ms
`5603ab2`	1268.47 ms	1280.73 ms	12.26 ms
`b728df4`	1287.43 ms	1293.94 ms	6.51 ms
`21d4150`	1252.86 ms	1280.55 ms	27.69 ms
`1edf30e`	1254.43 ms	1272.82 ms	18.39 ms
`ccc09e4`	1254.74 ms	1277.08 ms	22.34 ms
`a49594a`	1284.83 ms	1313.29 ms	28.45 ms
`c70e01a`	1273.00 ms	1299.12 ms	26.12 ms

App size

Revision	Plain	With Sentry	Diff
`21845e2`	8.15 MiB	9.12 MiB	991.34 KiB
`b47809a`	8.16 MiB	9.17 MiB	1.01 MiB
`49a149b`	8.15 MiB	9.12 MiB	986.26 KiB
`5603ab2`	8.15 MiB	9.12 MiB	990.57 KiB
`b728df4`	8.15 MiB	9.15 MiB	1020.72 KiB
`21d4150`	8.16 MiB	9.17 MiB	1.01 MiB
`1edf30e`	8.16 MiB	9.17 MiB	1.01 MiB
`ccc09e4`	8.16 MiB	9.16 MiB	1.01 MiB
`a49594a`	8.16 MiB	9.16 MiB	1.00 MiB
`c70e01a`	8.16 MiB	9.17 MiB	1.01 MiB

Previous results on branch: fix/duplicate-breadcrumbs-logging

Startup times

Revision	Plain	With Sentry	Diff
`40f188e`	1270.11 ms	1275.53 ms	5.42 ms
`8d570b0`	1260.57 ms	1286.88 ms	26.31 ms
`c910142`	1232.19 ms	1265.57 ms	33.39 ms
`a4d5fe6`	1267.67 ms	1285.23 ms	17.57 ms
`130de29`	1251.83 ms	1281.11 ms	29.28 ms
`91e64ae`	1242.90 ms	1254.87 ms	11.98 ms
`a638fd5`	1247.36 ms	1263.37 ms	16.01 ms

App size

Revision	Plain	With Sentry	Diff
`40f188e`	8.10 MiB	9.08 MiB	1004.40 KiB
`8d570b0`	8.10 MiB	9.08 MiB	1003.32 KiB
`c910142`	8.10 MiB	9.08 MiB	1004.28 KiB
`a4d5fe6`	8.10 MiB	9.08 MiB	1004.50 KiB
`130de29`	8.10 MiB	9.08 MiB	1003.55 KiB
`91e64ae`	8.10 MiB	9.08 MiB	1003.30 KiB
`a638fd5`	8.10 MiB	9.08 MiB	1004.35 KiB

codecov · 2023-02-14T08:07:22Z

Codecov Report

Patch coverage has no change and project coverage change: -0.96 ⚠️

Comparison is base (2d3b03d) 89.83% compared to head (340d779) 88.87%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1283      +/-   ##
==========================================
- Coverage   89.83%   88.87%   -0.96%     
==========================================
  Files         155      121      -34     
  Lines        5064     3810    -1254     
==========================================
- Hits         4549     3386    -1163     
+ Misses        515      424      -91

Impacted Files	Coverage Δ
...ib/src/integrations/load_contexts_integration.dart
.../src/user_interaction/user_interaction_widget.dart
...ter/lib/src/integrations/on_error_integration.dart
flutter/lib/src/sentry_flutter_options.dart
.../src/integrations/widgets_binding_integration.dart
flutter/lib/src/renderer/renderer.dart
...r/lib/src/integrations/screenshot_integration.dart
flutter/lib/src/sentry_native_channel.dart
flutter/lib/src/native_scope_observer.dart
...r/lib/src/integrations/native_sdk_integration.dart
... and 24 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

brustolin

LGTM

flutter/lib/src/integrations/load_contexts_integration.dart

flutter/ios/Classes/SentryFlutterPluginApple.swift

marandaneto

Thanks @denrase

marandaneto · 2023-02-28T14:39:55Z

flutter/test/integrations/init_native_sdk_integration_test.dart

@@ -53,7 +53,7 @@ void main() {
        'maxCacheItems': 30,
        'sendDefaultPii': false,
        'enableOutOfMemoryTracking': true,
-        'enableNdkScopeSync': false,
+        'enableNdkScopeSync': true,


@denrase is this intentional?

marandaneto and others added 3 commits February 8, 2023 17:17

fix test

c53fe92

Remove duplicated breadcrumbs when syncing with native layer

4a4ab31

use flutter foundation function to compare maps

e185593

add type argument

8d570b0

denrase added 2 commits February 14, 2023 09:05

add changelog entry

393ee9f

remove whitespace

91e64ae

denrase marked this pull request as ready for review February 14, 2023 09:11

denrase requested review from marandaneto, brustolin and krystofwoldrich as code owners February 14, 2023 09:11

brustolin approved these changes Feb 16, 2023

View reviewed changes

marandaneto reviewed Feb 27, 2023

View reviewed changes

flutter/lib/src/integrations/load_contexts_integration.dart Outdated Show resolved Hide resolved

marandaneto reviewed Feb 27, 2023

View reviewed changes

flutter/ios/Classes/SentryFlutterPluginApple.swift Outdated Show resolved Hide resolved

denrase added 2 commits February 27, 2023 17:26

take breadcrumbs from native if scope sync is disabled

130de29

use same date formatters with fallback as in cocoa sdk

a638fd5

denrase requested a review from marandaneto February 28, 2023 09:35

marandaneto approved these changes Feb 28, 2023

View reviewed changes

denrase and others added 7 commits February 28, 2023 10:58

remove obsolete test

55e5c99

sort native breadcrumbs

c910142

no need to sort

a4d5fe6

add explicit return types

40f188e

Merge branch 'main' into fix/duplicate-breadcrumbs-logging

29161ad

Merge branch 'main' of github.com:getsentry/sentry-dart

3a22de5

Merge branch 'main' into fix/duplicate-breadcrumbs-logging

57873cf

marandaneto reviewed Feb 28, 2023

View reviewed changes

rollback test

340d779

marandaneto merged commit d2f62d6 into main Feb 28, 2023

marandaneto deleted the fix/duplicate-breadcrumbs-logging branch February 28, 2023 15:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove duplicated breadcrumbs when syncing with iOS/macOS #1283

Remove duplicated breadcrumbs when syncing with iOS/macOS #1283

denrase commented Feb 13, 2023 •

edited

github-actions bot commented Feb 13, 2023 •

edited

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/duplicate-breadcrumbs-logging

Startup times

App size

github-actions bot commented Feb 13, 2023 •

edited

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/duplicate-breadcrumbs-logging

Startup times

App size

codecov bot commented Feb 14, 2023 •

edited

brustolin left a comment

marandaneto left a comment

marandaneto Feb 28, 2023

Remove duplicated breadcrumbs when syncing with iOS/macOS #1283

Remove duplicated breadcrumbs when syncing with iOS/macOS #1283

Conversation

denrase commented Feb 13, 2023 • edited

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Feb 13, 2023 • edited

Android Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/duplicate-breadcrumbs-logging

Startup times

App size

github-actions bot commented Feb 13, 2023 • edited

iOS Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: fix/duplicate-breadcrumbs-logging

Startup times

App size

codecov bot commented Feb 14, 2023 • edited

Codecov Report

brustolin left a comment

Choose a reason for hiding this comment

marandaneto left a comment

Choose a reason for hiding this comment

marandaneto Feb 28, 2023

Choose a reason for hiding this comment

denrase commented Feb 13, 2023 •

edited

github-actions bot commented Feb 13, 2023 •

edited

github-actions bot commented Feb 13, 2023 •

edited

codecov bot commented Feb 14, 2023 •

edited