[ES|QL] Technical preview in Discover and Dashboards #146971

stratoula · 2022-12-05T09:38:59Z

Summary

This PR is the first iteration of the new ES|QL language in kibana. The majority of the functionality is based on the existing functionality for SQL (which has already been merged). This builds on top of it to enable the functionality for ES|QL. We decided to remove SQL for now so this PR enables ES|QL and hides SQL. We are not removing the code for now.

Important notes:

This PR contains basic autocomplete functionality for the ES|QL commands. We want to improve it in follow up PRs
The majority of the tests for SQL were moved to work with ES|QL instead
The search strategy is a very simple endpoint for now as we don't have async search nor pagination
Now that we remove SQL, the ui for ES|QL selection has changed
The documentation for ESQL has been handled by the docs team so it is already reviewed and in sync with the official documentation
ES|QL is disabled in serverless projects for now

Changes from SQL:

The Discover histogram now is being created with ES|QL (using the date_trunc function). This gives it the ability to be saved on a dashboard and also to be edited inline.
ES|QL sometimes returns some warnings (on the search headers). For example when we are trying to date parse a string that doesnt contain a valid date). These warnings are also reported on the UI. There is a bug in ES and the warning doesnt come always https://github.com/elastic/elasticsearch-internal/issues/1465)
We have 2 types of commands:
- Transformational commands (keep, stats) -> when they exist in the query Discover should render the Table view (selected columns)
- Non transformational commands (all the others) -> in that case Discover renders the Document view
- ESQL switch on advanced settings is now on by default

Discover view with non transformational commands

Discover view with transformational commands

Missing

ESQL autocomplete is not perfect, we are going to work on it on a future PR. Specifically:
- There is a bug in autocomplete, sometimes writing a query overwrites the existing one, will deal with it in a future PR
- Further improvements

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

github-actions · 2022-12-05T09:39:12Z

Documentation preview:

✨ Changed pages

…-fix'

## Summary Autocomplete support for ESQL lang. ## Notes: Important: please do `yarn kbn clean & yarn kbn bootstrap` before testing. ## How to update syntax. `antlr` syntax was copied from `ES` and was slightly modified. In case if you need to update it please follow next steps: - modify `esql_parser.g4 `and/or `esql_lexer.g4` files - go to `kbn-monaco` package and execute `bazel clean & npm run build:antlr4ts:painless` - go to /painless_parser.ts file and revert the following change: <img width="478" alt="image" src="https://user-images.githubusercontent.com/20072247/209540586-bb77cad1-a6f0-42fa-9875-025bd4afbace.png"> - do `yarn kbn bootstrap` ### Checklist Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Stratoula Kalafateli <[email protected]>

…fix'

docs/management/advanced-options.asciidoc

Co-authored-by: Matthias Wilhelm <[email protected]>

davismcphee

Did an initial code review and it's looking really good overall! I didn't see anything that jumped out to me as an issue, and mostly just left some minor suggestions and nits for now. Will followup soon with feedback from my local testing.

src/plugins/data/common/search/expressions/esql.ts

src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.tsx

test/functional/apps/discover/group2/_sql_view.ts

packages/kbn-text-based-editor/src/helpers.ts

src/plugins/unified_search/public/dataview_picker/change_dataview.tsx

x-pack/plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx

x-pack/plugins/lens/public/datasources/text_based/utils.test.ts

stratoula · 2023-08-31T06:35:53Z

@davismcphee thanx a ton for the thorough code review! I have addressed everything here 6b75898

dej611

Code review 👍

Left few nitpick comments but not a blocker 🎉

packages/kbn-text-based-editor/src/text_based_languages_editor.styles.ts

src/plugins/discover/public/application/main/components/layout/discover_documents.tsx

stratoula · 2023-08-31T08:21:57Z

Thanx @dej611, applied your comments here d81042e

davismcphee

Fantastic work on this, it's so exiting to finally see ES|QL in Discover! I tested locally pretty extensively today and for the most part didn't encounter any broken functionality or major bugs. I did notice two main issues though:

