-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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 allowlist filter for Exception list data #112668
Add allowlist filter for Exception list data #112668
Conversation
Pinging @elastic/security-solution (Team: SecuritySolution) |
x-pack/plugins/security_solution/server/lib/telemetry/tasks/security_lists.ts
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
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.
Just a simple question about the types for the list telemetry.
trusted_application: TrustedApp[]; | ||
endpoint_exception: EndpointExceptionListItem[]; | ||
endpoint_event_filter: EndpointExceptionListItem[]; | ||
trusted_application: TelemetryEvent[]; |
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.
Interesting, why are these types changed to be less concrete?
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.
Ref this commit: be4c21f
The reasoning around this was 2 fold:
TrustedApp
andEndpointExceptionListItem
(renamedExceptionListItem
) were consolidated, so these would have been 3ExceptionListItem
. Using a type we don't own has caused the issue we discussed in the backref issue.- The reason for the cast to
TelemetryEvent
is to allow for the allow list filtering without overcomplicating the logic or introducing generics (which wouldn't be easy to do / test)
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.
Ah ok, that makes sense.
Thanks @donaherc. Good q! |
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @pjhampton |
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 thx for answering the Qs
* Add allowlist filter to exception list telemetry. * Refactor away union type. * Bump interval back to 24h. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💔 Backport failed
Successful backport PRs will be merged automatically after passing CI. To backport manually run: |
* Add allowlist filter to exception list telemetry. * Refactor away union type. * Bump interval back to 24h. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Pete Hampton <pjhampton@users.noreply.github.com>
* Add allowlist filter for Exception list data (#112668) * Add allowlist filter to exception list telemetry. * Refactor away union type. * Bump interval back to 24h. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> * Fix linting issue Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
Filters out non-required information for downstream analysis.
Checklist
Delete any items that are not applicable to this PR.
For maintainers