fix(replay): Improve capture of errorIds/traceIds #8678
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report 📦
|
mydea
force-pushed
the
fn/limit-replay-errors
branch
from
eae9ae5 to
8031e7f
Compare
We limit to max. 100 IDs being captured per replay, and ensure we do not capture stuff when replay has been disabled in the meanwhile.
mydea
force-pushed
the
fn/limit-replay-errors
branch
from
8031e7f to
3aa5c07
Compare
billyvg
approved these changes
Jul 31, 2023
| // In error mode, _context gets cleared on every checkout | ||
| // We limit to max. 100 transactions linked | ||
| if (event.contexts && event.contexts.trace && event.contexts.trace.trace_id && replayContext.traceIds.size < 100) { | ||
| replayContext.traceIds.add(event.contexts.trace.trace_id as string); |
There was a problem hiding this comment.
This is good for now, but was thinking in general, how can we communicate the behavior to customers?
Maybe a "custom" breadcrumb? I think I want to document our recording "protocol" a bit with all the different breadcrumb types we have.
There was a problem hiding this comment.
IMHO the "real" solution to this is to stop using this at all and make the connection on the backend, e.g. update the errorIds server side in a job after the replay is completed or similar 🤔
|
We should generally only ever hit this max for buffered replays right? |
Realistically yes, I'd say! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We limit to max. 100 IDs being captured per replay, and ensure we do not capture stuff when replay has been disabled in the meanwhile.
Since our handlers are (sadly) all global, I guess it may have been possible for something "bad" to happen when replay has been disabled in the meanwhile 🤔
ref https://github.com/getsentry/team-replay/issues/131
ref #8672