Skip to content
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

Merged
merged 4 commits into from Mar 18, 2024

Conversation

richzw
Copy link
Contributor

@richzw richzw commented Mar 14, 2024

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

return
}
clonedR := r.Clone(r.Context())
r.Body = io.NopCloser(bytes.NewReader(body))
Copy link
Collaborator

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.

Copy link
Contributor Author

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.

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a 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 :).

@richzw
Copy link
Contributor Author

richzw commented Mar 16, 2024

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

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a 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!


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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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.


# 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.
Copy link
Collaborator

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

Suggested change
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.

Copy link
Contributor Author

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

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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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 Show resolved Hide resolved
}
```

2. Wrap the `logRequestBody` middleware with the `http.ServeMux`:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
2. Wrap the `logRequestBody` middleware with the `http.ServeMux`:
2. Wrap the gateway serve mux with the `logRequestBody` handler:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will fix it

2. Wrap the `logRequestBody` middleware with the `http.ServeMux`:

```go
mux := http.NewServeMux()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
mux := http.NewServeMux()
mux := runtime.NewServeMux()
// Register generated gateway handlers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the typo

@richzw
Copy link
Contributor Author

richzw commented Mar 17, 2024

Some minor edits to the doc page, but thank you!

Thank you very much for your detailed review of the document. Could you please review the update comment again? @johanbrandhorst

@johanbrandhorst johanbrandhorst merged commit 4319001 into grpc-ecosystem:main Mar 18, 2024
15 of 17 checks passed
@johanbrandhorst
Copy link
Collaborator

Thanks for your contribution!

cuixq pushed a commit to google/osv.dev that referenced this pull request May 22, 2024
[![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 [@&#8203;Place1](https://togithub.com/Place1) in
[grpc-ecosystem/grpc-gateway#3966
- Marshal non proto fields with indents by
[@&#8203;gknw](https://togithub.com/gknw) in
[grpc-ecosystem/grpc-gateway#4027
- chore: fix some comments by
[@&#8203;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 [@&#8203;richzw](https://togithub.com/richzw) in
[grpc-ecosystem/grpc-gateway#4108
- feat(bazel): surface --disable_default_responses in rule def by
[@&#8203;rajukrishnamurthy](https://togithub.com/rajukrishnamurthy) in
[grpc-ecosystem/grpc-gateway#4126
- integration: fix wrapping of response writer causing endless test loop
by [@&#8203;johanbrandhorst](https://togithub.com/johanbrandhorst) in
[grpc-ecosystem/grpc-gateway#4131
- Omit enum field when value is empty by
[@&#8203;jeromefroe](https://togithub.com/jeromefroe) in
[grpc-ecosystem/grpc-gateway#4180
- docs/mapping/examples.md: update CoreOS example GitHub branch to
`master` branch by
[@&#8203;mohamedawnallah](https://togithub.com/mohamedawnallah) in
[grpc-ecosystem/grpc-gateway#4198
- chore: remove repetitive word by
[@&#8203;mountcount](https://togithub.com/mountcount) in
[grpc-ecosystem/grpc-gateway#4183
- Fix Typo in examples.md by
[@&#8203;umakantv](https://togithub.com/umakantv) in
[grpc-ecosystem/grpc-gateway#4212
- Write Content-Length header if doForwardTrailers is not set by
[@&#8203;joshgarnett](https://togithub.com/joshgarnett) in
[grpc-ecosystem/grpc-gateway#4259
- Fixing TestOutgoingTrailerMatcher, which was non-deterministic by
[@&#8203;joshgarnett](https://togithub.com/joshgarnett) in
[grpc-ecosystem/grpc-gateway#4265
- Update README.md by [@&#8203;MakDon](https://togithub.com/MakDon) in
[grpc-ecosystem/grpc-gateway#4322
- fix(4245): setting appropriate log level for error logs by
[@&#8203;rajeshkarnena](https://togithub.com/rajeshkarnena) in
[grpc-ecosystem/grpc-gateway#4327
- fix: handle `X-Forwarded-*` headers correctly by
[@&#8203;haines](https://togithub.com/haines) in
[grpc-ecosystem/grpc-gateway#4334

#### New Contributors

- [@&#8203;Place1](https://togithub.com/Place1) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3966
- [@&#8203;avoidalone](https://togithub.com/avoidalone) made their first
contribution in
[grpc-ecosystem/grpc-gateway#4092
- [@&#8203;richzw](https://togithub.com/richzw) made their first
contribution in
[grpc-ecosystem/grpc-gateway#4108
- [@&#8203;rajukrishnamurthy](https://togithub.com/rajukrishnamurthy)
made their first contribution in
[grpc-ecosystem/grpc-gateway#4126
- [@&#8203;jeromefroe](https://togithub.com/jeromefroe) made their first
contribution in
[grpc-ecosystem/grpc-gateway#4180
- [@&#8203;mohamedawnallah](https://togithub.com/mohamedawnallah) made
their first contribution in
[grpc-ecosystem/grpc-gateway#4198
- [@&#8203;mountcount](https://togithub.com/mountcount) made their first
contribution in
[grpc-ecosystem/grpc-gateway#4183
- [@&#8203;umakantv](https://togithub.com/umakantv) made their first
contribution in
[grpc-ecosystem/grpc-gateway#4212
- [@&#8203;joshgarnett](https://togithub.com/joshgarnett) made their
first contribution in
[grpc-ecosystem/grpc-gateway#4259
- [@&#8203;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
([#&#8203;363](https://togithub.com/jekyll/jekyll-feed/issues/363))

##### Development Fixes

- Add Ruby 3.1 to the CI matrix
([#&#8203;365](https://togithub.com/jekyll/jekyll-feed/issues/365))

##### Minor Enhancements

- Allow disabling of jekyll-feed while in development
([#&#8203;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>`
([#&#8203;297](https://togithub.com/jekyll/jekyll-feed/issues/297))

##### Bug Fixes

- Fold private methods into the `:render` method as local variables
([#&#8203;327](https://togithub.com/jekyll/jekyll-feed/issues/327))
- Check `post.categories` instead of `post.category`
([#&#8203;357](https://togithub.com/jekyll/jekyll-feed/issues/357))
- Switched xml_escape for `<![CDATA[]]>` for post content
([#&#8203;332](https://togithub.com/jekyll/jekyll-feed/issues/332))

##### Development Fixes

- Add Ruby 3.0 to CI
([#&#8203;337](https://togithub.com/jekyll/jekyll-feed/issues/337))
- Lock RuboCop to v1.18.x
([#&#8203;348](https://togithub.com/jekyll/jekyll-feed/issues/348))
- Add workflow to release gem via GH Action
([#&#8203;355](https://togithub.com/jekyll/jekyll-feed/issues/355))

##### Documentation

- Use `.atom` extension in documented examples since we write an Atom
feed ([#&#8203;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-->
mx-psi pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request May 23, 2024
…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 [@&#8203;Place1](https://togithub.com/Place1) in
[grpc-ecosystem/grpc-gateway#3966
- Marshal non proto fields with indents by
[@&#8203;gknw](https://togithub.com/gknw) in
[grpc-ecosystem/grpc-gateway#4027
- chore: fix some comments by
[@&#8203;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 [@&#8203;richzw](https://togithub.com/richzw) in
[grpc-ecosystem/grpc-gateway#4108
- feat(bazel): surface --disable_default_responses in rule def by
[@&#8203;rajukrishnamurthy](https://togithub.com/rajukrishnamurthy) in
[grpc-ecosystem/grpc-gateway#4126
- integration: fix wrapping of response writer causing endless test loop
by [@&#8203;johanbrandhorst](https://togithub.com/johanbrandhorst) in
[grpc-ecosystem/grpc-gateway#4131
- Omit enum field when value is empty by
[@&#8203;jeromefroe](https://togithub.com/jeromefroe) in
[grpc-ecosystem/grpc-gateway#4180
- docs/mapping/examples.md: update CoreOS example GitHub branch to
`master` branch by
[@&#8203;mohamedawnallah](https://togithub.com/mohamedawnallah) in
[grpc-ecosystem/grpc-gateway#4198
- chore: remove repetitive word by
[@&#8203;mountcount](https://togithub.com/mountcount) in
[grpc-ecosystem/grpc-gateway#4183
- Fix Typo in examples.md by
[@&#8203;umakantv](https://togithub.com/umakantv) in
[grpc-ecosystem/grpc-gateway#4212
- Write Content-Length header if doForwardTrailers is not set by
[@&#8203;joshgarnett](https://togithub.com/joshgarnett) in
[grpc-ecosystem/grpc-gateway#4259
- Fixing TestOutgoingTrailerMatcher, which was non-deterministic by
[@&#8203;joshgarnett](https://togithub.com/joshgarnett) in
[grpc-ecosystem/grpc-gateway#4265
- Update README.md by [@&#8203;MakDon](https://togithub.com/MakDon) in
[grpc-ecosystem/grpc-gateway#4322
- fix(4245): setting appropriate log level for error logs by
[@&#8203;rajeshkarnena](https://togithub.com/rajeshkarnena) in
[grpc-ecosystem/grpc-gateway#4327
- fix: handle `X-Forwarded-*` headers correctly by
[@&#8203;haines](https://togithub.com/haines) in
[grpc-ecosystem/grpc-gateway#4334

#### New Contributors

- [@&#8203;Place1](https://togithub.com/Place1) made their first
contribution in
[grpc-ecosystem/grpc-gateway#3966
- [@&#8203;avoidalone](https://togithub.com/avoidalone) made their first
contribution in
[grpc-ecosystem/grpc-gateway#4092
- [@&#8203;richzw](https://togithub.com/richzw) made their first
contribution in
[grpc-ecosystem/grpc-gateway#4108
- [@&#8203;rajukrishnamurthy](https://togithub.com/rajukrishnamurthy)
made their first contribution in
[grpc-ecosystem/grpc-gateway#4126
- [@&#8203;jeromefroe](https://togithub.com/jeromefroe) made their first
contribution in
[grpc-ecosystem/grpc-gateway#4180
- [@&#8203;mohamedawnallah](https://togithub.com/mohamedawnallah) made
their first contribution in
[grpc-ecosystem/grpc-gateway#4198
- [@&#8203;mountcount](https://togithub.com/mountcount) made their first
contribution in
[grpc-ecosystem/grpc-gateway#4183
- [@&#8203;umakantv](https://togithub.com/umakantv) made their first
contribution in
[grpc-ecosystem/grpc-gateway#4212
- [@&#8203;joshgarnett](https://togithub.com/joshgarnett) made their
first contribution in
[grpc-ecosystem/grpc-gateway#4259
- [@&#8203;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>
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.

None yet

2 participants