-
Notifications
You must be signed in to change notification settings - Fork 566
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ý <pstibrany@gmail.com>
@@ -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ý <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
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ý <pstibrany@gmail.com>
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ý <pstibrany@gmail.com> * Update dskit, address review feedback. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * CHANGELOG.md Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Fix linter errors. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Fix go.mod Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Address review feedback. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> --------- Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
* Fix handling of HTTP error responses with binary data. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Update dskit, address review feedback. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * CHANGELOG.md Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Fix linter errors. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Fix go.mod Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Address review feedback. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> --------- Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
* 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ý <pstibrany@gmail.com> * Update dskit, address review feedback. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * CHANGELOG.md Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Fix linter errors. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Fix go.mod Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Address review feedback. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> --------- Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Fix go.mod. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> --------- Signed-off-by: Peter Štibraný <pstibrany@gmail.com>
* 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ý <pstibrany@gmail.com> * Update dskit, address review feedback. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * CHANGELOG.md Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Fix linter errors. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Fix go.mod Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Address review feedback. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> --------- Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Fix test failure because r293 doesn't have updated mapping yet. Signed-off-by: Peter Štibraný <pstibrany@gmail.com> * Downgrade dskit to version from r293 + our fix --------- Signed-off-by: Peter Štibraný <pstibrany@gmail.com> Co-authored-by: Ying WANG <ying.wang@grafana.com>
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.