-
-
Notifications
You must be signed in to change notification settings - Fork 77
chore(deps): Upgrade @sentry/node from v7 to v10.27.0 #1126
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1126 +/- ##
==========================================
+ Coverage 36.26% 36.28% +0.02%
==========================================
Files 144 144
Lines 17746 17753 +7
Branches 1457 1459 +2
==========================================
+ Hits 6435 6442 +7
Misses 11293 11293
Partials 18 18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if (rootSpan) { | ||
| rootSpan.setStatus({ | ||
| code: status === 0 ? 1 : 2, // 1 = ok/cancelled, 2 = error/aborted | ||
| message: status === 0 ? 'cancelled' : 'aborted', | ||
| }); | ||
| rootSpan.end(); |
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.
Bug: OpenTelemetry status code 1 is incorrectly used for 'cancelled' operations, reporting them as successful.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
The code sets rootSpan.setStatus({ code: 1, message: 'cancelled' }) when a user cancels an operation. According to the OpenTelemetry standard, which Sentry v10+ uses, status code 1 signifies Ok (success). This incorrect mapping causes cancelled operations to be reported as successful in the Sentry UI, leading to distorted telemetry metrics such as failure rates.
💡 Suggested Fix
Change the status code for 'cancelled' operations from 1 to an appropriate OpenTelemetry status code that reflects cancellation or a non-successful state, such as SpanStatusCode.UNSET or SpanStatusCode.ERROR with a specific attribute.
🤖 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/utils/clack/index.ts#L127-L132
Potential issue: The code sets `rootSpan.setStatus({ code: 1, message: 'cancelled' })`
when a user cancels an operation. According to the OpenTelemetry standard, which Sentry
v10+ uses, status code `1` signifies `Ok` (success). This incorrect mapping causes
cancelled operations to be reported as successful in the Sentry UI, leading to distorted
telemetry metrics such as failure rates.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 6291988
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.
I think 'ok' is fine here, given cancelled is not an error but a user decision.
Closes DEVEX-184