-
-
Notifications
You must be signed in to change notification settings - Fork 226
feat: Associate replays with errors and traces on Android #4133
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
Changes from all commits
3ef1422
9872376
b8d9452
ae6157e
6f848a3
05c674c
cf534b9
ff8019b
0989eca
b43f1bf
f6a8242
6386234
41ba57d
75997c6
32ba50b
efb200d
6d770cb
37b40c5
356d39d
66e3f80
edc0b9c
81c4f50
b50ae47
7b95524
330bd66
39e56fd
4f2f39c
4c2fc76
8ef8d66
eee7a20
4ce0049
c3ab0b4
cf96b60
b989d5c
54e5536
0b21161
369b529
438c4e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,15 +20,15 @@ internal class DynamicSamplingContext | |
| /// </summary> | ||
| public static readonly DynamicSamplingContext Empty = new(new Dictionary<string, string>().AsReadOnly()); | ||
|
|
||
| private DynamicSamplingContext( | ||
| SentryId traceId, | ||
| private DynamicSamplingContext(SentryId traceId, | ||
| string publicKey, | ||
| bool? sampled, | ||
| double? sampleRate = null, | ||
| double? sampleRand = null, | ||
| string? release = null, | ||
| string? environment = null, | ||
| string? transactionName = null) | ||
| string? transactionName = null, | ||
| IReplaySession? replaySession = null) | ||
| { | ||
| // Validate and set required values | ||
| if (traceId == SentryId.Empty) | ||
|
|
@@ -51,7 +51,7 @@ private DynamicSamplingContext( | |
| throw new ArgumentOutOfRangeException(nameof(sampleRand), "Arg invalid if < 0.0 or >= 1.0"); | ||
| } | ||
|
|
||
| var items = new Dictionary<string, string>(capacity: 8) | ||
| var items = new Dictionary<string, string>(capacity: 9) | ||
| { | ||
| ["trace_id"] = traceId.ToString(), | ||
| ["public_key"] = publicKey, | ||
|
|
@@ -88,12 +88,29 @@ private DynamicSamplingContext( | |
| items.Add("transaction", transactionName); | ||
| } | ||
|
|
||
| if (replaySession?.ActiveReplayId is { } replayId && replayId != SentryId.Empty) | ||
| { | ||
| items.Add("replay_id", replayId.ToString()); | ||
| } | ||
|
|
||
| Items = items; | ||
| } | ||
|
|
||
| public BaggageHeader ToBaggageHeader() => BaggageHeader.Create(Items, useSentryPrefix: true); | ||
|
|
||
| public static DynamicSamplingContext? CreateFromBaggageHeader(BaggageHeader baggage) | ||
| public DynamicSamplingContext WithReplayId(IReplaySession? replaySession) | ||
| { | ||
| if (replaySession?.ActiveReplayId is not { } replayId || replayId == SentryId.Empty) | ||
| { | ||
| return this; | ||
| } | ||
|
|
||
| var items = Items.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); | ||
| items["replay_id"] = replayId.ToString(); | ||
| return new DynamicSamplingContext(items); | ||
| } | ||
|
|
||
| public static DynamicSamplingContext? CreateFromBaggageHeader(BaggageHeader baggage, IReplaySession? replaySession) | ||
| { | ||
| var items = baggage.GetSentryMembers(); | ||
|
|
||
|
|
@@ -144,10 +161,19 @@ private DynamicSamplingContext( | |
| } | ||
| items.Add("sample_rand", rand.ToString("N4", CultureInfo.InvariantCulture)); | ||
| } | ||
|
|
||
| if (replaySession?.ActiveReplayId is { } replayId) | ||
| { | ||
| // Any upstream replay_id will be propagated only if the current process hasn't started it's own replay session. | ||
| // Otherwise we have to overwrite this as it's the only way to communicate the replayId to Sentry Relay. | ||
| // In Mobile apps this should never be a problem. | ||
| items["replay_id"] = replayId.ToString(); | ||
| } | ||
|
|
||
| return new DynamicSamplingContext(items); | ||
| } | ||
|
|
||
| public static DynamicSamplingContext CreateFromTransaction(TransactionTracer transaction, SentryOptions options) | ||
| public static DynamicSamplingContext CreateFromTransaction(TransactionTracer transaction, SentryOptions options, IReplaySession? replaySession) | ||
| { | ||
| // These should already be set on the transaction. | ||
| var publicKey = options.ParsedDsn.PublicKey; | ||
|
|
@@ -161,18 +187,18 @@ public static DynamicSamplingContext CreateFromTransaction(TransactionTracer tra | |
| var release = options.SettingLocator.GetRelease(); | ||
| var environment = options.SettingLocator.GetEnvironment(); | ||
|
|
||
| return new DynamicSamplingContext( | ||
| traceId, | ||
| return new DynamicSamplingContext(traceId, | ||
| publicKey, | ||
| sampled, | ||
| sampleRate, | ||
| sampleRand, | ||
| release, | ||
| environment, | ||
| transactionName); | ||
| transactionName, | ||
| replaySession); | ||
| } | ||
|
|
||
| public static DynamicSamplingContext CreateFromPropagationContext(SentryPropagationContext propagationContext, SentryOptions options) | ||
| public static DynamicSamplingContext CreateFromPropagationContext(SentryPropagationContext propagationContext, SentryOptions options, IReplaySession? replaySession) | ||
| { | ||
| var traceId = propagationContext.TraceId; | ||
| var publicKey = options.ParsedDsn.PublicKey; | ||
|
|
@@ -184,18 +210,20 @@ public static DynamicSamplingContext CreateFromPropagationContext(SentryPropagat | |
| publicKey, | ||
| null, | ||
| release: release, | ||
| environment: environment); | ||
| environment: environment, | ||
| replaySession: replaySession | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| internal static class DynamicSamplingContextExtensions | ||
| { | ||
| public static DynamicSamplingContext? CreateDynamicSamplingContext(this BaggageHeader baggage) | ||
| => DynamicSamplingContext.CreateFromBaggageHeader(baggage); | ||
| 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); | ||
|
Comment on lines
+221
to
+228
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of adding replay to all the factories, how about we use just the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather avoid The other option would be to change 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. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| #if __ANDROID__ | ||
| using Sentry.Android.Extensions; | ||
| #endif | ||
|
|
||
| namespace Sentry.Internal; | ||
|
|
||
| internal interface IReplaySession | ||
Flash0ver marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| public SentryId? ActiveReplayId { get; } | ||
| } | ||
|
|
||
| internal class ReplaySession : IReplaySession | ||
| { | ||
| public static readonly IReplaySession Instance = new ReplaySession(); | ||
|
|
||
| private ReplaySession() | ||
| { | ||
| } | ||
|
|
||
| public SentryId? ActiveReplayId | ||
| { | ||
| get | ||
| { | ||
| #if __ANDROID__ | ||
| // Check to see if a Replay ID is available | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the idea to add iOS here later? potentially Web for example if we were brave enough? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I was planning for at least iOS... |
||
| #endif | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.