-
Notifications
You must be signed in to change notification settings - Fork 514
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 gzip middleware to frontend HTTP endpoints #1080
Conversation
98753aa
to
97566c3
Compare
b3736a3
to
7ea7ec5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, but defer approval to another maintainer that is more familiar with the query path.
Saving this for after we cut 1.2. Will review then. 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, it works as expected :)
Without compression:
➜ curl -sv -o /dev/null localhost:3200/api/traces/05c8ce18fbc11e8264df92bd7d07f5e6 -w '%{size_download}'
* Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 3200 (#0)
> GET /api/traces/05c8ce18fbc11e8264df92bd7d07f5e6 HTTP/1.1
> Host: localhost:3200
> User-Agent: curl/7.64.1
> Accept: */*
>
< HTTP/1.1 200 OK
< Vary: Accept-Encoding
< Date: Wed, 27 Oct 2021 16:05:11 GMT
< Content-Type: text/plain; charset=utf-8
< Transfer-Encoding: chunked
<
{ [43159 bytes data]
* Connection #0 to host localhost left intact
* Closing connection 0
43138
With compression:
➜ curl --compressed -sv -o /dev/null localhost:3200/api/traces/05c8ce18fbc11e8264df92bd7d07f5e6 -w '%{size_download}'
* Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 3200 (#0)
> GET /api/traces/05c8ce18fbc11e8264df92bd7d07f5e6 HTTP/1.1
> Host: localhost:3200
> User-Agent: curl/7.64.1
> Accept: */*
> Accept-Encoding: deflate, gzip
>
< HTTP/1.1 200 OK
< Content-Encoding: gzip
< Content-Type: text/plain; charset=utf-8
< Vary: Accept-Encoding
< Date: Wed, 27 Oct 2021 16:05:18 GMT
< Transfer-Encoding: chunked
<
{ [7220 bytes data]
* Connection #0 to host localhost left intact
* Closing connection 0
7187
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Already tested this earlier.
Could you also take a look at my earlier comment? I think we can avoid adding (*client).QueryTraceWithResponse
.
e39adf1
to
f3d5645
Compare
I've pulled in the changes from main. Let me know if anything else here needs to be addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀 We just have to fix the changelog entry.
f3d5645
to
69ff963
Compare
CI is really upset with your latest changes, any idea what you did wrong here? 🤣 |
Oh my our ci is not happy. |
69ff963
to
ec1a613
Compare
I've rebased on main again. |
Thanks 👍 |
What this PR does:
Which issue(s) this PR fixes:
Fixes #891
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]