Skip to content

Conversation

@sergical
Copy link
Member

Closes DEVEX-184

@linear
Copy link

linear bot commented Nov 28, 2025

@sergical sergical requested a review from Lms24 November 28, 2025 20:59
@codecov
Copy link

codecov bot commented Nov 28, 2025

Codecov Report

❌ Patch coverage is 30.76923% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 36.28%. Comparing base (e0f50bc) to head (58367a3).

Files with missing lines Patch % Lines
src/telemetry.ts 14.70% 29 Missing ⚠️
src/utils/clack/index.ts 61.11% 7 Missing ⚠️
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              
Flag Coverage Δ
unit-tests 36.28% <30.76%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sergical sergical requested a review from Lms24 December 8, 2025 16:18
Comment on lines +127 to +132
if (rootSpan) {
rootSpan.setStatus({
code: status === 0 ? 1 : 2, // 1 = ok/cancelled, 2 = error/aborted
message: status === 0 ? 'cancelled' : 'aborted',
});
rootSpan.end();
Copy link

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

Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants