-
-
Notifications
You must be signed in to change notification settings - Fork 275
Add sentry.replay_id
to flutter logs
#3257
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
base: main
Are you sure you want to change the base?
Conversation
Relates to #3245
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3257 +/- ##
==========================================
+ Coverage 87.84% 89.74% +1.90%
==========================================
Files 290 96 -194
Lines 10003 3424 -6579
==========================================
- Hits 8787 3073 -5714
+ Misses 1216 351 -865 ☔ View full report in Codecov by Sentry. |
Android Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
6b69699 | 456.06 ms | 557.44 ms | 101.38 ms |
192b44c | 472.26 ms | 477.34 ms | 5.08 ms |
1f639ee | 429.98 ms | 476.60 ms | 46.62 ms |
6f47800 | 451.04 ms | 509.64 ms | 58.60 ms |
7cfee3b | 498.78 ms | 516.98 ms | 18.20 ms |
7cfbbd6 | 499.69 ms | 592.24 ms | 92.55 ms |
73dca78 | 476.53 ms | 522.21 ms | 45.68 ms |
32914d8 | 461.96 ms | 495.47 ms | 33.51 ms |
2cf9161 | 454.12 ms | 512.67 ms | 58.55 ms |
6bcdc99 | 440.23 ms | 435.77 ms | -4.46 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
6b69699 | 6.54 MiB | 7.70 MiB | 1.17 MiB |
192b44c | 13.93 MiB | 14.93 MiB | 1.00 MiB |
1f639ee | 13.93 MiB | 15.00 MiB | 1.06 MiB |
6f47800 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
7cfee3b | 6.54 MiB | 7.70 MiB | 1.17 MiB |
7cfbbd6 | 6.54 MiB | 7.70 MiB | 1.17 MiB |
73dca78 | 6.54 MiB | 7.69 MiB | 1.15 MiB |
32914d8 | 6.54 MiB | 7.70 MiB | 1.16 MiB |
2cf9161 | 6.54 MiB | 7.70 MiB | 1.16 MiB |
6bcdc99 | 13.93 MiB | 15.00 MiB | 1.06 MiB |
Previous results on branch: enha/logs-replay-id
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
9e888b6 | 460.09 ms | 462.35 ms | 2.26 ms |
f98c32f | 479.02 ms | 466.78 ms | -12.25 ms |
234bb86 | 453.89 ms | 457.28 ms | 3.39 ms |
1e63021 | 430.72 ms | 453.78 ms | 23.06 ms |
02f86c3 | 453.09 ms | 454.63 ms | 1.55 ms |
91562e2 | 464.50 ms | 455.70 ms | -8.80 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
9e888b6 | 13.93 MiB | 15.00 MiB | 1.06 MiB |
f98c32f | 13.93 MiB | 15.00 MiB | 1.06 MiB |
234bb86 | 13.93 MiB | 15.00 MiB | 1.06 MiB |
1e63021 | 13.93 MiB | 15.00 MiB | 1.06 MiB |
02f86c3 | 13.93 MiB | 15.00 MiB | 1.06 MiB |
91562e2 | 13.93 MiB | 15.00 MiB | 1.06 MiB |
iOS Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
aeb02f2 | 1244.29 ms | 1256.55 ms | 12.26 ms |
54acf91 | 1257.65 ms | 1277.96 ms | 20.31 ms |
73a3c38 | 1263.37 ms | 1277.90 ms | 14.53 ms |
944b773 | 1252.82 ms | 1254.08 ms | 1.27 ms |
827bf09 | 1261.86 ms | 1276.41 ms | 14.55 ms |
192b44c | 1269.08 ms | 1275.52 ms | 6.44 ms |
c1e775e | 1263.08 ms | 1275.32 ms | 12.24 ms |
cf443d2 | 1255.79 ms | 1248.38 ms | -7.40 ms |
81f83eb | 1259.53 ms | 1273.39 ms | 13.86 ms |
0fb3800 | 1256.60 ms | 1266.28 ms | 9.68 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
aeb02f2 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
54acf91 | 20.70 MiB | 22.46 MiB | 1.75 MiB |
73a3c38 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
944b773 | 5.53 MiB | 6.00 MiB | 479.98 KiB |
827bf09 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
192b44c | 5.53 MiB | 5.96 MiB | 444.33 KiB |
c1e775e | 20.70 MiB | 22.46 MiB | 1.75 MiB |
cf443d2 | 5.53 MiB | 6.00 MiB | 479.99 KiB |
81f83eb | 7.86 MiB | 9.44 MiB | 1.58 MiB |
0fb3800 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
Previous results on branch: enha/logs-replay-id
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
91562e2 | 1242.54 ms | 1254.08 ms | 11.54 ms |
f98c32f | 1256.22 ms | 1265.67 ms | 9.44 ms |
1e63021 | 1244.62 ms | 1246.37 ms | 1.76 ms |
9e888b6 | 1247.65 ms | 1245.32 ms | -2.33 ms |
234bb86 | 1241.00 ms | 1250.87 ms | 9.88 ms |
02f86c3 | 1262.54 ms | 1263.67 ms | 1.13 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
91562e2 | 5.53 MiB | 6.00 MiB | 480.92 KiB |
f98c32f | 5.53 MiB | 6.00 MiB | 483.36 KiB |
1e63021 | 5.53 MiB | 6.00 MiB | 483.21 KiB |
9e888b6 | 5.53 MiB | 6.00 MiB | 483.21 KiB |
234bb86 | 5.53 MiB | 6.00 MiB | 479.96 KiB |
02f86c3 | 5.53 MiB | 6.00 MiB | 480.79 KiB |
@stefanosiano Same here, review pls :) |
looks good, but i'll put it on hold, until the last point has been decided on: https://www.notion.so/sentry/Logs-Replays-Notes-2738b10e4b5d8035b027c6cb0fc57174 |
@stefanosiano Pls take a look if i interpreted the flag correctly. |
@denrase
In Android we have this distinction because the replay id is set in the scope when the replay is captured (and it's in session mode), and is only available in the replay integration (and not in the scope) when it's running in buffer mode. |
@stefanosiano Thank you, didn't have access to |
i guess it's fine if you try to cast the options of the scope to |
@stefanosiano AFAIK this only works in the Flutter repo, as the Dart repo does not know about the |
yeah right |
packages/flutter/lib/src/integrations/replay_log_integration.dart
Outdated
Show resolved
Hide resolved
@stefanosiano Took me a while to debug this and figure out how everything fits together. We have the following issues:
So the latter is currently incorrect I think. Need to think about how we wan't to tackle this. \cc @buenaflor |
@denrase sounds like we need some internal hybrids SDK api from cocoa for this |
I think it might be good to align with RN here |
@buenaflor Checked RN. Right now they are only setting the |
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: Replay ID Persistence Causes Log Association Errors
When the replay recorder stops, the _replayId
field isn't cleared, even though the scope's replayId
is. This inconsistency means the native binding's replayId
getter returns a stale ID, which could cause the ReplayLogIntegration
to incorrectly associate logs with a stopped replay.
packages/flutter/lib/src/native/java/sentry_native_java.dart#L58-L68
break; | |
case 'ReplayRecorder.stop': | |
hub.configureScope((s) { | |
// ignore: invalid_use_of_internal_member | |
s.replayId = null; | |
}); | |
final future = _replayRecorder?.stop(); | |
_replayRecorder = null; | |
await future; | |
is |
@buenaflor On Android this works as expected AFAIK, on iOS we can only detect full sessions or onError sessions as soon as they are set to the scope. @stefanosiano Do you know if this would be ok as a step between? RN also only sets |
var scopeReplayId: String? = null | ||
Sentry.configureScope { scope -> | ||
scopeReplayId = scope.replayId?.toString() | ||
} | ||
channel.invokeMethod( | ||
"ReplayRecorder.start", | ||
mapOf( | ||
"scope.replayId" to scopeReplayId, | ||
"replayId" to integration.getReplayId().toString(), |
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.
what's the difference between the two? (replay id from scope vs replay id from integration), seems a bit confusing to me if you don't know the context around this
📜 Description
sentry.replay_id
to structured logssentry._internal.replay_is_buffering
to structured logs💡 Motivation and Context
Closes #3245
💚 How did you test it?
Unit tests
📝 Checklist
sendDefaultPii
is enabledNote
Add
ReplayLogIntegration
to enrich Flutter structured logs with replay metadata and include it only on replay-supported platforms, with tests and changelog updates.ReplayLogIntegration
to addsentry.replay_id
andsentry._internal.replay_is_buffering
toSentryLog
viaOnBeforeCaptureLog
.ReplayLogIntegration
only whennative.supportsReplay
is true.replay_log_integration_test.dart
covering replay id and buffering flag behavior, registration, and cleanup.ReplayLogIntegration
only on replay-supported platforms.test/sentry_flutter_util.dart
assertion to check byruntimeType
for absent integrations.Written by Cursor Bugbot for commit 02f86c3. This will update automatically on new commits. Configure here.