[Discover] Breakdown support for fieldstats by mohamedhamed-ahmed · Pull Request #199028 · elastic/kibana

mohamedhamed-ahmed · 2024-11-05T17:04:44Z

closes #192700

📝 Summary

This PR add a new Add breakdown button to the field stats popover for all applicable fields.

🎥 Demo

Screen.Recording.2024-11-07.at.14.07.50.mov

jughosta

Left some comments but overall looks good!

packages/kbn-esql-utils/src/utils/esql_fields_utils.ts

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

packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx

…ithub.com/mohamedhamed-ahmed/kibana into 192700-fieldstats-popover-breakdown-field

packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.ts

packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx

…ithub.com/mohamedhamed-ahmed/kibana into 192700-fieldstats-popover-breakdown-field

elasticmachine · 2024-11-07T13:09:12Z

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

elasticmachine · 2024-11-07T13:09:12Z

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

stratoula · 2024-11-11T15:55:34Z

@mohamedhamed-ahmed do we want to disable the icon if the breakdown is already applied?

Not so important, just asking

mohamedhamed-ahmed · 2024-11-11T15:58:01Z

@mohamedhamed-ahmed do we want to disable the icon if the breakdown is already applied?

Had same question in mind, and discussed it with @jughosta but then we agreed to keep it and not hide it

stratoula

ES|QL changes LGTM, I also did a brief testing locally. I don't kow if we want to disable thye breakdown icon if the breakdown field has already been applied in the dropdown but I dont consider it a blocker. LGTM, thanx Mohamed, great job!

jughosta · 2024-11-11T16:53:49Z

packages/kbn-esql-utils/src/utils/esql_fields_utils.ts


