Skip to content
Merged
Prev Previous commit
Next Next commit
Unit tests
  • Loading branch information
jamescrosswell committed May 23, 2025
commit 6fc1ff90a5e32c1a6dff5beb076e903515054a42
12 changes: 8 additions & 4 deletions src/Sentry.AspNetCore/SentryTracingMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,11 @@ public async Task InvokeAsync(HttpContext context)
}
finally
{
if (transaction is not null)
if (transaction is UnsampledTransaction)
{
transaction.Finish();
}
else if (transaction is TransactionTracer tracer)
{
// The Transaction name was altered during the pipeline execution,
// That could be done by user interference or by some Event Capture
Expand Down Expand Up @@ -183,15 +187,15 @@ public async Task InvokeAsync(HttpContext context)
if (!string.IsNullOrEmpty(customTransactionName))
{
transaction.Name = $"{method} {customTransactionName}";
((TransactionTracer)transaction).NameSource = TransactionNameSource.Custom;
tracer.NameSource = TransactionNameSource.Custom;
}
else
{
// Finally, fallback to using the URL path.
// e.g. "GET /pets/1"
var path = context.Request.Path;
transaction.Name = $"{method} {path}";
((TransactionTracer)transaction).NameSource = TransactionNameSource.Url;
tracer.NameSource = TransactionNameSource.Url;
}
}

Expand All @@ -200,7 +204,7 @@ public async Task InvokeAsync(HttpContext context)
transaction.Finish(status);
}
// Status code not yet changed to 500 but an exception does exist
// so lets avoid passing the misleading 200 down and close only with
// so let's avoid passing the misleading 200 down and close only with
// the exception instance that will be inferred as errored.
else if (status == SpanStatus.Ok)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ private async Task<TransactionContext> StartOrContinueTraceAsync(FunctionContext
if (requestData is null)
{
// not an HTTP trigger
return SentrySdk.ContinueTrace((SentryTraceHeader?)null, (BaggageHeader?)null, transactionName, Operation);
return _hub.ContinueTrace((SentryTraceHeader?)null, (BaggageHeader?)null, transactionName, Operation);
}

var httpMethod = requestData.Method.ToUpperInvariant();
Expand Down
11 changes: 7 additions & 4 deletions src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ internal ITransactionTracer StartTransaction(
return NoOpTransaction.Instance;
}

bool? isSampled = null;
double? sampleRate = null;
var sampleRand = dynamicSamplingContext?.Items.TryGetValue("sample_rand", out var dscsampleRand) ?? false
? double.Parse(dscsampleRand, NumberStyles.Float, CultureInfo.InvariantCulture)
Expand All @@ -155,17 +156,20 @@ internal ITransactionTracer StartTransaction(
if (tracesSampler(samplingContext) is { } samplerSampleRate)
{
sampleRate = samplerSampleRate;
// The TracesSampler trumps all other sampling decisions (even the trace header)
isSampled = SampleRandHelper.IsSampled(sampleRand, sampleRate.Value);
}
}

// If the sampling decision isn't made by a trace sampler then fallback to Random sampling
// If the sampling decision isn't made by a trace sampler we check the trace header first (from the context) or
// finally fallback to Random sampling if the decision has been made by no other means
sampleRate ??= _options.TracesSampleRate ?? 0.0;
var isSampled = SampleRandHelper.IsSampled(sampleRand, sampleRate.Value);
isSampled ??= context.IsSampled ?? SampleRandHelper.IsSampled(sampleRand, sampleRate.Value);

// Make sure there is a replayId (if available) on the provided DSC (if any).
dynamicSamplingContext = dynamicSamplingContext?.WithReplayId(_replaySession);

if (!isSampled)
if (isSampled is false)
{
var unsampledTransaction = new UnsampledTransaction(this, context)
{
Expand All @@ -181,7 +185,6 @@ internal ITransactionTracer StartTransaction(

var transaction = new TransactionTracer(this, context)
{
IsSampled = true,
SampleRate = sampleRate,
SampleRand = sampleRand,
DynamicSamplingContext = dynamicSamplingContext // Default to the provided DSC
Expand Down
4 changes: 2 additions & 2 deletions src/Sentry/Internal/NoOpSpan.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ protected NoOpSpan()
public IReadOnlyDictionary<string, object?> Extra => ImmutableDictionary<string, object?>.Empty;
public IReadOnlyDictionary<string, object?> Data => ImmutableDictionary<string, object?>.Empty;
public DateTimeOffset StartTimestamp => default;
public DateTimeOffset? EndTimestamp => default;
public bool IsFinished => default;
public DateTimeOffset? EndTimestamp => null;
public virtual bool IsFinished => false;

public virtual string Operation
{
Expand Down
20 changes: 18 additions & 2 deletions src/Sentry/Internal/UnsampledTransaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,15 @@ public UnsampledTransaction(IHub hub, ITransactionContext context)

internal DynamicSamplingContext? DynamicSamplingContext { get; set; }

private bool _isFinished;
public override bool IsFinished => _isFinished;

public override IReadOnlyCollection<ISpan> Spans => _spans;

public override SpanId SpanId => _context.SpanId;

public override SentryId TraceId => _context.TraceId;

public override bool? IsSampled => false;

public double? SampleRate { get; set; }
Expand All @@ -53,8 +58,17 @@ public override void Finish()
{
_options?.LogDebug("Finishing unsampled transaction");

// Clear the transaction from the scope
_hub.ConfigureScope(scope => scope.ResetTransaction(this));
// Ensure the transaction is really cleared from the scope
// See: https://github.com/getsentry/sentry-dotnet/issues/4198
_isFinished = true;

// Clear the transaction from the scope and regenerate the Propagation Context, so new events don't have a
// trace context that is "older" than the transaction that just finished
_hub.ConfigureScope(scope =>
{
scope.ResetTransaction(this);
scope.SetPropagationContext(new SentryPropagationContext());
});

// Record the discarded events
var spanCount = Spans.Count + 1; // 1 for each span + 1 for the transaction itself
Expand Down Expand Up @@ -83,5 +97,7 @@ public override ISpan StartChild(string operation)
private class UnsampledSpan(UnsampledTransaction transaction) : NoOpSpan
{
public override ISpan StartChild(string operation) => transaction.StartChild(operation);

public override bool? IsSampled => false;
}
}
12 changes: 2 additions & 10 deletions src/Sentry/TransactionTracer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,7 @@ public SpanStatus? Status
}

/// <inheritdoc />
public bool? IsSampled
{
get => Contexts.Trace.IsSampled;
internal set
{
Contexts.Trace.IsSampled = value;
SampleRate ??= value == null ? null : value.Value ? 1.0 : 0.0;
}
}
public bool? IsSampled => true; // Implicitly if we instantiate this class then the transaction is sampled in

/// <summary>
/// The sample rate used for this transaction.
Expand Down Expand Up @@ -234,6 +226,7 @@ internal TransactionTracer(IHub hub, string name, string operation, TransactionN
/// </summary>
internal TransactionTracer(IHub hub, ITransactionContext context, TimeSpan? idleTimeout = null)
{
Debug.Assert(context.IsSampled ?? true, "context.IsSampled should always be true when creating a TransactionTracer");
_hub = hub;
_options = _hub.GetSentryOptions();
Name = context.Name;
Expand All @@ -244,7 +237,6 @@ internal TransactionTracer(IHub hub, ITransactionContext context, TimeSpan? idle
TraceId = context.TraceId;
Description = context.Description;
Status = context.Status;
IsSampled = context.IsSampled;
StartTimestamp = _stopwatch.StartDateTimeOffset;

if (context is TransactionContext transactionContext)
Expand Down
8 changes: 6 additions & 2 deletions test/Sentry.AspNet.Tests/HttpContextExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ public void StartSentryTransaction_CreatesValidTransaction()
using var _ = SentrySdk.UseHub(new Hub(
new SentryOptions
{
Dsn = ValidDsn
Dsn = ValidDsn,
TracesSampleRate = 1.0
},
Substitute.For<ISentryClient>()
));
Expand Down Expand Up @@ -57,7 +58,8 @@ public void FinishSentryTransaction_FinishesTransaction()
using var _ = SentrySdk.UseHub(new Hub(
new SentryOptions
{
Dsn = ValidDsn
Dsn = ValidDsn,
TracesSampleRate = 1.0
},
Substitute.For<ISentryClient>()
));
Expand All @@ -81,6 +83,7 @@ public void StartSentryTransaction_SendDefaultPii_set_to_true_sets_cookies()
new SentryOptions
{
Dsn = ValidDsn,
TracesSampleRate = 1.0,
SendDefaultPii = true
},
Substitute.For<ISentryClient>()
Expand All @@ -102,6 +105,7 @@ public void StartSentryTransaction_SendDefaultPii_set_to_true_does_not_set_cooki
new SentryOptions
{
Dsn = ValidDsn,
TracesSampleRate = 1.0,
SendDefaultPii = true
},
Substitute.For<ISentryClient>()
Expand Down
44 changes: 30 additions & 14 deletions test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public async Task Transaction_is_bound_on_the_scope_automatically()

var sentryClient = Substitute.For<ISentryClient>();

var hub = new Hub(new SentryOptions { Dsn = ValidDsn }, sentryClient);
var hub = new Hub(new SentryOptions { Dsn = ValidDsn, TracesSampleRate = 1.0 }, sentryClient);

var server = new TestServer(new WebHostBuilder()
.UseDefaultServiceProvider(di => di.EnableValidation())
Expand Down Expand Up @@ -105,8 +105,10 @@ public async Task Transaction_is_bound_on_the_scope_automatically()
transaction.NameSource.Should().Be(TransactionNameSource.Route);
}

[Fact]
public async Task Transaction_is_started_automatically_from_incoming_trace_header()
[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task Transaction_is_started_automatically_from_incoming_trace_header(bool isSampled)
{
// Arrange
var sentryClient = Substitute.For<ISentryClient>();
Expand All @@ -133,27 +135,41 @@ public async Task Transaction_is_started_automatically_from_incoming_trace_heade
var client = server.CreateClient();

// Act
var sentryTraceHeader = isSampled
? "75302ac48a024bde9a3b3734a82e36c8-1000000000000000-1"
: "75302ac48a024bde9a3b3734a82e36c8-1000000000000000-0";

using var request = new HttpRequestMessage(HttpMethod.Get, "/person/13")
{
Headers =
{
{"sentry-trace", "75302ac48a024bde9a3b3734a82e36c8-1000000000000000-0"}
{"sentry-trace", sentryTraceHeader}
}
};

await client.SendAsync(request);

// Assert
sentryClient.Received(1).CaptureTransaction(Arg.Is<SentryTransaction>(t =>
t.Name == "GET /person/{id}" &&
t.NameSource == TransactionNameSource.Route &&
t.TraceId == SentryId.Parse("75302ac48a024bde9a3b3734a82e36c8") &&
t.ParentSpanId == SpanId.Parse("1000000000000000") &&
t.IsSampled == false
),
Arg.Any<Scope>(),
Arg.Any<SentryHint>()
);
if (isSampled)
{
sentryClient.Received(1).CaptureTransaction(Arg.Is<SentryTransaction>(t =>
t.Name == "GET /person/{id}" &&
t.NameSource == TransactionNameSource.Route &&
t.TraceId == SentryId.Parse("75302ac48a024bde9a3b3734a82e36c8") &&
t.ParentSpanId == SpanId.Parse("1000000000000000") &&
t.IsSampled == true
),
Arg.Any<Scope>(),
Arg.Any<SentryHint>()
);
}
else
{
sentryClient.DidNotReceive().CaptureTransaction(Arg.Any<SentryTransaction>(),
Arg.Any<Scope>(),
Arg.Any<SentryHint>()
);
}
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,19 @@ public async Task Original_exception_rethrown()
[Fact]
public async Task Transaction_PropertiesAreSet()
{
// Arrange
var functionContext = Substitute.For<FunctionContext>();
var functionDefinition = Substitute.For<FunctionDefinition>();
functionContext.FunctionDefinition.Returns(functionDefinition);
functionDefinition.Name.Returns(nameof(HttpFunction));

var sut = _fixture.GetSut();

// Act
await sut.Invoke(functionContext, context => HttpFunction(null, context));

// Assert
var transaction = _fixture.Transaction;

transaction.Should().NotBeNull();
transaction.Name.Should().Be(functionDefinition.Name);
transaction.Operation.Should().Be("function");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,7 @@ public Fixture()

public ITransactionTracer StartTransaction(IHub hub, ITransactionContext context)
{
var transaction = new TransactionTracer(hub, context)
{
IsSampled = true
};
var transaction = new TransactionTracer(hub, context);
var (currentScope, _) = ScopeManager.GetCurrent();
currentScope.Transaction = transaction;
return transaction;
Expand Down
Loading