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
chore(examples): add one example to log request body when the status code is non 200 #4108
Conversation
return | ||
} | ||
clonedR := r.Clone(r.Context()) | ||
r.Body = io.NopCloser(bytes.NewReader(body)) |
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.
Is this necessary? I don't think the request body should be used after the handler returns.
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.
Is this necessary? I don't think the request body should be used after the handler returns.
Yes, you are right. This line is removed in the new commit. Sorry for the typo.
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.
Thanks for the update. Could you perhaps add a small section to the Operations section of our doc with a link to this middleware? Here is the docs site: https://grpc-ecosystem.github.io/grpc-gateway/docs/operations/ and the source is here: https://github.com/grpc-ecosystem/grpc-gateway/tree/main/docs/docs/operations. Just create a new file :).
The doc has been added. Could you please review this doc? @johanbrandhorst |
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.
Some minor edits to the doc page, but thank you!
docs/docs/operations/logging.md
Outdated
|
||
If you want to log the request body in the `customErrorHandler` middleware, unfortunately the request body has been consumed in the `customErrorHandler` middleware and can't be read again. To log the request body, you can use one middleware to buffer the request body before it's consumed. | ||
|
||
1. `logRequestBody` middleware,which logs the request body when the response status code is not 200. |
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.
1. `logRequestBody` middleware,which logs the request body when the response status code is not 200. | |
1. Create a `http.Handler` middleware. The `logRequestBody` example middleware logs the request body when the response status code is not 200. |
docs/docs/operations/logging.md
Outdated
|
||
# Logging the request body pattern for a request | ||
|
||
If you want to log the request body in the `customErrorHandler` middleware, unfortunately the request body has been consumed in the `customErrorHandler` middleware and can't be read again. To log the request body, you can use one middleware to buffer the request body before it's consumed. |
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.
I think we can be more general than this. How about
If you want to log the request body in the `customErrorHandler` middleware, unfortunately the request body has been consumed in the `customErrorHandler` middleware and can't be read again. To log the request body, you can use one middleware to buffer the request body before it's consumed. | |
If you want to log the request body of incoming requests, you will need to buffer the body before it reaches the gateway. To log the request body, you can use a middleware `http.Handler` to buffer the request body before it's consumed. |
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.
I will update the doc
docs/docs/operations/logging.md
Outdated
lw := newLogResponseWriter(w) | ||
body, err := io.ReadAll(r.Body) | ||
if err != nil { | ||
http.Error(w, fmt.Sprintf("grpc server read request body err %+v", err), http.StatusBadRequest) |
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.
http.Error(w, fmt.Sprintf("grpc server read request body err %+v", err), http.StatusBadRequest) | |
http.Error(w, fmt.Sprintf("failed to read body: %v", err), http.StatusBadRequest) |
docs/docs/operations/logging.md
Outdated
} | ||
``` | ||
|
||
2. Wrap the `logRequestBody` middleware with the `http.ServeMux`: |
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.
2. Wrap the `logRequestBody` middleware with the `http.ServeMux`: | |
2. Wrap the gateway serve mux with the `logRequestBody` handler: |
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.
I will fix it
docs/docs/operations/logging.md
Outdated
2. Wrap the `logRequestBody` middleware with the `http.ServeMux`: | ||
|
||
```go | ||
mux := http.NewServeMux() |
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.
mux := http.NewServeMux() | |
mux := runtime.NewServeMux() | |
// Register generated gateway handlers |
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.
Sorry for the typo
Thank you very much for your detailed review of the document. Could you please review the update comment again? @johanbrandhorst |
Thanks for your contribution! |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | Type | Update | |---|---|---|---|---|---|---|---| | [github.com/grpc-ecosystem/grpc-gateway/v2](https://togithub.com/grpc-ecosystem/grpc-gateway) | `v2.19.1` -> `v2.20.0` | [![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgrpc-ecosystem%2fgrpc-gateway%2fv2/v2.20.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fgrpc-ecosystem%2fgrpc-gateway%2fv2/v2.20.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fgrpc-ecosystem%2fgrpc-gateway%2fv2/v2.19.1/v2.20.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgrpc-ecosystem%2fgrpc-gateway%2fv2/v2.19.1/v2.20.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | require | minor | | | All locks refreshed | | | | | | lockFileMaintenance | | [jekyll-feed](https://togithub.com/jekyll/jekyll-feed) | `0.15.1` -> `0.17.0` | [![age](https://developer.mend.io/api/mc/badges/age/rubygems/jekyll-feed/0.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/rubygems/jekyll-feed/0.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/rubygems/jekyll-feed/0.15.1/0.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/rubygems/jekyll-feed/0.15.1/0.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | minor | --- ### Release Notes <details> <summary>grpc-ecosystem/grpc-gateway (github.com/grpc-ecosystem/grpc-gateway/v2)</summary> ### [`v2.20.0`](https://togithub.com/grpc-ecosystem/grpc-gateway/releases/tag/v2.20.0) [Compare Source](https://togithub.com/grpc-ecosystem/grpc-gateway/compare/v2.19.1...v2.20.0) #### What's Changed - api_visibility option now transitively hides request/response messages by [@​Place1](https://togithub.com/Place1) in [grpc-ecosystem/grpc-gateway#3966 - Marshal non proto fields with indents by [@​gknw](https://togithub.com/gknw) in [grpc-ecosystem/grpc-gateway#4027 - chore: fix some comments by [@​avoidalone](https://togithub.com/avoidalone) in [grpc-ecosystem/grpc-gateway#4092 - chore(examples): add one example to log request body when the status code is non 200 by [@​richzw](https://togithub.com/richzw) in [grpc-ecosystem/grpc-gateway#4108 - feat(bazel): surface --disable_default_responses in rule def by [@​rajukrishnamurthy](https://togithub.com/rajukrishnamurthy) in [grpc-ecosystem/grpc-gateway#4126 - integration: fix wrapping of response writer causing endless test loop by [@​johanbrandhorst](https://togithub.com/johanbrandhorst) in [grpc-ecosystem/grpc-gateway#4131 - Omit enum field when value is empty by [@​jeromefroe](https://togithub.com/jeromefroe) in [grpc-ecosystem/grpc-gateway#4180 - docs/mapping/examples.md: update CoreOS example GitHub branch to `master` branch by [@​mohamedawnallah](https://togithub.com/mohamedawnallah) in [grpc-ecosystem/grpc-gateway#4198 - chore: remove repetitive word by [@​mountcount](https://togithub.com/mountcount) in [grpc-ecosystem/grpc-gateway#4183 - Fix Typo in examples.md by [@​umakantv](https://togithub.com/umakantv) in [grpc-ecosystem/grpc-gateway#4212 - Write Content-Length header if doForwardTrailers is not set by [@​joshgarnett](https://togithub.com/joshgarnett) in [grpc-ecosystem/grpc-gateway#4259 - Fixing TestOutgoingTrailerMatcher, which was non-deterministic by [@​joshgarnett](https://togithub.com/joshgarnett) in [grpc-ecosystem/grpc-gateway#4265 - Update README.md by [@​MakDon](https://togithub.com/MakDon) in [grpc-ecosystem/grpc-gateway#4322 - fix(4245): setting appropriate log level for error logs by [@​rajeshkarnena](https://togithub.com/rajeshkarnena) in [grpc-ecosystem/grpc-gateway#4327 - fix: handle `X-Forwarded-*` headers correctly by [@​haines](https://togithub.com/haines) in [grpc-ecosystem/grpc-gateway#4334 #### New Contributors - [@​Place1](https://togithub.com/Place1) made their first contribution in [grpc-ecosystem/grpc-gateway#3966 - [@​avoidalone](https://togithub.com/avoidalone) made their first contribution in [grpc-ecosystem/grpc-gateway#4092 - [@​richzw](https://togithub.com/richzw) made their first contribution in [grpc-ecosystem/grpc-gateway#4108 - [@​rajukrishnamurthy](https://togithub.com/rajukrishnamurthy) made their first contribution in [grpc-ecosystem/grpc-gateway#4126 - [@​jeromefroe](https://togithub.com/jeromefroe) made their first contribution in [grpc-ecosystem/grpc-gateway#4180 - [@​mohamedawnallah](https://togithub.com/mohamedawnallah) made their first contribution in [grpc-ecosystem/grpc-gateway#4198 - [@​mountcount](https://togithub.com/mountcount) made their first contribution in [grpc-ecosystem/grpc-gateway#4183 - [@​umakantv](https://togithub.com/umakantv) made their first contribution in [grpc-ecosystem/grpc-gateway#4212 - [@​joshgarnett](https://togithub.com/joshgarnett) made their first contribution in [grpc-ecosystem/grpc-gateway#4259 - [@​rajeshkarnena](https://togithub.com/rajeshkarnena) made their first contribution in [grpc-ecosystem/grpc-gateway#4327 **Full Changelog**: grpc-ecosystem/grpc-gateway@v2.19.1...v2.20.0 </details> <details> <summary>jekyll/jekyll-feed (jekyll-feed)</summary> ### [`v0.17.0`](https://togithub.com/jekyll/jekyll-feed/blob/HEAD/History.markdown#0170--2022-10-14) [Compare Source](https://togithub.com/jekyll/jekyll-feed/compare/v0.16.0...v0.17.0) ##### Documentation - Update CI status badge ([#​363](https://togithub.com/jekyll/jekyll-feed/issues/363)) ##### Development Fixes - Add Ruby 3.1 to the CI matrix ([#​365](https://togithub.com/jekyll/jekyll-feed/issues/365)) ##### Minor Enhancements - Allow disabling of jekyll-feed while in development ([#​370](https://togithub.com/jekyll/jekyll-feed/issues/370)) ### [`v0.16.0`](https://togithub.com/jekyll/jekyll-feed/blob/HEAD/History.markdown#0160--2022-01-03) [Compare Source](https://togithub.com/jekyll/jekyll-feed/compare/v0.15.1...v0.16.0) ##### Minor Enhancements - Add support for `page.description` in front matter to become entry `<summary>` ([#​297](https://togithub.com/jekyll/jekyll-feed/issues/297)) ##### Bug Fixes - Fold private methods into the `:render` method as local variables ([#​327](https://togithub.com/jekyll/jekyll-feed/issues/327)) - Check `post.categories` instead of `post.category` ([#​357](https://togithub.com/jekyll/jekyll-feed/issues/357)) - Switched xml_escape for `<![CDATA[]]>` for post content ([#​332](https://togithub.com/jekyll/jekyll-feed/issues/332)) ##### Development Fixes - Add Ruby 3.0 to CI ([#​337](https://togithub.com/jekyll/jekyll-feed/issues/337)) - Lock RuboCop to v1.18.x ([#​348](https://togithub.com/jekyll/jekyll-feed/issues/348)) - Add workflow to release gem via GH Action ([#​355](https://togithub.com/jekyll/jekyll-feed/issues/355)) ##### Documentation - Use `.atom` extension in documented examples since we write an Atom feed ([#​359](https://togithub.com/jekyll/jekyll-feed/issues/359)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 6am on wednesday" in timezone Australia/Sydney, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/google/osv.dev). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ0YXJnZXRCcmFuY2giOiJtYXN0ZXIiLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->
…33162) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [github.com/grpc-ecosystem/grpc-gateway/v2](https://togithub.com/grpc-ecosystem/grpc-gateway) | `v2.19.1` -> `v2.20.0` | [![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgrpc-ecosystem%2fgrpc-gateway%2fv2/v2.20.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fgrpc-ecosystem%2fgrpc-gateway%2fv2/v2.20.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fgrpc-ecosystem%2fgrpc-gateway%2fv2/v2.19.1/v2.20.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgrpc-ecosystem%2fgrpc-gateway%2fv2/v2.19.1/v2.20.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>grpc-ecosystem/grpc-gateway (github.com/grpc-ecosystem/grpc-gateway/v2)</summary> ### [`v2.20.0`](https://togithub.com/grpc-ecosystem/grpc-gateway/releases/tag/v2.20.0) [Compare Source](https://togithub.com/grpc-ecosystem/grpc-gateway/compare/v2.19.1...v2.20.0) #### What's Changed - api_visibility option now transitively hides request/response messages by [@​Place1](https://togithub.com/Place1) in [grpc-ecosystem/grpc-gateway#3966 - Marshal non proto fields with indents by [@​gknw](https://togithub.com/gknw) in [grpc-ecosystem/grpc-gateway#4027 - chore: fix some comments by [@​avoidalone](https://togithub.com/avoidalone) in [grpc-ecosystem/grpc-gateway#4092 - chore(examples): add one example to log request body when the status code is non 200 by [@​richzw](https://togithub.com/richzw) in [grpc-ecosystem/grpc-gateway#4108 - feat(bazel): surface --disable_default_responses in rule def by [@​rajukrishnamurthy](https://togithub.com/rajukrishnamurthy) in [grpc-ecosystem/grpc-gateway#4126 - integration: fix wrapping of response writer causing endless test loop by [@​johanbrandhorst](https://togithub.com/johanbrandhorst) in [grpc-ecosystem/grpc-gateway#4131 - Omit enum field when value is empty by [@​jeromefroe](https://togithub.com/jeromefroe) in [grpc-ecosystem/grpc-gateway#4180 - docs/mapping/examples.md: update CoreOS example GitHub branch to `master` branch by [@​mohamedawnallah](https://togithub.com/mohamedawnallah) in [grpc-ecosystem/grpc-gateway#4198 - chore: remove repetitive word by [@​mountcount](https://togithub.com/mountcount) in [grpc-ecosystem/grpc-gateway#4183 - Fix Typo in examples.md by [@​umakantv](https://togithub.com/umakantv) in [grpc-ecosystem/grpc-gateway#4212 - Write Content-Length header if doForwardTrailers is not set by [@​joshgarnett](https://togithub.com/joshgarnett) in [grpc-ecosystem/grpc-gateway#4259 - Fixing TestOutgoingTrailerMatcher, which was non-deterministic by [@​joshgarnett](https://togithub.com/joshgarnett) in [grpc-ecosystem/grpc-gateway#4265 - Update README.md by [@​MakDon](https://togithub.com/MakDon) in [grpc-ecosystem/grpc-gateway#4322 - fix(4245): setting appropriate log level for error logs by [@​rajeshkarnena](https://togithub.com/rajeshkarnena) in [grpc-ecosystem/grpc-gateway#4327 - fix: handle `X-Forwarded-*` headers correctly by [@​haines](https://togithub.com/haines) in [grpc-ecosystem/grpc-gateway#4334 #### New Contributors - [@​Place1](https://togithub.com/Place1) made their first contribution in [grpc-ecosystem/grpc-gateway#3966 - [@​avoidalone](https://togithub.com/avoidalone) made their first contribution in [grpc-ecosystem/grpc-gateway#4092 - [@​richzw](https://togithub.com/richzw) made their first contribution in [grpc-ecosystem/grpc-gateway#4108 - [@​rajukrishnamurthy](https://togithub.com/rajukrishnamurthy) made their first contribution in [grpc-ecosystem/grpc-gateway#4126 - [@​jeromefroe](https://togithub.com/jeromefroe) made their first contribution in [grpc-ecosystem/grpc-gateway#4180 - [@​mohamedawnallah](https://togithub.com/mohamedawnallah) made their first contribution in [grpc-ecosystem/grpc-gateway#4198 - [@​mountcount](https://togithub.com/mountcount) made their first contribution in [grpc-ecosystem/grpc-gateway#4183 - [@​umakantv](https://togithub.com/umakantv) made their first contribution in [grpc-ecosystem/grpc-gateway#4212 - [@​joshgarnett](https://togithub.com/joshgarnett) made their first contribution in [grpc-ecosystem/grpc-gateway#4259 - [@​rajeshkarnena](https://togithub.com/rajeshkarnena) made their first contribution in [grpc-ecosystem/grpc-gateway#4327 **Full Changelog**: grpc-ecosystem/grpc-gateway@v2.19.1...v2.20.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector-contrib). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbImRlcGVuZGVuY2llcyIsInJlbm92YXRlYm90Il19--> --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>
References to other Issues or PRs
Add one sample to resolve the issue #3780
Have you read the Contributing Guidelines?
Brief description of what is fixed or changed
One outer wrapper to clone the request body, since the body of the request has been consumed in the customErrorHandler.
Other comments