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

max_content_length_* is not consistent on the wire compressed vs. uncompressed #35299

Open
kebroad opened this issue Sep 19, 2024 · 6 comments
Open
Labels
exporter/splunkhec needs triage New item requiring triage

Comments

@kebroad
Copy link

kebroad commented Sep 19, 2024

Component(s)

exporter/splunkhec

Describe the issue you're reporting

Upon using this exporter we have noticed that when one of the max_content_length_ configurations is set (ie. max_content_length_logs), the size of the payload on the wire may not be consistent depending on the disable_compression setting.

When true, the write function will return an error if its over capacity, whereas the compression write function seems like it will write it if its compressed size is under the max_content_length.

So, if I understand it correctly, when max_content_length_logs is 1MB, the uncompressed writer will ensure its raw size is under 1MB, but the gzip writer will ensure its compressed size is under 1MB.

Is this intentional? We are exporting to an endpoint that has size limits on the uncompressed data but, while we want to compress it, with variable compression rates its hard to determine exactly what content length limit we would want to configure.

@kebroad kebroad added the needs triage New item requiring triage label Sep 19, 2024
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@bderrly
Copy link

bderrly commented Sep 19, 2024

The part that was most surprising here was that the documentation led us to believe that the compression was going to be just before being sent over the wire. Thus our expectation was that that data buffer would be capped at the max_content_length_logs, in our case 5 MB, and then compressed and sent over the wire. As mentioned above, our log destination (Logscale) is expecting a maximum uncompressed payload of 5 MB.

From README.md:

disable_compression (default: false): Whether to disable gzip compression over HTTP.

@atoulme
Copy link
Contributor

atoulme commented Sep 19, 2024

The Content-Length header value is the size of the payload on the wire, not the uncompressed content.

If you want to configure the size of the uncompressed payload, we will need more work to support that.

@kebroad
Copy link
Author

kebroad commented Sep 19, 2024

If you want to configure the size of the uncompressed payload, we will need more work to support that.

Yes, this is essentially what we want. I wouldn't mind working on a PR for this. Would you want to add another configuration field for this use case or change the existing behavior of the max_content_length_ fields in the buffer writers, or something else?

@atoulme
Copy link
Contributor

atoulme commented Sep 19, 2024

I think that'd be something such as "payload_size". Note we also have a "max_event_size" configuration key. Those settings might play on each other. Do you want to have different sizes per signal, such as a key for log, metrics and traces? Or just one?

@kebroad
Copy link
Author

kebroad commented Sep 19, 2024

We are currently only using this exporter for logs, so one setting for all three would be fine.

Im thinking we could perhaps have a field like max_content_length_type, which could be compressed or raw.
compressed would be the equivalent of what we currently have, ie. if max_content_length_logs: 5000000, that would mean that the size of the compressed payload would have to be under 5MB. If it was raw at max_content_length_logs: 5000000, it would mean that the raw size of the payload must be under 5MB, which is then compressed to some lower size, then sent over the wire.

max_content_length_type would have no effect if disable_compression: true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/splunkhec needs triage New item requiring triage
Projects
None yet
Development

No branches or pull requests

3 participants