Compress otlp payload with zlib #87
Conversation
Fixes yangxikun#71 Also updates the otel-collector configuration file: * The `logging` exporter was renamed to `debug` * Jaeger now supports receiving OTLP traffic which makes things simpler
|
@plantfansam Could you review this PR? |
wperron
force-pushed
the
otlp-zlib
branch
2 times, most recently
from
01e73b6 to
fa1d69a
Compare
wperron
force-pushed
the
otlp-zlib
branch
2 times, most recently
from
82ff258 to
1cfc5d1
Compare
| -- the compression should be set to Best Compression and window size | ||
| -- should be set to 15+16, see reference below: | ||
| -- https://github.com/brimworks/lua-zlib/issues/4#issuecomment-26383801 | ||
| local deflate_stream = zlib.deflate(zlib.BEST_COMPRESSION, 15+16) |
There was a problem hiding this comment.
I was thinking about this a bit more ... I think we should figure out how to do the compression in the otlp exporter's call_collector method. This is because requests to the collector can retry multiple times, and we don't want to re-gzip the payload each time. That's what we do with protobuf encoding (see
There was a problem hiding this comment.
sounds good, shouldn't be hard 👍
There was a problem hiding this comment.
I moved the compression to the otlp exporter and adjusted the tests, I think it looks good, I am a bit worried that it pushes complexity into the worker init block but I'll you be the judge of that.
| } | ||
|
|
||
| return setmetatable(self, mt) | ||
| end | ||
|
|
||
| function _M.do_request(self, body) |
There was a problem hiding this comment.
Can we consider change do_request(self, body) to do_request(self, body, additional_headers).
Then call do_request(body, ["Content-Encoding": "gzip"]) if gzip is enabled.
| else | ||
| otel_global.metrics_reporter:record_value( | ||
| exporter_request_uncompressed_payload_size, string.len(pb_encoded_body)) | ||
| end |
There was a problem hiding this comment.
The gzip logic should move out while loop.
* another take on yangxikun#87
* another take on yangxikun#87
* another take on yangxikun#87
* another take on yangxikun#87
Adds gzip compression to the HTTP client.
Also updates the local otel-collector and docker-compose configurations, some of the configs have been renamed and OTLP support was added to Jaeger since the last update.
I've tested this locally, but I'm having trouble running the integration test suite, I'm hoping the GitHub Action will work OK.
Fixes #71