[Maps] add Top term aggregation by nreese · Pull Request #57875 · elastic/kibana

nreese · 2020-02-18T16:27:34Z

This PR adds Terms aggregation to aggregation selection list. This allows users to create maps with clusters and joins that can be styled by top term.

This feature is required for blended layer. This will allow clusters in blended layer to have roll-up style when documents are styled by category.

elasticmachine · 2020-02-18T16:27:37Z

Pinging @elastic/kibana-gis (Team:Geo)

thomasneirynck

So sweet getting these additional new features dialed in. It really matures the feature-set tremendously.

x-pack/legacy/plugins/maps/common/constants.ts

x-pack/legacy/plugins/maps/public/layers/util/es_agg_utils.js

x-pack/legacy/plugins/maps/public/layers/sources/es_term_source.js

x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js

x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js

thomasneirynck · 2020-02-19T20:57:58Z

x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/es_geo_grid_source.js

  async getGeoJsonWithMeta(layerName, searchFilters, registerCancelCallback) {
    const indexPattern = await this.getIndexPattern();
    const searchSource = await this._makeSearchSource(searchFilters, 0);
-    const aggConfigs = new AggConfigs(


+1 getting rid of agg-configs.

Probably can remove corresponding mock as well:

kibana/x-pack/legacy/plugins/maps/public/layers/sources/es_term_source.test.js

Line 11 in 6011af3

jest.mock('ui/agg_types', () => ({

cc @aaronjcaldwell I think we're only down to usage for bounds-agg after this.

kibana/x-pack/legacy/plugins/maps/public/layers/sources/es_source.js

Line 17 in 90aeae2

import { AggConfigs } from 'ui/agg_types';

We could probably remove in similar fashion

nreese · 2020-02-24T14:48:56Z

@elasticmachine merge upstream

thomasneirynck · 2020-02-24T19:47:15Z

Thanks for investigating the composite agg.

I would be hesitant to replace the existing implementation.

The reason for introducing composite-aggs is to support the top-terms nested aggregation for the fine-finest resolution of grid layers and to support top-terms in term-joining. Composite-aggs paginate through all the results, and are similar to _scroll in that respect. Paging through a result-set is generally not recommended for real-time user-requests, and more often applicable in offline workflows (e.g. reporting etc...). Using composite-agg would be a new paradigm for loading data from Elasticsearch in Maps (and Kibana).
Composite-aggs are not needed to support top-terms in context of the blended-layer feature [Maps] Blended layer that switches between documents and clusters #57879 (which is what triggered this PR in the first place). For blended-layers, Maps will default to the "coarse"-resolution (makes more sense for "clusters"/labeling anyway).
We have not had user-feedback in Maps yet, that the grid-layers or term-joining is hitting bucket-limit issues with the current-implementation for fine/finest.

I would only create a composite-aggs, if a top-terms is present in the metric-config. It seems like the most conservative choice. It would prevent us from pre-emptively fixing an issue that has not been raised, but still allow us to introduce the top-terms functionality more generically instead of limiting it to just using it in blended-layers.

Later, we can always still remove the default implementation if using composite-aggs did not introduce any unintended consequences.

nreese · 2020-02-24T20:14:48Z

I would only create a composite-aggs, if a top-terms is present in the metric-config. It seems like the most conservative choice.

I have added a separate path for when there are no term sub-aggregations.

x-pack/legacy/plugins/maps/public/layers/sources/es_pew_pew_source/convert_to_lines.test.js

x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/geo_tile_utils.js

x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/convert_to_geojson.test.js

x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/es_geo_grid_source.js

x-pack/legacy/plugins/maps/public/layers/sources/es_source.js

x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/es_geo_grid_source.js

x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/geo_tile_utils.js

x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js

thomasneirynck

Some small cleanup suggestions. Can you create a follow-up ticket for the getMetrics issue?

x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/es_geo_grid_source.js

x-pack/legacy/plugins/maps/public/layers/util/es_agg_utils.test.js

x-pack/legacy/plugins/maps/public/selectors/map_selectors.js

nreese · 2020-02-26T00:07:31Z

Can you create a follow-up ticket for the getMetrics issue?

Created #58560

thomasneirynck

One small comment left-over. Thanks, great functionality. not only to make blended-layers work, but also as stand-alone!

x-pack/legacy/plugins/maps/public/layers/util/es_agg_utils.ts

kibanamachine · 2020-02-26T03:13:44Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 104acf6

History

💚 Build #28926 succeeded ac8c04b
💔 Build #28894 failed 79b5a3a
💔 Build #28887 failed 7a0ffa8
💚 Build #28679 succeeded a126388
💔 Build #28657 failed 1aae2ea

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

* [Maps] add Top term aggregation * update pew-pew source to handle terms agg * make helper function for pulling values from bucket * update terms source * better join labels * categoricla meta * remove unused constant * remove unused changes * remove unused constant METRIC_SCHEMA_CONFIG * update jest expect * fix auto complete suggestions for top term * get category autocomplete working with style props from joins * pluck categorical style meta with real field name * mock MetricsEditor to fix jest test * review feedback * es_agg_utils.js to es_agg_utils.ts * typing updates * use composit agg to avoid search.buckets limit * i18n update and functional test fix * stop paging through results when request is aborted * remove unused file * do not use composite agg when no terms sub-aggregations * clean up * pass indexPattern to getValueAggsDsl * review feedback * more review feedback * ts-ignore for untyped imports in tests * more review feedback * add bucket.hasOwnProperty check Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* [Maps] add Top term aggregation * update pew-pew source to handle terms agg * make helper function for pulling values from bucket * update terms source * better join labels * categoricla meta * remove unused constant * remove unused changes * remove unused constant METRIC_SCHEMA_CONFIG * update jest expect * fix auto complete suggestions for top term * get category autocomplete working with style props from joins * pluck categorical style meta with real field name * mock MetricsEditor to fix jest test * review feedback * es_agg_utils.js to es_agg_utils.ts * typing updates * use composit agg to avoid search.buckets limit * i18n update and functional test fix * stop paging through results when request is aborted * remove unused file * do not use composite agg when no terms sub-aggregations * clean up * pass indexPattern to getValueAggsDsl * review feedback * more review feedback * ts-ignore for untyped imports in tests * more review feedback * add bucket.hasOwnProperty check Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

nreese added 6 commits February 18, 2020 05:48

[Maps] add Top term aggregation

30b03a9

update pew-pew source to handle terms agg

127b381

make helper function for pulling values from bucket

9e31333

update terms source

402358e

better join labels

66967ab

categoricla meta

1dff40c

nreese added Team:Geo Former Team Label for Geo Team. Now use Team:Presentation enhancement New value added to drive a business result v8.0.0 v7.7.0 labels Feb 18, 2020

nreese requested review from jsanz and thomasneirynck February 18, 2020 16:27

nreese requested a review from a team as a code owner February 18, 2020 16:27

nreese added 3 commits February 18, 2020 09:29

remove unused constant

19d3720

remove unused changes

b99138a

remove unused constant METRIC_SCHEMA_CONFIG

8aceadb

nreese added release_note:enhancement and removed enhancement New value added to drive a business result labels Feb 18, 2020

nreese added 6 commits February 18, 2020 11:18

Merge branch 'master' of github.com:elastic/kibana into top_terms

b90233d

update jest expect

6011af3

fix auto complete suggestions for top term

90aeae2

get category autocomplete working with style props from joins

28a854f

pluck categorical style meta with real field name

46af667

mock MetricsEditor to fix jest test

66c5f2b

nreese added the review label Feb 18, 2020

thomasneirynck reviewed Feb 19, 2020

View reviewed changes

nreese added 3 commits February 19, 2020 14:24

Merge branch 'master' of github.com:elastic/kibana into top_terms

2facf0a

review feedback

ecc64b0

es_agg_utils.js to es_agg_utils.ts

dd6f7a8

Merge branch 'master' into top_terms

c2c4feb

nreese mentioned this pull request Feb 24, 2020

Custom Bucket Sub Aggregations and Top Hits #19096

Closed

do not use composite agg when no terms sub-aggregations

572a97f

nreese mentioned this pull request Feb 24, 2020

[Maps] Add support for additional aggregations and fields for geohash & boundary layers #28040

Closed

nreese added 2 commits February 24, 2020 16:14

clean up

1aae2ea

pass indexPattern to getValueAggsDsl

a126388

thomasneirynck reviewed Feb 25, 2020

View reviewed changes

x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js Show resolved Hide resolved

thomasneirynck requested review from thomasneirynck February 25, 2020 16:51

nreese added 4 commits February 25, 2020 10:09

Merge branch 'master' of github.com:elastic/kibana into top_terms

4de141a

review feedback

7a0ffa8

more review feedback

79b5a3a

ts-ignore for untyped imports in tests

ac8c04b

thomasneirynck reviewed Feb 25, 2020

View reviewed changes

more review feedback

122491a

nreese requested a review from thomasneirynck February 26, 2020 00:04

nreese mentioned this pull request Feb 26, 2020

[Maps] clean-up getMetricFields from AbstractESSource #58560

Closed

thomasneirynck approved these changes Feb 26, 2020

View reviewed changes

x-pack/legacy/plugins/maps/public/layers/util/es_agg_utils.ts Show resolved Hide resolved

add bucket.hasOwnProperty check

104acf6

nreese merged commit 3212754 into elastic:master Feb 26, 2020

nreese mentioned this pull request Feb 26, 2020

[7.x] [Maps] add Top term aggregation (#57875) #58599

Merged

thomasneirynck mentioned this pull request Feb 26, 2020

Deprecate search.max_buckets? elastic/elasticsearch#51731

Closed

Conversation

nreese commented Feb 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Feb 18, 2020

Uh oh!

thomasneirynck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomasneirynck Feb 19, 2020

Choose a reason for hiding this comment

Uh oh!

nreese commented Feb 24, 2020

Uh oh!

thomasneirynck commented Feb 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nreese commented Feb 24, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomasneirynck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nreese commented Feb 26, 2020

Uh oh!

thomasneirynck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented Feb 26, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nreese commented Feb 18, 2020 •

edited

Loading

thomasneirynck commented Feb 24, 2020 •

edited

Loading