[exporter/splunkhec] Allow to drop log messages longer than a given max length

[atoulme]

Component(s)

exporter/splunkhec

Is your feature request related to a problem? Please describe.

Log records can have an indefinite size, and we want to make sure we don't overwhelm backends when we send logs.

Describe the solution you'd like

Allow the user to configure a max length for a log record of up to a fixed amount.
If a log record is longer than the max set, cut it into parts that fit in the limit, and add a field that identifies that this part of a series.

Describe alternatives you've considered

No response

Additional context

No response

[hvaghani221]

Hey @atoulme, I am not 100% sure of the requirements here. We can break events to fix the max limit, but we can not merge those events again into splunk AFAIK.

From the description, it looks like you are asking to add a unique id (and maybe the sequence number) to those broken events to group them together in the search.

For example,

{"fields":{}, "event":"event1"}
{"fields":{}, "event":"event2"}

It will be converted to:

{"fields":{"id":"id-1", "seq-no":1}, "event":"event1.1"}
{"fields":{"id":"id-1", "seq-no":2}, "event":"event1.2"}
{"fields":{"id":"id-1", "seq-no":3}, "event":"event1.3"}
{"fields":{"id":"id-2", "seq-no":1}, "event":"event2.1"}
{"fields":{"id":"id-2", "seq-no":2}, "event":"event2.2"}

Let me know if I am not interpreting it correctly.

[github-actions]

Pinging code owners for exporter/splunkhec: @atoulme @dmitryax. See Adding Labels via Comments if you do not have permissions to add labels yourself.

[atoulme]

No, we probably cannot merge them back, but I guess that is fine. Yes, the unique id and sequence number sounds good to me.

[shalper2]

I'll take a crack at this using @harshit-splunk 's comment as a jumping off point

[atoulme]

Talking about this with other folks, it appears splitting log messages should be done explicitly with a transformprocessor rather than try to do this in the exporter. We should abandon this approach.

[VihasMakwana]

signalfx/splunk-otel-collector-chart#696 (comment)
this says to apply a 5Mb limit on the payload sent, we could add another config, max_payload_size or something.
If the final payload size is > max_payload_size, drop it.
This basically means implementing maxEventSize (from inputs.conf) in our exporter.
This way we wouldn't overwhelm the backends.

[VihasMakwana]

@atoulme does this look good?
Can I raise a PR soon?

[atoulme]

lgtm, go for it.

[VihasMakwana]

@atoulme I think we can close this one as PR is merged

[dmitryax]

@atoulme @VihasMakwana, this issue sounds a bit different from the implementation done in #20752:

The issue and the name of the configuration MaxEventSize are supposed to be applied to individual log records. But #20752 is implemented as another limit for a batch of logs, the same as MaxContentLengthLogs but for uncompressed data.

Do we really need another batch limit with a confusing name? I'd suggest we make it applied to individual log records and keep MaxContentLengthLogs as the only batch limit. That will simplify the logic and make the MaxEventSize name clear. I can apply the change if there are no objections

[atoulme]

Ah, that is a bug then. MaxEventSize is only to catch events that are too long and has nothing to do with batching.

[atoulme]

Yes, this is not implemented properly, apologies. https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/20752/files#r1194214914 has the offending line.

[atoulme]

PR is out with the fix.