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
box_events: fingerprint the box.event_id field for doc deduplication #9498
Conversation
🚀 Benchmarks reportTo see the full report comment with |
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
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. I assume this ID is unique across Box accounts (in case users are running multiple instances of the integration).
The text in the docs could be interpreted that way, "The ID of the event object. You can use this to detect duplicate events". The language is not quite strong enough for me to be completely happy. We could inject the client ID into the ingest pipeline for inclusion in the fingerprint set and then remove it. I think this would make me happier, though this value is per application, not per owner, so it may move.
I am unable to find a definitively documented identifier that we could use, so this is probably the best we have. |
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.
I think this would make me happier, though this value is per application, not per owner, so it may move.
LGTM after adding client_id
👍🏼 although a test could be nice to have.
- remove: | ||
field: _conf.client_id | ||
ignore_missing: true |
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.
Is this required since there is a remove
on _conf
field later in the pipeline?
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
💚 Build Succeeded
History
cc @efd6 |
Package box_events - 2.8.0 containing this change is available at https://epr.elastic.co/search?package=box_events |
Proposed commit message
See https://developer.box.com/reference/resources/event/#param-event_id for field semantics.
Checklist
changelog.yml
file.Author's Checklist
How to test this PR locally
Related issues
Screenshots