Skip to content

[Security solution] Create new class instance for each use of LangChain chat model#190004

Merged
stephmilovic merged 1 commit intoelastic:mainfrom
stephmilovic:create_llm_instance
Aug 7, 2024
Merged

[Security solution] Create new class instance for each use of LangChain chat model#190004
stephmilovic merged 1 commit intoelastic:mainfrom
stephmilovic:create_llm_instance

Conversation

@stephmilovic
Copy link
Copy Markdown
Contributor

@stephmilovic stephmilovic commented Aug 6, 2024

Summary

Follow up from #189810
Related to #189771

We need to create a new class instance for each use of any LangChain chat model. This is necessary to avoid any potential side effects from shared state and a variety of bugs.

  1. Renames getLlmInstance to createLlmInstance
  2. Adds a comment to explain to always create a new instance of the LLM class and to never assign it to a variable.
  3. Removes anywhere llmClass was assigned to a variable (llm) and instead always calls createLlmInstance()

Tests

ESQLKnowledgeBaseTool AlertsCountTool OpenAndAcknowledgedAlertsTool Alert Summary
Azure 4o Azure 4o Azure 4o Azure 4o
Gemini Pro 1.5 Gemini Pro 1.5 Gemini Pro 1.5 Gemini Pro 1.5
Sonnet 3.5 Sonnet 3.5 Sonnet 3.5 Sonnet 3.5

@stephmilovic stephmilovic added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Security Generative AI Security Generative AI v8.15.0 v8.16.0 labels Aug 6, 2024
@stephmilovic stephmilovic requested a review from a team as a code owner August 6, 2024 19:35
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

✅ unchanged

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

Copy link
Copy Markdown
Contributor

@andrew-goldstein andrew-goldstein left a comment

Choose a reason for hiding this comment

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

Thanks for this fix @stephmilovic! 🙏
✅ Desk tested locally
LGTM

@stephmilovic
Copy link
Copy Markdown
Contributor Author

Another example, this will solve the titling step from invoking tools: https://smith.langchain.com/public/e34f8013-12cc-49c2-b68a-b42205322c9c/r

@stephmilovic stephmilovic merged commit 34f1f0f into elastic:main Aug 7, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 7, 2024
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 All backports created successfully

Status Branch Result
8.15

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Aug 7, 2024
…LangChain chat model (#190004) (#190096)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[Security solution] Create new class instance for each use of
LangChain chat model
(#190004)](#190004)

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

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

<!--BACKPORT [{"author":{"name":"Steph
Milovic","email":"stephanie.milovic@elastic.co"},"sourceCommit":{"committedDate":"2024-08-07T19:28:14Z","message":"[Security
solution] Create new class instance for each use of LangChain chat model
(#190004)","sha":"34f1f0f75b01df87720a2f81de46381dc3876378","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:
SecuritySolution","Team:Security Generative
AI","v8.15.0","v8.16.0"],"title":"[Security solution] Create new class
instance for each use of LangChain chat
model","number":190004,"url":"https://github.com/elastic/kibana/pull/190004","mergeCommit":{"message":"[Security
solution] Create new class instance for each use of LangChain chat model
(#190004)","sha":"34f1f0f75b01df87720a2f81de46381dc3876378"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"8.15","label":"v8.15.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/190004","number":190004,"mergeCommit":{"message":"[Security
solution] Create new class instance for each use of LangChain chat model
(#190004)","sha":"34f1f0f75b01df87720a2f81de46381dc3876378"}}]}]
BACKPORT-->

Co-authored-by: Steph Milovic <stephanie.milovic@elastic.co>
@mistic mistic added v8.15.1 and removed v8.15.0 labels Aug 8, 2024
@mistic
Copy link
Copy Markdown
Contributor

mistic commented Aug 8, 2024

This PR didn't make it on time of the latest v8.15 BC. Updating the labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Team:Security Generative AI Security Generative AI Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.1 v8.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants