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

[Obs AI Assistant] Fix knowledge base install status #205970

Closed
neptunian opened this issue Jan 8, 2025 · 3 comments · Fixed by #206130
Closed

[Obs AI Assistant] Fix knowledge base install status #205970

neptunian opened this issue Jan 8, 2025 · 3 comments · Fixed by #206130
Assignees
Labels
Team:Obs AI Assistant Observability AI Assistant

Comments

@neptunian
Copy link
Contributor

neptunian commented Jan 8, 2025

Before migrating to the inference api, the /setup route installed the model and polled ml.getTrainedModels only returning a response when the model was ready (see old code). Since migrating to using the inference endpoint, /setup now only returns the result from the create inference API and no longer polls for model readiness before returning.

When a user does not have a Gen AI connector setup we automatically trigger /setup after saving the connector and depended on the polling there. If there is a preconfigured connector, the user manually installs and we poll using /status instead. We should poll for /status in both cases to check for model readiness and not rely on /setup to indicate when the whole installation process is complete.

This is causing a UI issue where after /setup finishes, they are shown a message that the knowledge base hasn't been setup because /status reports the model is not ready because it is still being deployed.

screenshots of the problem state Image Image Image

Introduced in: #186499

I think it's also causing knowledge base tests to be flaky which still presume it can start using the knowledge base after /setup has responded with 200.

#205677
#205581

For the knowledge base tests, we'd probably need to add polling to /status, same as the UI, to check the model is ready so we can start running tests against the knowledge base.

@neptunian neptunian added the Team:Obs AI Assistant Observability AI Assistant label Jan 8, 2025
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ai-assistant (Team:Obs AI Assistant)

@neptunian neptunian self-assigned this Jan 9, 2025
@sorenlouv
Copy link
Member

sorenlouv commented Jan 9, 2025

The most straightforward option would be to add polling back to the /setup route.

The /status endpoint already does polling so I don't think we need two endpoints doing this.

I suggest leaving the /setup endpoint to creating the inference endpoint, and leaving the polling to the /status endpoint.

Like you say, the UI will then use the /status endpoint to reflect that state of the model. And API tests will need to retry the /status endpoint to ensure the model is ready before ingesting data.

@neptunian
Copy link
Contributor Author

neptunian commented Jan 9, 2025

To add to the description details, for whatever reason we only polled /status when a user initiated an install manually (previous install error or user has a pre configured connector and we dont automatically install) otherwise we depended on /setup to poll within itself. So polling /status did not happen when we auto installed. I'm not sure why it was done in two places instead of just relying on /status.

kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Jan 15, 2025
)

Resolves elastic#205970

Updates logic to account for knowledge base `/setup` no longer polling
for model readiness before returning.

