-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Discover] Breakdown support for fieldstats #199028
Merged
mohamedhamed-ahmed
merged 26 commits into
elastic:main
from
mohamedhamed-ahmed:192700-fieldstats-popover-breakdown-field
Nov 12, 2024
+307
−62
Merged
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
3c51e10
[Discover] Breakdown support for fieldstats
mohamedhamed-ahmed 3fad548
[CI] Auto-commit changed files from 'node scripts/yarn_deduplicate'
kibanamachine 3662347
Merge branch 'main' into 192700-fieldstats-popover-breakdown-field
mohamedhamed-ahmed 2c66f0d
code refactor
mohamedhamed-ahmed 064dd18
Merge branch '192700-fieldstats-popover-breakdown-field' of https://g…
mohamedhamed-ahmed 93ffa66
bug fix
mohamedhamed-ahmed 96848c1
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine 93b9894
revert code
mohamedhamed-ahmed 39dc06a
Merge branch '192700-fieldstats-popover-breakdown-field' of https://g…
mohamedhamed-ahmed 6f06144
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine 7297442
add unit tests
mohamedhamed-ahmed dfd7fb6
Merge branch '192700-fieldstats-popover-breakdown-field' of https://g…
mohamedhamed-ahmed 3dd24d4
Merge branch 'main' into 192700-fieldstats-popover-breakdown-field
mohamedhamed-ahmed 26c21e9
added component tests
mohamedhamed-ahmed a56102e
Merge branch 'main' into 192700-fieldstats-popover-breakdown-field
mohamedhamed-ahmed 2e13155
code refactor
mohamedhamed-ahmed e491c54
code refactor
mohamedhamed-ahmed 3b19865
Merge branch 'main' into 192700-fieldstats-popover-breakdown-field
mohamedhamed-ahmed 263eaab
export fix
mohamedhamed-ahmed 98ed647
icon change
mohamedhamed-ahmed f3d78dd
Merge branch 'main' into 192700-fieldstats-popover-breakdown-field
mohamedhamed-ahmed 923e305
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine 62c0e56
code refactor and tests
mohamedhamed-ahmed 31978dd
Tests
mohamedhamed-ahmed e0a313d
swap icon positions
mohamedhamed-ahmed f0e94eb
Merge branch 'main' into 192700-fieldstats-popover-breakdown-field
mohamedhamed-ahmed File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading status checks…
code refactor
commit 2e13155bf940b6b0e00b9e4b4eba05085f6ec464
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other cases like
histogram
I guess also would not work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are not supported by ESQL yet. What other options can this timeseriesMetric take? Let's evaluate first before doing the suggested change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are possible values for data view mode
kibana/x-pack/plugins/lens/public/types.ts
Line 111 in a854ff8
It does not exactly apply to ES|QL at the moment but it might in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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