From 7a0d04003381ec3b475454530cb2607f871c5a3a Mon Sep 17 00:00:00 2001 From: Dominik Rosiek Date: Wed, 1 Jun 2022 17:11:37 +0200 Subject: [PATCH 1/3] fix(pkg/stanza/input/file/reader): skip building fingerprint in case of configuration change If customer changes fingerprint_size while offset is behind the value, it is going to panic as it is unable to rebuild fingerprint Signed-off-by: Dominik Rosiek --- pkg/stanza/operator/input/file/file_test.go | 21 +++++++++++++++++++++ pkg/stanza/operator/input/file/reader.go | 10 +++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/pkg/stanza/operator/input/file/file_test.go b/pkg/stanza/operator/input/file/file_test.go index cc1b6b18d736..76bef59b1d52 100644 --- a/pkg/stanza/operator/input/file/file_test.go +++ b/pkg/stanza/operator/input/file/file_test.go @@ -864,6 +864,7 @@ func TestFileReader_FingerprintUpdated(t *testing.T) { // - Updates as a file is read // - Stops updating when the max fingerprint size is reached // - Stops exactly at max fingerprint size, regardless of content +// - Do not change size after fingerprint configuration change func TestFingerprintGrowsAndStops(t *testing.T) { t.Parallel() @@ -919,6 +920,26 @@ func TestFingerprintGrowsAndStops(t *testing.T) { reader.ReadToEnd(context.Background()) require.Equal(t, fileContent[:expectedFP], reader.Fingerprint.FirstBytes) } + + // Test fingerprint change + // Change fingerprint and try to read file again + // We do not expect fingerprint change + // We test both increasing and decreasing fingerprint size + reader.fileInput.fingerprintSize = maxFP * (lineLen / 3) + line := stringWithLength(lineLen-1) + "\n" + fileContent = append(fileContent, []byte(line)...) + + writeString(t, temp, line) + reader.ReadToEnd(context.Background()) + require.Equal(t, fileContent[:expectedFP], reader.Fingerprint.FirstBytes) + + reader.fileInput.fingerprintSize = maxFP / 2 + line = stringWithLength(lineLen-1) + "\n" + fileContent = append(fileContent, []byte(line)...) + + writeString(t, temp, line) + reader.ReadToEnd(context.Background()) + require.Equal(t, fileContent[:expectedFP], reader.Fingerprint.FirstBytes) }) } } diff --git a/pkg/stanza/operator/input/file/reader.go b/pkg/stanza/operator/input/file/reader.go index 04008838f3cd..343a8105c20f 100644 --- a/pkg/stanza/operator/input/file/reader.go +++ b/pkg/stanza/operator/input/file/reader.go @@ -226,11 +226,19 @@ func getScannerError(scanner *PositionalScanner) error { // Read from the file and update the fingerprint if necessary func (r *Reader) Read(dst []byte) (int, error) { - if len(r.Fingerprint.FirstBytes) == r.fileInput.fingerprintSize { + // Skip if fingerprint is already built + // or if fingerprint is behind Offset + if len(r.Fingerprint.FirstBytes) == r.fileInput.fingerprintSize || int(r.Offset) > len(r.Fingerprint.FirstBytes) { return r.file.Read(dst) } n, err := r.file.Read(dst) appendCount := min0(n, r.fileInput.fingerprintSize-int(r.Offset)) + // return for n == 0 or r.Offset >= r.fileInput.fingerprintSize + if appendCount == 0 { + return n, err + } + + // for appendCount==0, the following code would add `0` to fingerprint r.Fingerprint.FirstBytes = append(r.Fingerprint.FirstBytes[:r.Offset], dst[:appendCount]...) return n, err } From df8411108c43aae25c0864bf206bc14a628b3d69 Mon Sep 17 00:00:00 2001 From: Dominik Rosiek Date: Thu, 2 Jun 2022 08:29:09 +0200 Subject: [PATCH 2/3] chore: changelog Signed-off-by: Dominik Rosiek --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 019c86b789e0..11a1e0550dca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ - `prometheusexporter`: Converting monotonic Delta to Cumulative sums (#9919) - `statsdreceiver`: Update the lastIntervalTime for Counter metrics (#9919) - `resourcedetectionprocessor`: GCP resource detector now correctly detects region on Google App Engine standard (#10814) +- `pkg/stanza`: Skip building fingerprint in case of configuration change (#10485) ## v0.52.0 From 251c4f05e17b9a3e3f72c6b4367445282cc79cd7 Mon Sep 17 00:00:00 2001 From: Dominik Rosiek Date: Wed, 8 Jun 2022 07:26:02 +0200 Subject: [PATCH 3/3] feat(tests): extract fingerprint size change as separate test Signed-off-by: Dominik Rosiek --- pkg/stanza/operator/input/file/file_test.go | 69 ++++++++++++++++++++- 1 file changed, 67 insertions(+), 2 deletions(-) diff --git a/pkg/stanza/operator/input/file/file_test.go b/pkg/stanza/operator/input/file/file_test.go index 76bef59b1d52..d23a390ae042 100644 --- a/pkg/stanza/operator/input/file/file_test.go +++ b/pkg/stanza/operator/input/file/file_test.go @@ -864,7 +864,6 @@ func TestFileReader_FingerprintUpdated(t *testing.T) { // - Updates as a file is read // - Stops updating when the max fingerprint size is reached // - Stops exactly at max fingerprint size, regardless of content -// - Do not change size after fingerprint configuration change func TestFingerprintGrowsAndStops(t *testing.T) { t.Parallel() @@ -920,7 +919,73 @@ func TestFingerprintGrowsAndStops(t *testing.T) { reader.ReadToEnd(context.Background()) require.Equal(t, fileContent[:expectedFP], reader.Fingerprint.FirstBytes) } - + }) + } +} + +// This is same test like TestFingerprintGrowsAndStops, but with additional check for fingerprint size check +// Test that a fingerprint: +// - Starts empty +// - Updates as a file is read +// - Stops updating when the max fingerprint size is reached +// - Stops exactly at max fingerprint size, regardless of content +// - Do not change size after fingerprint configuration change +func TestFingerprintChangeSize(t *testing.T) { + t.Parallel() + + // Use a number with many factors. + // Sometimes fingerprint length will align with + // the end of a line, sometimes not. Test both. + maxFP := 360 + + // Use prime numbers to ensure variation in + // whether or not they are factors of maxFP + lineLens := []int{3, 5, 7, 11, 13, 17, 19, 23, 27} + + for _, lineLen := range lineLens { + t.Run(fmt.Sprintf("%d", lineLen), func(t *testing.T) { + t.Parallel() + operator, _, tempDir := newTestFileOperator(t, func(cfg *Config) { + cfg.FingerprintSize = helper.ByteSize(maxFP) + }, nil) + defer func() { + require.NoError(t, operator.Stop()) + }() + + temp := openTemp(t, tempDir) + tempCopy := openFile(t, temp.Name()) + fp, err := operator.NewFingerprint(temp) + require.NoError(t, err) + require.Equal(t, []byte(""), fp.FirstBytes) + + splitter, err := operator.getMultiline() + require.NoError(t, err) + + reader, err := operator.NewReader(temp.Name(), tempCopy, fp, splitter) + require.NoError(t, err) + defer reader.Close() + + // keep track of what has been written to the file + fileContent := []byte{} + + // keep track of expected fingerprint size + expectedFP := 0 + + // Write lines until file is much larger than the length of the fingerprint + for len(fileContent) < 2*maxFP { + expectedFP += lineLen + if expectedFP > maxFP { + expectedFP = maxFP + } + + line := stringWithLength(lineLen-1) + "\n" + fileContent = append(fileContent, []byte(line)...) + + writeString(t, temp, line) + reader.ReadToEnd(context.Background()) + require.Equal(t, fileContent[:expectedFP], reader.Fingerprint.FirstBytes) + } + // Test fingerprint change // Change fingerprint and try to read file again // We do not expect fingerprint change