-
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
[Security Solution][Detections] Update telemetry to use ML contract #71665
Conversation
This interface recently changed and we're now able to use the ML contract to retrieve these values. A few unnecessary arguments are stubbed as we're in a non-user, non-request context.
Conflicts: x-pack/plugins/ml/server/shared_services/providers/modules.ts x-pack/plugins/security_solution/server/usage/detections/detections_helpers.ts
Pinging @elastic/siem (Team:SIEM) |
This is more legible but still gets the point across; the intermediate variable was explicit but ultimately unnnecessary.
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
We're not calling different methods, so our mocks need to change slightly.
💚 Build SucceededBuild metrics
History
To update your PR or re-run it, just comment with: |
@@ -40,8 +41,14 @@ export function getModulesProvider({ | |||
request: KibanaRequest, | |||
savedObjectsClient: SavedObjectsClientContract | |||
) { | |||
const hasMlCapabilities = getHasMlCapabilities(request); | |||
let hasMlCapabilities: HasMlCapabilities; | |||
if (request.params === 'DummyKibanaRequest') { |
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 would just make DummyKibanaRequest
a shared constant somewhere so someone doesn't accidentally break this in the future 😂
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.
Yeah... uptime is doing something similar as well. We want to be consistent while discouraging this temporary workaround... if it's used in a third place maybe we move it upstream into getHasMlCapabilities
.
…lastic#71665) * Update security solution telemetry to use ML providers This interface recently changed and we're now able to use the ML contract to retrieve these values. A few unnecessary arguments are stubbed as we're in a non-user, non-request context. * Simplify our capabilities stub assignment This is more legible but still gets the point across; the intermediate variable was explicit but ultimately unnnecessary. * Update tests following telemetry refactor We're not calling different methods, so our mocks need to change slightly.
…71665) (#71733) * Update security solution telemetry to use ML providers This interface recently changed and we're now able to use the ML contract to retrieve these values. A few unnecessary arguments are stubbed as we're in a non-user, non-request context. * Simplify our capabilities stub assignment This is more legible but still gets the point across; the intermediate variable was explicit but ultimately unnnecessary. * Update tests following telemetry refactor We're not calling different methods, so our mocks need to change slightly.
Pinging @elastic/security-solution (Team: SecuritySolution) |
Summary
This interface recently changed and we're now able to use the ML contract to retrieve these values. A few unnecessary arguments are stubbed as we're in a non-user, non-request context.
For maintainers