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

Fix body size telemetry when ReadFrom is used #26

Merged
merged 3 commits into from
Mar 26, 2020
Merged

Conversation

ajlake
Copy link
Contributor

@ajlake ajlake commented Mar 26, 2020

This includes a small API break due to AccessHandler
hooks using int64 instead of int for size.

This includes a small API break due to AccessHandler
hooks using int64 instead of int for size.
Copy link
Member

@bluekeyes bluekeyes left a comment

Choose a reason for hiding this comment

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

I agree that the implementation here is close enough to the version from zenazn/goji that it's probably worth including the original license in a header comment in that file, probably with a not that it has been modified, in addition to the header for this project. At minimum, we should include a link to original source.

baseapp/metrics.go Outdated Show resolved Hide resolved
baseapp/middleware.go Outdated Show resolved Hide resolved
baseapp/proxy_writer.go Outdated Show resolved Hide resolved
baseapp/proxy_writer.go Outdated Show resolved Hide resolved
Copy link
Member

@bluekeyes bluekeyes left a comment

Choose a reason for hiding this comment

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

Two more stray hlog references in comments, but looks good otherwise.

baseapp/middleware.go Outdated Show resolved Hide resolved
baseapp/middleware.go Outdated Show resolved Hide resolved
@ajlake ajlake merged commit ae2587f into develop Mar 26, 2020
@ajlake ajlake deleted the ajlake/size branch March 26, 2020 22:59
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.

2 participants