forked from elastic/kibana
-
Couldn't load subscription status.
- Fork 0
[ES|QL] Correctly parse command option positions #3
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
Draft
vadimkibana
wants to merge
22
commits into
comment-printer-5
Choose a base branch
from
esql-option-location-3
base: comment-printer-5
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Summary I noticed that "september" doesn't look nice as it wraps:  I suggest we use a shorter format: 
…lastic#192866) Resolves: elastic/response-ops-team#150 This PR adds a new telemetry to the alerting plugin. The task collects the below data and saves in the task state. `count_alerts_total` total alerts of all time `count_alerts_by_rule_type` total alerts of all time by rule types Note: I tried to use the stats API as Brandon suggested in the issue, but it just returns the total number of alerts. We have to use aggregations for alerts by rule types. Therefore I didn't use it. ## To verify: - Change the task interval to 1m on [this line](https://github.com/elastic/kibana/pull/192866/files#diff-014c1a7c63ade0d0f548523ef161369fdeb21c589a8112f202b8086ca23af6fdL32) - Run Kibana - Create some rules that generates alerts. - Let them run for a while. - Check the saved alerting telemetry in the task state by using the below query. - `count_alerts_total`and `count_alerts_by_rule_type` should be populated ``` GET /.kibana_task_manager_*/_search { "query": { "term": { "task.taskType": "alerting_telemetry" } }, "size" : 1 } ```
## Summary This fixes Skipping flaky test to fix elastic#194044 A follow up ticket was created to re-enable it later: elastic#194046
…lastic#192665) ## Summary This PR updates the Knowledge Base Management Settings page to use the new `entries` API introduced in elastic#186566. Many thanks to @angorayc for her work on the Assistant Management Settings overhaul, and initial implementation of this new KB Management UI over in elastic#186847. <p align="center"> <img width="600" src="https://github.com/user-attachments/assets/0a82587e-f33c-45f1-9165-1a676d6db5fa" /> </p> ### Feature Flag & Setup The changes in this PR, as with the other [recent V2 KB enhancements](elastic#186566), are behind the following feature flag: ``` xpack.securitySolution.enableExperimental: - 'assistantKnowledgeBaseByDefault' ``` ~They also require a code change in the `AIAssistantService` to enable the new mapping (since setup happens on plugin start before FF registration), so be sure to update `fieldMap` to `knowledgeBaseFieldMapV2` below before testing:~ This is no longer the case as of [cdec104](elastic@cdec104). Just changing the above feature flag is now sufficient, just note that if upgrading and the KB was previously setup, you'll need to manually delete the data stream (`DELETE /_data_stream/.kibana-elastic-ai-assistant-knowledge-base-default`) or the management table will be littered with the old ESQL docs instead of being a single aggregate entry. Once configured, the new Knowledge Base Management Settings will become available in Stack Management. The old settings UI is currently still available via the Settings Modal, but will soon be removed and replaced with links to the new interface via the Assistant Settings Context Menu (replacing the existing `cog`). Please see the designs ([Security GenAI](https://www.figma.com/design/BMvpY9EhcPIaoOS7LSrkL0/%5B8.15%2C-%5D-GenAI-Security-Settings?node-id=51-25207&node-type=canvas&t=t3vZSPhMxQhScJVt-0) / [Unified AI Assistant](https://www.figma.com/design/xN20zMRNtMlirWB6n9n1xJ/Unified-AI-Assistant-Settings?node-id=0-1&node-type=canvas&t=3RDYE7h2DjLlFlcN-0)) for all changes. > [!IMPORTANT] > There are no migrations in place between the legacy and v2 KB mappings, so be sure to start with a clean ES data directory. ### Testing To aid with developing the UI, I took the opportunity to start fleshing out the KB Entries API integration tests. These live in [x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries](https://github.com/spong/kibana/tree/7ae6be136ad992b2163df13b55118556b01b6cb9/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries), and are currently configured to only run on `@ess`, as running `tiny_elser` in serverless and MKI environments can be tricky (more on that later). To start the server and run the tests, from the `x-pack/test/security_solution_api_integration/` directory run `yarn genai_kb_entries:server:ess`, and once started, `yarn genai_kb_entries:runner:ess`. ##### Changes in support of testing In order to setup the API integration tests for use with the Knowledge Base, some functional changes needed to be made to the assistant/config: 1. Since ELSER is a heavy model to run in CI, the ML folks have created `pt_tiny_elser` for use in testing. Unfortunately, the `getELSER()` helper off the `ml` client that we use to get the `modelld` for installing ELSER, ingest pipelines, etc, cannot be overridden ([elastic#193633](elastic#193633)), so we must have some other means of doing that. So to get things working in the test env, I've plumbed through an optional `modelId` override to the POST knowledge base route (`/ internal/ elastic_assistant/ knowledge_base/{resource?}?modelId=pt_tiny_elser`). This then overrides the aiAssistantService `getELSER()` function [when fetching](https://github.com/elastic/kibana/blob/645b3b863be16d70b8a7130a84b248c19729c340/x-pack/plugins/elastic_assistant/server/ai_assistant_service/index.ts#L334-L354) a `kbDataClient` using the request, which appears to be the only way to also trigger a reinitialization of the ingest pipeline (which required the `modelId`), since that usually only occurs on plugin start. If there is a cleaner way to perform this reinitialization, please let me know! 2. Turns out [`getService('ml').importTrainedModel()`](https://github.com/elastic/kibana/blob/f18224c6869ae52228da3764ca9a427106b872fb/x-pack/test/functional/services/ml/api.ts#L1575-L1587) can't be run in test env's with `ssl:true`, which is the default security config. You can read more about that issue in [elastic#193477](elastic#193477), but the current workaround is to turn off `ssl` for this specific test configuration, so that's why [`ess.config.ts`](https://github.com/spong/kibana/blob/cf73d4c7fcd69207a9625046456a94212da833c7/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries/trial_license_complete_tier/configs/ess.config.ts#L22) looks a little different. If there's a better way to manage this config, also please let me know! ##### Additional notes We don't currently have a `securityAssistant` API client/service to use in integration tests, so I've just been creating one-off functions using `supertest` for now. I don't have the bandwidth to work this now, but perhaps @MadameSheema / @muskangulati-qasource could lend a hand here? I did need to test multi-user and multi-space scenarios, so I ported over the same [auth helpers](https://github.com/elastic/kibana/tree/dc26f1012f35c2445028a87dcc8cb3f063e058b0/x-pack/test/security_solution_api_integration/test_suites/genai/knowledge_base/entries/utils/auth) I saw used in other suites. Would be nice if these were bundled into the client as well ala how the o11y folks have done it [here](https://github.com/elastic/kibana/blob/e9f23aa98e3abadd491be61b17e7daa3cc110cdb/x-pack/test/observability_ai_assistant_api_integration/tests/knowledge_base/knowledge_base.spec.ts#L27-L34). Perhaps this is also on the list of things for @maximpn to generate from OAS's.... 🙃 ### RBAC In plumbing the UI, I've tried to place `// TODO: KB-RBAC` tags in all the places I came across that will require an RBAC check/change. This includes some of the API integration tests, which I currently have skipped as they would fail without RBAC. ### Other notable changes * There are now dedicated `legacy` and `v2` helper functions when managing persistence/retrieval of knowledge base entries. This should help with tearing out the old KB later, and better readability now. * I've tried to remove dependency on the `ElasticsearchStore` as much as possible. The store's only use should now be within tools as a retriever [here](https://github.com/elastic/kibana/blob/de89153368848397df823c062e907a607d347dff/x-pack/plugins/elastic_assistant/server/routes/helpers.ts#L397-L405), and in post_evaluate [here](https://github.com/elastic/kibana/blob/de89153368848397df823c062e907a607d347dff/x-pack/plugins/elastic_assistant/server/routes/evaluate/post_evaluate.ts#L170-L179). If we adopt the new [`naturalLanguageToESQL`](elastic#192042) tool in `8.16` (or update our existing ESQL tool to use the `kbDataClient` for retrieval), we should be able to get rid of this entirely. * Added a [`spaces_roles_users_data.http`](https://github.com/elastic/kibana/blob/7447394fe39d5e2e098c266c14875d3aa17d3067/x-pack/packages/kbn-elastic-assistant-common/impl/utils/spaces_roles_users_data.http#L1) file for adding spaces, roles, users, and a sample `slackbot` index for use with [sample `IndexEntries` here](https://github.com/elastic/kibana/blob/7447394fe39d5e2e098c266c14875d3aa17d3067/x-pack/packages/kbn-elastic-assistant-common/impl/schemas/knowledge_base/entries/crud_knowledge_base_entries_route.http#L18-L56). ### // TODO In effort to make incremental progress and facilitate early knowledge share with @patrykkopycinski, I'm capping this PR where it's at, and so here are the remaining items to complete full integration of the new Knowledge Base Management Settings interface: - [ ] Support `Update` action - [ ] Move from `EuiInMemoryTable` - [ ] Finalize `Setup` UI - [ ] Cleanup `Save` loaders - [ ] Plumb through `{{knowledge_history}}` prompt template and include use's `required` entries All this work is behind the aforementioned feature flag and required code change, and this changeset has also been manually upgrade tested to ensure there are no issues that would impact the regularly scheduled serverless releases. This is more of a note to reviewers when testing that full functionality is not present. ### Checklist - [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials * Feature currently behind feature flag. Documentation to be added before flag is removed. Tracked in elastic/security-docs#5337 - [X] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
…astic#194060) ## Summary Updating the search "Dev Tools" link from using dev_tools:console to just dev_tools. The console deeplink is missing from the deeplinks definition sometimes and causing the side nav item to be removed.
…ng (elastic#193909) Fixes [https://github.com/elastic/kibana/security/code-scanning/456](https://github.com/elastic/kibana/security/code-scanning/456) To fix the problem, we need to ensure that backslashes are also escaped in the `value` string. This can be done by first replacing backslashes with double backslashes and then replacing double quotes with escaped double quotes. This ensures that all occurrences of backslashes and double quotes are properly escaped. - Modify the `value.replace` call to first escape backslashes and then escape double quotes. - The changes will be made in the `createFilterFromOptions` function, specifically on line 128. _Suggested fixes powered by Copilot Autofix. Review carefully before merging._ Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
## Summary Closes elastic#194051 As far as I can tell from the docs the `unlabeled` event sends information about the removed label. I'm not sure the best way to test this aside from merging the PR and adjusting the labels on this PR afterwards. Reverting if needed. Plan: 1. Merge PR 2. Add `backport:prev-minor` 3. Verify the workflow ran and _did not_ trigger a backport since `backport:skip` is still a label 4. Remove `backport:skip` 5. Verify the workflow ran and _did_ trigger a backport to `8.x` 6. Revert the labels and close the backport PR since it is not actually needed.
…194008) Relates to elastic/observability-dev#3873. This PR aims to enable filter type to be easily targeted from FullStory.
…t view (elastic#192299) - Closes elastic#163275 ## Summary With this PR users can search by field value (raw and formatted) in DocViewer flyout. <img width="569" alt="Screenshot 2024-09-06 at 20 49 57" src="https://github.com/user-attachments/assets/27e89017-4b8f-437b-9e00-b9a24eb88184"> <img width="544" alt="Screenshot 2024-09-06 at 20 50 09" src="https://github.com/user-attachments/assets/5086af87-df97-4fdb-84c3-f30c547678e0"> <img width="536" alt="Screenshot 2024-09-12 at 18 45 11" src="https://github.com/user-attachments/assets/abdfe31d-85a5-41de-bf05-88ecf0ac2b18"> ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: kibanamachine <[email protected]>
… compatible with 9.0.0 (elastic#193856) ## Summary Part of elastic/kibana-team#1113
…ions, connectors, rules, alerts, and cases plugins (elastic#193520) ## Summary Resolves elastic#192956 This PR adds - `access: internal` option to internal routes - `access: public` option to public routes It which will help restrict access of internal routes and allow users to access all public routes. This PR updates api routes of following `x-pack/plugins` - actions - alerting - cases - rule_registry - stack_connectors - triggers_actions_ui
## Summary
TL;DR
- Adds ability to parse out comments from source to AST.
- Adds ability for every AST node to have *decoration*—comments,
which can be attached from left, top, and right from the node.
- Implements routine which attached comments to AST nodes.
- In `BasicPrettyPrinter` adds support only for *left* and *right*
comment printing, as the basic printer prints only on one line.
- In `WrappingPrettyPrinter` adds support for all comment printing for
all AST nodes.
- Introduces a `Query` object and `query` AST node, which represent
thole query—the root node, list of commands.
- The ES|QL AST example plugin now displays the pretty-printed text
version.
### Comments
This PR introduced an optional `formatting` field for all AST nodes. In
the `formatting` field one can specify comment decorations from
different sides of a node.
When parsing, once can now specify the `{ withComments: true }` option,
which will collect all comments from the source while parsing using the
`collectDecorations` routine. It will then also call the
`attachDecorations`, which walks the AST and assigns each comment to
some AST node.
Further, traversal and pretty-print API have been updated to work with
comments:
- The `Walker` has been updated to be able to walk all comments from the
AST.
- The `BasicPrettyPrinter` adds support only for *left* and *right*
inline comment printing, as the basic printer prints only on one line.
- The `WrappingPrettyPrinter` adds support for all comment printing for
all AST nodes. It switches to line-break printing mode if it detects
there are comments with line breaks (those could be multi-line comments,
or single line comments—single line comments are always followed
by a line break). It also correctly inserts punctuation, when an AST
node is surrounded by comments.
### Parsing utils
All parsing utils have been moved to the `/parser` sub-folder.
Files in the `/parser` folder have been renamed as per Kibana convention
to reflect what is inside the file. For example, the `EsqlErrorListener`
class is in a file named `esql_error_listener.ts`.
A `Query` class and `ESQLAstQueryExpression` AST nodes have been
introduced. They represent the result of a full query parse. (Before
that, the AST root was just an array of command nodes, now the AST root
is represented by the `ESQLAstQueryExpression` node.)
### Builder
I have started the implementation of the `Builder` static class in the
`/builder` folder. It is simply a collection of stateless AST node
factories—functions which construct AST nodes.
Some of the `Builder` methods are already used by the parser, more will
follow. We will also use the `Builder` in upcoming [*Mutation
API*](elastic#191812).
### ES|QL Example Plugin
This PR sets up Storybook and implements few Storybook stories for the
ES|QL AST example plugin, run it with:
```
yarn storybook esql_ast_inspector
```
This PR updates the *ES|QL AST Explorer* example plugin. Start Kibana
with example plugins enabled:
```
yarn start --run-examples
```
And navigate to
[`/app/esql_ast_inspector`](http://localhost:5601/app/esql_ast_inspector)
to see the new example plugin UI.

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
### For maintainers
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
---------
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Closes elastic#192553
Checklist
Delete any items that are not applicable to this PR.
For maintainers