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.

Sign up for GitHub

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

Jump to bottom

[v9]: Flutter Web release health #2794

Open
wants to merge 67 commits into
base: main
Choose a base branch
from
Open

[v9]: Flutter Web release health #2794

wants to merge 67 commits into from

Conversation

buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Mar 13, 2025 •
edited
Loading

📜 Description

Session handling is implemented the same way as the JS Browser SDK.

  • Routing changes finish existing session and creates a new session
  • After event processing errored events trigger a web session update on specific conditions

This means using the SentryNavigatorObserver is a requirement for making release health work on Web otherwise we cannot observer route changes

💡 Motivation and Context

Part of v9, introduces Flutter Web Release Health support via JS SDK

Closes #2716

💚 How did you test it?

image

📝 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

@codecov Codecov
Copy link

codecov bot commented Mar 13, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 87.80488% with 10 lines in your changes missing coverage. Please review.

Project coverage is 88.59%. Comparing base (1107802) to head (9df9352).

Files with missing lines Patch % Lines
.../lib/src/integrations/web_session_integration.dart 86.95% 3 Missing ⚠️
dart/lib/src/sentry_client.dart 71.42% 2 Missing ⚠️
dart/lib/src/sentry_options.dart 66.66% 2 Missing ⚠️
flutter/lib/src/native/sentry_native_channel.dart 80.00% 2 Missing ⚠️
flutter/lib/src/sentry_flutter.dart 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2794      +/-   ##
==========================================
- Coverage   88.62%   88.59%   -0.04%     
==========================================
  Files         263      265       +2     
  Lines        8766     8847      +81     
==========================================
+ Hits         7769     7838      +69     
- Misses        997     1009      +12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 13, 2025 •
edited
Loading

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 487.24 ms 545.30 ms 58.05 ms
Size 6.44 MiB 7.56 MiB 1.12 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d10745a 318.86 ms 374.82 ms 55.96 ms
1b0c8a3 479.78 ms 521.18 ms 41.40 ms
824df58 436.68 ms 548.80 ms 112.12 ms
e6b16cd 523.24 ms 594.77 ms 71.53 ms
9f9f94f 331.04 ms 368.92 ms 37.88 ms
3392f27 486.67 ms 629.35 ms 142.68 ms
5aab4c5 326.12 ms 378.82 ms 52.70 ms
abcdba3 354.68 ms 399.04 ms 44.36 ms
32fa6b6 420.38 ms 497.90 ms 77.52 ms
b9da046 393.00 ms 455.52 ms 62.52 ms

App size

Revision Plain With Sentry Diff
d10745a 6.26 MiB 7.20 MiB 956.08 KiB
1b0c8a3 6.49 MiB 7.57 MiB 1.08 MiB
824df58 6.35 MiB 7.35 MiB 1021.71 KiB
e6b16cd 6.35 MiB 7.33 MiB 1005.63 KiB
9f9f94f 5.94 MiB 6.95 MiB 1.01 MiB
3392f27 6.46 MiB 7.48 MiB 1.02 MiB
5aab4c5 6.06 MiB 7.10 MiB 1.04 MiB
abcdba3 5.94 MiB 6.95 MiB 1.01 MiB
32fa6b6 6.35 MiB 7.35 MiB 1021.69 KiB
b9da046 6.06 MiB 7.03 MiB 990.30 KiB

Previous results on branch: v9-feat/js-release-health

Startup times

Revision Plain With Sentry Diff
97cf12e 476.20 ms 551.29 ms 75.08 ms
8877646 499.23 ms 560.47 ms 61.24 ms
4478d8f 483.96 ms 533.68 ms 49.72 ms
599dbee 454.68 ms 497.98 ms 43.30 ms
b4d0b7b 450.79 ms 492.04 ms 41.25 ms
7bbe0e7 468.78 ms 528.28 ms 59.50 ms
f9d8b56 437.36 ms 484.45 ms 47.09 ms
079f78a 497.67 ms 592.55 ms 94.88 ms
082f904 466.54 ms 519.15 ms 52.60 ms
8e609fe 418.40 ms 479.20 ms 60.80 ms

App size

Revision Plain With Sentry Diff
97cf12e 6.44 MiB 7.56 MiB 1.12 MiB
8877646 6.44 MiB 7.56 MiB 1.12 MiB
4478d8f 6.44 MiB 7.56 MiB 1.12 MiB
599dbee 6.44 MiB 7.56 MiB 1.12 MiB
b4d0b7b 6.44 MiB 7.56 MiB 1.12 MiB
7bbe0e7 6.44 MiB 7.56 MiB 1.12 MiB
f9d8b56 6.44 MiB 7.56 MiB 1.12 MiB
079f78a 6.44 MiB 7.56 MiB 1.12 MiB
082f904 6.44 MiB 7.56 MiB 1.12 MiB
8e609fe 6.44 MiB 7.56 MiB 1.12 MiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 13, 2025 •
edited
Loading

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1259.37 ms 1266.67 ms 7.30 ms
Size 8.43 MiB 9.98 MiB 1.55 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
22ed6cb 1277.41 ms 1283.76 ms 6.35 ms
d4120ac 1260.61 ms 1274.09 ms 13.47 ms
19a9adb 1249.21 ms 1258.31 ms 9.09 ms
b39b48c 1229.22 ms 1255.17 ms 25.95 ms
ccc09e4 1254.74 ms 1277.08 ms 22.34 ms
1edf30e 1254.43 ms 1272.82 ms 18.39 ms
b2cbbc8 1240.00 ms 1254.37 ms 14.37 ms
bd1b990 1209.43 ms 1226.96 ms 17.53 ms
8a7f528 1232.57 ms 1249.82 ms 17.24 ms
691aa3b 1265.57 ms 1282.13 ms 16.55 ms

App size

Revision Plain With Sentry Diff
22ed6cb 8.10 MiB 9.17 MiB 1.08 MiB
d4120ac 8.28 MiB 9.34 MiB 1.06 MiB
19a9adb 8.42 MiB 9.86 MiB 1.44 MiB
b39b48c 8.42 MiB 9.91 MiB 1.48 MiB
ccc09e4 8.16 MiB 9.16 MiB 1.01 MiB
1edf30e 8.16 MiB 9.17 MiB 1.01 MiB
b2cbbc8 8.10 MiB 9.18 MiB 1.08 MiB
bd1b990 8.32 MiB 9.38 MiB 1.06 MiB
8a7f528 8.10 MiB 9.16 MiB 1.06 MiB
691aa3b 8.16 MiB 9.17 MiB 1.01 MiB

Previous results on branch: v9-feat/js-release-health

Startup times

Revision Plain With Sentry Diff
7bbe0e7 1266.61 ms 1277.60 ms 10.99 ms
e4dbbee 1244.80 ms 1267.08 ms 22.29 ms
e4b20e6 1249.37 ms 1264.04 ms 14.67 ms
b0f0733 1260.08 ms 1278.54 ms 18.46 ms
8877646 1252.29 ms 1265.75 ms 13.46 ms
079f78a 1264.92 ms 1282.59 ms 17.67 ms
8825e1f 1245.86 ms 1265.96 ms 20.10 ms
f9d8b56 1253.02 ms 1265.48 ms 12.46 ms
12f5bb0 1261.33 ms 1280.34 ms 19.01 ms
599dbee 1258.40 ms 1266.94 ms 8.53 ms

App size

Revision Plain With Sentry Diff
7bbe0e7 8.43 MiB 9.98 MiB 1.55 MiB
e4dbbee 8.43 MiB 9.98 MiB 1.55 MiB
e4b20e6 8.43 MiB 9.98 MiB 1.55 MiB
b0f0733 8.43 MiB 9.98 MiB 1.55 MiB
8877646 8.43 MiB 9.98 MiB 1.55 MiB
079f78a 8.43 MiB 9.98 MiB 1.55 MiB
8825e1f 8.43 MiB 9.98 MiB 1.55 MiB
f9d8b56 8.43 MiB 9.98 MiB 1.55 MiB
12f5bb0 8.43 MiB 9.98 MiB 1.55 MiB
599dbee 8.43 MiB 9.98 MiB 1.55 MiB

@buenaflor buenaflor changed the title v9: Flutter Web release health (draft) [v9]: Flutter Web release health (draft) Mar 13, 2025
denrase
denrase reviewed Mar 17, 2025
View reviewed changes
Copy link
Collaborator

@denrase denrase left a comment

Choose a reason for hiding this comment

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

Mainly questions. Also, session tracking is only supposed to work is users are using the navigation router or did i just miss where it is also started? Is this not different to iOS/Android, where sessions are bound to overall app lifecyle?

@buenaflor buenaflor marked this pull request as ready for review March 18, 2025 13:24
@buenaflor buenaflor requested a review from denrase March 18, 2025 14:32
@buenaflor
Copy link
Contributor Author

@denrase do you mind giving it one last review 🙏

denrase
denrase approved these changes Mar 20, 2025
View reviewed changes
Copy link
Collaborator

@denrase denrase left a comment

Choose a reason for hiding this comment

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

LGTM. Only CI issues remain.

@buenaflor
Copy link
Contributor Author

Changed the implementation to make it easier configurable for the user and unified:

  • The WebSessionIntegration is added automatically for Web
  • It is not enabled until SentryNavigatorObserver is initialized
  • Once SentryNavigatorObserver is initialized it enables it by executing the enable() method
  • This will setup the necessary classes to allow session handling

Benefit of this is the integration is removable and we can track adoption of this due to adding it as an integration

@buenaflor buenaflor requested a review from denrase March 21, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krystofwoldrich krystofwoldrich Awaiting requested review from krystofwoldrich krystofwoldrich is a code owner

@stefanosiano stefanosiano Awaiting requested review from stefanosiano stefanosiano is a code owner

@denrase denrase Awaiting requested review from denrase

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[v9] Flutter Web release health support via JS SDK
2 participants
@buenaflor @denrase