-
Notifications
You must be signed in to change notification settings - Fork 569
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
Fix reporting of HTTP error messages with binary body content #8363
Fix reporting of HTTP error messages with binary body content #8363
Conversation
Signed-off-by: Peter Štibraný <[email protected]>
@@ -46,13 +47,17 @@ const ( | |||
maxErrMsgLen = 1024 | |||
) | |||
|
|||
type OTLPHandlerLimits interface { |
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 interface was extracted to simplify test setup, and is otherwise unrelated to the PR.
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
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.
Great work!! See my comments though; I think there might be a left behind comment, and a missing wg.Add(1)
.
Signed-off-by: Peter Štibraný <[email protected]>
Thanks for review Arve. I've fixed your latest findings. |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-8363-to-r293 origin/r293
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x b9055992dbadb486e0e682bf02a4eebdcdf8774f
# Push it to GitHub
git push --set-upstream origin backport-8363-to-r293
git switch main
# Remove the local backport branch
git branch -D backport-8363-to-r293 Then, create a pull request where the |
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-8363-to-r294 origin/r294
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x b9055992dbadb486e0e682bf02a4eebdcdf8774f
# Push it to GitHub
git push --set-upstream origin backport-8363-to-r294
git switch main
# Remove the local backport branch
git branch -D backport-8363-to-r294 Then, create a pull request where the |
* Fix handling of HTTP error responses with binary data. Signed-off-by: Peter Štibraný <[email protected]> * Update dskit, address review feedback. Signed-off-by: Peter Štibraný <[email protected]> * CHANGELOG.md Signed-off-by: Peter Štibraný <[email protected]> * Fix linter errors. Signed-off-by: Peter Štibraný <[email protected]> * Fix go.mod Signed-off-by: Peter Štibraný <[email protected]> * Address review feedback. Signed-off-by: Peter Štibraný <[email protected]> --------- Signed-off-by: Peter Štibraný <[email protected]>
* Fix handling of HTTP error responses with binary data. Signed-off-by: Peter Štibraný <[email protected]> * Update dskit, address review feedback. Signed-off-by: Peter Štibraný <[email protected]> * CHANGELOG.md Signed-off-by: Peter Štibraný <[email protected]> * Fix linter errors. Signed-off-by: Peter Štibraný <[email protected]> * Fix go.mod Signed-off-by: Peter Štibraný <[email protected]> * Address review feedback. Signed-off-by: Peter Štibraný <[email protected]> --------- Signed-off-by: Peter Štibraný <[email protected]>
* Fix reporting of HTTP error messages with binary body content (#8363) * Fix handling of HTTP error responses with binary data. Signed-off-by: Peter Štibraný <[email protected]> * Update dskit, address review feedback. Signed-off-by: Peter Štibraný <[email protected]> * CHANGELOG.md Signed-off-by: Peter Štibraný <[email protected]> * Fix linter errors. Signed-off-by: Peter Štibraný <[email protected]> * Fix go.mod Signed-off-by: Peter Štibraný <[email protected]> * Address review feedback. Signed-off-by: Peter Štibraný <[email protected]> --------- Signed-off-by: Peter Štibraný <[email protected]> * Fix go.mod. Signed-off-by: Peter Štibraný <[email protected]> --------- Signed-off-by: Peter Štibraný <[email protected]>
* Fix reporting of HTTP error messages with binary body content (#8363) * Fix handling of HTTP error responses with binary data. Signed-off-by: Peter Štibraný <[email protected]> * Update dskit, address review feedback. Signed-off-by: Peter Štibraný <[email protected]> * CHANGELOG.md Signed-off-by: Peter Štibraný <[email protected]> * Fix linter errors. Signed-off-by: Peter Štibraný <[email protected]> * Fix go.mod Signed-off-by: Peter Štibraný <[email protected]> * Address review feedback. Signed-off-by: Peter Štibraný <[email protected]> --------- Signed-off-by: Peter Štibraný <[email protected]> * Fix test failure because r293 doesn't have updated mapping yet. Signed-off-by: Peter Štibraný <[email protected]> * Downgrade dskit to version from r293 + our fix --------- Signed-off-by: Peter Štibraný <[email protected]> Co-authored-by: Ying WANG <[email protected]>
What this PR does
PR #8227 introduced sending of binary HTTP response bodies to OTLP requests. Unfortunately our
httpgrpc.Server
implementation tries to use such response bodies as error message instatus.Status
objects, and marshalling such object then fails.This PR fixes that by adding support for specifying different error message to use in
status.Status
object (PR in dskit TBD), and fixing OTLP handler to use this support.Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]
.about-versioning.md
updated with experimental features.