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] Supports counter fields in Discover sidebar #186154

Closed
wants to merge 4 commits into from

Conversation

stratoula
Copy link
Contributor

Summary

In 8.14 we added partial support of counter fields (you have to convert them to long first to use) but in the sidebar they were reported as unknown.

This PR asserts the correct kibana type for these fields.

image image

The fields statistics still dont work for this field but I am tracking this work here #186077

@stratoula stratoula added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.15.0 Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana labels Jun 13, 2024
@stratoula stratoula changed the title [ES|QL] Support counter fields in Discover sidebar [ES|QL] Supports counter fields in Discover sidebar Jun 13, 2024
@stratoula stratoula marked this pull request as ready for review June 13, 2024 06:30
@stratoula stratoula requested a review from a team as a code owner June 13, 2024 06:30
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-esql (Team:ESQL)

@stratoula
Copy link
Contributor Author

/ci

@jughosta
Copy link
Contributor

The changes works great in Discover ES|QL! I wonder how it would affect other parts of Kibana.

Currently, for data view fields the code relies on field.timeSeriesMetric being counter while field.type is number. With this change ES|QL column type is going to be counter and once transformed into a data view field format field.type becomes counter. Both formats make sense on it's own but this creates some inconsistency with what we already have and it can cause some unexpected behaviour in other apps which rely only on timeSeriesMetric.

From field caps:
Screenshot 2024-06-13 at 10 02 14

From ES|QL response:
Screenshot 2024-06-13 at 10 23 27

What if we create a helper function which would convert an ES|QL column into a DataViewField field and preserve the current structure with timeSeriesMetric?

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

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

id before after diff
aiops 549.9KB 550.5KB +580.0B
apm 3.5MB 3.5MB +20.0B
cloudSecurityPosture 450.1KB 450.2KB +20.0B
datasetQuality 186.4KB 186.4KB +20.0B
dataViewManagement 136.5KB 136.5KB +20.0B
discover 809.2KB 809.3KB +116.0B
enterpriseSearch 2.7MB 2.7MB +20.0B
esqlDataGrid 118.7KB 118.7KB +20.0B
eventAnnotationListing 296.2KB 296.2KB +20.0B
lens 1.5MB 1.5MB +116.0B
lists 142.3KB 142.3KB +20.0B
logsExplorer 247.2KB 247.2KB +20.0B
ml 4.1MB 4.1MB +232.0B
presentationUtil 85.4KB 85.5KB +20.0B
securitySolution 13.6MB 13.6MB +580.0B
slo 860.6KB 860.7KB +116.0B
stackAlerts 79.7KB 79.8KB +116.0B
transform 479.5KB 479.5KB +20.0B
unifiedDocViewer 165.4KB 165.4KB +40.0B
unifiedHistogram 66.9KB 66.9KB +20.0B
unifiedSearch 221.5KB 221.6KB +20.0B
total +2.1KB

Page load bundle

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

id before after diff
data 420.1KB 420.3KB +116.0B
dataViewFieldEditor 26.3KB 26.3KB +20.0B
dataViews 61.6KB 61.6KB +20.0B
dataVisualizer 23.0KB 23.1KB +20.0B
fieldFormats 63.0KB 63.0KB +20.0B
maps 54.2KB 54.3KB +116.0B
total +312.0B

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

@stratoula
Copy link
Contributor Author

stratoula commented Jun 13, 2024

@jughosta I didn't know about that, thanx. As we unfortunately rely on dataviews what you are proposing makes sense but I am not 100% certain I understand the implementation.

I will close this PR and I created a new issue to track your proposal #186160. It will be better if someone from the team would help here as I am not 100% certain of how this will work but I would love to see a PoC.

@kertal and @davismcphee could you possibly prioritize it? 🙏

@stratoula stratoula closed this Jun 13, 2024
@davismcphee
Copy link
Contributor

@stratoula just a heads up that it looks like @jughosta has been looking into this here: #186292.

@stratoula
Copy link
Contributor Author

Fantastic ❤️

jughosta added a commit that referenced this pull request Jul 9, 2024
- Closes #186160

## Summary

This PR adds a new util to help with converting ES|QL column into data
view field representation
https://github.com/elastic/kibana/blob/9d63332c74523b00f2b9056352a5b3a86eaf2b75/packages/kbn-data-view-utils/src/utils/convert_to_data_view_field.ts#L13

This allows to handle counter fields in a more predicable way despite of
the different format of ES|QL column data.
#186154 (comment)

<img width="1988" alt="Screenshot 2024-07-03 at 13 48 20"
src="https://github.com/elastic/kibana/assets/1415710/14ce9cd8-8a02-4f3c-8845-c19c30079a75">

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
Co-authored-by: Matthias Wilhelm <[email protected]>
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 9, 2024
- Closes elastic#186160

## Summary

