enhancement(kubernetes_logs source): Allow specification of a maximum line size to be applied after merging instead of just before #22582

ganelo · 2025-03-03T21:51:15Z

Summary

Add a new config field max_merged_line_bytes which allows limiting maximum line size even when auto_partial_merge is enabled (see #22581).

Change Type

Bug fix
New feature
Non-functional (chore, refactoring, docs)
Performance

Is this a breaking change?

Yes
No

How did you test this PR?

Added unit tests to exercise the new configuration.

Does this PR include user facing changes?

Yes. Please add a changelog fragment based on our guidelines.
No. A maintainer will apply the "no-changelog" label to this PR.

Checklist

Please read our Vector contributor resources.
- make check-all is a good command to run locally. This check is
  defined here. Some of these
  checks might not be relevant to your PR. For Rust changes, at the very least you should run:
  - cargo fmt --all
  - cargo clippy --workspace --all-targets -- -D warnings
  - cargo nextest run --workspace (alternatively, you can run cargo test --all)
If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
run dd-rust-license-tool write to regenerate the license inventory and commit the changes (if any). More details here.

References

Closes: kubernetes_logs source permits arbitrarily large lines due to interaction of auto_partial_merge and max_line_bytes #22581

…ments around

ganelo · 2025-03-05T15:49:31Z

Tagging @pront / @jszwedko explicitly since it doesn't look like anyone was auto-added as a reviewer.

…ize and max_merged_line_size

changelog.d/22581_max_merged_line_bytes.feature.md

Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com>

… to be partial; fix test

pront

Thanks @ganelo, this mostly looks reasonable.

src/sources/kubernetes_logs/mod.rs

src/internal_events/kubernetes_logs.rs

src/sources/kubernetes_logs/partial_events_merger.rs

src/sources/kubernetes_logs/mod.rs

var/log/pods/default/checkpoints.json

pront · 2025-03-31T19:15:41Z

lib/file-source/src/buffer.rs

        total_read += used;

        if !discarding && buf.len() > max_size {
-            warn!(


I am not very familiar with this code. I wonder why this warn! was moved only to be emitted later.

This was part of plumbing through the actual contents that were being dropped so they could be included in the error.

src/sources/kubernetes_logs/mod.rs

src/sources/kubernetes_logs/partial_events_merger.rs

pront · 2025-03-31T19:28:06Z

src/sources/kubernetes_logs/partial_events_merger.rs

+        let mut bytes_mut = BytesMut::new();
        if let Some(bucket) = self.buckets.get_mut(file) {
+            // don't bother continuing to process new partial events that match existing ones that are already too big
+            if bucket.too_big {


Hmm I wonder if we can maintain a separate data structure for these buckets so that we don't mix with them with the emitted ones.

Not totally sure I follow - can you elaborate what the concern is that you're wanting to mitigate?

Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com>

… state struct

pront · 2025-04-08T15:06:02Z

This is starting to look great, will do a final review shortly.

…bytes

ganelo · 2025-05-27T18:04:25Z

Hey @pront - just checking whether there's anything further you need from me on this?

pront · 2025-06-10T20:04:26Z

Hey @pront - just checking whether there's anything further you need from me on this?

Apologies for the long delay, I was away for 2+ weeks. My plan is to review this / give prompt feedback and include it in the next release.

Copilot

Pull Request Overview

This PR adds a new configuration option, max_merged_line_bytes, to the kubernetes_logs source so that users can limit the size of merged log lines even when auto_partial_merge is enabled. Key changes include updates to the CUE reference documentation, modifications to the partial events merger logic in Rust to enforce the new limit (along with corresponding tests), and adjustments to internal events and file source modules for consistent error reporting.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
website/cue/reference/components/sources/base/kubernetes_logs.cue	Added documentation for the max_merged_line_bytes field.
src/sources/kubernetes_logs/partial_events_merger.rs	Integrated checking for the new max_merged_line_bytes and adjusted event merging and filtering behavior.
src/sources/kubernetes_logs/mod.rs	Updated configuration parsing and adjusted max_line_bytes value based on the new max_merged_line_bytes setting.
src/internal_events/kubernetes_logs.rs	Added a new internal event for merged lines that exceed the configured limit.
src/internal_events/file.rs, lib/file-source/*	Made supportive changes in error emitting and tests to properly handle the updated behavior for oversized lines.
changelog.d/22581_max_merged_line_bytes.feature.md	Added a changelog entry describing the new feature.

pront

Thanks and apologies for the delays. Left a couple of nits.

lib/file-source/src/buffer.rs

src/sources/kubernetes_logs/mod.rs

… the whole buffer and then truncating, use more idiomatic Rust for handling both configured and unconfigured cases of max_merged_line_bytes

ganelo · 2025-06-11T19:29:57Z

Thanks and apologies for the delays. Left a couple of nits.

@pront - all good! Thanks for the review, pushed up requested changes.

ganelo · 2025-06-11T19:42:33Z

@pront - sorry, mind enabling automerge again? Had to push a fix for failing spelling check coming from master

pront · 2025-06-11T19:55:43Z

@pront - sorry, mind enabling automerge again? Had to push a fix for failing spelling check coming from master

Sure. Spell checker failures do not block PRs. But thanks for fixing it anyway.

ganelo · 2025-06-11T19:58:39Z

@pront - sorry, mind enabling automerge again? Had to push a fix for failing spelling check coming from master

Sure. Spell checker failures do not block PRs. But thanks for fixing it anyway.

Oops, good to know, will keep in mind for next time.

…lice

ganelo · 2025-06-12T15:35:58Z

@pront - I ended up having to push a fix for one of the nit-prompted changes

… line size to be applied after merging instead of just before (vectordotdev#22582) * Add config for maximum allowed line size after merging * Add warns when we drop partial logs for being too big; shift some comments around * Add changelog * Format * Increment component_discarded_events_total on violation of max_line_size and max_merged_line_size * Update changelog.d/22581_max_merged_line_bytes.feature.md Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com> * Don't emit expired events that are too big nor ones that don't appear to be partial; fix test * Fix another test * Update src/sources/kubernetes_logs/mod.rs Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com> * Update src/sources/kubernetes_logs/mod.rs Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com> * Remove inadvertently added file * Include Value rather than Event in error struct * Rename field in bucket struct * Move max_merged_line_bytes from being a param to being a field on the state struct * Make new config field optional, defaulting to old behavior * Format * Appease check-events * docs regen * Tweak wording of doc; emit only first 1k bytes of dropped lines in error * Rename fields for clarity * Per PR feedback: copy just the initial 1000 bytes rather than cloning the whole buffer and then truncating, use more idiomatic Rust for handling both configured and unconfigured cases of max_merged_line_bytes * Allow spelling of already-merged changelog filename * Don't try to include more characters than there actually are in the slice * Don't just get enough capacity, make sure length matches too * Formatting --------- Co-authored-by: Orri Ganel <oganel@palantir.com> Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com>

pront · 2025-06-13T13:10:24Z

Happy to see this merged!

oganel added 4 commits February 27, 2025 15:26

Add config for maximum allowed line size after merging

1013e9f

Add warns when we drop partial logs for being too big; shift some com…

2b3ea0a

…ments around

Add changelog

cd9d607

Format

dd5fad0

ganelo requested a review from a team as a code owner March 3, 2025 21:51

github-actions bot added the domain: sources Anything related to the Vector's sources label Mar 3, 2025

ganelo mentioned this pull request Mar 3, 2025

kubernetes_logs source permits arbitrarily large lines due to interaction of auto_partial_merge and max_line_bytes #22581

Closed

pront self-assigned this Mar 7, 2025

Increment component_discarded_events_total on violation of max_line_s…

354008c

…ize and max_merged_line_size

pront reviewed Mar 13, 2025

View reviewed changes

changelog.d/22581_max_merged_line_bytes.feature.md Outdated Show resolved Hide resolved

Update changelog.d/22581_max_merged_line_bytes.feature.md

0d1b806

Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com>

ganelo requested a review from pront March 14, 2025 14:18

pront requested a review from Copilot March 27, 2025 20:00

This comment was marked as outdated.

Sign in to view

oganel added 2 commits March 27, 2025 17:24

Don't emit expired events that are too big nor ones that don't appear…

8e0c994

… to be partial; fix test

Fix another test

cae7b01

pront reviewed Mar 31, 2025

View reviewed changes

ganelo and others added 6 commits March 31, 2025 15:33

Update src/sources/kubernetes_logs/mod.rs

bc23d35

Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com>

Update src/sources/kubernetes_logs/mod.rs

62bce6c

Co-authored-by: Pavlos Rontidis <pavlos.rontidis@gmail.com>

Remove inadvertently added file

4e93bc9

Include Value rather than Event in error struct

f9d7ca8

Rename field in bucket struct

6e58258

Move max_merged_line_bytes from being a param to being a field on the…

e551a4c

… state struct

jszwedko mentioned this pull request Apr 3, 2025

auto_partial_merge doesn't work in kubernetes source #19767

Open

oganel added 2 commits April 3, 2025 14:31

Make new config field optional, defaulting to old behavior

7bac39a

Format

4975ebe

pront self-requested a review April 8, 2025 15:06

Merge remote-tracking branch 'origin/master' into og/max_merged_line_…

af11a17

…bytes

iadjivon approved these changes May 27, 2025

View reviewed changes

pront requested a review from Copilot June 10, 2025 20:12

Copilot AI reviewed Jun 10, 2025

View reviewed changes

pront approved these changes Jun 11, 2025

View reviewed changes

lib/file-source/src/buffer.rs Outdated Show resolved Hide resolved

src/sources/kubernetes_logs/mod.rs Show resolved Hide resolved

Per PR feedback: copy just the initial 1000 bytes rather than cloning…

08b7292

… the whole buffer and then truncating, use more idiomatic Rust for handling both configured and unconfigured cases of max_merged_line_bytes

pront enabled auto-merge June 11, 2025 19:31

oganel added 2 commits June 11, 2025 15:41

Merge branch 'master' into og/max_merged_line_bytes

bfe4a25

Allow spelling of already-merged changelog filename

335f6e2

auto-merge was automatically disabled June 11, 2025 19:41
Head branch was pushed to by a user without write access

pront enabled auto-merge June 11, 2025 19:55

Don't try to include more characters than there actually are in the s…

e4efae0

…lice

auto-merge was automatically disabled June 11, 2025 20:22
Head branch was pushed to by a user without write access

Don't just get enough capacity, make sure length matches too

36b4c77

pront enabled auto-merge June 12, 2025 17:57

Formatting

ffcfcc3

auto-merge was automatically disabled June 12, 2025 20:13
Head branch was pushed to by a user without write access

pront enabled auto-merge June 12, 2025 20:15

pront added this pull request to the merge queue Jun 12, 2025

Merged via the queue into vectordotdev:master with commit f31839b Jun 12, 2025
43 checks passed

vparfonov mentioned this pull request Jul 22, 2025

Allow specification of a maximum line size to be applied after merging ViaQ/vector#216

Merged

enhancement(kubernetes_logs source): Allow specification of a maximum line size to be applied after merging instead of just before #22582

enhancement(kubernetes_logs source): Allow specification of a maximum line size to be applied after merging instead of just before #22582

Uh oh!

Conversation

ganelo commented Mar 3, 2025

Summary

Change Type

Is this a breaking change?

How did you test this PR?

Does this PR include user facing changes?

Checklist

References

Uh oh!

ganelo commented Mar 5, 2025

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

pront left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pront Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

ganelo Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pront Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

ganelo Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

pront commented Apr 8, 2025

Uh oh!

ganelo commented May 27, 2025

Uh oh!

pront commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

pront left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ganelo commented Jun 11, 2025

Uh oh!

ganelo commented Jun 11, 2025

Uh oh!

pront commented Jun 11, 2025

Uh oh!

ganelo commented Jun 11, 2025

Uh oh!

ganelo commented Jun 12, 2025

Uh oh!

Uh oh!

pront commented Jun 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pront commented Jun 10, 2025 •

edited

Loading