fix: OpenTelemetry transactions for failed requests keep their route name and otel context#5310
fix: OpenTelemetry transactions for failed requests keep their route name and otel context#5310jamescrosswell wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5310 +/- ##
==========================================
- Coverage 74.17% 74.17% -0.01%
==========================================
Files 508 508
Lines 18353 18356 +3
Branches 3586 3587 +1
==========================================
+ Hits 13614 13615 +1
- Misses 3867 3869 +2
Partials 872 872 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…name and otel context With the OpenTelemetry integration, transactions are created by the SentrySpanProcessor on Activity start and only get their final name, operation and otel/response contexts on Activity end (parsed from http.route etc.). When a request threw an unhandled exception, Hub.CaptureEvent finished the scope's transaction as Aborted immediately after capturing the event — before the Activity ended — so the OTel enrichment was never applied. This caused failed requests to be reported under the raw activity name (e.g. "Microsoft.AspNetCore.Hosting.HttpRequestIn") with transaction source "custom" and no otel context, collapsing all failures into a single bucket with a 0% failure rate. Skip the early abort for OTel-instrumented transactions; the SentrySpanProcessor owns their lifecycle and finishes them with the correct name/operation/status when the Activity ends. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6cc572a to
2bac415
Compare
| _options.LogDebug( | ||
| "Not ending OpenTelemetry transaction as Aborted; it is finished by the SentrySpanProcessor."); | ||
| } | ||
| else | ||
| { |
There was a problem hiding this comment.
Bug: If SentrySpanProcessor.OnEnd is not called after an exception, the associated OTel transaction will never be finished or sent, leading to lost data.
Severity: MEDIUM
Suggested Fix
Implement a fallback mechanism, such as a timeout, to ensure transactions are eventually finished and reported even if SentrySpanProcessor.OnEnd is not called. Consider adding documentation about the implicit dependency on the OnEnd event firing.
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/Internal/Hub.cs#L673-L677
Potential issue: The code change skips finishing OpenTelemetry-instrumented transactions
during unhandled exceptions, instead relying on `SentrySpanProcessor.OnEnd` to complete
them. However, the `OnEnd` event is not guaranteed to fire in all scenarios, especially
in non-ASP.NET Core contexts or with certain framework-level exceptions. If `OnEnd` is
not called, the transaction will never be finished or sent to Sentry, remaining in
memory until garbage collected. This leads to the silent loss of performance data for
failed requests without any fallback mechanism.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
OnEnd is already the only path that finishes OTel transactions — every successful request relies on it, not just failed ones, so this doesn't add a new dependency. The cases where it wouldn't fire are (1) the Activity being sampled out, where we deliberately don't send a transaction, and (2) a process-terminating crash, where the flush is unreliable regardless and the only thing we'd have sent is the malformed transaction this PR removes.
For the ASP.NET Core path in the issue, the host always stops the activity, so OnEnd runs. A timeout fallback would just resurrect the early-finish bug.
Closes #5091
Summary
Fixes the issue where, with the OpenTelemetry integration, transactions for failed requests were reported under the raw activity name (e.g.
Microsoft.AspNetCore.Hosting.HttpRequestIn) instead of the route (e.g.GET /Test/BadEndpoint).Problem
With OTel instrumentation,
SentrySpanProcessorcreates the transaction on Activity start and only applies the final name, operation andotel/responsecontexts on Activity end (parsed fromhttp.routeetc., which aren't available at start).When a request threw an unhandled exception,
Hub.CaptureEventfinished the scope's transaction asAbortedimmediately after capturing the exception event — before the Activity ended — synchronously capturing/sending it. By the timeSentrySpanProcessor.OnEndran, the transaction was already finished, so all the OTel enrichment (route-based name,http.serveroperation,otel/responsecontexts) was discarded.This early-abort is correct for the native Sentry.AspNetCore tracing path (where the name is set up-front), but wrong when OTel owns the transaction lifecycle.
Fix
Skip the early
Abortedfinish for OTel-instrumented transactions. TheSentrySpanProcessorowns their lifecycle and finishes them with the correct name/operation/status when the Activity ends.Verification
Reproduced with the customer's minimal repro on .NET 10.
POST/GET BadEndpoint(throws → 500):Microsoft.AspNetCore.Hosting.HttpRequestInGET Test/BadEndpointMicrosoft.AspNetCore.Hosting.HttpRequestInhttp.serverotel/responsecontextAdded a regression test:
CaptureEvent_TerminalUnhandledException_DoesNotAbortOpenTelemetryTransaction.