-
-
Notifications
You must be signed in to change notification settings - Fork 501
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
Default to internal_error rather than unknown_error #2473
Conversation
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.
This makes sense, thanks. Could you also add a simple spec that covers this change to span_processor_spec.rb?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2473 +/- ##
=======================================
Coverage 98.19% 98.19%
=======================================
Files 128 128
Lines 4833 4833
=======================================
Hits 4746 4746
Misses 87 87
|
@solnic Done. The new spec cover all type of errors handled by |
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.
Looks good, thank you! Please update the top-level CHANGELOG.md too, and I'll be able to merge it in 🙂
0f55255
to
1e0bc06
Compare
@solnic Done! |
When using
sentry-opentelemetry
to trace code that isn't related to an HTTP call, theSpanProcessor
will set the status of aTransaction
to be"unknown_error"
, which is benign, but within Sentry UI this status is not interpreted as an error at all.The result is that all the impacted transactions appears to have 0% failure rate, etc. Changing it to
"internal_error"
prevents this, and also is in line with other sentry SDKs behaviour.Sidebar: The
SpanEvent
metadata around exceptions seems to be dropped in the conversion process toTransaction
, which is too bad.