-  return true;
+export const isESQLFieldGroupable = (field: FieldSpec): boolean => {
+  if (field.timeSeriesMetric === 'counter') return false;


Suggested change

if (field.timeSeriesMetric === 'counter') return false;

if (field.timeSeriesMetric) return false;

Other cases like histogram I guess also would not work.

They are not supported by ESQL yet. What other options can this timeseriesMetric take? Let's evaluate first before doing the suggested change

Here are possible values for data view mode

kibana/x-pack/plugins/lens/public/types.ts

Line 111 in a854ff8

timeSeriesMetric?: 'histogram' | 'summary' | 'gauge' | 'counter' | 'position';

It does not exactly apply to ES|QL at the moment but it might in the future.

I see, thanx for sharing Julia.

In the columns function we check for counter related metrics as only these are partially supported by ESQL and only for these we know for sure that they are not groupable.

We have 2 options. Either keep it as it is (which aligns with the above function) or change both.

I personally prefer the first option. (Aligns with the existing column function too). When more tsdb metrics will be added we should test (and update if is the case the isGroupable functionality). But I dont have a strong opinion, mostly a preference

packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx

packages/kbn-unified-field-list/src/containers/unified_field_list_item/field_list_item.tsx

...ages/kbn-unified-field-list/src/containers/unified_field_list_sidebar/field_list_sidebar.tsx

jughosta

Looks great, thanks! 👍

elasticmachine · 2024-11-12T11:37:25Z

⏳ Build in-progress

Buildkite Build
Commit: f0e94eb
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-199028-f0e94eb21b62

History

💚 Build #249967 succeeded 31978dd
💔 Build #249910 failed 62c0e56
💚 Build #249865 succeeded 923e305
💛 Build #249459 was flaky 98ed647
💔 Build #249454 failed 3b19865

kibanamachine · 2024-11-12T12:26:56Z

Starting backport for target branches: 8.x

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

closes elastic#192700 ## 📝 Summary This PR add a new `Add breakdown` button to the field stats popover for all applicable fields. ## 🎥 Demo https://github.com/user-attachments/assets/d647189c-9b04-4127-a4fd-f9764babe46e --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 7369442)

kibanamachine · 2024-11-12T12:31:50Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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

Questions ?

Please refer to the Backport tool documentation

kibanamachine · 2024-11-13T12:49:02Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

# Backport This will backport the following commits from `main` to `8.x`: - [[Discover] Breakdown support for fieldstats (#199028)](#199028)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: mohamedhamed-ahmed <mohamed.ahmed@elastic.co>

closes elastic#192700 ## 📝 Summary This PR add a new `Add breakdown` button to the field stats popover for all applicable fields. ## 🎥 Demo https://github.com/user-attachments/assets/d647189c-9b04-4127-a4fd-f9764babe46e --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

[Discover] Breakdown support for fieldstats

3c51e10

mohamedhamed-ahmed added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting labels Nov 5, 2024

kibanamachine and others added 2 commits November 5, 2024 17:15

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

3fad548

Merge branch 'main' into 192700-fieldstats-popover-breakdown-field

3662347

jughosta reviewed Nov 6, 2024

View reviewed changes

mohamedhamed-ahmed added 2 commits November 6, 2024 10:35

code refactor

2c66f0d

Merge branch '192700-fieldstats-popover-breakdown-field' of https://g…

064dd18

…ithub.com/mohamedhamed-ahmed/kibana into 192700-fieldstats-popover-breakdown-field

mohamedhamed-ahmed commented Nov 6, 2024

View reviewed changes

packages/kbn-data-view-utils/src/utils/convert_to_data_table_column.ts Outdated Show resolved Hide resolved

packages/kbn-unified-field-list/src/components/field_popover/field_popover_header.tsx Outdated Show resolved Hide resolved

mohamedhamed-ahmed and others added 4 commits November 6, 2024 10:59

bug fix

93ffa66

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

96848c1

revert code

93b9894

Merge branch '192700-fieldstats-popover-breakdown-field' of https://g…

39dc06a

…ithub.com/mohamedhamed-ahmed/kibana into 192700-fieldstats-popover-breakdown-field

mohamedhamed-ahmed added backport:prev-minor and removed backport:skip This PR does not require backporting labels Nov 6, 2024

elastic deleted a comment from elasticmachine Nov 6, 2024

kibanamachine and others added 5 commits November 6, 2024 17:11

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

6f06144

add unit tests

7297442

Merge branch '192700-fieldstats-popover-breakdown-field' of https://g…

dfd7fb6

…ithub.com/mohamedhamed-ahmed/kibana into 192700-fieldstats-popover-breakdown-field

Merge branch 'main' into 192700-fieldstats-popover-breakdown-field

3dd24d4

added component tests

26c21e9

mohamedhamed-ahmed marked this pull request as ready for review November 7, 2024 13:08

mohamedhamed-ahmed requested review from a team as code owners November 7, 2024 13:08

mohamedhamed-ahmed added Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// Team:obs-onboarding Observability Onboarding Team labels Nov 7, 2024

mohamedhamed-ahmed requested a review from jughosta November 7, 2024 13:09

code refactor and tests

62c0e56

mohamedhamed-ahmed requested review from jughosta, stratoula and tonyghiani November 11, 2024 11:54

tonyghiani approved these changes Nov 11, 2024

View reviewed changes

Tests

31978dd

stratoula approved these changes Nov 11, 2024

View reviewed changes

jughosta reviewed Nov 11, 2024

View reviewed changes

mohamedhamed-ahmed added 2 commits November 12, 2024 11:32

swap icon positions

e0a313d

Merge branch 'main' into 192700-fieldstats-popover-breakdown-field

f0e94eb

jughosta approved these changes Nov 12, 2024

View reviewed changes

mohamedhamed-ahmed merged commit 7369442 into elastic:main Nov 12, 2024

kibanamachine added the v9.0.0 label Nov 12, 2024

kibanamachine mentioned this pull request Nov 12, 2024

[8.x] [Discover] Breakdown support for fieldstats (#199028) #199780

Merged

This was referenced Nov 12, 2024

[Security Solution] Add Alert Suppression editable component #198673

Merged

chore(slo): Migrate to server-route-repository #198726

Merged

kibanamachine mentioned this pull request Nov 13, 2024

[ES|QL] Improve STATS command summary extraction #199796

Merged

3 tasks

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 13, 2024

kibanamachine added v8.17.0 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Nov 13, 2024

	if (field.timeSeriesMetric === 'counter') return false;
	if (field.timeSeriesMetric) return false;

Conversation

mohamedhamed-ahmed commented Nov 5, 2024 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Summary

🎥 Demo

Uh oh!

jughosta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Nov 7, 2024

Uh oh!

elasticmachine commented Nov 7, 2024

Uh oh!

stratoula commented Nov 11, 2024

Uh oh!

mohamedhamed-ahmed commented Nov 11, 2024

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

jughosta Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

stratoula Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

jughosta Nov 11, 2024

Choose a reason for hiding this comment

Uh oh!

stratoula Nov 11, 2024 • 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!

Uh oh!

Uh oh!

Uh oh!

jughosta left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 12, 2024

⏳ Build in-progress

History

Uh oh!

kibanamachine commented Nov 12, 2024

Uh oh!

kibanamachine commented Nov 12, 2024

💚 All backports created successfully

Questions ?

Uh oh!

kibanamachine commented Nov 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mohamedhamed-ahmed commented Nov 5, 2024 •

edited by kibanamachine

Loading

stratoula Nov 11, 2024 •

edited

Loading