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

Extract AI assistant to package #194552

Merged
merged 30 commits into from
Oct 10, 2024

Conversation

sphilipse
Copy link
Member

@sphilipse sphilipse commented Oct 1, 2024

Summary

This extracts the Observability AI Assistant into a shared package so Search and Observability can both consume it.

A few notes:

This still relies on significantly tight coupling with the Obs AI assistant plugin, which we will want to slowly decouple over time. It means that currently to consume this in multiple places, you need to provide a number of plugins for useKibana. Hopefully we can get rid of that and replace them with props eventually and make the interface a little less plugin-dependent.

@sphilipse sphilipse added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 v8.16.0 backport:version Backport to applied version labels Team:Search labels Oct 1, 2024
@sphilipse sphilipse marked this pull request as ready for review October 1, 2024 15:57
@sphilipse sphilipse requested review from a team as code owners October 1, 2024 15:57
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:Obs AI Assistant Observability AI Assistant labels Oct 1, 2024
@elasticmachine
Copy link
Contributor

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

@sphilipse sphilipse requested review from afharo and removed request for afharo October 2, 2024 12:44
Copy link
Member

@afharo afharo left a comment

Choose a reason for hiding this comment

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

LGTM

@kibana-ci
Copy link
Collaborator

kibana-ci commented Oct 7, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
observabilityAIAssistant 105 106 +1
observabilityAIAssistantApp 247 251 +4
searchAssistant 4 173 +169
total +174

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/ai-assistant - 64 +64
observabilityAIAssistant 291 292 +1
total +65

Async chunks

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

id before after diff
observabilityAIAssistantApp 154.3KB 153.9KB -435.0B
searchAssistant 0.0B 97.1KB +97.1KB
total +96.7KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/ai-assistant - 1 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
observabilityAIAssistant 46.1KB 46.3KB +183.0B
observabilityAIAssistantApp 8.6KB 8.7KB +53.0B
searchAssistant 1.5KB 4.9KB +3.4KB
total +3.6KB
Unknown metric groups

API count

id before after diff
@kbn/ai-assistant - 65 +65
observabilityAIAssistant 293 294 +1
total +66

async chunk count

id before after diff
searchAssistant 0 1 +1

ESLint disabled line counts

id before after diff
@kbn/ai-assistant - 8 +8
observabilityAIAssistantApp 14 10 -4
total +4

miscellaneous assets size

id before after diff
observabilityAIAssistant 0.0B 92.9KB +92.9KB
observabilityAIAssistantApp 92.9KB 0.0B -92.9KB
searchAssistant 0.0B 161.8KB +161.8KB
total +161.8KB

Total ESLint disabled count

id before after diff
@kbn/ai-assistant - 8 +8
observabilityAIAssistantApp 14 10 -4
total +4

History

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

Copy link
Member

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

Apologies for the late feedback & thanks for doing this! My main concern is how we deal with context, perhaps we should get rid of useKibana().services and instead put the stuff we need on the AIAssistantAppServiceContext or whatever it is called?

Copy link
Member

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

approving but would like to see ? changed to ! so we can actually tell if something breaks

@sphilipse sphilipse enabled auto-merge (squash) October 10, 2024 11:13
@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 10, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
observabilityAIAssistant 105 106 +1
observabilityAIAssistantApp 247 250 +3
searchAssistant 4 172 +168
total +172

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/ai-assistant - 62 +62
observabilityAIAssistant 291 292 +1
total +63

Async chunks

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

id before after diff
observabilityAIAssistantApp 154.4KB 153.7KB -676.0B
searchAssistant 0.0B 96.9KB +96.9KB
total +96.3KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/ai-assistant - 1 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
observabilityAIAssistant 46.1KB 46.2KB +155.0B
observabilityAIAssistantApp 8.6KB 8.7KB +52.0B
searchAssistant 1.5KB 4.9KB +3.4KB
total +3.6KB
Unknown metric groups

API count

id before after diff
@kbn/ai-assistant - 62 +62
observabilityAIAssistant 293 294 +1
total +63

async chunk count

id before after diff
searchAssistant 0 1 +1

ESLint disabled line counts

id before after diff
@kbn/ai-assistant - 8 +8
observabilityAIAssistantApp 14 10 -4
total +4

miscellaneous assets size

