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

add max_batch_size to recombine instructions #712

Closed
wants to merge 2 commits into from

Conversation

atoulme
Copy link
Contributor

@atoulme atoulme commented Mar 30, 2023

Sets values for max_batch_size (default is 1000) for crio to 8, containerd and docker to 16 respectively.

@atoulme atoulme requested review from a team as code owners March 30, 2023 00:12
@matthewmodestino
Copy link

Suggest perhaps we go with max_log_size first to see if we can just set the size to 1MB vs the different settings for docker and cri-o/containerd.

https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/stanza/docs/operators/recombine.md

If we do go with max_batch_size docker splits at 16384 and cri-o/containerd is 8192, so I would flip your numbers. 8 for docker, 16 for cri-o/containerd.

Lets be sure to capture the behaviour of 20MB+ logs, i assume we would flush then recombine the next batch from the big log together. Will help us identify them in Splunk.

@atoulme
Copy link
Contributor Author

atoulme commented Mar 30, 2023

OK, I'll rework this.

@atoulme atoulme force-pushed the add_max_batch_size_recombine branch from e500fad to e439b79 Compare March 30, 2023 00:40
@atoulme
Copy link
Contributor Author

atoulme commented Mar 30, 2023

Let's go first with #713 as indicated by @matthewmodestino

@atoulme
Copy link
Contributor Author

atoulme commented Mar 30, 2023

Closing in favor of #713.

@atoulme atoulme closed this Mar 30, 2023
@atoulme atoulme deleted the add_max_batch_size_recombine branch October 18, 2023 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants