-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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 listPrefix in awsS3WriteCommitPrefix #31776
Conversation
continue | ||
} | ||
|
||
var latestStoredTime time.Time | ||
keys[state.ID] = struct{}{} | ||
latestStoredTime, ok := latestStoredTimeByBucket[state.Bucket] | ||
latestStoredTime, ok := latestStoredTimeByBucketAndListPrefix[state.Bucket+state.ListPrefix] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is indeed both a bugfix and a breaking change
from the elastic side the only integrations that does that is the one on cisco managed s3 buckets, that's indeed the bugged integration that will fix
but in the case of users using custom setup with a single bucket and multiple list prefixes it will be a breaking change as in the measure that s3 objects could be ingested again (and probably fails ingestion because of same document ids)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for such a quick reaction to the issue!
/test |
* add listPrefix in awsS3WriteCommitPrefix * linting and changelog * linting * try fixing integration test * try fixing integration test (cherry picked from commit ee38609) # Conflicts: # x-pack/filebeat/input/awss3/input_benchmark_test.go
* add listPrefix in awsS3WriteCommitPrefix * linting and changelog * linting * try fixing integration test * try fixing integration test (cherry picked from commit ee38609) # Conflicts: # x-pack/filebeat/input/awss3/input_benchmark_test.go
* add listPrefix in awsS3WriteCommitPrefix (#31776) * add listPrefix in awsS3WriteCommitPrefix * linting and changelog * linting * try fixing integration test * try fixing integration test (cherry picked from commit ee38609) # Conflicts: # x-pack/filebeat/input/awss3/input_benchmark_test.go * fix backport * remove duplicates in test Co-authored-by: Andrea Spacca <andrea.spacca@elastic.co>
) * add listPrefix in awsS3WriteCommitPrefix (#31776) * add listPrefix in awsS3WriteCommitPrefix * linting and changelog * linting * try fixing integration test * try fixing integration test (cherry picked from commit ee38609) # Conflicts: # x-pack/filebeat/input/awss3/input_benchmark_test.go * fix backport * remove duplicates in test Co-authored-by: Andrea Spacca <andrea.spacca@elastic.co>
* add listPrefix in awsS3WriteCommitPrefix * linting and changelog * linting * try fixing integration test * try fixing integration test (cherry picked from commit ee38609)
* add listPrefix in awsS3WriteCommitPrefix (#31776) * add listPrefix in awsS3WriteCommitPrefix * linting and changelog * linting * try fixing integration test * try fixing integration test (cherry picked from commit ee38609) * Update CHANGELOG.next.asciidoc * Update CHANGELOG.next.asciidoc Co-authored-by: Andrea Spacca <andrea.spacca@elastic.co>
* add listPrefix in awsS3WriteCommitPrefix * linting and changelog * linting * try fixing integration test * try fixing integration test
Bug
What does this PR do?
Fix
awsS3WriteCommitPrefix
bug when using a single bucket with multiple listPrefix on aws-s3 s3 direct inputWhy is it important?
Regarding the Cisco Umbrella integration via the elastic agent, a customer ingesting using the Cisco Managed S3 and have near 40 individual integrations setup for different customers. However they found logs to be missing as though the agent failed to read them, not even partial parsing of a log. I checked the files within an S3 bucket to make sure they were written by Cisco and they were.
Checklist
- [ ] I have made corresponding changes to the documentation- [ ] I have made corresponding change to the default configuration filesCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs