[x-pack][filebeat] incorporate LastModified into s3 event _id generation #42078

stefans-elastic · 2024-12-17T10:38:16Z

Proposed commit message

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Disruptive User Impact

Author's Checklist

[ ]

How to test this PR locally

Related issues

Closes #42040

Use cases

Screenshots

Logs

mergify · 2024-12-17T10:39:23Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @stefans-elastic? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit

mergify · 2024-12-17T10:39:23Z

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.

tetianakravchenko

@stefans-elastic I think LastModified should be added to the debug log at https://github.com/elastic/beats/pull/42078/files#diff-f345fd6a1f5ea9523117d4ead2e5f1d13fb82eb1c65a089fd34fcdd514916a96R119, as now it is used for id

could you please also add a test for this change?

…d-change

x-pack/filebeat/input/awss3/s3_objects.go

x-pack/filebeat/input/azureblobstorage/input_test.go

x-pack/filebeat/input/gcs/input_test.go

CHANGELOG.next.asciidoc

tetianakravchenko · 2024-12-18T08:45:38Z

x-pack/filebeat/input/gcs/input_test.go

@@ -662,7 +663,7 @@ func Test_StorageClient(t *testing.T) {
 					if !tt.checkJSON {
 						val, err = got.Fields.GetValue("message")
 						assert.NoError(t, err)
-						assert.True(t, tt.expected[val.(string)])
+						assert.True(t, tt.expected[strings.ReplaceAll(val.(string), "\r", "")])


why do we need this change in this PR?

it's because windows treating line breaks as \r\n instead of \n which results in tests failing on windows

mergify · 2024-12-18T08:46:40Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b aws-s3-id-change upstream/aws-s3-id-change
git merge upstream/main
git push upstream aws-s3-id-change

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>

…d-change

stefans-elastic · 2024-12-19T12:06:26Z

Tests are passing now so I think this PR is ready for review

tetianakravchenko · 2024-12-19T13:33:55Z

@stefans-elastic I see that you initially added unit test for this PR 3041ff4 but then reverted, is it intentional?

stefans-elastic · 2024-12-19T13:48:43Z

@stefans-elastic I see that you initially added unit test for this PR 3041ff4 but then reverted, is it intentional?

the implementation has changed so the test wasn't correct anymore

stefans-elastic · 2024-12-19T14:16:22Z

@stefans-elastic I see that you initially added unit test for this PR 3041ff4 but then reverted, is it intentional?

the implementation has changed so the test wasn't correct anymore

@tetianakravchenko I've added a new test

tetianakravchenko

LGTM

mergify · 2024-12-19T16:36:51Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b aws-s3-id-change upstream/aws-s3-id-change
git merge upstream/main
git push upstream aws-s3-id-change

…d-change

…ion (#42078) * [x-pack][filebeat] incorporate LastModified into s3 event _id generation * update PR id in changelog * log LastModified value * add unit test * fix unit tests on windows * use UnixMilli instead of time string * change s3 event _id format * fix changelog * Update x-pack/filebeat/input/gcs/input_test.go Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co> * Update x-pack/filebeat/input/azureblobstorage/input_test.go Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co> * revert windows test fix * add unit test --------- Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co> (cherry picked from commit 58a5369) # Conflicts: # x-pack/filebeat/input/awss3/s3_objects.go

…to s3 event _id generation (#42132) * [x-pack][filebeat] incorporate LastModified into s3 event _id generation (#42078) * [x-pack][filebeat] incorporate LastModified into s3 event _id generation * update PR id in changelog * log LastModified value * add unit test * fix unit tests on windows * use UnixMilli instead of time string * change s3 event _id format * fix changelog * Update x-pack/filebeat/input/gcs/input_test.go Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co> * Update x-pack/filebeat/input/azureblobstorage/input_test.go Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co> * revert windows test fix * add unit test --------- Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co> (cherry picked from commit 58a5369) # Conflicts: # x-pack/filebeat/input/awss3/s3_objects.go * resolve conflicts * linter fix --------- Co-authored-by: stefans-elastic <stefan.stas@elastic.co>

[x-pack][filebeat] incorporate LastModified into s3 event _id generation

74bfc78

stefans-elastic added enhancement Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team labels Dec 17, 2024

stefans-elastic self-assigned this Dec 17, 2024

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Dec 17, 2024

mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Dec 17, 2024

update PR id in changelog

68de656

stefans-elastic marked this pull request as ready for review December 17, 2024 10:40

stefans-elastic requested a review from a team as a code owner December 17, 2024 10:40

tetianakravchenko reviewed Dec 17, 2024

View reviewed changes

tetianakravchenko requested a review from a team December 17, 2024 11:21

stefans-elastic added 4 commits December 17, 2024 13:56

Merge branch 'main' of github.com:stefans-elastic/beats into aws-s3-i…

faaf7bb

…d-change

log LastModified value

30bc1f4

add unit test

3041ff4

fix unit tests on windows

32a6966

stefans-elastic requested a review from a team as a code owner December 17, 2024 15:11

andrewkroh reviewed Dec 17, 2024

View reviewed changes

x-pack/filebeat/input/awss3/s3_objects.go Outdated Show resolved Hide resolved

stefans-elastic added 2 commits December 17, 2024 17:44

use UnixMilli instead of time string

94c1054

change s3 event _id format

64af26c

efd6 reviewed Dec 17, 2024

View reviewed changes

x-pack/filebeat/input/azureblobstorage/input_test.go Outdated Show resolved Hide resolved

x-pack/filebeat/input/gcs/input_test.go Outdated Show resolved Hide resolved

tetianakravchenko reviewed Dec 18, 2024

View reviewed changes

stefans-elastic and others added 5 commits December 18, 2024 11:35

merge main

eefba38

fix changelog

c642bc3

Update x-pack/filebeat/input/gcs/input_test.go

47d432a

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>

Update x-pack/filebeat/input/azureblobstorage/input_test.go

b01f431

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>

Merge branch 'main' of github.com:stefans-elastic/beats into aws-s3-i…

3127957

…d-change

revert windows test fix

d623d34

stefans-elastic requested review from efd6, andrewkroh and tetianakravchenko December 19, 2024 12:06

add unit test

32c0dd4

tetianakravchenko approved these changes Dec 19, 2024

View reviewed changes

tetianakravchenko requested a review from zmoog December 19, 2024 16:29

andrewkroh approved these changes Dec 19, 2024

View reviewed changes

andrewkroh mentioned this pull request Dec 19, 2024

[Filebeat] aws-s3 - Document _id generation behavior #42127

Merged

6 tasks

mergify bot mentioned this pull request Dec 19, 2024

[8.x](backport #42127) [Filebeat] aws-s3 - Document _id generation behavior #42130

Merged

6 tasks

Merge branch 'main' of github.com:stefans-elastic/beats into aws-s3-i…

569d097

…d-change

stefans-elastic merged commit 58a5369 into elastic:main Dec 20, 2024
22 checks passed

stefans-elastic deleted the aws-s3-id-change branch December 20, 2024 09:56

mergify bot mentioned this pull request Dec 20, 2024

[8.x](backport #42078) [x-pack][filebeat] incorporate LastModified into s3 event _id generation #42132

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[x-pack][filebeat] incorporate LastModified into s3 event _id generation #42078

[x-pack][filebeat] incorporate LastModified into s3 event _id generation #42078

stefans-elastic commented Dec 17, 2024 •

edited

Loading

mergify bot commented Dec 17, 2024

mergify bot commented Dec 17, 2024

tetianakravchenko left a comment •

edited

Loading

tetianakravchenko Dec 18, 2024

stefans-elastic Dec 18, 2024

mergify bot commented Dec 18, 2024

stefans-elastic commented Dec 19, 2024

tetianakravchenko commented Dec 19, 2024

stefans-elastic commented Dec 19, 2024

stefans-elastic commented Dec 19, 2024

tetianakravchenko left a comment

mergify bot commented Dec 19, 2024

[x-pack][filebeat] incorporate LastModified into s3 event _id generation #42078

[x-pack][filebeat] incorporate LastModified into s3 event _id generation #42078

Conversation

stefans-elastic commented Dec 17, 2024 • edited Loading

Proposed commit message

Checklist

Disruptive User Impact

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

mergify bot commented Dec 17, 2024

mergify bot commented Dec 17, 2024

tetianakravchenko left a comment • edited Loading

Choose a reason for hiding this comment

tetianakravchenko Dec 18, 2024

Choose a reason for hiding this comment

stefans-elastic Dec 18, 2024

Choose a reason for hiding this comment

mergify bot commented Dec 18, 2024

stefans-elastic commented Dec 19, 2024

tetianakravchenko commented Dec 19, 2024

stefans-elastic commented Dec 19, 2024

stefans-elastic commented Dec 19, 2024

tetianakravchenko left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 19, 2024

stefans-elastic commented Dec 17, 2024 •

edited

Loading

tetianakravchenko left a comment •

edited

Loading