From f0417f9c2ac0932dde2d913be22c70f77e9ed168 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 7 Oct 2020 14:49:48 +0200 Subject: [PATCH 1/4] allow sorting by median --- src/plugins/data/common/search/aggs/agg_config.ts | 9 +++++++++ src/plugins/data/common/search/aggs/agg_type.ts | 9 +++++++++ src/plugins/data/common/search/aggs/buckets/terms.ts | 5 ++--- src/plugins/data/common/search/aggs/metrics/median.ts | 3 +++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/plugins/data/common/search/aggs/agg_config.ts b/src/plugins/data/common/search/aggs/agg_config.ts index 201e9f1ec402c..910c79f5dd0d7 100644 --- a/src/plugins/data/common/search/aggs/agg_config.ts +++ b/src/plugins/data/common/search/aggs/agg_config.ts @@ -400,6 +400,15 @@ export class AggConfig { return this.params.field; } + /** + * Returns the bucket path containing the main value the agg will produce + * (e.g. for sum of bytes it will point to the sum, for median it will point + * to the 50 percentile in the percentile multi value bucket) + */ + getValueBucketPath() { + return this.type.getValueBucketPath(this); + } + makeLabel(percentageMode = false) { if (this.params.customLabel) { return this.params.customLabel; diff --git a/src/plugins/data/common/search/aggs/agg_type.ts b/src/plugins/data/common/search/aggs/agg_type.ts index 1e3839038b0f7..3ffac0c12eb22 100644 --- a/src/plugins/data/common/search/aggs/agg_type.ts +++ b/src/plugins/data/common/search/aggs/agg_type.ts @@ -60,6 +60,7 @@ export interface AggTypeConfig< getSerializedFormat?: (agg: TAggConfig) => SerializedFieldFormat; getValue?: (agg: TAggConfig, bucket: any) => any; getKey?: (bucket: any, key: any, agg: TAggConfig) => any; + getValueBucketPath?: (agg: TAggConfig) => string; } // TODO need to make a more explicit interface for this @@ -210,6 +211,10 @@ export class AggType< return this.params.find((p: TParam) => p.name === name); }; + getValueBucketPath = (agg: TAggConfig) => { + return agg.id; + }; + /** * Generic AggType Constructor * @@ -233,6 +238,10 @@ export class AggType< this.createFilter = config.createFilter; } + if (config.getValueBucketPath) { + this.getValueBucketPath = config.getValueBucketPath; + } + if (config.params && config.params.length && config.params[0] instanceof BaseParamType) { this.params = config.params as TParam[]; } else { diff --git a/src/plugins/data/common/search/aggs/buckets/terms.ts b/src/plugins/data/common/search/aggs/buckets/terms.ts index 1363d38748c8b..3d543e6c5f574 100644 --- a/src/plugins/data/common/search/aggs/buckets/terms.ts +++ b/src/plugins/data/common/search/aggs/buckets/terms.ts @@ -41,7 +41,6 @@ import { export const termsAggFilter = [ '!top_hits', '!percentiles', - '!median', '!std_dev', '!derivative', '!moving_avg', @@ -198,14 +197,14 @@ export const getTermsBucketAgg = () => return; } - const orderAggId = orderAgg.id; + const orderAggPath = orderAgg.getValueBucketPath(); if (orderAgg.parentId && aggs) { orderAgg = aggs.byId(orderAgg.parentId); } output.subAggs = (output.subAggs || []).concat(orderAgg); - order[orderAggId] = dir; + order[orderAggPath] = dir; }, }, { diff --git a/src/plugins/data/common/search/aggs/metrics/median.ts b/src/plugins/data/common/search/aggs/metrics/median.ts index 7b48a664b5fb9..a189461020915 100644 --- a/src/plugins/data/common/search/aggs/metrics/median.ts +++ b/src/plugins/data/common/search/aggs/metrics/median.ts @@ -42,6 +42,9 @@ export const getMedianMetricAgg = () => { values: { field: aggConfig.getFieldDisplayName() }, }); }, + getValueBucketPath(aggConfig) { + return `${aggConfig.id}.50`; + }, params: [ { name: 'field', From 10e833830a90867a8b42dad4fa7a486db5c2cb9e Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 7 Oct 2020 15:18:05 +0200 Subject: [PATCH 2/4] fix unit test --- .../public/components/controls/order_agg.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/vis_default_editor/public/components/controls/order_agg.test.tsx b/src/plugins/vis_default_editor/public/components/controls/order_agg.test.tsx index 4c843791153b0..f1497631b66c5 100644 --- a/src/plugins/vis_default_editor/public/components/controls/order_agg.test.tsx +++ b/src/plugins/vis_default_editor/public/components/controls/order_agg.test.tsx @@ -106,7 +106,7 @@ describe('OrderAggParamEditor component', () => { mount(); - expect(setValue).toHaveBeenCalledWith('agg5'); + expect(setValue).toHaveBeenCalledWith('agg3'); }); it('defaults to the _key metric if no agg is compatible', () => { From d53c0fface2c2d9cf9f44a9433831b02b2740751 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 7 Oct 2020 16:48:56 +0200 Subject: [PATCH 3/4] update docs --- ...-data-public.aggconfig.getvaluebucketpath.md | 17 +++++++++++++++++ ...bana-plugin-plugins-data-public.aggconfig.md | 1 + src/plugins/data/public/public.api.md | 1 + 3 files changed, 19 insertions(+) create mode 100644 docs/development/plugins/data/public/kibana-plugin-plugins-data-public.aggconfig.getvaluebucketpath.md diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.aggconfig.getvaluebucketpath.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.aggconfig.getvaluebucketpath.md new file mode 100644 index 0000000000000..5616064ddaa0a --- /dev/null +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.aggconfig.getvaluebucketpath.md @@ -0,0 +1,17 @@ + + +[Home](./index.md) > [kibana-plugin-plugins-data-public](./kibana-plugin-plugins-data-public.md) > [AggConfig](./kibana-plugin-plugins-data-public.aggconfig.md) > [getValueBucketPath](./kibana-plugin-plugins-data-public.aggconfig.getvaluebucketpath.md) + +## AggConfig.getValueBucketPath() method + +Returns the bucket path containing the main value the agg will produce (e.g. for sum of bytes it will point to the sum, for median it will point to the 50 percentile in the percentile multi value bucket) + +Signature: + +```typescript +getValueBucketPath(): string; +``` +Returns: + +`string` + diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.aggconfig.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.aggconfig.md index ceb90cffbf6ca..d4a8eddf51cfc 100644 --- a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.aggconfig.md +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.aggconfig.md @@ -47,6 +47,7 @@ export declare class AggConfig | [getResponseAggs()](./kibana-plugin-plugins-data-public.aggconfig.getresponseaggs.md) | | | | [getTimeRange()](./kibana-plugin-plugins-data-public.aggconfig.gettimerange.md) | | | | [getValue(bucket)](./kibana-plugin-plugins-data-public.aggconfig.getvalue.md) | | | +| [getValueBucketPath()](./kibana-plugin-plugins-data-public.aggconfig.getvaluebucketpath.md) | | Returns the bucket path containing the main value the agg will produce (e.g. for sum of bytes it will point to the sum, for median it will point to the 50 percentile in the percentile multi value bucket) | | [isFilterable()](./kibana-plugin-plugins-data-public.aggconfig.isfilterable.md) | | | | [makeLabel(percentageMode)](./kibana-plugin-plugins-data-public.aggconfig.makelabel.md) | | | | [nextId(list)](./kibana-plugin-plugins-data-public.aggconfig.nextid.md) | static | Calculate the next id based on the ids in this list {array} list - a list of objects with id properties | diff --git a/src/plugins/data/public/public.api.md b/src/plugins/data/public/public.api.md index 13f658b562d39..10a43a4e31f7e 100644 --- a/src/plugins/data/public/public.api.md +++ b/src/plugins/data/public/public.api.md @@ -128,6 +128,7 @@ export class AggConfig { getTimeRange(): import("../../../public").TimeRange | undefined; // (undocumented) getValue(bucket: any): any; + getValueBucketPath(): string; // (undocumented) id: string; // (undocumented) From bff6f1660e684105181dfd83b4f323d7414af6d3 Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Wed, 14 Oct 2020 11:19:55 +0200 Subject: [PATCH 4/4] add some tests --- .../common/search/aggs/buckets/terms.test.ts | 45 +++++++++++++++++++ .../common/search/aggs/metrics/median.test.ts | 6 +++ 2 files changed, 51 insertions(+) diff --git a/src/plugins/data/common/search/aggs/buckets/terms.test.ts b/src/plugins/data/common/search/aggs/buckets/terms.test.ts index 2c5be00c8afea..8f645b4712c7f 100644 --- a/src/plugins/data/common/search/aggs/buckets/terms.test.ts +++ b/src/plugins/data/common/search/aggs/buckets/terms.test.ts @@ -18,6 +18,7 @@ */ import { AggConfigs } from '../agg_configs'; +import { METRIC_TYPES } from '../metrics'; import { mockAggTypesRegistry } from '../test_helpers'; import { BUCKET_TYPES } from './bucket_agg_types'; @@ -133,5 +134,49 @@ describe('Terms Agg', () => { expect(params.include).toStrictEqual([1.1, 2, 3.33]); expect(params.exclude).toStrictEqual([4, 5.555, 6]); }); + + test('uses correct bucket path for sorting by median', () => { + const indexPattern = { + id: '1234', + title: 'logstash-*', + fields: { + getByName: () => field, + filter: () => [field], + }, + } as any; + + const field = { + name: 'field', + indexPattern, + }; + + const aggConfigs = new AggConfigs( + indexPattern, + [ + { + id: 'test', + params: { + field: { + name: 'string_field', + type: 'string', + }, + orderAgg: { + type: METRIC_TYPES.MEDIAN, + params: { + field: { + name: 'number_field', + type: 'number', + }, + }, + }, + }, + type: BUCKET_TYPES.TERMS, + }, + ], + { typesRegistry: mockAggTypesRegistry() } + ); + const { [BUCKET_TYPES.TERMS]: params } = aggConfigs.aggs[0].toDsl(); + expect(params.order).toEqual({ 'test-orderAgg.50': 'desc' }); + }); }); }); diff --git a/src/plugins/data/common/search/aggs/metrics/median.test.ts b/src/plugins/data/common/search/aggs/metrics/median.test.ts index f3f2d157ebafc..42298586cb68f 100644 --- a/src/plugins/data/common/search/aggs/metrics/median.test.ts +++ b/src/plugins/data/common/search/aggs/metrics/median.test.ts @@ -63,6 +63,12 @@ describe('AggTypeMetricMedianProvider class', () => { expect(dsl.median.percentiles.percents).toEqual([50]); }); + it('points to right value within multi metric for value bucket path', () => { + expect(aggConfigs.byId(METRIC_TYPES.MEDIAN)!.getValueBucketPath()).toEqual( + `${METRIC_TYPES.MEDIAN}.50` + ); + }); + it('converts the response', () => { const agg = aggConfigs.getResponseAggs()[0];