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

[ES|QL] Edits query in the dashboard #167754

Closed
wants to merge 42 commits into from

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Oct 2, 2023

Summary

Part of #165928
Closes #144498

Allows the user to edit the ES|QL query from the dashboard. Also allows the user to select one of the suggestions.

image image

Testing

Navigate to Discover ES|QL mode and save a Lens chart to a dashboard. Click the edit Visualization.

Important notes

  • We can very easily enable suggestions for the dataview panels but I am going to do it on a follow up PR to keep this PR clean
  • Creation is going to be added on a follow up PR
  • Warnings are not rendered in the editor because I am using the limit 0 for performance reasons. We need to find another way to depict them via the embeddable or store. It will be on a follow up PR.
  • Errors are being displayed though. The user is not allowed to apply the changes when an error occurs.

Running queries which don't return numeric fields

In these cases (i.e. from logstash-* | keep clientip we are returning a table. I had to change the datatable logic for text based datasource to not depend to isBucketed flag. This is something we had foreseen from the beginning of text based languages

image

Running queries which return a lot of fields

For queries with many fields Lens is going to suggest a huge table trying to add the fields to the different dimensions. This is not something we want:

  • not performant
  • user possibly will start removing fields from the dimensions
  • this table is unreadable

For this reason we decided to select the first 5 fields and then the user can easily adjust the dimensions with the fields they want.

image

Checklist

stratoula added a commit that referenced this pull request Oct 18, 2023
… popover (#169193)

## Summary

Leftover from my ES|QL implementation. We don't need to rerun the query
when a user clicks the warning popover. I am removing this here.

Note: I still keep it on the footer component because I will need this
here #167754
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 18, 2023
… popover (elastic#169193)

## Summary

Leftover from my ES|QL implementation. We don't need to rerun the query
when a user clicks the warning popover. I am removing this here.

Note: I still keep it on the footer component because I will need this
here elastic#167754

(cherry picked from commit 2ebb325)
hop-dev pushed a commit to hop-dev/kibana that referenced this pull request Oct 18, 2023
… popover (elastic#169193)

## Summary

Leftover from my ES|QL implementation. We don't need to rerun the query
when a user clicks the warning popover. I am removing this here.

Note: I still keep it on the footer component because I will need this
here elastic#167754
kibanamachine referenced this pull request Oct 18, 2023
…g/error popover (#169193) (#169214)

# Backport

This will backport the following commits from `main` to `8.11`:
- [[ES|QL] Do not refresh the query when a user clicks the warning/error
popover (#169193)](#169193)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Stratoula
Kalafateli","email":"efstratia.kalafateli@elastic.co"},"sourceCommit":{"committedDate":"2023-10-18T09:43:50Z","message":"[ES|QL]
Do not refresh the query when a user clicks the warning/error popover
(#169193)\n\n## Summary\r\n\r\nLeftover from my ES|QL implementation. We
don't need to rerun the query\r\nwhen a user clicks the warning popover.
I am removing this here.\r\n\r\nNote: I still keep it on the footer
component because I will need this\r\nhere
https://github.com/elastic/kibana/pull/167754","sha":"2ebb325d24cef862295008e881c11367e0c00519","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Visualizations","release_note:skip","backport:prev-minor","v8.11.0","Feature:ES|QL","v8.12.0"],"number":169193,"url":"https://github.com/elastic/kibana/pull/169193","mergeCommit":{"message":"[ES|QL]
Do not refresh the query when a user clicks the warning/error popover
(#169193)\n\n## Summary\r\n\r\nLeftover from my ES|QL implementation. We
don't need to rerun the query\r\nwhen a user clicks the warning popover.
I am removing this here.\r\n\r\nNote: I still keep it on the footer
component because I will need this\r\nhere
https://github.com/elastic/kibana/pull/167754","sha":"2ebb325d24cef862295008e881c11367e0c00519"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/169193","number":169193,"mergeCommit":{"message":"[ES|QL]
Do not refresh the query when a user clicks the warning/error popover
(#169193)\n\n## Summary\r\n\r\nLeftover from my ES|QL implementation. We
don't need to rerun the query\r\nwhen a user clicks the warning popover.
I am removing this here.\r\n\r\nNote: I still keep it on the footer
component because I will need this\r\nhere
https://github.com/elastic/kibana/pull/167754","sha":"2ebb325d24cef862295008e881c11367e0c00519"}}]}]
BACKPORT-->

Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
@@ -142,6 +144,7 @@ export const useLensSuggestions = ({
currentSuggestion: histogramSuggestion ?? currentSuggestion,
suggestionUnsupported: !currentSuggestion && !histogramSuggestion && isPlainRecord,
isOnHistogramMode: Boolean(histogramSuggestion),
histogramQuery: histogramQuery.current ? { esql: histogramQuery.current } : undefined,
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ This is done to communicate correctly the histogram query (as it is not the same that Discover uses aka the unified search query)

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 1133 1136 +3

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/text-based-editor 21 10 -11
lens 526 531 +5
textBasedLanguages 20 9 -11
total -17

Async chunks

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

id before after diff
lens 1.4MB 1.4MB +7.1KB
textBasedLanguages 148.9KB 150.2KB +1.4KB
unifiedHistogram 50.3KB 50.5KB +193.0B
total +8.6KB

Page load bundle

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

id before after diff
lens 37.2KB 37.4KB +148.0B
Unknown metric groups

API count

id before after diff
@kbn/text-based-editor 22 25 +3
lens 625 630 +5
textBasedLanguages 20 23 +3
total +11

History

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

@stratoula
Copy link
Contributor Author

closing in favor of #169911

@stratoula stratoula closed this Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens] Adjust the visualizations to work without the isBucketed flag
3 participants