Skip to content

proxy: don't mark ingest.queue_send spans ERROR for expected 4xx body rejections#178

Open
arseniycodes wants to merge 1 commit into
mainfrom
ash/proxy-queue-send-span-noise
Open

proxy: don't mark ingest.queue_send spans ERROR for expected 4xx body rejections#178
arseniycodes wants to merge 1 commit into
mainfrom
ash/proxy-queue-send-span-noise

Conversation

@arseniycodes

@arseniycodes arseniycodes commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Problem

When a client sends an empty or oversized OTLP body, enqueueStream throws EmptyBodyError / PayloadTooLargeError. These are handled client errors: the outer catch maps them to 400/413 via handleIngestBodyError, the S3 multipart upload is aborted for the oversized case, and the request lifecycle completes normally. But the inner catch inside the ingest.queue_send span unconditionally called recordException + setStatus(ERROR), so every such request produced an ERROR span — and a false-positive incident — for an operation working exactly as designed. The surrounding spans (ingest.forward, auth.validate, the server span) all stay Unset, confirming nothing actually failed.

Fix

  • body-capture.ts: new isExpectedBodyError(err) predicate next to the two error classes.
  • index.ts: the ingest.queue_send catch skips recordException/setStatus(ERROR) for expected rejections and still rethrows so the outer handler produces the 4xx. Genuine enqueue failures (SQS/S3 errors) are unchanged and still mark the span ERROR.

Testing

  • New isExpectedBodyError unit test in body-capture.test.ts (matches both rejections, rejects generic errors/undefined) — written first, red, then green.
  • Full proxy suite: tsx --test src/*.test.ts — 89 pass, 0 fail.
  • tsc --noEmit clean.

Summary by cubic

Stop marking ingest.queue_send spans as ERROR for expected 4xx body rejections (empty or oversized OTLP payloads). This removes false-positive incidents while keeping real SQS/S3 enqueue failures marked as ERROR.

  • Bug Fixes
    • Added isExpectedBodyError alongside EmptyBodyError and PayloadTooLargeError.
    • Updated index.ts to skip recordException and setStatus(ERROR) for expected rejections; still rethrows so the outer handler returns 400/413.
    • Added unit test for isExpectedBodyError; full proxy test suite passes.

Written for commit 3025064. Summary will update on new commits.

Review in cubic

… rejections

EmptyBodyError and PayloadTooLargeError thrown by enqueueStream are handled
client errors: the outer handler maps them to 400/413 and the request
completes normally (for oversized bodies the S3 multipart upload is aborted
first). The inner catch on the ingest.queue_send span still recorded the
exception and set ERROR status unconditionally, so every empty or oversized
payload produced an ERROR span — and a false-positive incident — for an
operation working as designed.

Add isExpectedBodyError alongside the error classes in body-capture.ts and
skip recordException/setStatus(ERROR) for those two rejections. Genuine
enqueue failures (SQS/S3 errors) still mark the span ERROR.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

Re-trigger cubic

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.

1 participant