[ES|QL] [Discover] Keeps the histogram config on time change by stratoula · Pull Request #208053 · elastic/kibana

stratoula · 2025-01-23T13:27:51Z

Summary

Closes #198749

Checklist

Unit or functional tests were updated or added to match the most common scenarios

stratoula · 2025-01-24T08:27:22Z

    return appendToESQLQuery(
      safeQuery,
-      `| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp${breakdown}${sortBy} | rename timestamp as \`${dataView.timeFieldName} every ${queryInterval}\``
+      `| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp${breakdown}${sortBy}`


ℹ️ Remove the rename from here, let the lens suggestions api to handle this

…lens_vis_service.ts

elasticmachine · 2025-01-24T11:20:55Z

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

jughosta

This is a very nice improvement to make!

Can you please check the case when user already had some Discover sessions with saved custom vis configuration? When I exported a session from another instance and imported it into an instance with this branch, my customization got reset.

jughosta · 2025-01-24T12:54:15Z

    return appendToESQLQuery(
      safeQuery,
-      `| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp${breakdown}${sortBy} | rename timestamp as \`${dataView.timeFieldName} every ${queryInterval}\``
+      `| EVAL timestamp=DATE_TRUNC(${queryInterval}, ${dataView.timeFieldName}) | stats results = count(*) by timestamp${breakdown}${sortBy}`


Can we exclude the time interval too? Otherwise, it stays locked when saved:

You mean when saved in a dashboard, right? Same problem exists in main too. If you save the histogram in the dashboard, the xaxis label doesnt change, the user needs to update the rename in the query.

Are you suggesting to exculde the label at all? To not mention the interval at all in the x axis?

No exactly. In my case I saved the session + vis when my interval was 30 sec. Then I changed the time range to 1 year. The chart still shows "every 30 seconds". And it's also empty for some reason (as on the screenshot above).

hmm I can't reproduce. Here I have a discover session, I change the time and the label updates

Can you customize the chart before saving the session?

Have you made changes to your viz maybe and then saved? I think the problem is there. I will take a look to check if it is something easy to fix 👍

Yeah, looks like it. Thanks!

This change should fix it a3e4495

stratoula · 2025-01-24T13:59:30Z

Can you please check the case when user already had some Discover sessions with saved custom vis configuration? When I exported a session from another instance and imported it into an instance with this branch, my customization got reset.

Julia unfortunately is not going to work for existing saved searches because the SO has been saved with the name/id that has the timestamp with the interval.

jughosta · 2025-01-27T13:22:58Z

+  columns: TextBasedLayerColumn[],
+  dateFieldLabel: string
+) => {
+  const dateColumn = columns.find((column) => column.columnId === 'timestamp');


What if current data set has a real timestamp field, would not it conflict with this additional logic?

These are the columns used by the histogram and not the entire dataset, so this is the column we create with the eval

Alright! Can we create a const for timestamp and share between files?

Here it is Jul 84cef91

jughosta · 2025-01-27T13:24:13Z

+          fieldName: c.variable ? `?${c.variable}` : c.id,
          variable: c.variable,
+          label: c.name,
+          customLabel: c.id !== c.name,


Can it be hasCustomLabel?

Noticed that the custom label gets reset when interval changes. Not sure if it's okay.

This is an already existing property so I cant rename it unfortunately.

The custom label will reset here yes but I think that is ok as it happens only for the discover histogram. I dont expect users to want to give a custom label tbh. If we think that they need to, then we need to find another solution and not rely on the custom label of the Lens charts.

jughosta · 2025-01-27T13:27:49Z

        index: 'e3465e67bdeced2befff9f9dca7ecf9c48504cad68a10efd881f4c7dd5ade28a',
        query: {
-          esql: 'from kibana_sample_data_logs | limit 10 | EVAL timestamp=DATE_TRUNC(30 second, @timestamp) | stats results = count(*) by timestamp | rename timestamp as `@timestamp every 30 second`',
+          esql: 'from kibana_sample_data_logs | limit 10 | EVAL timestamp=DATE_TRUNC(30 second, @timestamp) | stats results = count(*) by timestamp',


Is it correct that when adding to a dashboard, the chart does not show the same long time range?

This is happening also in main Julia. It is due to an ES limitation and irrelevant with this PR

Hm, how do we have a different view on Discover page then?

It seems so, not exactly sure why it looks differently in Discover though 🤔 . I was referring to this btw elastic/elasticsearch#111483

I think is because of this Jul https://github.com/elastic/kibana/blob/main/src/platform/plugins/shared/unified_histogram/public/chart/hooks/use_lens_props.ts#L91

Since dashboard knows the current time range then it could pass it to Lens and it would be fixed on Dashboard page too even without ES support, right?

I think the viz team wants to solve it when we have this elastic/elasticsearch#111483 from ES side. Right @markov00 ?

That's the idea, but this doesn't blocks us from specifying directly the timerange in the chart domain until this feature is available in ESQL

yes I agree, we should.

jughosta

Data discovery changes LGTM 👍

Please note that this PR will reset existing customizations for ES|QL histogram which were saved in saved search SO.

elasticmachine · 2025-02-03T11:07:53Z

💚 Build Succeeded

Buildkite Build
Commit: 373bb52

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
`lens`	1.6MB	1.6MB	+35.0B
`unifiedHistogram`	71.0KB	70.9KB	-84.0B
total			-49.0B

Page load bundle

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

id	before	after	diff
`unifiedHistogram`	10.3KB	10.5KB	+234.0B

History

💛 Build #271504 was flaky 13e1e8f
💛 Build #270551 was flaky 15eac7e
💔 Build #270528 failed 84cef91
💛 Build #270423 was flaky 789caea
💚 Build #270355 succeeded a3e4495
💛 Build #269901 was flaky b24854a

nickofthyme

Vis changes LGTM

kibanamachine · 2025-02-03T16:01:41Z

Starting backport for target branches: 8.18, 8.x, 9.0

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

…#208053) ## Summary Closes elastic#198749 ![meow](https://github.com/user-attachments/assets/2cb2ff53-49f9-414e-985f-c0acd3945078) ### Checklist - [ ] [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 (cherry picked from commit b25f236)

kibanamachine · 2025-02-03T16:07:50Z

💚 All backports created successfully

Status	Branch	Result
✅	8.18
✅	8.x
✅	9.0

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…208053) (#209357) # Backport This will backport the following commits from `main` to `8.x`: - [[ES|QL] [Discover] Keeps the histogram config on time change (#208053)](#208053)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>

…208053) (#209356) # Backport This will backport the following commits from `main` to `8.18`: - [[ES|QL] [Discover] Keeps the histogram config on time change (#208053)](#208053)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>

…208053) (#209358) # Backport This will backport the following commits from `main` to `9.0`: - [[ES|QL] [Discover] Keeps the histogram config on time change (#208053)](#208053)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>

stratoula added 2 commits January 23, 2025 14:27

[ES|QL] [Discover] Keeps the histogram config on time change

34269c2

Fixes unit tests

c5dac35

stratoula commented Jan 24, 2025

View reviewed changes

Comment thread src/platform/plugins/shared/unified_histogram/public/services/lens_vis_service.ts Outdated

Update src/platform/plugins/shared/unified_histogram/public/services/…

b24854a

…lens_vis_service.ts

stratoula marked this pull request as ready for review January 24, 2025 11:20

stratoula requested review from a team as code owners January 24, 2025 11:20

stratoula added the Feature:ES|QL ES|QL related features in Kibana label Jan 24, 2025

jughosta reviewed Jan 24, 2025

View reviewed changes

stratoula added 3 commits January 27, 2025 09:07

Merge branch 'main' into fix-keep-esql-chart-conf-on-time-change

00a007d

Fixes the wrong label in a saved session

a3e4495

Merge branch 'main' into fix-keep-esql-chart-conf-on-time-change

789caea

jughosta reviewed Jan 27, 2025

View reviewed changes

stratoula added 2 commits January 27, 2025 15:29

Use a constant for timestamp column

84cef91

Fix wrong import

15eac7e

jughosta approved these changes Jan 28, 2025

View reviewed changes

stratoula added 2 commits January 29, 2025 13:05

Merge branch 'main' into fix-keep-esql-chart-conf-on-time-change

13e1e8f

Merge branch 'main' into fix-keep-esql-chart-conf-on-time-change

373bb52

stratoula added the v8.19.0 label Feb 3, 2025

nickofthyme approved these changes Feb 3, 2025

View reviewed changes

stratoula merged commit b25f236 into elastic:main Feb 3, 2025

kibanamachine added the v9.1.0 label Feb 3, 2025

kibanamachine mentioned this pull request Feb 3, 2025

[8.18] [ES|QL] [Discover] Keeps the histogram config on time change (#208053) #209356

Merged

kibanamachine mentioned this pull request Feb 3, 2025

[8.x] [ES|QL] [Discover] Keeps the histogram config on time change (#208053) #209357

Merged

kibanamachine mentioned this pull request Feb 3, 2025

[9.0] [ES|QL] [Discover] Keeps the histogram config on time change (#208053) #209358

Merged

Conversation

stratoula commented Jan 23, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticmachine commented Jan 24, 2025

Uh oh!

jughosta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stratoula Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jughosta Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jughosta Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stratoula commented Jan 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stratoula Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stratoula Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jughosta left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Feb 3, 2025

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

stratoula commented Jan 23, 2025 •

edited by kibanamachine

Loading

stratoula Jan 24, 2025 •

edited

Loading

jughosta Jan 24, 2025 •

edited

Loading

jughosta Jan 24, 2025 •

edited

Loading

stratoula Jan 27, 2025 •

edited

Loading

stratoula Jan 27, 2025 •

edited

Loading