- Currently we only poll `/status` if user manually installs the
knowledge base. In cases where we auto installed, such as after
successfully setting up a connector, we depended on `/setup` to poll
internally. Since the latter was removed, we need to always poll
`/status`, otherwise user could potentially be in the state where
`setup` has finished (considered installed) but `status` still reports
not ready and we show the install message again (see screenshots in
elastic#205970)
- Currently if an install is in progress and user closes the flyout, the
progress state is lost. These changes should continue to reflect the
installation progress in the UI.
- Renames variables and adds comments for easier readability
- adds unit test to component that handles the install UI state,
`WelcomeMessageKnowledgeBase`

---------

Co-authored-by: Søren Louv-Jansen <[email protected]>
(cherry picked from commit 06526fe)
kibanamachine added a commit that referenced this issue Jan 15, 2025
…) (#206777)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Obs AI Assistant] fix knowledge base installation state
(#206130)](#206130)

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

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

<!--BACKPORT [{"author":{"name":"Sandra
G","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-15T14:38:07Z","message":"[Obs
AI Assistant] fix knowledge base installation state
(#206130)\n\nResolves
https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to
account for knowledge base `/setup` no longer polling\r\nfor model
readiness before returning.\r\n\r\n- Currently we only poll `/status` if
user manually installs the\r\nknowledge base. In cases where we auto
installed, such as after\r\nsuccessfully setting up a connector, we
depended on `/setup` to poll\r\ninternally. Since the latter was
removed, we need to always poll\r\n`/status`, otherwise user could
potentially be in the state where\r\n`setup` has finished (considered
installed) but `status` still reports\r\nnot ready and we show the
install message again (see screenshots
in\r\nhttps://github.com//issues/205970)\r\n- Currently if
an install is in progress and user closes the flyout, the\r\nprogress
state is lost. These changes should continue to reflect
the\r\ninstallation progress in the UI.\r\n- Renames variables and adds
comments for easier readability\r\n- adds unit test to component that
handles the install UI
state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen
<[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","ci:cloud-deploy","Team:Obs
AI
Assistant","ci:project-deploy-elasticsearch","ci:project-deploy-observability","backport:version","v8.18.0","v8.17.1"],"title":"[Obs
AI Assistant] fix knowledge base installation
state","number":206130,"url":"https://github.com/elastic/kibana/pull/206130","mergeCommit":{"message":"[Obs
AI Assistant] fix knowledge base installation state
(#206130)\n\nResolves
https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to
account for knowledge base `/setup` no longer polling\r\nfor model
readiness before returning.\r\n\r\n- Currently we only poll `/status` if
user manually installs the\r\nknowledge base. In cases where we auto
installed, such as after\r\nsuccessfully setting up a connector, we
depended on `/setup` to poll\r\ninternally. Since the latter was
removed, we need to always poll\r\n`/status`, otherwise user could
potentially be in the state where\r\n`setup` has finished (considered
installed) but `status` still reports\r\nnot ready and we show the
install message again (see screenshots
in\r\nhttps://github.com//issues/205970)\r\n- Currently if
an install is in progress and user closes the flyout, the\r\nprogress
state is lost. These changes should continue to reflect
the\r\ninstallation progress in the UI.\r\n- Renames variables and adds
comments for easier readability\r\n- adds unit test to component that
handles the install UI
state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen
<[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1"}},"sourceBranch":"main","suggestedTargetBranches":["8.x","8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/206130","number":206130,"mergeCommit":{"message":"[Obs
AI Assistant] fix knowledge base installation state
(#206130)\n\nResolves
https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to
account for knowledge base `/setup` no longer polling\r\nfor model
readiness before returning.\r\n\r\n- Currently we only poll `/status` if
user manually installs the\r\nknowledge base. In cases where we auto
installed, such as after\r\nsuccessfully setting up a connector, we
depended on `/setup` to poll\r\ninternally. Since the latter was
removed, we need to always poll\r\n`/status`, otherwise user could
potentially be in the state where\r\n`setup` has finished (considered
installed) but `status` still reports\r\nnot ready and we show the
install message again (see screenshots
in\r\nhttps://github.com//issues/205970)\r\n- Currently if
an install is in progress and user closes the flyout, the\r\nprogress
state is lost. These changes should continue to reflect
the\r\ninstallation progress in the UI.\r\n- Renames variables and adds
comments for easier readability\r\n- adds unit test to component that
handles the install UI
state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen
<[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Sandra G <[email protected]>
neptunian added a commit to neptunian/kibana that referenced this issue Jan 15, 2025
)

Resolves elastic#205970

Updates logic to account for knowledge base `/setup` no longer polling
for model readiness before returning.

- Currently we only poll `/status` if user manually installs the
knowledge base. In cases where we auto installed, such as after
successfully setting up a connector, we depended on `/setup` to poll
internally. Since the latter was removed, we need to always poll
`/status`, otherwise user could potentially be in the state where
`setup` has finished (considered installed) but `status` still reports
not ready and we show the install message again (see screenshots in
elastic#205970)
- Currently if an install is in progress and user closes the flyout, the
progress state is lost. These changes should continue to reflect the
installation progress in the UI.
- Renames variables and adds comments for easier readability
- adds unit test to component that handles the install UI state,
`WelcomeMessageKnowledgeBase`

---------

Co-authored-by: Søren Louv-Jansen <[email protected]>
(cherry picked from commit 06526fe)

# Conflicts:
#	x-pack/packages/kbn-ai-assistant/src/chat/welcome_message_knowledge_base.test.tsx
#	x-pack/plugins/observability_solution/observability_ai_assistant/server/routes/knowledge_base/route.ts
neptunian added a commit that referenced this issue Jan 15, 2025
) (#206837)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Obs AI Assistant] fix knowledge base installation state
(#206130)](#206130)

<!--- Backport version: 9.6.4 -->

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

<!--BACKPORT [{"author":{"name":"Sandra
G","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-15T14:38:07Z","message":"[Obs
AI Assistant] fix knowledge base installation state
(#206130)\n\nResolves
https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to
account for knowledge base `/setup` no longer polling\r\nfor model
readiness before returning.\r\n\r\n- Currently we only poll `/status` if
user manually installs the\r\nknowledge base. In cases where we auto
installed, such as after\r\nsuccessfully setting up a connector, we
depended on `/setup` to poll\r\ninternally. Since the latter was
removed, we need to always poll\r\n`/status`, otherwise user could
potentially be in the state where\r\n`setup` has finished (considered
installed) but `status` still reports\r\nnot ready and we show the
install message again (see screenshots
in\r\nhttps://github.com//issues/205970)\r\n- Currently if
an install is in progress and user closes the flyout, the\r\nprogress
state is lost. These changes should continue to reflect
the\r\ninstallation progress in the UI.\r\n- Renames variables and adds
comments for easier readability\r\n- adds unit test to component that
handles the install UI
state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen
<[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","ci:cloud-deploy","Team:Obs
AI
Assistant","ci:project-deploy-elasticsearch","ci:project-deploy-observability","backport:version","v8.18.0","v8.17.1"],"title":"[Obs
AI Assistant] fix knowledge base installation
state","number":206130,"url":"https://github.com/elastic/kibana/pull/206130","mergeCommit":{"message":"[Obs
AI Assistant] fix knowledge base installation state
(#206130)\n\nResolves
https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to
account for knowledge base `/setup` no longer polling\r\nfor model
readiness before returning.\r\n\r\n- Currently we only poll `/status` if
user manually installs the\r\nknowledge base. In cases where we auto
installed, such as after\r\nsuccessfully setting up a connector, we
depended on `/setup` to poll\r\ninternally. Since the latter was
removed, we need to always poll\r\n`/status`, otherwise user could
potentially be in the state where\r\n`setup` has finished (considered
installed) but `status` still reports\r\nnot ready and we show the
install message again (see screenshots
in\r\nhttps://github.com//issues/205970)\r\n- Currently if
an install is in progress and user closes the flyout, the\r\nprogress
state is lost. These changes should continue to reflect
the\r\ninstallation progress in the UI.\r\n- Renames variables and adds
comments for easier readability\r\n- adds unit test to component that
handles the install UI
state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen
<[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1"}},"sourceBranch":"main","suggestedTargetBranches":["8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/206130","number":206130,"mergeCommit":{"message":"[Obs
AI Assistant] fix knowledge base installation state
(#206130)\n\nResolves
https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to
account for knowledge base `/setup` no longer polling\r\nfor model
readiness before returning.\r\n\r\n- Currently we only poll `/status` if
user manually installs the\r\nknowledge base. In cases where we auto
installed, such as after\r\nsuccessfully setting up a connector, we
depended on `/setup` to poll\r\ninternally. Since the latter was
removed, we need to always poll\r\n`/status`, otherwise user could
potentially be in the state where\r\n`setup` has finished (considered
installed) but `status` still reports\r\nnot ready and we show the
install message again (see screenshots
in\r\nhttps://github.com//issues/205970)\r\n- Currently if
an install is in progress and user closes the flyout, the\r\nprogress
state is lost. These changes should continue to reflect
the\r\ninstallation progress in the UI.\r\n- Renames variables and adds
comments for easier readability\r\n- adds unit test to component that
handles the install UI
state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen
<[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/206777","number":206777,"state":"OPEN"},{"branch":"8.17","label":"v8.17.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
viduni94 pushed a commit to viduni94/kibana that referenced this issue Jan 23, 2025
)

Resolves elastic#205970

Updates logic to account for knowledge base `/setup` no longer polling
for model readiness before returning.

- Currently we only poll `/status` if user manually installs the
knowledge base. In cases where we auto installed, such as after
successfully setting up a connector, we depended on `/setup` to poll
internally. Since the latter was removed, we need to always poll
`/status`, otherwise user could potentially be in the state where
`setup` has finished (considered installed) but `status` still reports
not ready and we show the install message again (see screenshots in
elastic#205970)
- Currently if an install is in progress and user closes the flyout, the
progress state is lost. These changes should continue to reflect the
installation progress in the UI.
- Renames variables and adds comments for easier readability
- adds unit test to component that handles the install UI state,
`WelcomeMessageKnowledgeBase`

---------

Co-authored-by: Søren Louv-Jansen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Obs AI Assistant Observability AI Assistant
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants