Skip to content

feat!: don't trace 301, 305-399 or 401-404 responses by default #5632

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.

Sign up for GitHub

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

Merged
jamescrosswell merged 2 commits into from
Sep 29, 2026
Merged

feat!: don't trace 301, 305-399 or 401-404 responses by default #5632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c44561f
feat!: don't trace redirects or 401-404 responses by default
jamescrosswell Sep 28, 2026
b83738e
fix: keep tracing 302 and 303 responses by default
jamescrosswell Sep 28, 2026
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
10 changes: 8 additions & 2 deletions src/Sentry/SentryOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,9 +949,15 @@ public IDiagnosticLogger? DiagnosticLogger

/// <summary>
/// <para>Transactions will be dropped if the HTTP Response status code matches any of the configured ranges.</para>
/// <para>Defaults to an empty collection (all transactions are captured regardless of status code).</para>
/// <para>Defaults to 301, 305-399 and 401-404. 302 and 303 are traced, as they commonly redirect after a successful form POST.</para>
/// <para>Clear the collection to capture transactions regardless of status code.</para>
/// </summary>
public IList<HttpStatusCodeRange> TraceIgnoreStatusCodes { get; set; } = [];
public IList<HttpStatusCodeRange> TraceIgnoreStatusCodes { get; set; } = new List<HttpStatusCodeRange>
{
301,
(305, 399),
(401, 404)
};

Comment on lines 954 to 961

@sentry sentry Bot Sep 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Configuring TraceIgnoreStatusCodes via a configuration file incorrectly replaces the new default values instead of appending to them, causing more transactions to be traced.
Severity: MEDIUM

Suggested Fix

Update the configuration binding logic to merge user-provided values from the configuration file with the default values, rather than replacing them. The logic should append values from BindableSentryOptions.TraceIgnoreStatusCodes to the existing options.TraceIgnoreStatusCodes list.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/Sentry/SentryOptions.cs#L954-L961

Potential issue: The configuration binding logic for `TraceIgnoreStatusCodes` replaces
the entire default list if any values are provided in a configuration file, such as
`appsettings.json`. This is due to a null-coalescing assignment (`??`) that was not
updated to handle the new, non-empty default list. As a result, users who customize this
setting to add their own status codes will unintentionally lose the new default ignored
codes (e.g., 401-404). This can lead to an unexpected increase in traced transactions
and higher span quota consumption, as status codes that were intended to be ignored by
default will now be traced.

Also affects:

  • test/Sentry.Tests/BindableSentryOptionsTests.cs:26~67

Did we get this right? 👍 / 👎 to inform future reviews.

@jamescrosswell jamescrosswell Sep 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a bug - it's by design. We provide some defaults but people can override these via configuration. Otherwise there would be no way to remove any of our defaults via configuration (it would have to be configured in code).

// The default failed request target list will match anything, but adding to the list should clear that.
private Lazy<IList<StringOrRegex>> _failedRequestTargets = new(() =>
Expand Down
38 changes: 38 additions & 0 deletions test/Sentry.AspNetCore.Tests/IntegrationMockedBackgroundWorker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,4 +338,42 @@ public async Task Environment_BothOnOptionsAndEnvVar_ValueFromOption()
.Environment == expected
));
}

[Fact]
public async Task NotFoundRequest_ByDefault_NoTransactionCaptured()
{
Configure = o => o.TracesSampleRate = 1;
ConfigureApp = app => app.UseSentryTracing();

Build();
_ = await HttpClient.GetAsync("/");
_ = await HttpClient.GetAsync("/missing");

_ = Worker.Received(1).EnqueueEnvelope(Arg.Is<Envelope>(e => HasTransaction(e, "GET /")));
_ = Worker.DidNotReceive().EnqueueEnvelope(Arg.Is<Envelope>(e => HasTransaction(e, "GET /missing")));
}

[Fact]
public async Task NotFoundRequest_TraceIgnoreStatusCodesCleared_TransactionCaptured()
{
Configure = o =>
{
o.TracesSampleRate = 1;
o.TraceIgnoreStatusCodes.Clear();
};
ConfigureApp = app => app.UseSentryTracing();

Build();
_ = await HttpClient.GetAsync("/missing");

_ = Worker.Received(1).EnqueueEnvelope(Arg.Is<Envelope>(e => HasTransaction(e, "GET /missing")));
}

private static bool HasTransaction(Envelope envelope, string name) =>
envelope.Items
.Select(i => i.Payload)
.OfType<JsonSerializable>()
.Select(i => i.Source)
.OfType<SentryTransaction>()
.Any(t => t.Name == name);
}
37 changes: 37 additions & 0 deletions test/Sentry.Tests/BindableSentryOptionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,42 @@ public void ApplyTo_SetsOptionsFromConfig()
// Assert
AssertContainsExpectedPropertyValues(actual);
}

[Fact]
public void ApplyTo_TraceIgnoreStatusCodesNotConfigured_KeepsDefault()
{
// Arrange
var actual = new SentryOptions();
var bindable = new BindableSentryOptions();

// Act
new ConfigurationBuilder().Build().Bind(bindable);
bindable.ApplyTo(actual);

// Assert
actual.TraceIgnoreStatusCodes.Should().Equal(new SentryOptions().TraceIgnoreStatusCodes);
}

[Fact]
public void ApplyTo_TraceIgnoreStatusCodesConfigured_ReplacesDefault()
{
// Arrange
var actual = new SentryOptions();
var bindable = new BindableSentryOptions();
var config = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string>
{
["TraceIgnoreStatusCodes:0"] = "404",
["TraceIgnoreStatusCodes:1"] = "500"
})
.Build();

// Act
config.Bind(bindable);
bindable.ApplyTo(actual);

// Assert
actual.TraceIgnoreStatusCodes.Should().Equal(new HttpStatusCodeRange(404), new HttpStatusCodeRange(500));
}
}
#endif
57 changes: 47 additions & 10 deletions test/Sentry.Tests/TraceIgnoreStatusCodeTransactionProcessorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,8 @@ namespace Sentry.Tests;

public class TraceIgnoreStatusCodeTransactionProcessorTests
{
private static SentryOptions OptionsWithIgnoredCodes(params HttpStatusCodeRange[] ranges)
{
var options = new SentryOptions();
foreach (var range in ranges)
{
options.TraceIgnoreStatusCodes.Add(range);
}
return options;
}
private static SentryOptions OptionsWithIgnoredCodes(params HttpStatusCodeRange[] ranges) =>
new() { TraceIgnoreStatusCodes = [.. ranges] };

private static SentryTransaction TransactionWithStatusCode(int statusCode)
{
Expand All @@ -21,11 +14,55 @@ private static SentryTransaction TransactionWithStatusCode(int statusCode)
return transaction;
}

[Theory]
[InlineData(301)]
[InlineData(305)]
[InlineData(307)]
[InlineData(308)]
[InlineData(399)]
[InlineData(401)]
[InlineData(403)]
[InlineData(404)]
public void Process_DefaultOptions_DropsIgnoredStatusCode(int statusCode)
{
// Arrange
var processor = new TraceIgnoreStatusCodeTransactionProcessor(new SentryOptions());
var transaction = TransactionWithStatusCode(statusCode);

// Act
var result = processor.Process(transaction);

// Assert
result.Should().BeNull();
}

[Theory]
[InlineData(200)]
[InlineData(300)]
[InlineData(302)]
[InlineData(303)]
[InlineData(304)]
[InlineData(400)]
[InlineData(405)]
[InlineData(500)]
public void Process_DefaultOptions_ReturnsTransaction(int statusCode)
{
// Arrange
var processor = new TraceIgnoreStatusCodeTransactionProcessor(new SentryOptions());
var transaction = TransactionWithStatusCode(statusCode);

// Act
var result = processor.Process(transaction);

// Assert
result.Should().BeSameAs(transaction);
}

[Fact]
public void Process_EmptyIgnoreList_ReturnsTransaction()
{
// Arrange
var options = new SentryOptions();
var options = OptionsWithIgnoredCodes();
var processor = new TraceIgnoreStatusCodeTransactionProcessor(options);
var transaction = TransactionWithStatusCode(404);

Expand Down
Loading