Skip to content
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

Add Sampling Decision to Trace Envelope Header #2495

Merged
merged 6 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@

## Unreleased

### Features

- Added Sampling Decision to Trace Envelope Header ([#2495](https://github.com/getsentry/sentry-dotnet/pull/2495))

### Fixes

- Fixes baggage propagation when an exception is thrown from middleware ([#2487](https://github.com/getsentry/sentry-dotnet/pull/2487))
- Fixed baggage propagation when an exception is thrown from middleware ([#2487](https://github.com/getsentry/sentry-dotnet/pull/2487))
- Fix Durable Functions preventing orchestrators from completing ([#2491](https://github.com/getsentry/sentry-dotnet/pull/2491))
- Re-enable HubTests.FlushOnDispose_SendsEnvelope ([#2492](https://github.com/getsentry/sentry-dotnet/pull/2492))

Expand Down
13 changes: 13 additions & 0 deletions src/Sentry/DynamicSamplingContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ internal class DynamicSamplingContext
private DynamicSamplingContext(
SentryId traceId,
string publicKey,
bool? sampled,
double sampleRate,
string? release = null,
string? environment = null,
Expand Down Expand Up @@ -51,6 +52,11 @@ private DynamicSamplingContext(
["sample_rate"] = sampleRate.ToString(CultureInfo.InvariantCulture)
};

if (sampled.HasValue)
{
items.Add("sampled", sampled.Value ? "true" : "false");
}

// Set optional values
if (!string.IsNullOrWhiteSpace(release))
{
Expand Down Expand Up @@ -95,6 +101,11 @@ private DynamicSamplingContext(
return null;
}

if (items.TryGetValue("sampled", out var sampledString) && !bool.TryParse(sampledString, out _))
{
return null;
}

if (!items.TryGetValue("sample_rate", out var sampleRate) ||
!double.TryParse(sampleRate, NumberStyles.Float, CultureInfo.InvariantCulture, out var rate) ||
rate is < 0.0 or > 1.0)
Expand All @@ -110,6 +121,7 @@ public static DynamicSamplingContext CreateFromTransaction(TransactionTracer tra
// These should already be set on the transaction.
var publicKey = Dsn.Parse(options.Dsn!).PublicKey;
var traceId = transaction.TraceId;
var sampled = transaction.IsSampled;
var sampleRate = transaction.SampleRate!.Value;
var userSegment = transaction.User.Segment;
var transactionName = transaction.NameSource.IsHighQuality() ? transaction.Name : null;
Expand All @@ -121,6 +133,7 @@ public static DynamicSamplingContext CreateFromTransaction(TransactionTracer tra
return new DynamicSamplingContext(
traceId,
publicKey,
sampled,
sampleRate,
release,
environment,
Expand Down
2 changes: 1 addition & 1 deletion src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ internal ITransaction StartTransaction(
}

// Use the provided DSC, or create one based on this transaction.
// This must be done AFTER the sampling decision has been made.
// DSC creation must be done AFTER the sampling decision has been made.
transaction.DynamicSamplingContext =
dynamicSamplingContext ?? transaction.CreateDynamicSamplingContext(_options);

Expand Down
41 changes: 36 additions & 5 deletions test/Sentry.Tests/DynamicSamplingContextTests.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using Xunit.Sdk;

namespace Sentry.Tests;

public class DynamicSamplingContextTests
Expand Down Expand Up @@ -142,6 +144,22 @@ public void CreateFromBaggage_SampleRate_TooHigh()
Assert.Null(dsc);
}

[Fact]
public void CreateFromBaggage_Sampled_MalFormed()
{
var baggage = BaggageHeader.Create(new List<KeyValuePair<string, string>>
{
{"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"},
{"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"},
{"sentry-sample_rate", "1.0"},
{"sentry-sampled", "foo"},
});

var dsc = baggage.CreateDynamicSamplingContext();

Assert.Null(dsc);
}

[Fact]
public void CreateFromBaggage_Valid_Minimum()
{
Expand All @@ -168,6 +186,7 @@ public void CreateFromBaggage_Valid_Complete()
{
{"sentry-trace_id", "43365712692146d08ee11a729dfbcaca"},
{"sentry-public_key", "d4d82fc1c2c4032a83f3a29aa3a3aff"},
{"sentry-sampled", "true"},
{"sentry-sample_rate", "1.0"},
{"sentry-release", "test@1.0.0+abc"},
{"sentry-environment", "production"},
Expand All @@ -178,9 +197,10 @@ public void CreateFromBaggage_Valid_Complete()
var dsc = baggage.CreateDynamicSamplingContext();

Assert.NotNull(dsc);
Assert.Equal(7, dsc.Items.Count);
Assert.Equal(baggage.Members.Count, dsc.Items.Count);
Assert.Equal("43365712692146d08ee11a729dfbcaca", Assert.Contains("trace_id", dsc.Items));
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("test@1.0.0+abc", Assert.Contains("release", dsc.Items));
Assert.Equal("production", Assert.Contains("environment", dsc.Items));
Expand Down Expand Up @@ -210,8 +230,11 @@ public void ToBaggageHeader()
Assert.Equal(original.Members, result.Members);
}

[Fact]
public void CreateFromTransaction()
[Theory]
[InlineData(false)]
[InlineData(true)]
[InlineData(null)]
public void CreateFromTransaction(bool? isSampled)
{
var options = new SentryOptions
{
Expand All @@ -230,7 +253,7 @@ public void CreateFromTransaction()
{
Name = "GET /person/{id}",
NameSource = TransactionNameSource.Route,
IsSampled = true,
IsSampled = isSampled,
SampleRate = 0.5,
User =
{
Expand All @@ -241,9 +264,17 @@ public void CreateFromTransaction()
var dsc = transaction.CreateDynamicSamplingContext(options);

Assert.NotNull(dsc);
Assert.Equal(7, dsc.Items.Count);
Assert.Equal(isSampled.HasValue ? 8 : 7, dsc.Items.Count);
Assert.Equal(traceId.ToString(), Assert.Contains("trace_id", dsc.Items));
Assert.Equal("d4d82fc1c2c4032a83f3a29aa3a3aff", Assert.Contains("public_key", dsc.Items));
if (transaction.IsSampled is { } sampled)
{
Assert.Equal(sampled ? "true" : "false", Assert.Contains("sampled", dsc.Items));
}
else
{
Assert.DoesNotContain("sampled", dsc.Items);
}
Assert.Equal("0.5", Assert.Contains("sample_rate", dsc.Items));
Assert.Equal("foo@2.4.5", Assert.Contains("release", dsc.Items));
Assert.Equal("staging", Assert.Contains("environment", dsc.Items));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[
[
{
Header: {
event_id: Guid_1,
Expand All @@ -10,6 +10,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_2,
transaction: my transaction
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_3,
transaction: my transaction
}
Expand Down Expand Up @@ -144,6 +145,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_3,
transaction: my transaction
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_3,
transaction: my transaction
}
Expand Down Expand Up @@ -144,6 +145,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_3,
transaction: my transaction
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_3,
transaction: my transaction
}
Expand Down Expand Up @@ -144,6 +145,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_3,
transaction: my transaction
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_3,
transaction: my transaction
}
Expand Down Expand Up @@ -144,6 +145,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_3,
transaction: my transaction
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_3,
transaction: my transaction
}
Expand Down Expand Up @@ -144,6 +145,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_3,
transaction: my transaction
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[
[
{
Header: {
event_id: Guid_1,
Expand All @@ -10,6 +10,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_2,
transaction: my transaction
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[
[
{
Header: {
event_id: Guid_1,
Expand All @@ -10,6 +10,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_2,
transaction: my transaction
}
Expand Down Expand Up @@ -51,6 +52,7 @@
public_key: d4d82fc1c2c4032a83f3a29aa3a3aff,
release: release,
sample_rate: 1,
sampled: true,
trace_id: Guid_2,
transaction: my transaction
}
Expand Down