-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Migrates security solution usage collector es client from legacy to new #86853
Migrates security solution usage collector es client from legacy to new #86853
Conversation
…than the deprecated legacy client
Pinging @elastic/siem (Team:SIEM) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@michaelolo24 would you mind taking a look at these changes or redirect the PR to someone who could please? |
Will do! @rylnd should be in the loop as well |
@@ -4,12 +4,11 @@ | |||
* you may not use this file except in compliance with the Elastic License. | |||
*/ | |||
|
|||
import { SearchParams } from 'elasticsearch'; |
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.
Guessing this was an accidental import @rylnd?
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.
Its use was replaced by the custom RuleSearchParams
below!
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.
Looks good to me!
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just 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.
LGTM! Thanks for the update @TinaHeiligers !
@@ -4,12 +4,11 @@ | |||
* you may not use this file except in compliance with the Elastic License. | |||
*/ | |||
|
|||
import { SearchParams } from 'elasticsearch'; |
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.
Its use was replaced by the custom RuleSearchParams
below!
}; | ||
}; | ||
} | ||
interface RuleSearchParams { |
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.
Nit: since ruleSearchOptions
is an internal variable and its type is already constrained by its usage I would be inclined to make its type implicit and remove RuleSearchParams
. However, there's nothing wrong with the explicit interface here, either.
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'll keep it explicit for now and leave it up to your team to remove if that's what you prefer.
…y to new (#86853) (#87237) Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
Pinging @elastic/security-solution (Team: SecuritySolution) |
Summary
Updates the security solution usage collector
fetch
method's elasticsearch client to the new client provided by the usage_collector'sfetch
context.Part of #86358
Checklist
For maintainers