id before after diff
observabilityAIAssistant 0.0B 92.9KB +92.9KB
observabilityAIAssistantApp 92.9KB 0.0B -92.9KB
searchAssistant 0.0B 161.8KB +161.8KB
total +161.8KB

Total ESLint disabled count

id before after diff
@kbn/ai-assistant - 8 +8
observabilityAIAssistantApp 14 10 -4
total +4

History

@sphilipse sphilipse merged commit 8a3a059 into elastic:main Oct 10, 2024
48 of 49 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

You might need to backport the following PRs to 8.x:
- [Cloud Security] Refactoring tests (#195675)
- [Authz] Adjusted forbidden message for new security route configuration (#195368)
- [Logs ML] Check permissions before granting access to Logs ML pages (#195278)
- [Inventory] Check permissions before registering the Inventory plugin in observabilityShared navigation (#195557)
- Remove kbn-ace, ace and brace dependencies (#195703)
- Fix theme switch success toast layout (#195717)
- [[ES

Manual backport

To create the backport manually run:

node scripts/backport --pr 194552

Questions ?

Please refer to the Backport tool documentation

@sphilipse
Copy link
Member Author

💚 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

sphilipse added a commit to sphilipse/kibana that referenced this pull request Oct 12, 2024
## Summary

This extracts the Observability AI Assistant into a shared package so
Search and Observability can both consume it.

A few notes:

This still relies on significantly tight coupling with the Obs AI
assistant plugin, which we will want to slowly decouple over time. It
means that currently to consume this in multiple places, you need to
provide a number of plugins for useKibana. Hopefully we can get rid of
that and replace them with props eventually and make the interface a
little less plugin-dependent.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 8a3a059)

# Conflicts:
#	.github/CODEOWNERS
sphilipse added a commit that referenced this pull request Oct 12, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [Extract AI assistant to package
(#194552)](#194552)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Sander
Philipse","email":"94373878+sphilipse@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-10-10T13:11:49Z","message":"Extract
AI assistant to package (#194552)\n\n## Summary\r\n\r\nThis extracts the
Observability AI Assistant into a shared package so\r\nSearch and
Observability can both consume it.\r\n\r\nA few notes:\r\n\r\nThis still
relies on significantly tight coupling with the Obs AI\r\nassistant
plugin, which we will want to slowly decouple over time. It\r\nmeans
that currently to consume this in multiple places, you need
to\r\nprovide a number of plugins for useKibana. Hopefully we can get
rid of\r\nthat and replace them with props eventually and make the
interface a\r\nlittle less
plugin-dependent.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"8a3a05927bdbe264c491b4034ff5d81674f3db73","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Search","Team:Obs
AI
Assistant","ci:project-deploy-observability","v8.16.0","backport:version"],"number":194552,"url":"https://github.com/elastic/kibana/pull/194552","mergeCommit":{"message":"Extract
AI assistant to package (#194552)\n\n## Summary\r\n\r\nThis extracts the
Observability AI Assistant into a shared package so\r\nSearch and
Observability can both consume it.\r\n\r\nA few notes:\r\n\r\nThis still
relies on significantly tight coupling with the Obs AI\r\nassistant
plugin, which we will want to slowly decouple over time. It\r\nmeans
that currently to consume this in multiple places, you need
to\r\nprovide a number of plugins for useKibana. Hopefully we can get
rid of\r\nthat and replace them with props eventually and make the
interface a\r\nlittle less
plugin-dependent.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"8a3a05927bdbe264c491b4034ff5d81674f3db73"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194552","number":194552,"mergeCommit":{"message":"Extract
AI assistant to package (#194552)\n\n## Summary\r\n\r\nThis extracts the
Observability AI Assistant into a shared package so\r\nSearch and
Observability can both consume it.\r\n\r\nA few notes:\r\n\r\nThis still
relies on significantly tight coupling with the Obs AI\r\nassistant
plugin, which we will want to slowly decouple over time. It\r\nmeans
that currently to consume this in multiple places, you need
to\r\nprovide a number of plugins for useKibana. Hopefully we can get
rid of\r\nthat and replace them with props eventually and make the
interface a\r\nlittle less
plugin-dependent.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"8a3a05927bdbe264c491b4034ff5d81674f3db73"}},{"branch":"8.x","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:Obs AI Assistant Observability AI Assistant Team:Search v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants