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.

Sign up for GitHub

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

Jump to bottom

[TSVB] Disable using top_hits in pipeline aggregations #82278

Merged
merged 8 commits into from
Nov 10, 2020
Merged

[TSVB] Disable using top_hits in pipeline aggregations #82278

merged 8 commits into from
Nov 10, 2020

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Nov 2, 2020 •
edited
Loading

Summary

Closes #44294.

This PR makes it impossible in the TSVB UI to use top_hits inside the bucket_script aggregation because they do not work together from Elasticsearch side. So when the user has a top_hit aggregation it won't appear on the Bucket script metric dropdown so he won't be able to use it.

Screenshot 2020-11-03 at 10 40 52 AM

I also saw that when I use Top Hit with a number field I see the following server error:
Screenshot 2020-11-03 at 10 59 35 AM

so I have also fixed this in this PR

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula stratoula requested a review from alexwizp November 3, 2020 08:41
@stratoula stratoula added Feature:TSVB TSVB (Time Series Visual Builder) v7.11.0 release_note:fix v8.0.0 labels Nov 3, 2020
alexwizp
alexwizp reviewed Nov 3, 2020
View reviewed changes
Copy link
Contributor

@alexwizp alexwizp left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Your changes work correctly for the described issue. But I see the same error message for other types of Parent Pipeline Aggregations.
Looks like we should disable the whole group.

image

@stratoula
Copy link
Contributor Author

Good point @alexwizp. Let me look into this

@stratoula
Copy link
Contributor Author

@alexwizp it doesn't work with Sibling Pipeline Aggregations so I excluded it from them too 🙂

@stratoula stratoula marked this pull request as ready for review November 4, 2020 08:33
@stratoula stratoula requested a review from a team November 4, 2020 08:33
@stratoula stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Nov 4, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id before after diff
visTypeTimeseries 1.7MB 1.7MB +245.0B

History

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

@mbondyra
Copy link
Contributor

mbondyra commented Nov 10, 2020 •
edited
Loading

Tested on Chrome, the top_hits is blocked from using in all parent and sibling aggregations just like expected. The only thing I'd see as improvement is to change the title and description of this PR if anyone would look for it in the future (something like [TSVB] Disable using top_hits in pipeline aggregations)

@stratoula stratoula changed the title [TSVB] Disable using top_hits in bucket script aggregations [TSVB] Disable using top_hits in pipeline aggregations Nov 10, 2020
@stratoula stratoula merged commit a7a83fd into elastic:master Nov 10, 2020
stratoula added a commit to stratoula/kibana that referenced this pull request Nov 10, 2020
@stratoula @kibanamachine
[TSVB] Disable using top_hits in pipeline aggregations (elastic#82278)
2f4073a 
* [TSVB] Disable using top_hits in bucket script aggregations

* remove console message

* Correct schema for metrics size

* Chanhe hardcoded agg with the exported for the METRIC_TYPES var

* Exclude top_hit agg from all Sibling Pipeline Aggregations and all Parent Pipeline Aggregations

Co-authored-by: Kibana Machine <[email protected]>
@stratoula stratoula mentioned this pull request Nov 10, 2020
Merged
stratoula added a commit that referenced this pull request Nov 10, 2020
@stratoula @kibanamachine
[TSVB] Disable using top_hits in pipeline aggregations (#82278) (#83042)
59bdf4a 
* [TSVB] Disable using top_hits in bucket script aggregations

* remove console message

* Correct schema for metrics size

* Chanhe hardcoded agg with the exported for the METRIC_TYPES var

* Exclude top_hit agg from all Sibling Pipeline Aggregations and all Parent Pipeline Aggregations

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
phillipb added a commit to phillipb/kibana that referenced this pull request Nov 10, 2020
@phillipb
Merge branch 'bootstrap-node-details-overlay' of github.com:phillipb/…
4134c8c 
…kibana into bootstrap-node-details-overlay

* 'bootstrap-node-details-overlay' of github.com:phillipb/kibana: (49 commits)
  [Security Solution] Fix DNS Network table query (elastic#82778)
  [Workplace Search] Consolidate groups routes (elastic#83015)
  Adds cloud links to user menu (elastic#82803)
  [Security Solution][Detections] - follow up cleanup on auto refresh rules (elastic#83023)
  [App Search] Added the log retention panel to the Settings page (elastic#82982)
  [Maps] show icon when layer is filtered by time and allow layers to ignore global time range (elastic#83006)
  [DOCS] Consolidates drilldown pages (elastic#82081)
  [Maps] add on-prem EMS config (elastic#82525)
  migrate i18n mixin to KP (elastic#81799)
  [bundle optimization] fix imports of react-use lib (elastic#82847)
  [Discover] Add metric on adding filter (elastic#82961)
  [Lens] Performance refactoring for indexpattern fast lookup and Operation support matrix computation (elastic#82829)
  skip flaky suite (elastic#82804)
  Fix SO query for searching across spaces (elastic#83025)
  renaming built-in alerts to Stack Alerts (elastic#82873)
  [TSVB] Disable using top_hits in pipeline aggregations (elastic#82278)
  [Visualizations] Remove kui usage (elastic#82810)
  [Visualizations] Make the icon buttons labels more descriptive (elastic#82585)
  [Lens] Do not reset formatting when switching between custom ranges and auto histogram (elastic#82694)
  Fix ilm navigation (elastic#81664)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 10, 2020
@gmmorris
Merge branch 'alerts/stack-alerts-public' of github.com:gmmorris/kiba…
0104cb6 
…na into alerts/stack-alerts-public

* 'alerts/stack-alerts-public' of github.com:gmmorris/kibana:
  [Security Solution] Fix DNS Network table query (elastic#82778)
  [Workplace Search] Consolidate groups routes (elastic#83015)
  Adds cloud links to user menu (elastic#82803)
  [Security Solution][Detections] - follow up cleanup on auto refresh rules (elastic#83023)
  [App Search] Added the log retention panel to the Settings page (elastic#82982)
  [Maps] show icon when layer is filtered by time and allow layers to ignore global time range (elastic#83006)
  [DOCS] Consolidates drilldown pages (elastic#82081)
  [Maps] add on-prem EMS config (elastic#82525)
  migrate i18n mixin to KP (elastic#81799)
  [bundle optimization] fix imports of react-use lib (elastic#82847)
  [Discover] Add metric on adding filter (elastic#82961)
  [Lens] Performance refactoring for indexpattern fast lookup and Operation support matrix computation (elastic#82829)
  skip flaky suite (elastic#82804)
  Fix SO query for searching across spaces (elastic#83025)
  renaming built-in alerts to Stack Alerts (elastic#82873)
  [TSVB] Disable using top_hits in pipeline aggregations (elastic#82278)
  [Visualizations] Remove kui usage (elastic#82810)
  [Visualizations] Make the icon buttons labels more descriptive (elastic#82585)
  [Lens] Do not reset formatting when switching between custom ranges and auto histogram (elastic#82694)
:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexwizp alexwizp alexwizp approved these changes

@mbondyra mbondyra mbondyra approved these changes

Assignees
No one assigned
Labels
Feature:TSVB TSVB (Time Series Visual Builder) release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[TSVB] Disable using top_hits in bucket script aggregations
5 participants
@stratoula @elasticmachine @kibanamachine @mbondyra @alexwizp