-
Notifications
You must be signed in to change notification settings - Fork 471
aws: optimise cloudtrail field retention work #14441
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
base: main
Are you sure you want to change the base?
Conversation
🚀 Benchmarks reportTo see the full report comment with |
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.
Great work! A few comments, but I really love the consistency changes!
packages/aws/data_stream/cloudtrail/elasticsearch/ingest_pipeline/default.yml
Show resolved
Hide resolved
packages/aws/data_stream/cloudtrail/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/aws/data_stream/cloudtrail/elasticsearch/ingest_pipeline/default.yml
Show resolved
Hide resolved
/test |
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
@efd6 qq: Can you use the |
It's not clear to me how that would be achieved. |
Hey @efd6 - if they remove fields will this have an impact on dashboards/rules? If so, can we make a plan to document this somewhere? |
@cpascale43 This change does not remove fields. That was already done in #14236. This just changes the time that the removal is done, from being a post facto remove, to a preemptive non-add. The change linked above shows the documentation that is presented to the user. |
This change is already starting to rot due to the complexity of the code here. |
This is the optimisation described in the TODO that was to be done.
/test |
|
💚 Build Succeeded
History
cc @efd6 |
Proposed commit message
Note
Best reviewed commitwise.
Checklist
changelog.yml
file.Author's Checklist
How to test this PR locally
Related issues
Screenshots