Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send trace origin #1534

Merged
merged 29 commits into from
Jul 17, 2023
Merged

Send trace origin #1534

merged 29 commits into from
Jul 17, 2023

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Jun 26, 2023

📜 Description

Sentry trace origin for spans and contexts.

See https://develop.sentry.dev/sdk/performance/trace-origin

💡 Motivation and Context

Closes #1420

💚 How did you test it?

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

@github-actions
Copy link
Contributor

github-actions bot commented Jun 26, 2023

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 340.69 ms 406.58 ms 65.89 ms
Size 6.16 MiB 7.14 MiB 1004.21 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
a609134 350.12 ms 404.12 ms 54.00 ms
633cf2e 289.36 ms 340.38 ms 51.02 ms
ef31c7f 311.39 ms 359.33 ms 47.94 ms
7f2b01d 304.94 ms 345.71 ms 40.78 ms
379d7a8 327.10 ms 355.39 ms 28.29 ms
8d64376 302.88 ms 356.84 ms 53.96 ms
f2db4ec 372.46 ms 469.72 ms 97.26 ms
453e1bc 320.41 ms 372.73 ms 52.32 ms
56810ff 309.72 ms 352.26 ms 42.54 ms
211a7aa 324.19 ms 393.26 ms 69.07 ms

App size

Revision Plain With Sentry Diff
a609134 5.94 MiB 6.95 MiB 1.01 MiB
633cf2e 5.94 MiB 6.92 MiB 1001.53 KiB
ef31c7f 6.06 MiB 7.09 MiB 1.03 MiB
7f2b01d 5.94 MiB 6.95 MiB 1.01 MiB
379d7a8 5.94 MiB 6.95 MiB 1.01 MiB
8d64376 5.94 MiB 6.96 MiB 1.02 MiB
f2db4ec 6.06 MiB 7.03 MiB 990.27 KiB
453e1bc 5.94 MiB 6.95 MiB 1.01 MiB
56810ff 5.94 MiB 6.92 MiB 1001.71 KiB
211a7aa 6.06 MiB 7.03 MiB 997.24 KiB

Previous results on branch: feat/trace-origin

Startup times

Revision Plain With Sentry Diff
0addffe 308.02 ms 391.16 ms 83.14 ms
98192ae 287.72 ms 344.59 ms 56.87 ms
1988ebe 283.81 ms 382.80 ms 98.99 ms
af45407 297.10 ms 350.04 ms 52.94 ms
8d011af 306.82 ms 359.51 ms 52.69 ms
1ff48ca 297.29 ms 367.74 ms 70.45 ms
9b84879 333.94 ms 400.44 ms 66.50 ms
8d41c70 347.82 ms 407.70 ms 59.89 ms
5fe8c6e 332.54 ms 376.02 ms 43.48 ms
77f6137 296.45 ms 355.78 ms 59.33 ms

App size

Revision Plain With Sentry Diff
0addffe 6.16 MiB 7.14 MiB 1003.75 KiB
98192ae 6.16 MiB 7.14 MiB 1003.74 KiB
1988ebe 6.16 MiB 7.14 MiB 1004.21 KiB
af45407 6.16 MiB 7.14 MiB 1004.21 KiB
8d011af 6.16 MiB 7.14 MiB 1003.75 KiB
1ff48ca 6.16 MiB 7.14 MiB 1004.21 KiB
9b84879 6.16 MiB 7.14 MiB 1003.75 KiB
8d41c70 6.16 MiB 7.14 MiB 1004.21 KiB
5fe8c6e 6.16 MiB 7.14 MiB 1003.74 KiB
77f6137 6.16 MiB 7.14 MiB 1003.74 KiB

@github-actions
Copy link
Contributor

github-actions bot commented Jun 26, 2023

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against bb4030d

@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +6.44 🎉

Comparison is base (6a40d32) 87.20% compared to head (bb4030d) 93.65%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1534      +/-   ##
==========================================
+ Coverage   87.20%   93.65%   +6.44%     
==========================================
  Files          15       57      +42     
  Lines         430     1891    +1461     
==========================================
+ Hits          375     1771    +1396     
- Misses         55      120      +65     
Impacted Files Coverage Δ
.../lib/src/navigation/sentry_navigator_observer.dart 98.94% <ø> (ø)
dio/lib/src/sentry_transformer.dart 96.42% <100.00%> (+0.27%) ⬆️
dio/lib/src/tracing_client_adapter.dart 89.65% <100.00%> (+0.36%) ⬆️
flutter/lib/src/sentry_asset_bundle.dart 94.61% <100.00%> (ø)
sqflite/lib/src/sentry_batch.dart 94.73% <100.00%> (+0.19%) ⬆️
sqflite/lib/src/sentry_database.dart 77.50% <100.00%> (+1.18%) ⬆️
sqflite/lib/src/sentry_database_executor.dart 97.61% <100.00%> (+0.22%) ⬆️
sqflite/lib/src/sentry_sqflite.dart 81.25% <100.00%> (+1.25%) ⬆️
...flite/lib/src/sentry_sqflite_database_factory.dart 73.68% <100.00%> (+1.46%) ⬆️