This PR adds a new util to help with converting ES|QL column into data
view field representation
https://github.com/elastic/kibana/blob/9d63332c74523b00f2b9056352a5b3a86eaf2b75/packages/kbn-data-view-utils/src/utils/convert_to_data_view_field.ts#L13

This allows to handle counter fields in a more predicable way despite of
the different format of ES|QL column data.
elastic#186154 (comment)

<img width="1988" alt="Screenshot 2024-07-03 at 13 48 20"
src="https://github.com/elastic/kibana/assets/1415710/14ce9cd8-8a02-4f3c-8845-c19c30079a75">

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
Co-authored-by: Matthias Wilhelm <[email protected]>
(cherry picked from commit 692b656)
kibanamachine added a commit that referenced this pull request Aug 6, 2024
# Backport

This will backport the following commits from `main` to `8.15`:
- [[ES|QL] Support counter fields
(#186292)](#186292)

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

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

<!--BACKPORT [{"author":{"name":"Julia
Rechkunova","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-09T10:32:48Z","message":"[ES|QL]
Support counter fields (#186292)\n\n- Closes
https://github.com/elastic/kibana/issues/186160\r\n\r\n##
Summary\r\n\r\nThis PR adds a new util to help with converting ES|QL
column into data\r\nview field
representation\r\nhttps://github.com/elastic/kibana/blob/9d63332c74523b00f2b9056352a5b3a86eaf2b75/packages/kbn-data-view-utils/src/utils/convert_to_data_view_field.ts#L13\r\n\r\nThis
allows to handle counter fields in a more predicable way despite
of\r\nthe different format of ES|QL column
data.\r\nhttps://github.com//pull/186154#issuecomment-2164973060\r\n\r\n<img
width=\"1988\" alt=\"Screenshot 2024-07-03 at 13 48
20\"\r\nsrc=\"https://github.com/elastic/kibana/assets/1415710/14ce9cd8-8a02-4f3c-8845-c19c30079a75\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by: Stratoula
Kalafateli <[email protected]>\r\nCo-authored-by: Matthias
Wilhelm
<[email protected]>","sha":"692b656f9c247b438951a946f3f1123d36ac00a6","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:DataDiscovery","backport:prev-minor","Feature:ES|QL","v8.16.0"],"title":"[ES|QL]
Support counter
fields","number":186292,"url":"https://github.com/elastic/kibana/pull/186292","mergeCommit":{"message":"[ES|QL]
Support counter fields (#186292)\n\n- Closes
https://github.com/elastic/kibana/issues/186160\r\n\r\n##
Summary\r\n\r\nThis PR adds a new util to help with converting ES|QL
column into data\r\nview field
representation\r\nhttps://github.com/elastic/kibana/blob/9d63332c74523b00f2b9056352a5b3a86eaf2b75/packages/kbn-data-view-utils/src/utils/convert_to_data_view_field.ts#L13\r\n\r\nThis
allows to handle counter fields in a more predicable way despite
of\r\nthe different format of ES|QL column
data.\r\nhttps://github.com//pull/186154#issuecomment-2164973060\r\n\r\n<img
width=\"1988\" alt=\"Screenshot 2024-07-03 at 13 48
20\"\r\nsrc=\"https://github.com/elastic/kibana/assets/1415710/14ce9cd8-8a02-4f3c-8845-c19c30079a75\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by: Stratoula
Kalafateli <[email protected]>\r\nCo-authored-by: Matthias
Wilhelm
<[email protected]>","sha":"692b656f9c247b438951a946f3f1123d36ac00a6"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/186292","number":186292,"mergeCommit":{"message":"[ES|QL]
Support counter fields (#186292)\n\n- Closes
https://github.com/elastic/kibana/issues/186160\r\n\r\n##
Summary\r\n\r\nThis PR adds a new util to help with converting ES|QL
column into data\r\nview field
representation\r\nhttps://github.com/elastic/kibana/blob/9d63332c74523b00f2b9056352a5b3a86eaf2b75/packages/kbn-data-view-utils/src/utils/convert_to_data_view_field.ts#L13\r\n\r\nThis
allows to handle counter fields in a more predicable way despite
of\r\nthe different format of ES|QL column
data.\r\nhttps://github.com//pull/186154#issuecomment-2164973060\r\n\r\n<img
width=\"1988\" alt=\"Screenshot 2024-07-03 at 13 48
20\"\r\nsrc=\"https://github.com/elastic/kibana/assets/1415710/14ce9cd8-8a02-4f3c-8845-c19c30079a75\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>\r\nCo-authored-by:
Elastic Machine
<[email protected]>\r\nCo-authored-by: Stratoula
Kalafateli <[email protected]>\r\nCo-authored-by: Matthias
Wilhelm
<[email protected]>","sha":"692b656f9c247b438951a946f3f1123d36ac00a6"}}]}]
BACKPORT-->

Co-authored-by: Julia Rechkunova <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:ES|QL ES|QL related features in Kibana release_note:skip Skip the PR/issue when compiling release notes Team:ESQL ES|QL related features in Kibana v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants