Skip to content
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] [GenAI] [Detections] Ask security assistant to help diagnose rule execution errors #166778

Merged
merged 14 commits into from
Nov 16, 2023

Conversation

dhurley14
Copy link
Contributor

@dhurley14 dhurley14 commented Sep 19, 2023

Summary

Thanks @spong for the speedy assistance with getting this code-complete!

Utilizing the Security Assistant to provide some suggested mediation steps for rule errors could help customers to better self-diagnose rule errors. Thus, enhancing their experience with the Security Solution and potentially reducing new support tickets.

Error on rule details page:
threshold_rule_exception_error

Response from security assistant:
threshold_rule_exception_assistant_resolved

Available for warnings too:
assistant_error_help_warning

Includes the rule name and data sources for pre-built rules for additional information to generate a slightly more helpful response:

pre_built_rule_name_data_source

@dhurley14 dhurley14 self-assigned this Sep 19, 2023
@dhurley14 dhurley14 added review release_note:feature Makes this part of the condensed release notes Team:Detection Engine Security Solution Detection Engine Area Feature:GenAI v8.11.0 labels Sep 19, 2023
@dhurley14 dhurley14 added v8.12.0 and removed v8.11.0 labels Nov 8, 2023
@dhurley14 dhurley14 marked this pull request as ready for review November 9, 2023 18:32
@dhurley14 dhurley14 requested review from a team as code owners November 9, 2023 18:32
@spong spong requested review from spong and removed request for nikitaindik November 9, 2023 20:37
Comment on lines 76 to 88
<EuiButton color={color} size="s">
<NewChat
category="detection-rules"
color={color}
conversationId={i18nAssistant.DETECTION_RULES_CONVERSATION_ID}
description={i18n.ASK_ASSISTANT_DESCRIPTION}
getPromptContext={getPromptContext}
suggestedUserPrompt={i18n.ASK_ASSISTANT_USER_PROMPT}
tooltip={i18n.ASK_ASSISTANT_TOOLTIP}
>
{i18n.ASK_ASSISTANT_ERROR_BUTTON}
</NewChat>
</EuiButton>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bug here where the button will still show for users that don't have the assistant enabled. Adding useAssistantAvailability and wrapping with a check for hasAssistantPrivilege. Would be nice for us to do this within the NewChat component so it doesn't always need to be wrapped, but will need to think how we can go about this since the privileges live on the Security Solution side:

Detection_rules__SIEM__-_Kibana

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked out, tested locally, and LGTM! Thanks so much for putting this together @dhurley14! And a belated congrats on the 👶 😉!!

I did find two bugs in testing:

One around RBAC where the button would show if the assistant wasn't enabled for the user. I pushed a fix for this -- we have a hook for determining assistant availability, so I just wrapped the button in this check and all is good! 👍

The second bug I saw was when engaging with the assistant and the conversation has no connector selected. In this instance the prompt message is not set since the text area is disabled. This is pre-existing issue, and is present with all the other NewChat implementations, so just something we'll need to fix on the assistant side.

Anyway, with that, I think we're all good to merge this. Thanks again and hope you had fun integrating with the assistant! 😀

Comment on lines +24 to +25
/** Optionally specify color of empty button */
color?: 'text' | 'accent' | 'primary' | 'success' | 'warning' | 'danger';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome -- thanks for expanding the functionality here! 🙂

@spong spong added the Feature:Security Assistant Security Assistant label Nov 16, 2023
@spong spong enabled auto-merge (squash) November 16, 2023 19:45
@spong spong merged commit 18d65c4 into elastic:main Nov 16, 2023
27 checks passed
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Investigations - Security Solution Cypress Tests #2 / Correlation tab should update timeline after removing eql should update timeline after removing eql

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 12.8MB 12.8MB +1.6KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @dhurley14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:GenAI Feature:Security Assistant Security Assistant release_note:feature Makes this part of the condensed release notes review Team:Detection Engine Security Solution Detection Engine Area v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants