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 assistant] Use inference connector in security AI features #204505

Merged
merged 70 commits into from
Jan 8, 2025

Conversation

stephmilovic
Copy link
Contributor

@stephmilovic stephmilovic commented Dec 16, 2024

Summary

Enables new inference connector in the AI Assistant and Attack Discovery. To use with Attack Discovery, you must increase the rate limit of the connector and set the number of alerts sent to minimum (50).

Only preconfigured inference connectors will be shown in Security until all the providers will support unified completion.

To test

  1. update the value for inferenceConnectorOn to true in x-pack/platform/plugins/shared/stack_connectors/common/experimental_features.ts
  2. Create an inference connector using OpenAI creds. Configure the inference endpoint for completion and name the endpoint openai-completion-preconfig
  3. Now that the inference endpoint is created, add a preconfigured connector with the same credentials.
  4. Select the preconfigured selector in AI Assistant. Ensure the UI configured connector does not show
  5. Test the assistant with streaming and without, invoking tools. Ensure it works!
  6. Go to attack discovery. Ensure you have alerts. Ensure the minimum number of alerts is configured (50). Run attack discovery with the inference preconfigured connector selected. Ensure it works!

YulNaumenko and others added 30 commits November 14, 2024 11:51
…:YulNaumenko/kibana into ai-connector-inference-completion-openai
…:YulNaumenko/kibana into ai-connector-inference-completion-openai
@stephmilovic stephmilovic added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Security Generative AI Security Generative AI v8.18.0 labels Jan 6, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@stephmilovic
Copy link
Contributor Author

@elasticmachine merge upstream

@pmuellr
Copy link
Member

pmuellr commented Jan 7, 2025

To use with Attack Discovery, you must increase the rate limit of the connector and set the number of alerts sent to minimum (50)

Has anything changed w/r/t the "queued" nature of the connector; specifically that we don't use task manager to arrange to run the connector "later". Asking since it has not be "queued" in the past, which means it is somewhat exempt from some ZDT/BWC issues (the thorny ones).

@stephmilovic
Copy link
Contributor Author

@elasticmachine merge upstream

@stephmilovic
Copy link
Contributor Author

To use with Attack Discovery, you must increase the rate limit of the connector and set the number of alerts sent to minimum (50)

Has anything changed w/r/t the "queued" nature of the connector; specifically that we don't use task manager to arrange to run the connector "later". Asking since it has not be "queued" in the past, which means it is somewhat exempt from some ZDT/BWC issues (the thorny ones).

No, we’re not using task manager to queue inference tasks

Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

ResponseOps changes LGTM, assuming the connectors are not queued via task manager ... (left a question on that in the PR comments)

}),
tap((line) => {
if ('error' in line) {
throw new Error(line.error.message);
throw new Error(line.error.message ?? line.error.reason ?? 'Unknown error');
Copy link
Member

Choose a reason for hiding this comment

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

I suggest using || instead of ??, as ?? will allow "" to pass through as the value:

> x = ""
''
> y = ""
''
> x ?? y ?? 'abc'
''
> x || y || 'abc'
'abc'

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

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
integrationAssistant 878.3KB 882.8KB +4.5KB
securitySolution 22.1MB 22.1MB +93.0B
total +4.6KB
Unknown metric groups

ESLint disabled line counts

id before after diff
@kbn/elastic-assistant 15 16 +1

Total ESLint disabled count

id before after diff
@kbn/elastic-assistant 16 17 +1

History

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM! Great work!
Very exciting to see the puzzle peaces are coming together.

@stephmilovic stephmilovic merged commit c6501da into elastic:main Jan 8, 2025
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12673749960

@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@nchaulet
Copy link
Member

nchaulet commented Jan 9, 2025

Looks like fleet Cypress test are broken since that PR was merged
Screenshot 2025-01-09 at 11 34 45 AM

with an error useAssistantContext must be used within a AssistantProvider in useLoadConnectors

@stephmilovic
Copy link
Contributor Author

stephmilovic commented Jan 9, 2025

Looks like fleet Cypress test are broken since that PR was merged

Sorry about this! @bhapas has a fix in this PR: #206111

kibanamachine added a commit that referenced this pull request Jan 9, 2025
…tures (#204505) (#205923)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security assistant] Use inference connector in security AI features
(#204505)](#204505)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Steph
Milovic","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-08T15:30:15Z","message":"[Security
assistant] Use inference connector in security AI features
(#204505)","sha":"c6501da809c5ff8dc5f16076205ec65abaffcb54","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","v9.0.0","Team:
SecuritySolution","backport:prev-minor","Team:Security Generative
AI","v8.18.0"],"title":"[Security assistant] Use inference connector in
security AI
features","number":204505,"url":"https://github.com/elastic/kibana/pull/204505","mergeCommit":{"message":"[Security
assistant] Use inference connector in security AI features
(#204505)","sha":"c6501da809c5ff8dc5f16076205ec65abaffcb54"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/204505","number":204505,"mergeCommit":{"message":"[Security
assistant] Use inference connector in security AI features
(#204505)","sha":"c6501da809c5ff8dc5f16076205ec65abaffcb54"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Steph Milovic <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:enhancement Team:Security Generative AI Security Generative AI Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants