Repository navigation
fix(opentelemetry source): return 4xx instead of 500 for rejected HTTP requests - #26509
Andrew-Hinson wants to merge 6 commits into
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
…upported-content-type # Conflicts: # src/sources/opentelemetry/http.rs
|
FYI, I merged master to resolve a conflict with #26457. Since that PR moved delivery failures out of handle_rejection, I dropped the now-unused Status check. |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
…, and 405 responses
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92e67d0e16
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
thomasqueirozb
left a comment
There was a problem hiding this comment.
Thanks for working with me on this one and for the important callout!
Summary
The
opentelemetrysource's OTLP/HTTP rejection handler returned500 Internal Server Errorfor every warp rejection that wasn't an
ErrorMessage, with the rejection'sDebugoutput(e.g.
Rejection(InvalidHeader { name: "content-type" })) as the message. So a clientsending the wrong content type got what looked like a server crash.
This PR maps those rejections to the right client errors:
Content-Typeis now415 Unsupported Media TypePOSTmethod now returns405 Method Not Allowed404 Not FoundAnything else still returns 500. This is the minimal fix suggested in the issue; accepting
OTLP/HTTP JSON would be a separate feature.
One thing I left out of scope: a malformed
content-encodingheader still returns 500.Most of this change was written with Claude Opus 5.5; I reviewed and tested it.
References
Closes: #26456
Vector configuration
How did you test this PR?
Added
http_rejections_return_client_error_status_codes, which fails on master and passeswith this change:
All OpenTelemetry source tests pass (Rust 1.96.1, Arch Linux).
I also ran a local build with the config above and sent requests with
curl:Content-Type: application/jsonContent-Type/v1/logs/v1/nopeapplication/x-protobufDoes this PR include user facing changes?
changelog.d/26456_opentelemetry_http_rejection_status.fix.md