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

contrib/internal/httputil: return correct wrapped response writer #1078

Merged
merged 10 commits into from
Dec 16, 2021

Conversation

renanrt
Copy link
Contributor

@renanrt renanrt commented Dec 3, 2021

We should return the wrapped response writer and NOT the original one.

In response to this issue.

@knusbaum knusbaum changed the title Return correct wrapped response writer writer contrib/internal/httputil: return correct wrapped response writer Dec 4, 2021
@knusbaum knusbaum added this to the 1.35.0 milestone Dec 4, 2021
Copy link
Contributor

@knusbaum knusbaum left a comment

Choose a reason for hiding this comment

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

One nit about import order for the linter.

contrib/internal/httputil/trace_gen.go Outdated Show resolved Hide resolved
@renanrt renanrt requested a review from knusbaum December 5, 2021 20:24
Copy link
Contributor

@Julio-Guerra Julio-Guerra 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 contributing :-)

I just added the test that was missing and would have caught the issue you fix here.

The patch you propose is actually incorrect: trace_gen.go is a generated file and you should instead fix make_responsewriter.go and then run go run ./make_responsewriter.go > trace_gen.go to fix trace_gen.go.

Thank you :-)

gbbr
gbbr previously requested changes Dec 6, 2021
contrib/internal/httputil/trace_gen.go Show resolved Hide resolved
@renanrt
Copy link
Contributor Author

renanrt commented Dec 14, 2021

Updated the file that generates the code, tested and it seems to be working fine!

@felixge felixge modified the milestones: 1.35.0, 1.36.0 Dec 14, 2021
@Julio-Guerra Julio-Guerra modified the milestones: 1.36.0, 1.35.0 Dec 14, 2021
@Julio-Guerra Julio-Guerra dismissed stale reviews from gbbr and knusbaum December 16, 2021 10:54

Done

@Julio-Guerra Julio-Guerra merged commit 846a1f2 into DataDog:v1 Dec 16, 2021
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

5 participants