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

Distributor: Loki API can receive gzipped JSON #3291

Merged
merged 2 commits into from
Feb 5, 2021
Merged

Distributor: Loki API can receive gzipped JSON #3291

merged 2 commits into from
Feb 5, 2021

Conversation

ukolovda
Copy link
Contributor

@ukolovda ukolovda commented Feb 4, 2021

What this PR does / why we need it:

This PR allow send gzipped JSON to push endpoint.

HTTP server recognize Content-Encoding: gzip request header (for JSON only) and unpack request body.
This option can reduce network traffic (5-30 times) and can be useful for sending logs to remote server.

Which issue(s) this PR fixes:

Fixes #3205

Special notes for your reviewer:

Checklist

  • Documentation added
  • Tests updated

@codecov-io
Copy link

Codecov Report

Merging #3291 (9be841d) into master (d667dd2) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3291      +/-   ##
==========================================
- Coverage   62.64%   62.62%   -0.03%     
==========================================
  Files         201      201              
  Lines       17110    17123      +13     
==========================================
+ Hits        10719    10723       +4     
- Misses       5431     5442      +11     
+ Partials      960      958       -2     
Impacted Files Coverage Δ
pkg/distributor/http.go 0.00% <0.00%> (ø)
pkg/querier/queryrange/downstreamer.go 95.29% <0.00%> (-2.36%) ⬇️
pkg/promtail/targets/file/tailer.go 78.57% <0.00%> (+5.35%) ⬆️

@ukolovda ukolovda changed the title WIP: Distributor: Loki API can receive gzipped JSON Distributor: Loki API can receive gzipped JSON Feb 4, 2021
@ukolovda ukolovda marked this pull request as ready for review February 4, 2021 15:13
@@ -97,6 +117,7 @@ func ParseRequest(r *http.Request) (*logproto.PushRequest, error) {
"msg", "push request parsed",
"path", r.URL.Path,
"contentType", contentType,
"contentEncoding", contentEncoding,
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Copy link
Member

@owen-d owen-d left a comment

Choose a reason for hiding this comment

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

This looks great, thanks!

I notice we don't have an http_test.go file, so I won't ask that you include some additional testing for this unless you'd like to.

Follow ups for @cyriltovena & myself:

  • use gzip pools
  • test gzip endcoded proto & json bodies

@ukolovda
Copy link
Contributor Author

ukolovda commented Feb 5, 2021

This looks great, thanks!

I notice we don't have an http_test.go file, so I won't ask that you include some additional testing for this unless you'd like to.

Follow ups for @CyrilPeponnet & myself:

* use gzip pools

* test gzip endcoded proto & json bodies

Thank you!
I'm beginning in Go and didn't make tests for HTTP server yet...
I made manual tests, then deploy patched version to my server. It works fine.

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@cyriltovena cyriltovena merged commit 2a51fb4 into grafana:master Feb 5, 2021
cyriltovena pushed a commit to cyriltovena/loki that referenced this pull request Feb 15, 2021
* Push API recognize HTTP header "Content-Encoding: gzip" and upload gzipped JSON

* Fix documentation mistake
Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

We did broke something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please add compression ability in push API
4 participants