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

metrics.Request receives an already drained *http.Response #258

Closed
lggomez opened this issue Oct 14, 2021 · 2 comments
Closed

metrics.Request receives an already drained *http.Response #258

lggomez opened this issue Oct 14, 2021 · 2 comments
Assignees
Labels
stale triage me I really want to be triaged. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@lggomez
Copy link

lggomez commented Oct 14, 2021

Is your feature request related to a problem? Please describe.
This is somewhat related to #245 in regards of the usefulness of the current metrics.Request interface

As of now, during the client APIcall execution the http response body is decoded before calling the EndRequest method, thus sending a drained response to it:

err = json.NewDecoder(httpResp.Body).Decode(resp)
requestMetrics.EndRequest(ctx, err, httpResp, httpResp.Header.Get("x-goog-maps-metro-area"))

I get that this favors performance but also reduces the efficacy of these metrics interfaces in the first place since, for instance, any manual logging or introspection of the raw JSON payload is not possible

Describe the solution you'd like
I'd like to be able to get the raw JSON response from the executed request

Describe alternatives you've considered
I think a simple, non-obtrusive way would be to extend the Request interface with a hook method like the following, to be called before performing the JSON decoding:

 OnBeforeDecode(httpResp *http.Response)

I get that this is a breaking change on the interface level but an empty implementation should have a negligible performance impact and for debugging or logging scenarios the following pattern can be applied to intercept request payloads:

data, _ := ioutil.ReadAll(httpResp.Body)
log.Println(data) // do w/e is needed with the body data
httpResp.Body = ioutil.NopCloser(bytes.NewBuffer(data)) // restore the drained body
@lggomez lggomez added triage me I really want to be triaged. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Oct 14, 2021
@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. Please comment here if it is still valid so that we can reprioritize. Thank you!

@stale stale bot added the stale label Apr 16, 2022
@stale
Copy link

stale bot commented Nov 2, 2022

Closing this. Please reopen if you believe it should be addressed. Thank you for your contribution.

@stale stale bot closed this as completed Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale triage me I really want to be triaged. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants