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

Add support for decompression of HTTP responses #764

Merged
merged 1 commit into from
Apr 19, 2021

Conversation

mem
Copy link
Contributor

@mem mem commented Mar 16, 2021

If the module configuration specifies the "compression" option
blackbox_exporter will try to decompress the response using the
specified algorithm. If the response is not compressed using that
algorithm, the probe will fail.

It validates that the "Accept-Encoding" header is either absent, or that
it specifies the same algorithm as the "compression" option. If the
"Accept-Encoding" header is present but it specifies a different
algorithm, the probe will fail.

If the compression option is not used, probe_http_content_length and
probe_http_uncompressed_body_length will have the same value
corresponding to the original content length. If the compression option
is used and the content can be decompressed, probe_http_content_length
will report the original content length as it currently does, and
probe_http_uncompressed_body_length will report the length of the body
after decompression as expected.

Fixes #684

Signed-off-by: Marcelo E. Magallon [email protected]

@mem mem force-pushed the handle_compressed_payload branch 2 times, most recently from 605beb8 to 0dc0058 Compare March 16, 2021 20:27
@mem mem requested a review from roidelapluie March 16, 2021 20:34
prober/http.go Outdated Show resolved Hide resolved
prober/http.go Outdated Show resolved Hide resolved
prober/http.go Outdated Show resolved Hide resolved
@roidelapluie
Copy link
Member

Thanks for this! I have left a few comments.

@mem mem force-pushed the handle_compressed_payload branch from 6e1d94e to c48a0ad Compare March 31, 2021 19:08
prober/http.go Outdated Show resolved Hide resolved
@roidelapluie
Copy link
Member

Thanks, I have added one last comment.

@mem mem requested a review from roidelapluie April 16, 2021 15:09
@mem
Copy link
Contributor Author

mem commented Apr 16, 2021

Thanks, I have added one last comment.

Thank you! I think I have addressed everything. Can you take another look?

prober/http.go Outdated Show resolved Hide resolved
Copy link
Member

@roidelapluie roidelapluie left a comment

Choose a reason for hiding this comment

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

LGTM. I do not even think we should check the params like you did because there may be non-standard servers implementations that would use other formats, but for which users might still want to use with this exporter, but let's keep them and gather users feedback.

I have added a minor comment.

If the module configuration specifies the "compression" option
blackbox_exporter will try to decompress the response using the
specified algorithm. If the response is not compressed using that
algorithm, the probe will fail.

It validates that the "Accept-Encoding" header is either absent, or that
it specifies the same algorithm as the "compression" option. If the
"Accept-Encoding" header is present but it specifies a different
algorithm, the probe will fail.

If the compression option is *not* used, probe_http_content_length and
probe_http_uncompressed_body_length will have the same value
corresponding to the original content length. If the compression option
is used and the content can be decompressed, probe_http_content_length
will report the original content length as it currently does, and
probe_http_uncompressed_body_length will report the length of the body
after decompression as expected.

Fixes #684

Signed-off-by: Marcelo E. Magallon <[email protected]>
@mem mem force-pushed the handle_compressed_payload branch from e9569ef to 2d378fe Compare April 19, 2021 22:09
@mem
Copy link
Contributor Author

mem commented Apr 19, 2021

LGTM. I do not even think we should check the params like you did because there may be non-standard servers implementations that would use other formats, but for which users might still want to use with this exporter, but let's keep them and gather users feedback.

I have added a minor comment.

Cool, thank you!

@mem mem merged commit cdab0e7 into master Apr 19, 2021
@mem mem deleted the handle_compressed_payload branch April 19, 2021 22:23
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.

Handling of compressed HTTP responses
2 participants