-
-
Notifications
You must be signed in to change notification settings - Fork 267
fix: TTID/TTFD transaction for the root page #3099
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3099 +/- ##
==========================================
- Coverage 87.71% 87.66% -0.05%
==========================================
Files 287 288 +1
Lines 9797 9813 +16
==========================================
+ Hits 8593 8603 +10
- Misses 1204 1210 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
iOS Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2d34233 | 1258.19 ms | 1268.92 ms | 10.73 ms |
6f47800 | 1247.52 ms | 1259.37 ms | 11.85 ms |
0fb45d0 | 1273.24 ms | 1286.44 ms | 13.19 ms |
ec78888 | 1251.37 ms | 1269.40 ms | 18.04 ms |
93b7728 | 1247.23 ms | 1264.87 ms | 17.64 ms |
b6c8720 | 1252.65 ms | 1266.61 ms | 13.96 ms |
6ba4675 | 1223.12 ms | 1238.17 ms | 15.04 ms |
2cf9161 | 1248.33 ms | 1266.55 ms | 18.22 ms |
640ad0c | 1241.04 ms | 1253.96 ms | 12.92 ms |
73dca78 | 1246.65 ms | 1265.42 ms | 18.76 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2d34233 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
6f47800 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
0fb45d0 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
ec78888 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
93b7728 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
b6c8720 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
6ba4675 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
2cf9161 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
640ad0c | 7.86 MiB | 9.44 MiB | 1.58 MiB |
73dca78 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
Previous results on branch: fix/web-ttid-for-root
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b44fc1a | 1254.94 ms | 1271.08 ms | 16.14 ms |
1b80841 | 1265.81 ms | 1284.39 ms | 18.58 ms |
def5eaf | 1233.83 ms | 1255.49 ms | 21.66 ms |
9759b94 | 1253.45 ms | 1259.83 ms | 6.39 ms |
4cdba90 | 1257.39 ms | 1269.10 ms | 11.71 ms |
26ad5f9 | 1246.24 ms | 1260.35 ms | 14.10 ms |
38b3b80 | 1243.75 ms | 1257.02 ms | 13.27 ms |
e585f0e | 1246.02 ms | 1259.16 ms | 13.14 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b44fc1a | 7.86 MiB | 9.44 MiB | 1.58 MiB |
1b80841 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
def5eaf | 7.86 MiB | 9.44 MiB | 1.58 MiB |
9759b94 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
4cdba90 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
26ad5f9 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
38b3b80 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
e585f0e | 7.86 MiB | 9.44 MiB | 1.58 MiB |
Android Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
0fb3800 | 465.64 ms | 536.77 ms | 71.13 ms |
cc4e375 | 426.15 ms | 482.34 ms | 56.19 ms |
0fb45d0 | 482.79 ms | 554.02 ms | 71.23 ms |
6ba4675 | 499.80 ms | 632.43 ms | 132.63 ms |
93b7728 | 475.28 ms | 489.13 ms | 13.86 ms |
b6c8720 | 457.41 ms | 519.04 ms | 61.63 ms |
ec78888 | 457.94 ms | 519.96 ms | 62.02 ms |
2cf9161 | 454.12 ms | 512.67 ms | 58.55 ms |
4481076 | 484.08 ms | 505.70 ms | 21.61 ms |
793f4dc | 462.68 ms | 544.21 ms | 81.53 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
0fb3800 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
cc4e375 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
0fb45d0 | 6.54 MiB | 7.70 MiB | 1.17 MiB |
6ba4675 | 6.54 MiB | 7.53 MiB | 1015.26 KiB |
93b7728 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
b6c8720 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
ec78888 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
2cf9161 | 6.54 MiB | 7.70 MiB | 1.16 MiB |
4481076 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
793f4dc | 6.54 MiB | 7.69 MiB | 1.15 MiB |
Previous results on branch: fix/web-ttid-for-root
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
4cdba90 | 489.49 ms | 575.90 ms | 86.41 ms |
1b80841 | 484.56 ms | 580.90 ms | 96.33 ms |
def5eaf | 472.78 ms | 498.09 ms | 25.31 ms |
9759b94 | 473.33 ms | 564.86 ms | 91.53 ms |
38b3b80 | 491.90 ms | 524.98 ms | 33.08 ms |
b44fc1a | 463.17 ms | 546.85 ms | 83.68 ms |
e585f0e | 478.98 ms | 571.94 ms | 92.96 ms |
26ad5f9 | 490.73 ms | 588.22 ms | 97.49 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
4cdba90 | 6.54 MiB | 7.70 MiB | 1.16 MiB |
1b80841 | 6.54 MiB | 7.70 MiB | 1.16 MiB |
def5eaf | 6.54 MiB | 7.70 MiB | 1.16 MiB |
9759b94 | 6.54 MiB | 7.70 MiB | 1.17 MiB |
38b3b80 | 6.54 MiB | 7.70 MiB | 1.16 MiB |
b44fc1a | 6.54 MiB | 7.70 MiB | 1.17 MiB |
e585f0e | 6.54 MiB | 7.69 MiB | 1.15 MiB |
26ad5f9 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Debug vs Release Null Handling Mismatch
The assert(transactionId != null);
statement in the currentDisplay()
method contradicts the immediate subsequent null check (if (transactionId == null) return null;
). This causes debug builds to crash when transactionId
is legitimately null (e.g., no active transaction, tracing disabled, or before app start integrations run), while release builds correctly handle this case by returning null. This creates inconsistent behavior between debug and release environments.
flutter/lib/src/sentry_flutter.dart#L294-L298
sentry-dart/flutter/lib/src/sentry_flutter.dart
Lines 294 to 298 in b44fc1a
final transactionId = options.timeToDisplayTracker.transactionId; | |
assert(transactionId != null); | |
if (transactionId == null) { | |
return null; | |
} |
Was this report helpful? Give feedback by reacting with 👍 or 👎
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good when CI is ready.
🚨 Detected changes in high risk code 🚨High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:
|
@sentry review |
@sentry review |
💡 Motivation and Context
Transaction for root pages in Web, Windows and Linux platforms were previously not created although they should be
Also this fixes an issue where the native app start integration was not an idle transaction
💚 How did you test it?
Unit test, manual tests
📝 Checklist
sendDefaultPii
is enabled🔮 Next steps