... and 40 files with indirect coverage changes

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

@github-actions
Copy link
Contributor

github-actions bot commented Jun 26, 2023

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1230.38 ms 1240.27 ms 9.89 ms
Size 8.29 MiB 9.37 MiB 1.08 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b49bf00 1248.00 ms 1260.35 ms 12.35 ms
d301b11 1260.61 ms 1272.06 ms 11.45 ms
c1bb00f 1265.14 ms 1290.85 ms 25.71 ms
eb1a7c1 1281.25 ms 1295.40 ms 14.15 ms
9c5aec6 1266.51 ms 1274.65 ms 8.14 ms
4efee31 1270.33 ms 1285.75 ms 15.42 ms
8d64376 1260.92 ms 1289.32 ms 28.40 ms
873fb42 1261.00 ms 1285.92 ms 24.92 ms
df16b96 1255.24 ms 1259.40 ms 4.16 ms
5aab4c5 1247.08 ms 1271.17 ms 24.09 ms

App size

Revision Plain With Sentry Diff
b49bf00 8.10 MiB 9.08 MiB 1004.36 KiB
d301b11 8.10 MiB 9.07 MiB 1000.82 KiB
c1bb00f 8.09 MiB 9.07 MiB 1001.06 KiB
eb1a7c1 8.15 MiB 9.13 MiB 1000.10 KiB
9c5aec6 8.15 MiB 9.12 MiB 986.23 KiB
4efee31 8.15 MiB 9.12 MiB 991.35 KiB
8d64376 8.16 MiB 9.17 MiB 1.01 MiB
873fb42 8.16 MiB 9.17 MiB 1.01 MiB
df16b96 8.10 MiB 9.16 MiB 1.06 MiB
5aab4c5 8.10 MiB 9.16 MiB 1.07 MiB

Previous results on branch: feat/trace-origin

Startup times

Revision Plain With Sentry Diff
8d011af 1241.12 ms 1241.19 ms 0.07 ms
5fe8c6e 1263.21 ms 1266.28 ms 3.06 ms
8d41c70 1242.39 ms 1265.90 ms 23.51 ms
af45407 1237.60 ms 1247.61 ms 10.01 ms
98192ae 1243.92 ms 1254.69 ms 10.78 ms
9b84879 1234.08 ms 1237.78 ms 3.69 ms
cf9ff62 1252.06 ms 1263.49 ms 11.43 ms
77f6137 1210.02 ms 1229.45 ms 19.43 ms
1988ebe 1231.31 ms 1252.55 ms 21.24 ms
0addffe 1217.14 ms 1225.30 ms 8.16 ms

App size

Revision Plain With Sentry Diff
8d011af 8.29 MiB 9.37 MiB 1.08 MiB
5fe8c6e 8.29 MiB 9.37 MiB 1.08 MiB
8d41c70 8.29 MiB 9.37 MiB 1.08 MiB
af45407 8.29 MiB 9.37 MiB 1.08 MiB
98192ae 8.29 MiB 9.37 MiB 1.08 MiB
9b84879 8.29 MiB 9.37 MiB 1.08 MiB
cf9ff62 8.29 MiB 9.37 MiB 1.08 MiB
77f6137 8.29 MiB 9.37 MiB 1.08 MiB
1988ebe 8.29 MiB 9.37 MiB 1.08 MiB
0addffe 8.29 MiB 9.37 MiB 1.08 MiB

@denrase denrase marked this pull request as ready for review June 26, 2023 13:17
@marandaneto
Copy link
Contributor

@denrase not sure if you looked at but here we go for iOS implementation https://github.com/getsentry/sentry-cocoa/pull/2957/files

dart/lib/src/hub.dart Outdated Show resolved Hide resolved
@marandaneto
Copy link
Contributor

Left a few comments, @philipphofmann is OOO but maybe @brustolin can chime in since he reviewed the iOS bits?

@denrase
Copy link
Collaborator Author

denrase commented Jun 27, 2023

@marandaneto I did check out the iOS PR. To what are you referring with "we go with iOS implementation"? What am I missing? 🤔

@denrase denrase requested a review from marandaneto June 27, 2023 11:26
@marandaneto
Copy link
Contributor

@marandaneto I did check out the iOS PR. To what are you referring with "we go with iOS implementation"? What am I missing? 🤔

Nothing, all good, sorry for the confusion, just wanted to ask because the iOS PR isn't linked to #1420 directly.

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let's just address the last @philipphofmann comment.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @denrase.

@marandaneto
Copy link
Contributor

marandaneto commented Jul 17, 2023

@denrase I see a failing test (only on chrome flutter test --platform chrome):

test/widgets_binding_observer_test.dart: WidgetsBindingObserver lifecycle breadcrumbs (failed)

@marandaneto marandaneto enabled auto-merge (squash) July 17, 2023 12:36
@marandaneto marandaneto merged commit 1e094d3 into main Jul 17, 2023
@marandaneto marandaneto deleted the feat/trace-origin branch July 17, 2023 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send span origin
4 participants