-
Notifications
You must be signed in to change notification settings - Fork 482
feat: do not report error stack for status code errors #3837
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
Conversation
BenchmarksBenchmark execution time: 2025-09-24 14:35:03 Comparing candidate commit 4fefb03 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 2 metrics, 0 unstable metrics. scenario:BenchmarkSetTagStringPtr-24
|
ErrorStack = "error.stack" | ||
|
||
// ErrorHandlingStack specifies the stack dump when the error is captured. | ||
ErrorHandlingStack = "error.handling_stack" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are adding these two new tags, what are the implications on the ingestion side? Especially for error tracking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new tags will change the behavior of error tags on spans; handling_stack
should already be handled from the error tracking side, but we just haven't been taking advantage of it earlier. The fingerprints of spans will change after we release this PR, so we should give them a heads-up before so that they can adjust in the backend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation
/merge |
View all feedbacks in Devflow UI.
This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
devflow unqueued this merge request: It did not become mergeable within the expected time |
|
/merge |
View all feedbacks in Devflow UI.
The expected merge time in
|
What does this PR do?
ErrorNoStackTrace
, to report errors without capturing the stack trace for our UI.error.stack
toerror.handling_stack
, movingerror.details
intoerror.stack
, and deprecatingerror.details
.Motivation
We were creating errors and reporting the stack for status code errors. For new error stack features (#3709), this causes us to create two error stacks: one starting from
SetTag
and one started from error creation. To avoid this, and also because reporting these error stacks is superfluous, we want to prevent reporting these stacks to hide them from our UI.For information on what each tag represents, refer to this (internal) page.
IMPORTANT: this might be considered a breaking change for Error Tracking. Please check with the ET team before releasing this feature.
Reviewer's Checklist
./scripts/lint.sh
locally.Unsure? Have a question? Request a review!