-
-
Notifications
You must be signed in to change notification settings - Fork 226
Propagate Sampling Seed #3951
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
Propagate Sampling Seed #3951
Changes from 1 commit
02593ec
3e7034d
c70ca4e
0b22582
79ca7e6
6feb7b7
7ee8924
54d3293
e486d84
03d948f
724880d
be64839
8fea485
a363f00
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| using Sentry.Internal; | ||
| using Sentry.Internal.Extensions; | ||
|
|
||
| namespace Sentry; | ||
|
|
@@ -111,6 +112,21 @@ private DynamicSamplingContext( | |
| return null; | ||
| } | ||
|
|
||
| // See https://develop.sentry.dev/sdk/telemetry/traces/#propagated-random-value | ||
| if (items.TryGetValue("sample_rand", out var sampleRand)) | ||
|
Member
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. odd that we validate this to be within a range as a float but then we make it a string then try to parse it back out.
Collaborator
Author
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'm not sure I follow. It may or may not be present as a string in the baggage header. If it's present then here we try to cast it to a float and ensure it's in the correct range. If it's present but malformed then we don't create the DSC from the baggage header. |
||
| { | ||
| if (!double.TryParse(sampleRand, NumberStyles.Float, CultureInfo.InvariantCulture, out var rand) || | ||
| rand is < 0.0 or >= 1.0) | ||
|
Member
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. didn't we validate this above before adding it to
Collaborator
Author
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. This method is creating a DSC from the baggage header. The BaggageHeader is a pretty simple class that just pulls stuff directly off a certain HTTP header, without any strong opinions about what the items in that header should be. Other vendors might be putting things on that header so we just propagate it, pretty much verbatim (we might add things to it but we never remove things from it). But we can't control what will be on that baggage header in this SDK. A request could be made to our SDK with a malformed sentry-sample_rand item in the BaggageHeader. |
||
| { | ||
| return null; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| var seed = FnvHash.ComputeHash(traceId); | ||
| var rand = new Random(seed).NextDouble() * rate; | ||
| items.Add("sample_rand", rand.ToString("N4", CultureInfo.InvariantCulture)); | ||
| } | ||
| return new DynamicSamplingContext(items); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,6 +142,104 @@ public void CreateFromBaggage_SampleRate_TooHigh() | |
| Assert.Null(dsc); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void CreateFromBaggage_SampleRand_Invalid() | ||
| { | ||
| var baggage = BaggageHeader.Create(new List<KeyValuePair<string, string>> | ||
| { | ||
| {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, | ||
| {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, | ||
| {"sentry-sample_rate", "1.0"}, | ||
| {"sentry-sample_rand", "not-a-number"}, | ||
| }); | ||
|
|
||
| var dsc = baggage.CreateDynamicSamplingContext(); | ||
|
|
||
| Assert.Null(dsc); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void CreateFromBaggage_SampleRand_TooLow() | ||
| { | ||
| var baggage = BaggageHeader.Create(new List<KeyValuePair<string, string>> | ||
| { | ||
| {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, | ||
| {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, | ||
| {"sentry-sample_rate", "1.0"}, | ||
| {"sentry-sample_rand", "-0.1"} | ||
| }); | ||
|
|
||
| var dsc = baggage.CreateDynamicSamplingContext(); | ||
|
|
||
| Assert.Null(dsc); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void CreateFromBaggage_SampleRand_TooHigh() | ||
| { | ||
| var baggage = BaggageHeader.Create(new List<KeyValuePair<string, string>> | ||
| { | ||
| {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, | ||
| {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, | ||
| {"sentry-sample_rate", "1.0"}, | ||
| {"sentry-sample_rand", "1.0"} // Must be less than 1 | ||
| }); | ||
|
|
||
| var dsc = baggage.CreateDynamicSamplingContext(); | ||
|
|
||
| Assert.Null(dsc); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void CreateFromBaggage_NotSampledNoSampleRand_GeneratesSampleRand() | ||
| { | ||
| var baggage = BaggageHeader.Create(new List<KeyValuePair<string, string>> | ||
| { | ||
| {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, | ||
| {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, | ||
| {"sentry-sample_rate", "0.5"} | ||
| }); | ||
|
|
||
| var dsc = baggage.CreateDynamicSamplingContext(); | ||
|
|
||
| using var scope = new AssertionScope(); | ||
| Assert.NotNull(dsc); | ||
| var sampleRandItem = Assert.Contains("sample_rand", dsc.Items); | ||
| var sampleRand = double.Parse(sampleRandItem, NumberStyles.Float, CultureInfo.InvariantCulture); | ||
| Assert.True(sampleRand >= 0.0); | ||
| Assert.True(sampleRand < 1.0); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData("true")] | ||
| public void CreateFromBaggage_SampledNoSampleRand_GeneratesConsistentSampleRand(string sampled) | ||
| { | ||
| var baggage = BaggageHeader.Create(new List<KeyValuePair<string, string>> | ||
| { | ||
| {"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"}, | ||
| {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, | ||
| {"sentry-sample_rate", "0.5"}, | ||
| {"sentry-sampled", sampled}, | ||
| }); | ||
|
|
||
| var dsc = baggage.CreateDynamicSamplingContext(); | ||
|
|
||
| using var scope = new AssertionScope(); | ||
| Assert.NotNull(dsc); | ||
| var sampleRandItem = Assert.Contains("sample_rand", dsc.Items); | ||
| var sampleRand = double.Parse(sampleRandItem, NumberStyles.Float, CultureInfo.InvariantCulture); | ||
| if (sampled == "true") | ||
| { | ||
| Assert.True(sampleRand >= 0.0); | ||
| Assert.True(sampleRand < 0.5); | ||
| } | ||
| else | ||
| { | ||
| Assert.True(sampleRand >= 0.5); | ||
| Assert.True(sampleRand < 1.0); | ||
| } | ||
| } | ||
|
|
||
| [Fact] | ||
| public void CreateFromBaggage_Sampled_MalFormed() | ||
| { | ||
|
|
@@ -186,6 +284,7 @@ public void CreateFromBaggage_Valid_Complete() | |
| {"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"}, | ||
| {"sentry-sampled", "true"}, | ||
| {"sentry-sample_rate", "1.0"}, | ||
| {"sentry-sample_rand", "0.1234"}, | ||
| {"sentry-release", "[email protected]+abc"}, | ||
| {"sentry-environment", "production"}, | ||
| {"sentry-user_segment", "Group B"}, | ||
|
|
@@ -200,6 +299,7 @@ public void CreateFromBaggage_Valid_Complete() | |
| Assert.Equal("d4d82fc1c2c4032a83f3a29aa3a3aff", Assert.Contains("public_key", dsc.Items)); | ||
| Assert.Equal("true", Assert.Contains("sampled", dsc.Items)); | ||
| Assert.Equal("1.0", Assert.Contains("sample_rate", dsc.Items)); | ||
| Assert.Equal("0.1234", Assert.Contains("sample_rand", dsc.Items)); | ||
| Assert.Equal("[email protected]+abc", Assert.Contains("release", dsc.Items)); | ||
| Assert.Equal("production", Assert.Contains("environment", dsc.Items)); | ||
| Assert.Equal("Group B", Assert.Contains("user_segment", dsc.Items)); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.