feat: Associate replays with errors and traces on Android #4133
Conversation
Part of #2136 Associate Errors and Traces with the active Session Replay (if one exists) on Android.
|
| var replayId = JavaSdk.ScopesAdapter.Instance?.Options?.ReplayController?.ReplayId?.ToSentryId(); | ||
| return (replayId is { } id && id != SentryId.Empty) ? id : null; | ||
| #else | ||
| return null; |
There was a problem hiding this comment.
Is the idea to add iOS here later? potentially Web for example if we were brave enough?
There was a problem hiding this comment.
Yeah, I was planning for at least iOS...
There was a problem hiding this comment.
approving to unblock, lets see if @vaind has comments then we can fix in follow up PRs
| public static DynamicSamplingContext? CreateDynamicSamplingContext(this BaggageHeader baggage, IReplaySession? replaySession = null) | ||
| => DynamicSamplingContext.CreateFromBaggageHeader(baggage, replaySession); | ||
|
|
||
| public static DynamicSamplingContext CreateDynamicSamplingContext(this TransactionTracer transaction, SentryOptions options) | ||
| => DynamicSamplingContext.CreateFromTransaction(transaction, options); | ||
| public static DynamicSamplingContext CreateDynamicSamplingContext(this TransactionTracer transaction, SentryOptions options, IReplaySession? replaySession) | ||
| => DynamicSamplingContext.CreateFromTransaction(transaction, options, replaySession); | ||
|
|
||
| public static DynamicSamplingContext CreateDynamicSamplingContext(this SentryPropagationContext propagationContext, SentryOptions options) | ||
| => DynamicSamplingContext.CreateFromPropagationContext(propagationContext, options); | ||
| public static DynamicSamplingContext CreateDynamicSamplingContext(this SentryPropagationContext propagationContext, SentryOptions options, IReplaySession? replaySession) | ||
| => DynamicSamplingContext.CreateFromPropagationContext(propagationContext, options, replaySession); |
There was a problem hiding this comment.
Instead of adding replay to all the factories, how about we use just the WithReplayId()?
There was a problem hiding this comment.
for reference/example, in dart, updating from scope is done like this: https://github.com/getsentry/sentry-dart/blob/e401af3bfe7268f6ad94bbf592fc81525eda264b/dart/lib/src/sentry_baggage.dart#L96-L113
There was a problem hiding this comment.
I'd rather avoid WithReplayId where possible as it has to recreate the DynamicSamplingContext in order to add the replay id... that would result in us creating the DynamicSamplingContext twice for each of the factories if there was an active replay session 😞
The other option would be to change DynamicSamplingContext.Items from a read only dictionary to a mutable dictionary. Most of the stuff in there really shouldn't be messed with once it's created (we're supposed to be propagating stuff verbatim), so until now it's been nice to have this be immutable.
All of this stuff is internal, so we can easily refactor in a separate PR whenever we like, if we want to go with a different coding style.
Part of #2136
Associate Errors and Traces with the active Session Replay (if one exists) on Android.
Protocol
Although there is a new Replay context, I found that just setting the replay_id on that context (e.g. for an event) did not result in the event showing in the errors pane for session replays.
What was recommended, and work better, is to store the
replay_idon the DSC (which will accompany all events) and let Relay unpack it from there. The DSC specification mentionsreplay_idas an optional property on the dynamic sampling context.Getting the Replay ID
There are three ways the DSC can be created:
In all 3 cases, we check to see if there's a Session Replay actively being recorded. If there is, we either set (or overwrite) the
replay_idthat int he DSC. If there's not, we just propagate whatever is in the header.This is a bit different to what we normally do with the baggage headers, so there are quite a few new scenarios to unit test.