You mentioned this in the summary regarding autocomplete issues:

There is a bug in autocomplete, sometimes writing a query overwrites the existing one, will deal with it in a future PR

I ran into this quite often in my testing where after autocompleting, the ES|QL editor would be completely bugged out and overwrite characters, not accept characters, lose syntax highlighting, or even get out of sync. It sounds like this is a known issue with plans to improve it, so I won't block the merge over it, but as a user it was quite frustrating, so hopefully we can get it sorted quickly.

I also noticed a lot of scenarios where duplicate requests are being fired in ES|QL mode, sometimes up to 3 extra fetches at once. Again since it's technical preview, I'll leave it up to you to decide if this is something we're ok living with for now (data views seem unaffected which is the main thing to me), but I would definitely suggest we prioritize addressing this soon.

I see there's already an issue for some of the extra fetches in #165192, but I noticed a number of other scenarios in my testing too:

1 extra fetch on chart hide
1 extra fetch on page reload when a chart is visible
1 extra fetch when changing from an index where the vis is hidden to one where it's shown
3 extra fetches when navigating back from Dashboard to Discover after adding an ES|QL Discover vis
3 extra fetches when saving or opening a saved search

Maybe we can add these cases to #165192 and expand it into an issue for addressing the extra fetch issue more broadly? I may be able to help with some of this once I can find some time for it.

One thing I noticed that I think existed before this PR but would be good to make a note to fix is the disabled time picker tooltip uses esql instead of ES|QL:

Another thing that would be good followup work for another PR is adding some details to the ES|QL request for the inspector. I saw you mentioned the ES|QL search implementation is pretty basic for now, maybe something to consider when evolving this:

Otherwise I created a very small PR with minor cleanup of the remaining references to SQL I found in Discover and Unified Histogram (no functionality changes): #165426. Feel free to merge to this PR if you want, or I can point it at main instead once this is merged.

And lastly I left a couple of suggestions for places where additional unit tests would be ideal. I get there's some urgency on this PR so again I won't block on it, but it would be great to consider improving the testing in those areas when we can.

My only other feedback is that it would be great to get a quick review from @lukasolson on the search changes if possible since he's most familiar with the area on our team.

Thanks again for all the effort on this @stratoula! This really is an amazing feature and milestone for Kibana and Discover 🙌

src/plugins/unified_histogram/public/layout/hooks/use_lens_suggestions.ts

src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts

stratoula · 2023-09-01T06:57:40Z

@davismcphee thanx a lot for the review

I also noticed a lot of scenarios where duplicate requests are being fired in ES|QL mode, sometimes up to 3 extra fetches at once.
I updated the issue. We should def look into this but I will need some help so we can look into it together when you have some time

One thing I noticed that I think existed before this PR but would be good to make a note to fix is the disabled time picker tooltip uses esql instead of ES|QL:

Updated!

Another thing that would be good followup work for another PR is adding some details to the ES|QL request for the inspector. I saw you mentioned the ES|QL search implementation is pretty basic for now, maybe something to consider when evolving this:

The ES team wants to work on async search of the esql api. At this point we can discuss together how we want to implement it with some help from @lukasolson as he knows this area better and we can revisit the inspector. We can also have a follow up PR if Lukas think that we can do something better on the search changes.

Otherwise I created a very small PR with minor cleanup of the remaining references to SQL I found in Discover and Unified Histogram (no functionality changes)

Thanx a ton for this! I will merge when CI is green

And lastly I left a couple of suggestions for places where additional unit tests would be ideal. I get there's some urgency on this PR so again I won't block on it, but it would be great to consider improving the testing in those areas when we can.

I created an issue and we can deal with it on followup PR(s)

stratoula · 2023-09-01T07:30:56Z

@elasticmachine merge upstream

## Summary This PR updates the remaining references to SQL with ES|QL in Discover and Unified Histogram. Co-authored-by: Stratoula Kalafateli <[email protected]>

kibana-ci · 2023-09-01T08:48:47Z

