-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
Fix: App start time span no longer created if too long #3299
Conversation
@krystofwoldrich the transaction will look different on the cases where this issue was happening, is this considered a break change? |
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3ffcddd | 302.92 ms | 315.80 ms | 12.88 ms |
3853f43 | 329.68 ms | 346.32 ms | 16.64 ms |
0db0c72 | 372.12 ms | 386.00 ms | 13.88 ms |
d197b5c+dirty | 338.94 ms | 354.87 ms | 15.93 ms |
abb7058 | 370.27 ms | 389.58 ms | 19.31 ms |
76d1baf+dirty | 335.72 ms | 355.52 ms | 19.80 ms |
0677344 | 327.74 ms | 337.14 ms | 9.40 ms |
9c48b2c | 349.24 ms | 385.96 ms | 36.72 ms |
457e29f | 398.10 ms | 421.39 ms | 23.29 ms |
8900e1a+dirty | 430.68 ms | 456.13 ms | 25.44 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3ffcddd | 17.73 MiB | 19.75 MiB | 2.02 MiB |
3853f43 | 17.73 MiB | 19.81 MiB | 2.08 MiB |
0db0c72 | 17.73 MiB | 19.75 MiB | 2.02 MiB |
d197b5c+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
abb7058 | 17.73 MiB | 19.83 MiB | 2.10 MiB |
76d1baf+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
0677344 | 17.73 MiB | 19.81 MiB | 2.07 MiB |
9c48b2c | 17.73 MiB | 19.80 MiB | 2.07 MiB |
457e29f | 17.73 MiB | 19.84 MiB | 2.10 MiB |
8900e1a+dirty | 17.73 MiB | 19.75 MiB | 2.01 MiB |
iOS (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
27ef4ee+dirty | 1293.52 ms | 1296.08 ms | 2.56 ms |
0677344+dirty | 1276.70 ms | 1300.07 ms | 23.37 ms |
3853f43+dirty | 1221.82 ms | 1242.64 ms | 20.82 ms |
ad6c299+dirty | 1244.76 ms | 1260.10 ms | 15.34 ms |
34aba08+dirty | 1276.78 ms | 1308.52 ms | 31.74 ms |
d197b5c+dirty | 1217.61 ms | 1242.66 ms | 25.05 ms |
9433f35+dirty | 1246.94 ms | 1271.45 ms | 24.52 ms |
76d1baf+dirty | 1244.10 ms | 1268.52 ms | 24.42 ms |
80b2ce3+dirty | 1265.92 ms | 1268.60 ms | 2.69 ms |
457e29f+dirty | 1253.94 ms | 1269.18 ms | 15.24 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
27ef4ee+dirty | 2.36 MiB | 2.85 MiB | 500.03 KiB |
0677344+dirty | 2.36 MiB | 2.85 MiB | 496.81 KiB |
3853f43+dirty | 2.36 MiB | 2.85 MiB | 499.81 KiB |
ad6c299+dirty | 2.36 MiB | 2.84 MiB | 488.85 KiB |
34aba08+dirty | 2.36 MiB | 2.85 MiB | 495.32 KiB |
d197b5c+dirty | 2.36 MiB | 2.82 MiB | 462.86 KiB |
9433f35+dirty | 2.36 MiB | 2.85 MiB | 499.80 KiB |
76d1baf+dirty | 2.36 MiB | 2.82 MiB | 469.45 KiB |
80b2ce3+dirty | 2.36 MiB | 2.84 MiB | 486.98 KiB |
457e29f+dirty | 2.36 MiB | 2.87 MiB | 520.67 KiB |
Previous results on branch: feat/drop-long-spam
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
f557bbc+dirty | 1243.20 ms | 1245.72 ms | 2.52 ms |
1feaf66+dirty | 1226.18 ms | 1237.58 ms | 11.40 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
f557bbc+dirty | 2.36 MiB | 2.87 MiB | 520.57 KiB |
1feaf66+dirty | 2.36 MiB | 2.87 MiB | 520.78 KiB |
Android (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
27ef4ee+dirty | 296.71 ms | 351.00 ms | 54.29 ms |
0677344+dirty | 288.40 ms | 391.44 ms | 103.04 ms |
3853f43+dirty | 278.12 ms | 338.72 ms | 60.60 ms |
ad6c299+dirty | 336.47 ms | 362.89 ms | 26.42 ms |
34aba08+dirty | 331.79 ms | 376.69 ms | 44.91 ms |
d197b5c+dirty | 258.75 ms | 313.61 ms | 54.86 ms |
9433f35+dirty | 265.50 ms | 336.08 ms | 70.58 ms |
76d1baf+dirty | 339.02 ms | 408.65 ms | 69.63 ms |
80b2ce3+dirty | 271.29 ms | 316.47 ms | 45.18 ms |
457e29f+dirty | 591.49 ms | 612.96 ms | 21.47 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
27ef4ee+dirty | 7.15 MiB | 8.08 MiB | 959.49 KiB |
0677344+dirty | 7.15 MiB | 8.07 MiB | 949.80 KiB |
3853f43+dirty | 7.15 MiB | 8.08 MiB | 959.34 KiB |
ad6c299+dirty | 7.15 MiB | 8.04 MiB | 912.17 KiB |
34aba08+dirty | 7.15 MiB | 8.07 MiB | 946.13 KiB |
d197b5c+dirty | 7.15 MiB | 8.09 MiB | 962.72 KiB |
9433f35+dirty | 7.15 MiB | 8.08 MiB | 959.34 KiB |
76d1baf+dirty | 7.15 MiB | 8.09 MiB | 964.41 KiB |
80b2ce3+dirty | 7.15 MiB | 8.04 MiB | 911.02 KiB |
457e29f+dirty | 7.15 MiB | 8.10 MiB | 981.29 KiB |
Previous results on branch: feat/drop-long-spam
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
f557bbc+dirty | 363.43 ms | 389.14 ms | 25.71 ms |
1feaf66+dirty | 353.53 ms | 385.16 ms | 31.63 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
f557bbc+dirty | 7.15 MiB | 8.10 MiB | 980.71 KiB |
1feaf66+dirty | 7.15 MiB | 8.11 MiB | 983.87 KiB |
iOS (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
27ef4ee+dirty | 1236.41 ms | 1244.90 ms | 8.49 ms |
0677344+dirty | 1252.52 ms | 1254.08 ms | 1.56 ms |
3853f43+dirty | 1271.74 ms | 1278.04 ms | 6.30 ms |
ad6c299+dirty | 1248.50 ms | 1248.88 ms | 0.38 ms |
34aba08+dirty | 1268.58 ms | 1276.80 ms | 8.22 ms |
d197b5c+dirty | 1234.80 ms | 1249.20 ms | 14.40 ms |
9433f35+dirty | 1232.24 ms | 1232.74 ms | 0.50 ms |
76d1baf+dirty | 1245.00 ms | 1257.76 ms | 12.76 ms |
80b2ce3+dirty | 1245.12 ms | 1262.04 ms | 16.92 ms |
457e29f+dirty | 1256.71 ms | 1258.50 ms | 1.79 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
27ef4ee+dirty | 2.92 MiB | 3.41 MiB | 503.72 KiB |
0677344+dirty | 2.92 MiB | 3.41 MiB | 500.94 KiB |
3853f43+dirty | 2.92 MiB | 3.41 MiB | 503.54 KiB |
ad6c299+dirty | 2.92 MiB | 3.40 MiB | 494.12 KiB |
34aba08+dirty | 2.92 MiB | 3.41 MiB | 499.03 KiB |
d197b5c+dirty | 2.92 MiB | 3.37 MiB | 464.41 KiB |
9433f35+dirty | 2.92 MiB | 3.41 MiB | 503.55 KiB |
76d1baf+dirty | 2.92 MiB | 3.38 MiB | 475.74 KiB |
80b2ce3+dirty | 2.92 MiB | 3.40 MiB | 492.75 KiB |
457e29f+dirty | 2.92 MiB | 3.43 MiB | 524.75 KiB |
Previous results on branch: feat/drop-long-spam
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
f557bbc+dirty | 1272.71 ms | 1282.96 ms | 10.25 ms |
1feaf66+dirty | 1276.73 ms | 1284.22 ms | 7.49 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
f557bbc+dirty | 2.92 MiB | 3.43 MiB | 524.56 KiB |
1feaf66+dirty | 2.92 MiB | 3.43 MiB | 524.81 KiB |
@lucas-zimerman Yes, they are different, but I believe this should be a fix, as over minute start-up span is definitely a bug. |
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.
This looks good and removes the app start spans longer than the threshold, but the overall transaction is still the same length as the start span would be included. Which I believe is unwanted.
These two transaction start times should be also inside the threshold condition in the _addAppStartData
method.
startTimestamp: appStartTimeSeconds, |
transaction.startTimestamp = this._awaitingAppStartData.appStartTime / 1000; |
@lucas-zimerman Thanks for the update, now the transactions have the correct start timestamp when the app start time is not added. 👍 I just have a few suggestions (non blocking) on how to avoid checking the duration at 3 different places. I believe we can do use the one check in the |
📢 Type of change
📜 Description
💡 Motivation and Context
Fixes #3044 / Long span messing the measures of alerts.
💚 How did you test it?
froze app start and released after the timeout. Before and after this change can be seen on the following transactions:
https://sentry-sdks.sentry.io/discover/sentry-react-native:6fbe53e73d9048c194169f1228a1bd55/?field=title&field=event.type&field=project&field=user.display&field=timestamp&field=replayId&homepage=true&id=21705&name=&project=5428561&query=&sort=-timestamp&statsPeriod=1h&topEvents=5&yAxis=count()
https://sentry-sdks.sentry.io/discover/sentry-react-native:c764f2fd956e40349e79434d98bcb4ff/?field=title&field=event.type&field=project&field=user.display&field=timestamp&field=replayId&homepage=true&id=21705&name=&project=5428561&query=&sort=-timestamp&statsPeriod=1h&topEvents=5&yAxis=count()
📝 Checklist
sendDefaultPII
is enabled🔮 Next steps