From 0458e613a5957f10d1699de144933e46cccf9815 Mon Sep 17 00:00:00 2001 From: Dan Jaglowski Date: Thu, 25 Jan 2024 14:58:21 -0600 Subject: [PATCH 1/2] [pkg/stanza] Fix recombine operator's 'overwrite_with' --- ...pkg-stanza-recombine-overwrite-with-2.yaml | 32 +++++++++++++++++++ .../pkg-stanza-recombine-overwrite-with.yaml | 27 ++++++++++++++++ .../transformer/recombine/recombine.go | 6 ++-- .../transformer/recombine/recombine_test.go | 23 +++++++++++-- 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100755 .chloggen/pkg-stanza-recombine-overwrite-with-2.yaml create mode 100755 .chloggen/pkg-stanza-recombine-overwrite-with.yaml diff --git a/.chloggen/pkg-stanza-recombine-overwrite-with-2.yaml b/.chloggen/pkg-stanza-recombine-overwrite-with-2.yaml new file mode 100755 index 000000000000..93d4488dc782 --- /dev/null +++ b/.chloggen/pkg-stanza-recombine-overwrite-with-2.yaml @@ -0,0 +1,32 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: pkg/stanza + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Invert recombine operator's 'overwrite_with' default value. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [30783] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + Previously, the default value was `oldest`, meaning that the recombine operator _should_ emit the + first entry from each batch (with the recombined field). However, the actual behavior was inverted. + This fixes the bug but also inverts the default setting so as to effectively cancel out the bug fix + for users who were not using this setting. For users who were explicitly setting `overwrite_with`, + the corrects the intended behavior. + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/.chloggen/pkg-stanza-recombine-overwrite-with.yaml b/.chloggen/pkg-stanza-recombine-overwrite-with.yaml new file mode 100755 index 000000000000..13c56cc519e4 --- /dev/null +++ b/.chloggen/pkg-stanza-recombine-overwrite-with.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: pkg/stanza + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fix bug where recombine operator's 'overwrite_with' condition was inverted. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [30783] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/pkg/stanza/operator/transformer/recombine/recombine.go b/pkg/stanza/operator/transformer/recombine/recombine.go index 0a67d7fe4a82..ffdf86897a9a 100644 --- a/pkg/stanza/operator/transformer/recombine/recombine.go +++ b/pkg/stanza/operator/transformer/recombine/recombine.go @@ -40,7 +40,7 @@ func NewConfigWithID(operatorID string) *Config { MaxBatchSize: 1000, MaxSources: 1000, CombineWith: defaultCombineWith, - OverwriteWith: "oldest", + OverwriteWith: "newest", ForceFlushTimeout: 5 * time.Second, SourceIdentifier: entry.NewAttributeField("file.path"), } @@ -344,9 +344,9 @@ func (r *Transformer) flushSource(source string, deleteSource bool) error { entries := batch.entries if r.overwriteWithOldest { - base = entries[0] - } else { base = entries[len(entries)-1] + } else { + base = entries[0] } // Set the recombined field on the entry diff --git a/pkg/stanza/operator/transformer/recombine/recombine_test.go b/pkg/stanza/operator/transformer/recombine/recombine_test.go index 9887d28cc537..90bb29a47abe 100644 --- a/pkg/stanza/operator/transformer/recombine/recombine_test.go +++ b/pkg/stanza/operator/transformer/recombine/recombine_test.go @@ -127,6 +127,25 @@ func TestTransformer(t *testing.T) { entryWithBody(t2, "test2"), entryWithBody(t2, "test1"), }, + []*entry.Entry{ + entryWithBody(t1, "test1\ntest2"), + }, + }, + { + "ThreeEntriesFirstOldest", + func() *Config { + cfg := NewConfig() + cfg.CombineField = entry.NewBodyField() + cfg.IsFirstEntry = "body == 'test1'" + cfg.OutputIDs = []string{"fake"} + cfg.OverwriteWith = "oldest" + return cfg + }(), + []*entry.Entry{ + entryWithBody(t1, "test1"), + entryWithBody(t2, "test2"), + entryWithBody(t2, "test1"), + }, []*entry.Entry{ entryWithBody(t2, "test1\ntest2"), }, @@ -159,7 +178,7 @@ func TestTransformer(t *testing.T) { cfg.CombineField = entry.NewBodyField() cfg.IsFirstEntry = "body == 'file1'" cfg.OutputIDs = []string{"fake"} - cfg.OverwriteWith = "newest" + cfg.OverwriteWith = "oldest" return cfg }(), []*entry.Entry{ @@ -267,7 +286,7 @@ func TestTransformer(t *testing.T) { cfg.CombineField = entry.NewBodyField("message") cfg.CombineWith = "" cfg.IsLastEntry = "body.logtag == 'F'" - cfg.OverwriteWith = "newest" + cfg.OverwriteWith = "oldest" cfg.ForceFlushTimeout = 100 * time.Millisecond cfg.OutputIDs = []string{"fake"} return cfg From 789566ba8912981ae79ef93d93a12649d722a65b Mon Sep 17 00:00:00 2001 From: Daniel Jaglowski Date: Thu, 25 Jan 2024 15:24:13 -0600 Subject: [PATCH 2/2] Update .chloggen/pkg-stanza-recombine-overwrite-with-2.yaml Co-authored-by: Yang Song --- .chloggen/pkg-stanza-recombine-overwrite-with-2.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/pkg-stanza-recombine-overwrite-with-2.yaml b/.chloggen/pkg-stanza-recombine-overwrite-with-2.yaml index 93d4488dc782..ac63aa545c75 100755 --- a/.chloggen/pkg-stanza-recombine-overwrite-with-2.yaml +++ b/.chloggen/pkg-stanza-recombine-overwrite-with-2.yaml @@ -20,7 +20,7 @@ subtext: | first entry from each batch (with the recombined field). However, the actual behavior was inverted. This fixes the bug but also inverts the default setting so as to effectively cancel out the bug fix for users who were not using this setting. For users who were explicitly setting `overwrite_with`, - the corrects the intended behavior. + this corrects the intended behavior. # If your change doesn't affect end users or the exported elements of any package, # you should instead start your pull request title with [chore] or use the "Skip Changelog" label.