💔 Build Failed

Failed CI Steps

Test Failures

[job] [logs] Serverless Observability Tests / serverless common API security/api_keys route access internal "before all" hook for "create"
[job] [logs] Serverless Observability Tests / serverless common UI home page has project header

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`data`	472	476	+4
`textBasedLanguages`	51	53	+2
`unifiedHistogram`	61	62	+1
`unifiedSearch`	214	213	-1
total			+6

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/es-query`	195	199	+4
`@kbn/monaco`	69	72	+3
`@kbn/text-based-editor`	14	15	+1
`data`	2583	2584	+1
`expressions`	1746	1749	+3
`textBasedLanguages`	17	18	+1
total			+13

Async chunks

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

id	before	after	diff
`discover`	504.0KB	504.6KB	+591.0B
`lens`	1.4MB	1.4MB	+114.0B
`textBasedLanguages`	68.6KB	130.1KB	+61.5KB
`unifiedHistogram`	46.8KB	48.5KB	+1.8KB
`unifiedSearch`	215.4KB	214.6KB	-794.0B
total			+63.2KB

Page load bundle

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

id	before	after	diff
`data`	404.6KB	408.3KB	+3.7KB
`discover`	29.6KB	29.6KB	+1.0B
`kbnUiSharedDeps-srcJs`	2.2MB	2.2MB	+17.8KB
`textBasedLanguages`	5.0KB	5.5KB	+490.0B
`unifiedSearch`	34.8KB	34.8KB	+60.0B
total			+22.1KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/es-query`	255	259	+4
`@kbn/monaco`	71	74	+3
`@kbn/text-based-editor`	15	16	+1
`data`	3310	3311	+1
`expressions`	2205	2208	+3
`textBasedLanguages`	17	18	+1
total			+13

async chunk count

id	before	after	diff
`textBasedLanguages`	2	3	+1
`unifiedSearch`	21	20	-1
total			-0

ESLint disabled line counts

id	before	after	diff
`unifiedSearch`	25	24	-1

Total ESLint disabled count

id	before	after	diff
`unifiedSearch`	25	24	-1

History

💔 Build #155535 failed d6b65c4
💔 Build #155526 failed 5a5a20b
💛 Build #155086 was flaky d81042e
💔 Build #155066 failed 6b75898

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

stratoula · 2023-09-01T09:21:09Z

I am merging it as the serverless obs tests failures are irrelevant with this PR changes

[ESQL] Enable in Discover and Lens

551b23d

stratoula mentioned this pull request Dec 5, 2022

[ESQL] Integration with Lens and Discover #145803

Closed

8 tasks

stratoula and others added 24 commits December 5, 2022 11:46

Fix check

4097f02

Further checks

bd99349

Fix file name

60c2e97

Merge with main and resolve conflicts

a4ba500

Fix theme

5fd2a0a

Merge branch 'main' into feature-esql

6bb412e

Merge branch 'main' into feature-esql

e51766e

Merge with main and resolve conflicts

f6b2854

Enable inspector requests for ESQL

007958e

Merge remote-tracking branch 'upstream/main' into feature-esql

e9e9240

[CI] Auto-commit changed files from 'node scripts/ts_project_linter -…

0caf41c

…-fix'

Merge with main and resolve conflicts

6112321

Merge branch 'main' into feature-esql

c9c80c4

Fix the wrong decoration when the code is populated with ...

3c557f3

Merge with main and resolve conflicts

4214810

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

d812ac7

…fix'

Fix by command autocomplete

07fed9e

Make autocomplete smarter

fc0f244

Make the text dynamic

9e2689d

Fix translations

9c9b6d8

Fix checks

834426d

Merge branch 'main' into feature-esql

f1b5a8d

Fix autocomplete in multilines

945a5c7

stratoula marked this pull request as ready for review January 23, 2023 07:14

stratoula requested review from a team as code owners January 23, 2023 07:14

Merge branch 'main' into feature-esql

f110566

rshen91 approved these changes Aug 29, 2023

View reviewed changes

ninoslavmiskovic reviewed Aug 30, 2023

View reviewed changes

docs/management/advanced-options.asciidoc Show resolved Hide resolved

kertal reviewed Aug 30, 2023

View reviewed changes

docs/management/advanced-options.asciidoc Outdated Show resolved Hide resolved

Update docs/management/advanced-options.asciidoc

d8565bc

Co-authored-by: Matthias Wilhelm <[email protected]>

stratoula mentioned this pull request Aug 30, 2023

[Unified histogram][ES|QL] Extra fetches in specific scenarios #165192

Open

stratoula added 2 commits August 30, 2023 11:40

Add a todo in the request_count FT

7527020

Merge branch 'main' into feature-esql

b937d2a

davismcphee reviewed Aug 31, 2023

View reviewed changes

Address code review comments

6b75898

Merge branch 'main' into feature-esql

2089884

dej611 approved these changes Aug 31, 2023

View reviewed changes

packages/kbn-text-based-editor/src/text_based_languages_editor.styles.ts Outdated Show resolved Hide resolved

src/plugins/discover/public/application/main/components/layout/discover_documents.tsx Outdated Show resolved Hide resolved

Apply PR comments

d81042e

davismcphee approved these changes Sep 1, 2023

View reviewed changes

stratoula added 2 commits September 1, 2023 09:34

Merge branch 'main' into feature-esql

34a8d0a

Update tooltip to display es|ql

5a5a20b

stratoula mentioned this pull request Sep 1, 2023

[ES|QL] Add more tests in unified_histogram #165431

Closed

Merge branch 'main' into feature-esql

d6b65c4

sorenlouv mentioned this pull request Sep 1, 2023

[APM] Add ES|QL support #165432

Open

Replace the remaining references to SQL with ES|QL (#165426)

a2d9f7f

## Summary This PR updates the remaining references to SQL with ES|QL in Discover and Unified Histogram. Co-authored-by: Stratoula Kalafateli <[email protected]>

stratoula merged commit 5d68129 into main Sep 1, 2023

stratoula deleted the feature-esql branch September 1, 2023 09:22

kertal mentioned this pull request Sep 12, 2023

[Discover] Group by field(s) #103008

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ES|QL] Technical preview in Discover and Dashboards #146971

[ES|QL] Technical preview in Discover and Dashboards #146971

stratoula commented Dec 5, 2022 •

edited

Loading

github-actions bot commented Dec 5, 2022

davismcphee left a comment

stratoula commented Aug 31, 2023

dej611 left a comment

stratoula commented Aug 31, 2023

davismcphee left a comment

stratoula commented Sep 1, 2023 •

edited

Loading

stratoula commented Sep 1, 2023

kibana-ci commented Sep 1, 2023 •

edited

Loading

API count

async chunk count

ESLint disabled line counts

Total ESLint disabled count

stratoula commented Sep 1, 2023

[ES|QL] Technical preview in Discover and Dashboards #146971

[ES|QL] Technical preview in Discover and Dashboards #146971

Conversation

stratoula commented Dec 5, 2022 • edited Loading

Summary

Important notes:

Changes from SQL:

Missing

Checklist

github-actions bot commented Dec 5, 2022

davismcphee left a comment

Choose a reason for hiding this comment

stratoula commented Aug 31, 2023

dej611 left a comment

Choose a reason for hiding this comment

stratoula commented Aug 31, 2023

davismcphee left a comment

Choose a reason for hiding this comment

stratoula commented Sep 1, 2023 • edited Loading

stratoula commented Sep 1, 2023

kibana-ci commented Sep 1, 2023 • edited Loading

💔 Build Failed

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

async chunk count

ESLint disabled line counts

Total ESLint disabled count

History

stratoula commented Sep 1, 2023

stratoula commented Dec 5, 2022 •

edited

Loading

stratoula commented Sep 1, 2023 •

edited

Loading

kibana-ci commented Sep 1, 2023 •

edited

Loading