From be740b6776834a77d51f8e2c94e8e1e2552b2860 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 12 Oct 2022 10:28:04 +0200 Subject: [PATCH 01/18] :sparkles: First pass with UI for random sampling --- .../data/common/search/aggs/agg_configs.ts | 6 + .../data/common/search/aggs/agg_types.ts | 2 + .../common/search/aggs/aggs_service.test.ts | 2 + .../search/aggs/buckets/bucket_agg_types.ts | 1 + .../data/common/search/aggs/buckets/index.ts | 2 + .../search/aggs/buckets/random_sampler.ts | 51 ++++++ .../aggs/buckets/random_sampler_fn.test.ts | 34 ++++ .../search/aggs/buckets/random_sampler_fn.ts | 82 +++++++++ .../lib/sibling_pipeline_agg_helper.ts | 8 +- src/plugins/data/common/search/aggs/types.ts | 5 + .../config_panel/dimension_container.scss | 45 ----- .../config_panel/dimension_container.tsx | 142 +--------------- .../config_panel/flyout_container.scss | 48 ++++++ .../config_panel/flyout_container.tsx | 159 ++++++++++++++++++ .../editor_frame/config_panel/layer_panel.tsx | 93 +++++++++- .../indexpattern_datasource/indexpattern.tsx | 13 ++ .../public/indexpattern_datasource/types.ts | 1 + x-pack/plugins/lens/public/types.ts | 10 +- 18 files changed, 518 insertions(+), 186 deletions(-) create mode 100644 src/plugins/data/common/search/aggs/buckets/random_sampler.ts create mode 100644 src/plugins/data/common/search/aggs/buckets/random_sampler_fn.test.ts create mode 100644 src/plugins/data/common/search/aggs/buckets/random_sampler_fn.ts create mode 100644 x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/flyout_container.scss create mode 100644 x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/flyout_container.tsx diff --git a/src/plugins/data/common/search/aggs/agg_configs.ts b/src/plugins/data/common/search/aggs/agg_configs.ts index c61ca69e0c6df..74abe5949376c 100644 --- a/src/plugins/data/common/search/aggs/agg_configs.ts +++ b/src/plugins/data/common/search/aggs/agg_configs.ts @@ -55,6 +55,8 @@ export interface AggConfigsOptions { hierarchical?: boolean; aggExecutionContext?: AggTypesDependencies['aggExecutionContext']; partialRows?: boolean; + probability: number; + samplerSeed?: number; } export type CreateAggConfigParams = Assign; @@ -248,6 +250,10 @@ export class AggConfigs { }; }); } + if (this.opts.probability !== 1) { + // inject the random sampler here as agg root + // console.log('Has random sampling in here'); + } const requestAggs = this.getRequestAggs(); const aggsWithDsl = requestAggs.filter((agg) => !agg.type.hasNoDsl).length; const timeSplitIndex = this.getAll().findIndex( diff --git a/src/plugins/data/common/search/aggs/agg_types.ts b/src/plugins/data/common/search/aggs/agg_types.ts index 43e3df6100a75..dc1063f7f3b28 100644 --- a/src/plugins/data/common/search/aggs/agg_types.ts +++ b/src/plugins/data/common/search/aggs/agg_types.ts @@ -69,6 +69,7 @@ export const getAggTypes = () => ({ { name: BUCKET_TYPES.GEOHASH_GRID, fn: buckets.getGeoHashBucketAgg }, { name: BUCKET_TYPES.GEOTILE_GRID, fn: buckets.getGeoTitleBucketAgg }, { name: BUCKET_TYPES.SAMPLER, fn: buckets.getSamplerBucketAgg }, + { name: BUCKET_TYPES.RANDOM_SAMPLER, fn: buckets.getRandomSamplerBucketAgg }, { name: BUCKET_TYPES.DIVERSIFIED_SAMPLER, fn: buckets.getDiversifiedSamplerBucketAgg }, ], }); @@ -90,6 +91,7 @@ export const getAggTypesFunctions = () => [ buckets.aggMultiTerms, buckets.aggRareTerms, buckets.aggSampler, + buckets.aggRandomSampler, buckets.aggDiversifiedSampler, metrics.aggAvg, metrics.aggBucketAvg, diff --git a/src/plugins/data/common/search/aggs/aggs_service.test.ts b/src/plugins/data/common/search/aggs/aggs_service.test.ts index f0425e460ae0f..6563fe30c81c8 100644 --- a/src/plugins/data/common/search/aggs/aggs_service.test.ts +++ b/src/plugins/data/common/search/aggs/aggs_service.test.ts @@ -69,6 +69,7 @@ describe('Aggs service', () => { "geotile_grid", "sampler", "diversified_sampler", + "random_sampler", "foo", ] `); @@ -123,6 +124,7 @@ describe('Aggs service', () => { "geotile_grid", "sampler", "diversified_sampler", + "random_sampler", ] `); expect(bStart.types.getAll().metrics.map((t) => t.name)).toMatchInlineSnapshot(` diff --git a/src/plugins/data/common/search/aggs/buckets/bucket_agg_types.ts b/src/plugins/data/common/search/aggs/buckets/bucket_agg_types.ts index fcfbb432e3055..f8227650fe7d6 100644 --- a/src/plugins/data/common/search/aggs/buckets/bucket_agg_types.ts +++ b/src/plugins/data/common/search/aggs/buckets/bucket_agg_types.ts @@ -23,4 +23,5 @@ export enum BUCKET_TYPES { DATE_HISTOGRAM = 'date_histogram', SAMPLER = 'sampler', DIVERSIFIED_SAMPLER = 'diversified_sampler', + RANDOM_SAMPLER = 'random_sampler', } diff --git a/src/plugins/data/common/search/aggs/buckets/index.ts b/src/plugins/data/common/search/aggs/buckets/index.ts index 000dcd5382b56..bb5b8f61b2f27 100644 --- a/src/plugins/data/common/search/aggs/buckets/index.ts +++ b/src/plugins/data/common/search/aggs/buckets/index.ts @@ -48,4 +48,6 @@ export * from './sampler_fn'; export * from './sampler'; export * from './diversified_sampler_fn'; export * from './diversified_sampler'; +export * from './random_sampler_fn'; +export * from './random_sampler'; export { SHARD_DELAY_AGG_NAME } from './shard_delay'; diff --git a/src/plugins/data/common/search/aggs/buckets/random_sampler.ts b/src/plugins/data/common/search/aggs/buckets/random_sampler.ts new file mode 100644 index 0000000000000..8d4dfbee08d5a --- /dev/null +++ b/src/plugins/data/common/search/aggs/buckets/random_sampler.ts @@ -0,0 +1,51 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { i18n } from '@kbn/i18n'; +import { BucketAggType } from './bucket_agg_type'; +import { BaseAggParams } from '../types'; +import { aggRandomSamplerFnName } from './random_sampler_fn'; + +export const RANDOM_SAMPLER_AGG_NAME = 'random_sampler'; + +const title = i18n.translate('data.search.aggs.buckets.randomSamplerTitle', { + defaultMessage: 'Random Sampler', + description: 'Random sampler aggregation title', +}); + +export interface AggParamsRandomSampler extends BaseAggParams { + /** + * The sampling probability + */ + probability: number; + /** + * The seed to generate the random sampling of documents. (optional) + */ + seed?: number; +} + +/** + * A filtering aggregation used to limit any sub aggregations' processing to a sample of the top-scoring documents. + */ +export const getRandomSamplerBucketAgg = () => + new BucketAggType({ + name: RANDOM_SAMPLER_AGG_NAME, + title, + customLabels: false, + expressionName: aggRandomSamplerFnName, + params: [ + { + name: 'probability', + type: 'number', + }, + { + name: 'seed', + type: 'number', + }, + ], + }); diff --git a/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.test.ts b/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.test.ts new file mode 100644 index 0000000000000..b14e9b1631896 --- /dev/null +++ b/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.test.ts @@ -0,0 +1,34 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { functionWrapper } from '../test_helpers'; +import { aggRandomSampler } from './random_sampler_fn'; + +describe('aggRandomSampler', () => { + const fn = functionWrapper(aggRandomSampler()); + + test('includes params when they are provided', () => { + const actual = fn({ + id: 'random_sampler', + schema: 'bucket', + probability: 0.1, + }); + + expect(actual.value).toMatchInlineSnapshot(` + Object { + "enabled": true, + "id": "random_sampler", + "params": Object { + "probability": 0.1, + }, + "schema": "bucket", + "type": "random_sampler", + } + `); + }); +}); diff --git a/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.ts b/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.ts new file mode 100644 index 0000000000000..0094f94dab151 --- /dev/null +++ b/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.ts @@ -0,0 +1,82 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { i18n } from '@kbn/i18n'; +import { ExpressionFunctionDefinition } from '@kbn/expressions-plugin/common'; +import { AggExpressionFunctionArgs, AggExpressionType, BUCKET_TYPES } from '..'; +import { RANDOM_SAMPLER_AGG_NAME } from './random_sampler'; + +export const aggRandomSamplerFnName = 'aggRandomSampler'; + +type Input = any; +type Arguments = AggExpressionFunctionArgs; + +type Output = AggExpressionType; +type FunctionDefinition = ExpressionFunctionDefinition< + typeof aggRandomSamplerFnName, + Input, + Arguments, + Output +>; + +export const aggRandomSampler = (): FunctionDefinition => ({ + name: aggRandomSamplerFnName, + help: i18n.translate('data.search.aggs.function.buckets.randomSampler.help', { + defaultMessage: 'Generates a serialized agg config for a Random sampler agg', + }), + type: 'agg_type', + args: { + id: { + types: ['string'], + help: i18n.translate('data.search.aggs.buckets.randomSampler.id.help', { + defaultMessage: 'ID for this aggregation', + }), + }, + enabled: { + types: ['boolean'], + default: true, + help: i18n.translate('data.search.aggs.buckets.randomSampler.enabled.help', { + defaultMessage: 'Specifies whether this aggregation should be enabled', + }), + }, + schema: { + types: ['string'], + help: i18n.translate('data.search.aggs.buckets.randomSampler.schema.help', { + defaultMessage: 'Schema to use for this aggregation', + }), + }, + probability: { + types: ['number'], + help: i18n.translate('data.search.aggs.buckets.randomSampler.probability.help', { + defaultMessage: 'The sampling probability', + }), + }, + seed: { + types: ['number'], + help: i18n.translate('data.search.aggs.buckets.randomSampler.seed.help', { + defaultMessage: 'The seed to generate the random sampling of documents.', + }), + }, + }, + fn: (input, args) => { + const { id, enabled, schema, ...rest } = args; + + return { + type: 'agg_type', + value: { + id, + enabled, + schema, + type: RANDOM_SAMPLER_AGG_NAME, + params: { + ...rest, + }, + }, + }; + }, +}); diff --git a/src/plugins/data/common/search/aggs/metrics/lib/sibling_pipeline_agg_helper.ts b/src/plugins/data/common/search/aggs/metrics/lib/sibling_pipeline_agg_helper.ts index 243a119847a2c..c50eb36468e9f 100644 --- a/src/plugins/data/common/search/aggs/metrics/lib/sibling_pipeline_agg_helper.ts +++ b/src/plugins/data/common/search/aggs/metrics/lib/sibling_pipeline_agg_helper.ts @@ -31,7 +31,13 @@ const metricAggFilter: string[] = [ '!filtered_metric', '!single_percentile', ]; -const bucketAggFilter: string[] = ['!filter', '!sampler', '!diversified_sampler', '!multi_terms']; +const bucketAggFilter: string[] = [ + '!filter', + '!sampler', + '!diversified_sampler', + '!random_sampler', + '!multi_terms', +]; export const siblingPipelineType = i18n.translate( 'data.search.aggs.metrics.siblingPipelineAggregationsSubtypeTitle', diff --git a/src/plugins/data/common/search/aggs/types.ts b/src/plugins/data/common/search/aggs/types.ts index bc35e46d8da7a..19ecd1eaa866d 100644 --- a/src/plugins/data/common/search/aggs/types.ts +++ b/src/plugins/data/common/search/aggs/types.ts @@ -109,8 +109,10 @@ import { AggParamsMovingAvgSerialized, AggParamsSerialDiffSerialized, AggParamsTopHitSerialized, + aggRandomSampler, } from '.'; import { AggParamsSampler } from './buckets/sampler'; +import { AggParamsRandomSampler } from './buckets/random_sampler'; import { AggParamsDiversifiedSampler } from './buckets/diversified_sampler'; import { AggParamsSignificantText } from './buckets/significant_text'; import { aggTopMetrics } from './metrics/top_metrics_fn'; @@ -185,6 +187,7 @@ interface SerializedAggParamsMapping { [BUCKET_TYPES.MULTI_TERMS]: AggParamsMultiTermsSerialized; [BUCKET_TYPES.RARE_TERMS]: AggParamsRareTerms; [BUCKET_TYPES.SAMPLER]: AggParamsSampler; + [BUCKET_TYPES.RANDOM_SAMPLER]: AggParamsRandomSampler; [BUCKET_TYPES.DIVERSIFIED_SAMPLER]: AggParamsDiversifiedSampler; [METRIC_TYPES.AVG]: AggParamsAvg; [METRIC_TYPES.CARDINALITY]: AggParamsCardinality; @@ -230,6 +233,7 @@ export interface AggParamsMapping { [BUCKET_TYPES.MULTI_TERMS]: AggParamsMultiTerms; [BUCKET_TYPES.RARE_TERMS]: AggParamsRareTerms; [BUCKET_TYPES.SAMPLER]: AggParamsSampler; + [BUCKET_TYPES.RANDOM_SAMPLER]: AggParamsRandomSampler; [BUCKET_TYPES.DIVERSIFIED_SAMPLER]: AggParamsDiversifiedSampler; [METRIC_TYPES.AVG]: AggParamsAvg; [METRIC_TYPES.CARDINALITY]: AggParamsCardinality; @@ -265,6 +269,7 @@ export interface AggFunctionsMapping { aggFilter: ReturnType; aggFilters: ReturnType; aggSignificantTerms: ReturnType; + aggRandomSampler: ReturnType; aggIpRange: ReturnType; aggDateRange: ReturnType; aggRange: ReturnType; diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss index b8ae7fa515190..deb353e7a1df5 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.scss @@ -1,48 +1,3 @@ -@import '@elastic/eui/src/components/flyout/variables'; -@import '@elastic/eui/src/components/flyout/mixins'; - -.lnsDimensionContainer { - // Use the EuiFlyout style - @include euiFlyout; - // But with custom positioning to keep it within the sidebar contents - animation: euiFlyout $euiAnimSpeedNormal $euiAnimSlightResistance; - left: 0; - max-width: none !important; - z-index: $euiZContentMenu; - - @include euiBreakpoint('l', 'xl') { - height: 100% !important; - position: absolute; - top: 0 !important; - } - - .lnsFrameLayout__sidebar-isFullscreen & { - border-left: $euiBorderThin; // Force border regardless of theme in fullscreen - box-shadow: none; - } -} - -.lnsDimensionContainer__header { - padding: $euiSize; - - .lnsFrameLayout__sidebar-isFullscreen & { - display: none; - } -} - -.lnsDimensionContainer__content { - @include euiYScroll; - flex: 1; -} - -.lnsDimensionContainer__footer { - padding: $euiSize; - - .lnsFrameLayout__sidebar-isFullscreen & { - display: none; - } -} - .lnsBody--overflowHidden { overflow: hidden; } diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx index 9d71e74eff473..13ba032f9b902 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_container.tsx @@ -7,44 +7,12 @@ import './dimension_container.scss'; -import React, { useState, useEffect, useCallback } from 'react'; -import { - EuiFlyoutHeader, - EuiFlyoutFooter, - EuiTitle, - EuiButtonIcon, - EuiButtonEmpty, - EuiFlexGroup, - EuiFlexItem, - EuiFocusTrap, -} from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; -import { DONT_CLOSE_DIMENSION_CONTAINER_ON_CLICK_CLASS } from '../../../utils'; - -function fromExcludedClickTarget(event: Event) { - for ( - let node: HTMLElement | null = event.target as HTMLElement; - node !== null; - node = node!.parentElement - ) { - if ( - node.classList!.contains(DONT_CLOSE_DIMENSION_CONTAINER_ON_CLICK_CLASS) || - node.classList!.contains('euiBody-hasPortalContent') || - node.getAttribute('data-euiportal') === 'true' - ) { - return true; - } - } - return false; -} +import React from 'react'; +import { FlyoutContainer } from './flyout_container'; export function DimensionContainer({ - isOpen, - groupLabel, - handleClose, panel, - isFullscreen, - panelRef, + ...props }: { isOpen: boolean; handleClose: () => boolean; @@ -53,107 +21,5 @@ export function DimensionContainer({ isFullscreen: boolean; panelRef: (el: HTMLDivElement) => void; }) { - const [focusTrapIsEnabled, setFocusTrapIsEnabled] = useState(false); - - const closeFlyout = useCallback(() => { - const canClose = handleClose(); - if (canClose) { - setFocusTrapIsEnabled(false); - } - return canClose; - }, [handleClose]); - - useEffect(() => { - document.body.classList.toggle('lnsBody--overflowHidden', isOpen); - return () => { - if (isOpen) { - setFocusTrapIsEnabled(false); - } - document.body.classList.remove('lnsBody--overflowHidden'); - }; - }, [isOpen]); - - if (!isOpen) { - return null; - } - - return ( -
- { - if (isFullscreen || fromExcludedClickTarget(event)) { - return; - } - closeFlyout(); - }} - onEscapeKey={closeFlyout} - > -
{ - if (isOpen) { - // EuiFocusTrap interferes with animating elements with absolute position: - // running this onAnimationEnd, otherwise the flyout pushes content when animating - setFocusTrapIsEnabled(true); - } - }} - > - - - - -

- - {i18n.translate('xpack.lens.configure.configurePanelTitle', { - defaultMessage: '{groupLabel}', - values: { - groupLabel, - }, - })} - -

- - - - - - - - - -
{panel}
- - - - {i18n.translate('xpack.lens.dimensionContainer.close', { - defaultMessage: 'Close', - })} - - -
- -
- ); + return {panel}; } diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/flyout_container.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/flyout_container.scss new file mode 100644 index 0000000000000..b08eb6281fa0e --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/flyout_container.scss @@ -0,0 +1,48 @@ +@import '@elastic/eui/src/components/flyout/variables'; +@import '@elastic/eui/src/components/flyout/mixins'; + +.lnsDimensionContainer { + // Use the EuiFlyout style + @include euiFlyout; + // But with custom positioning to keep it within the sidebar contents + animation: euiFlyout $euiAnimSpeedNormal $euiAnimSlightResistance; + left: 0; + max-width: none !important; + z-index: $euiZContentMenu; + + @include euiBreakpoint('l', 'xl') { + height: 100% !important; + position: absolute; + top: 0 !important; + } + + .lnsFrameLayout__sidebar-isFullscreen & { + border-left: $euiBorderThin; // Force border regardless of theme in fullscreen + box-shadow: none; + } +} + +.lnsDimensionContainer__header { + padding: $euiSize; + + .lnsFrameLayout__sidebar-isFullscreen & { + display: none; + } +} + +.lnsDimensionContainer__content { + @include euiYScroll; + flex: 1; +} + +.lnsDimensionContainer__footer { + padding: $euiSize; + + .lnsFrameLayout__sidebar-isFullscreen & { + display: none; + } +} + +.lnsBody--overflowHidden { + overflow: hidden; +} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/flyout_container.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/flyout_container.tsx new file mode 100644 index 0000000000000..dba2ffc73db01 --- /dev/null +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/flyout_container.tsx @@ -0,0 +1,159 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import './flyout_container.scss'; + +import React, { useState, useEffect, useCallback } from 'react'; +import { + EuiFlyoutHeader, + EuiFlyoutFooter, + EuiTitle, + EuiButtonIcon, + EuiButtonEmpty, + EuiFlexGroup, + EuiFlexItem, + EuiFocusTrap, +} from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { DONT_CLOSE_DIMENSION_CONTAINER_ON_CLICK_CLASS } from '../../../utils'; + +function fromExcludedClickTarget(event: Event) { + for ( + let node: HTMLElement | null = event.target as HTMLElement; + node !== null; + node = node!.parentElement + ) { + if ( + node.classList!.contains(DONT_CLOSE_DIMENSION_CONTAINER_ON_CLICK_CLASS) || + node.classList!.contains('euiBody-hasPortalContent') || + node.getAttribute('data-euiportal') === 'true' + ) { + return true; + } + } + return false; +} + +export function FlyoutContainer({ + isOpen, + groupLabel, + handleClose, + isFullscreen, + panelRef, + children, +}: { + isOpen: boolean; + handleClose: () => boolean; + children: React.ReactElement | null; + groupLabel: string; + isFullscreen: boolean; + panelRef: (el: HTMLDivElement) => void; +}) { + const [focusTrapIsEnabled, setFocusTrapIsEnabled] = useState(false); + + const closeFlyout = useCallback(() => { + const canClose = handleClose(); + if (canClose) { + setFocusTrapIsEnabled(false); + } + return canClose; + }, [handleClose]); + + useEffect(() => { + document.body.classList.toggle('lnsBody--overflowHidden', isOpen); + return () => { + if (isOpen) { + setFocusTrapIsEnabled(false); + } + document.body.classList.remove('lnsBody--overflowHidden'); + }; + }, [isOpen]); + + if (!isOpen) { + return null; + } + + return ( +
+ { + if (isFullscreen || fromExcludedClickTarget(event)) { + return; + } + closeFlyout(); + }} + onEscapeKey={closeFlyout} + > +
{ + if (isOpen) { + // EuiFocusTrap interferes with animating elements with absolute position: + // running this onAnimationEnd, otherwise the flyout pushes content when animating + setFocusTrapIsEnabled(true); + } + }} + > + + + + +

+ + {i18n.translate('xpack.lens.configure.configurePanelTitle', { + defaultMessage: '{groupLabel}', + values: { + groupLabel, + }, + })} + +

+ + + + + + + + + +
{children}
+ + + + {i18n.translate('xpack.lens.dimensionContainer.close', { + defaultMessage: 'Close', + })} + + +
+ +
+ ); +} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index 3d1068ebd521f..3a63b24b992d0 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -16,6 +16,9 @@ import { EuiFormRow, EuiText, EuiIconTip, + EuiRange, + EuiIcon, + EuiToolTip, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { LayerActions } from './layer_actions'; @@ -44,11 +47,14 @@ import { } from '../../../state_management'; import { onDropForVisualization } from './buttons/drop_targets_utils'; import { getSharedActions } from './layer_actions/layer_actions'; +import { FlyoutContainer } from './flyout_container'; const initialActiveDimensionState = { isNew: false, }; +const samplingValue = [0.0001, 0.001, 0.01, 0.1, 1]; + export function LayerPanel( props: Exclude & { activeVisualization: Visualization; @@ -80,6 +86,8 @@ export function LayerPanel( const [activeDimension, setActiveDimension] = useState( initialActiveDimensionState ); + const [isPanelSettingsOpen, setPanelSettingsOpen] = useState(false); + const [hideTooltip, setHideTooltip] = useState(false); const { @@ -110,6 +118,7 @@ export function LayerPanel( }, [activeVisualization.id]); const panelRef = useRef(null); + const settingsPanelRef = useRef(null); const registerLayerRef = useCallback( (el) => registerNewLayerRef(layerId, el), [layerId, registerNewLayerRef] @@ -317,7 +326,14 @@ export function LayerPanel( ...(activeVisualization.getSupportedActionsForLayer?.( layerId, visualizationState, - updateVisualization + updateVisualization, + () => setPanelSettingsOpen(true) + ) || []), + ...(layerDatasource?.getSupportedActionsForLayer?.( + layerId, + layerDatasourceState, + (newState) => updateDatasource(datasourceId, newState), + () => setPanelSettingsOpen(true) ) || []), ...getSharedActions({ activeVisualization, @@ -333,17 +349,25 @@ export function LayerPanel( [ activeVisualization, core, + datasourceId, isOnlyLayer, isTextBasedLanguage, + layerDatasource, + layerDatasourceState, layerId, layerIndex, onCloneLayer, onRemoveLayer, + updateDatasource, updateVisualization, visualizationState, ] ); + const samplingIndex = samplingValue.findIndex( + (v) => v === layerDatasourceState.layers[layerId].sampling + ); + return ( <>
@@ -650,6 +674,73 @@ export function LayerPanel( })}
+ (settingsPanelRef.current = el)} + isOpen={isPanelSettingsOpen} + isFullscreen={false} + groupLabel={i18n.translate('xpack.lens.editorFrame.layerSettingsTitle', { + defaultMessage: 'Layer settings', + })} + handleClose={() => { + // update the current layer settings + setPanelSettingsOpen(false); + return true; + }} + > +
+
+ + + {i18n.translate('xpack.lens.xyChart.randomSampling.label', { + defaultMessage: 'Random Sampling', + })} + + + + } + > + -1 ? samplingIndex : samplingValue.length - 1} + onChange={(e) => { + updateDatasource(datasourceId, { + ...layerDatasourceState, + layers: { + ...layerDatasourceState.layers, + [layerId]: { + ...layerDatasourceState.layers[layerId], + sampling: samplingValue[Number(e.target.value)], + }, + }, + }); + }} + showInput={false} + showRange={false} + showTicks + step={1} + min={0} + max={samplingValue.length - 1} + ticks={samplingValue.map((v, i) => ({ label: `${v}`, value: i }))} + /> + +
+
+ (panelRef.current = el)} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx index fb31c3c1a9a71..89a4233702f29 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx @@ -443,6 +443,18 @@ export function getIndexPatternDatasource({ getDropProps, onDrop, + getSupportedActionsForLayer(layerId, state, _, openLayerSettings) { + return [ + { + displayName: i18n.translate('xpack.lens.indexPattern.layerSettingsAction', { + defaultMessage: 'Layer settings', + }), + execute: openLayerSettings, + icon: 'gear', + isCompatible: true, + }, + ]; + }, getCustomWorkspaceRenderer: ( state: IndexPatternPrivateState, @@ -794,5 +806,6 @@ function blankLayer(indexPatternId: string) { indexPatternId, columns: {}, columnOrder: [], + sampling: 4, }; } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/types.ts b/x-pack/plugins/lens/public/indexpattern_datasource/types.ts index 5ff9b183d2579..4140ea076649c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/types.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/types.ts @@ -54,6 +54,7 @@ export interface IndexPatternLayer { indexPatternId: string; // Partial columns represent the temporary invalid states incompleteColumns?: Record; + sampling?: number; } export interface IndexPatternPersistedState { diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 29aff3d428690..2fba9acb6aaa5 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -439,6 +439,13 @@ export interface Datasource { * Get all the used DataViews from state */ getUsedDataViews: (state: T) => string[]; + + getSupportedActionsForLayer?: ( + layerId: string, + state: T, + setState: StateSetter, + openLayerSettings: () => void + ) => LayerAction[]; } export interface DatasourceFixAction { @@ -984,7 +991,8 @@ export interface Visualization { getSupportedActionsForLayer?: ( layerId: string, state: T, - setState: StateSetter + setState: StateSetter, + openLayerSettings: () => void ) => LayerAction[]; /** returns the type string of the given layer */ getLayerType: (layerId: string, state?: T) => LayerType | undefined; From b45c6de9d0732861bf2e0a51cf09a7f07b16d501 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 12 Oct 2022 18:57:34 +0200 Subject: [PATCH 02/18] :sparkles: Initial working version --- .../data/common/search/aggs/agg_configs.ts | 26 +++++++++++++++---- .../common/search/aggs/utils/time_splits.ts | 8 +++--- .../search/expressions/esaggs/esaggs_fn.ts | 17 ++++++++++++ .../expressions/esaggs/request_handler.ts | 1 + .../data/common/search/tabify/tabify.ts | 4 ++- .../data/common/search/tabify/types.ts | 1 + .../data/public/search/expressions/esaggs.ts | 7 ++++- .../datasources/form_based/to_expression.ts | 2 ++ 8 files changed, 55 insertions(+), 11 deletions(-) diff --git a/src/plugins/data/common/search/aggs/agg_configs.ts b/src/plugins/data/common/search/aggs/agg_configs.ts index 74abe5949376c..1db856893c126 100644 --- a/src/plugins/data/common/search/aggs/agg_configs.ts +++ b/src/plugins/data/common/search/aggs/agg_configs.ts @@ -109,6 +109,10 @@ export class AggConfigs { return this.opts.partialRows ?? false; } + public get samplerConfig() { + return { probability: this.opts.probability, seed: this.opts.samplerSeed }; + } + setTimeFields(timeFields: string[] | undefined) { this.timeFields = timeFields; } @@ -227,10 +231,20 @@ export class AggConfigs { } toDsl(): Record { - const dslTopLvl = {}; + const dslTopLvl: Record = {}; let dslLvlCursor: Record; let nestedMetrics: Array<{ config: AggConfig; dsl: Record }> | []; + if (this.opts.probability !== 1) { + dslTopLvl.sampling = { + random_sampler: { + probability: this.opts.probability, + seed: this.opts.samplerSeed, + }, + aggs: {}, + }; + } + const timeShifts = this.getTimeShifts(); const hasMultipleTimeShifts = Object.keys(timeShifts).length > 1; @@ -250,10 +264,6 @@ export class AggConfigs { }; }); } - if (this.opts.probability !== 1) { - // inject the random sampler here as agg root - // console.log('Has random sampling in here'); - } const requestAggs = this.getRequestAggs(); const aggsWithDsl = requestAggs.filter((agg) => !agg.type.hasNoDsl).length; const timeSplitIndex = this.getAll().findIndex( @@ -264,6 +274,10 @@ export class AggConfigs { if (!dslLvlCursor) { // start at the top level dslLvlCursor = dslTopLvl; + // when sampling jump directly to the aggs + if (this.opts.probability !== 1) { + dslLvlCursor = dslLvlCursor.sampling.aggs; + } } else { const prevConfig: AggConfig = list[i - 1]; const prevDsl = dslLvlCursor[prevConfig.id]; @@ -537,6 +551,8 @@ export class AggConfigs { metricsAtAllLevels: this.hierarchical, partialRows: this.partialRows, aggs: this.aggs.map((agg) => buildExpression(agg.toExpressionAst())), + probability: this.opts.probability, + samplerSeed: this.opts.samplerSeed, }), ]).toAst(); } diff --git a/src/plugins/data/common/search/aggs/utils/time_splits.ts b/src/plugins/data/common/search/aggs/utils/time_splits.ts index c2fe8aaca0fb2..b1262683446f3 100644 --- a/src/plugins/data/common/search/aggs/utils/time_splits.ts +++ b/src/plugins/data/common/search/aggs/utils/time_splits.ts @@ -427,11 +427,11 @@ export function insertTimeShiftSplit( const timeRange = aggConfigs.timeRange; const filters: Record = {}; const timeField = aggConfigs.timeFields[0]; + const timeFilter = getTime(aggConfigs.indexPattern, timeRange, { + fieldName: timeField, + forceNow: aggConfigs.forceNow, + }) as RangeFilter; Object.entries(timeShifts).forEach(([key, shift]) => { - const timeFilter = getTime(aggConfigs.indexPattern, timeRange, { - fieldName: timeField, - forceNow: aggConfigs.forceNow, - }) as RangeFilter; if (timeFilter) { filters[key] = { range: { diff --git a/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts b/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts index c8296b3c06557..c659b225aacea 100644 --- a/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts +++ b/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts @@ -32,6 +32,8 @@ interface Arguments { metricsAtAllLevels?: boolean; partialRows?: boolean; timeFields?: string[]; + probability: number; + samplerSeed?: number; } export type EsaggsExpressionFunctionDefinition = ExpressionFunctionDefinition< @@ -94,6 +96,21 @@ export const getEsaggsMeta: () => Omit defaultMessage: 'Provide time fields to get the resolved time ranges for the query', }), }, + probability: { + types: ['number'], + default: 1, + help: i18n.translate('data.search.functions.esaggs.probability.help', { + defaultMessage: + 'The probability that a document will be included in the aggregated data. Uses random sampler.', + }), + }, + samplerSeed: { + types: ['number'], + help: i18n.translate('data.search.functions.esaggs.samplerSeed.help', { + defaultMessage: + 'The seed to generate the random sampling of documents. Uses random sampler.', + }), + }, }, }); diff --git a/src/plugins/data/common/search/expressions/esaggs/request_handler.ts b/src/plugins/data/common/search/expressions/esaggs/request_handler.ts index 03512bcd2e270..630a50c01adee 100644 --- a/src/plugins/data/common/search/expressions/esaggs/request_handler.ts +++ b/src/plugins/data/common/search/expressions/esaggs/request_handler.ts @@ -133,6 +133,7 @@ export const handleRequest = ({ const tabifyParams = { metricsAtAllLevels: aggs.hierarchical, partialRows: aggs.partialRows, + samplerConfig: aggs.samplerConfig, timeRange: parsedTimeRange ? { from: parsedTimeRange.min, to: parsedTimeRange.max, timeFields: allTimeFields } : undefined, diff --git a/src/plugins/data/common/search/tabify/tabify.ts b/src/plugins/data/common/search/tabify/tabify.ts index ea26afd30129a..969d530e55fe1 100644 --- a/src/plugins/data/common/search/tabify/tabify.ts +++ b/src/plugins/data/common/search/tabify/tabify.ts @@ -147,7 +147,9 @@ export function tabifyAggResponse( const write = new TabbedAggResponseWriter(aggConfigs, respOpts || {}); const topLevelBucket: AggResponseBucket = { - ...esResponse.aggregations, + ...(aggConfigs.samplerConfig.probability !== 1 + ? esResponse.aggregations.sampling + : esResponse.aggregations), doc_count: esResponse.aggregations?.doc_count || esResponse.hits?.total, }; diff --git a/src/plugins/data/common/search/tabify/types.ts b/src/plugins/data/common/search/tabify/types.ts index bf0a99725e2ab..cfeb3052d33db 100644 --- a/src/plugins/data/common/search/tabify/types.ts +++ b/src/plugins/data/common/search/tabify/types.ts @@ -26,6 +26,7 @@ export interface TabbedResponseWriterOptions { metricsAtAllLevels: boolean; partialRows: boolean; timeRange?: TimeRangeInformation; + samplerConfig: { probability: number; seed?: number }; } /** @internal */ diff --git a/src/plugins/data/public/search/expressions/esaggs.ts b/src/plugins/data/public/search/expressions/esaggs.ts index d944dcd5c1a5d..b82401d4d8caf 100644 --- a/src/plugins/data/public/search/expressions/esaggs.ts +++ b/src/plugins/data/public/search/expressions/esaggs.ts @@ -48,7 +48,12 @@ export function getFunctionDefinition({ const aggConfigs = aggs.createAggConfigs( indexPattern, args.aggs?.map((agg) => agg.value) ?? [], - { hierarchical: args.metricsAtAllLevels, partialRows: args.partialRows } + { + hierarchical: args.metricsAtAllLevels, + partialRows: args.partialRows, + probability: args.probability, + samplerSeed: args.samplerSeed, + } ); const { handleEsaggsRequest } = await import('../../../common/search/expressions'); diff --git a/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts b/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts index fc77aa6520bd0..ff038b0939e6a 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts @@ -392,6 +392,8 @@ function getExpressionForLayer( metricsAtAllLevels: false, partialRows: false, timeFields: allDateHistogramFields, + probability: layer.sampling || 1, + // TODO: add samplerSeed here from search session }).toAst(), { type: 'function', From 1448a22e5728d6c7ae1ac5116d9c128343ccefe8 Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 13 Oct 2022 17:41:18 +0200 Subject: [PATCH 03/18] :fire: Remove unused stuff --- .../data/common/search/aggs/agg_types.ts | 2 - .../common/search/aggs/aggs_service.test.ts | 2 - .../search/aggs/buckets/bucket_agg_types.ts | 1 - .../data/common/search/aggs/buckets/index.ts | 2 - .../search/aggs/buckets/random_sampler.ts | 51 ------------ .../aggs/buckets/random_sampler_fn.test.ts | 34 -------- .../search/aggs/buckets/random_sampler_fn.ts | 82 ------------------- .../lib/sibling_pipeline_agg_helper.ts | 8 +- src/plugins/data/common/search/aggs/types.ts | 5 -- 9 files changed, 1 insertion(+), 186 deletions(-) delete mode 100644 src/plugins/data/common/search/aggs/buckets/random_sampler.ts delete mode 100644 src/plugins/data/common/search/aggs/buckets/random_sampler_fn.test.ts delete mode 100644 src/plugins/data/common/search/aggs/buckets/random_sampler_fn.ts diff --git a/src/plugins/data/common/search/aggs/agg_types.ts b/src/plugins/data/common/search/aggs/agg_types.ts index dc1063f7f3b28..43e3df6100a75 100644 --- a/src/plugins/data/common/search/aggs/agg_types.ts +++ b/src/plugins/data/common/search/aggs/agg_types.ts @@ -69,7 +69,6 @@ export const getAggTypes = () => ({ { name: BUCKET_TYPES.GEOHASH_GRID, fn: buckets.getGeoHashBucketAgg }, { name: BUCKET_TYPES.GEOTILE_GRID, fn: buckets.getGeoTitleBucketAgg }, { name: BUCKET_TYPES.SAMPLER, fn: buckets.getSamplerBucketAgg }, - { name: BUCKET_TYPES.RANDOM_SAMPLER, fn: buckets.getRandomSamplerBucketAgg }, { name: BUCKET_TYPES.DIVERSIFIED_SAMPLER, fn: buckets.getDiversifiedSamplerBucketAgg }, ], }); @@ -91,7 +90,6 @@ export const getAggTypesFunctions = () => [ buckets.aggMultiTerms, buckets.aggRareTerms, buckets.aggSampler, - buckets.aggRandomSampler, buckets.aggDiversifiedSampler, metrics.aggAvg, metrics.aggBucketAvg, diff --git a/src/plugins/data/common/search/aggs/aggs_service.test.ts b/src/plugins/data/common/search/aggs/aggs_service.test.ts index 6563fe30c81c8..f0425e460ae0f 100644 --- a/src/plugins/data/common/search/aggs/aggs_service.test.ts +++ b/src/plugins/data/common/search/aggs/aggs_service.test.ts @@ -69,7 +69,6 @@ describe('Aggs service', () => { "geotile_grid", "sampler", "diversified_sampler", - "random_sampler", "foo", ] `); @@ -124,7 +123,6 @@ describe('Aggs service', () => { "geotile_grid", "sampler", "diversified_sampler", - "random_sampler", ] `); expect(bStart.types.getAll().metrics.map((t) => t.name)).toMatchInlineSnapshot(` diff --git a/src/plugins/data/common/search/aggs/buckets/bucket_agg_types.ts b/src/plugins/data/common/search/aggs/buckets/bucket_agg_types.ts index f8227650fe7d6..fcfbb432e3055 100644 --- a/src/plugins/data/common/search/aggs/buckets/bucket_agg_types.ts +++ b/src/plugins/data/common/search/aggs/buckets/bucket_agg_types.ts @@ -23,5 +23,4 @@ export enum BUCKET_TYPES { DATE_HISTOGRAM = 'date_histogram', SAMPLER = 'sampler', DIVERSIFIED_SAMPLER = 'diversified_sampler', - RANDOM_SAMPLER = 'random_sampler', } diff --git a/src/plugins/data/common/search/aggs/buckets/index.ts b/src/plugins/data/common/search/aggs/buckets/index.ts index bb5b8f61b2f27..000dcd5382b56 100644 --- a/src/plugins/data/common/search/aggs/buckets/index.ts +++ b/src/plugins/data/common/search/aggs/buckets/index.ts @@ -48,6 +48,4 @@ export * from './sampler_fn'; export * from './sampler'; export * from './diversified_sampler_fn'; export * from './diversified_sampler'; -export * from './random_sampler_fn'; -export * from './random_sampler'; export { SHARD_DELAY_AGG_NAME } from './shard_delay'; diff --git a/src/plugins/data/common/search/aggs/buckets/random_sampler.ts b/src/plugins/data/common/search/aggs/buckets/random_sampler.ts deleted file mode 100644 index 8d4dfbee08d5a..0000000000000 --- a/src/plugins/data/common/search/aggs/buckets/random_sampler.ts +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { i18n } from '@kbn/i18n'; -import { BucketAggType } from './bucket_agg_type'; -import { BaseAggParams } from '../types'; -import { aggRandomSamplerFnName } from './random_sampler_fn'; - -export const RANDOM_SAMPLER_AGG_NAME = 'random_sampler'; - -const title = i18n.translate('data.search.aggs.buckets.randomSamplerTitle', { - defaultMessage: 'Random Sampler', - description: 'Random sampler aggregation title', -}); - -export interface AggParamsRandomSampler extends BaseAggParams { - /** - * The sampling probability - */ - probability: number; - /** - * The seed to generate the random sampling of documents. (optional) - */ - seed?: number; -} - -/** - * A filtering aggregation used to limit any sub aggregations' processing to a sample of the top-scoring documents. - */ -export const getRandomSamplerBucketAgg = () => - new BucketAggType({ - name: RANDOM_SAMPLER_AGG_NAME, - title, - customLabels: false, - expressionName: aggRandomSamplerFnName, - params: [ - { - name: 'probability', - type: 'number', - }, - { - name: 'seed', - type: 'number', - }, - ], - }); diff --git a/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.test.ts b/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.test.ts deleted file mode 100644 index b14e9b1631896..0000000000000 --- a/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.test.ts +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { functionWrapper } from '../test_helpers'; -import { aggRandomSampler } from './random_sampler_fn'; - -describe('aggRandomSampler', () => { - const fn = functionWrapper(aggRandomSampler()); - - test('includes params when they are provided', () => { - const actual = fn({ - id: 'random_sampler', - schema: 'bucket', - probability: 0.1, - }); - - expect(actual.value).toMatchInlineSnapshot(` - Object { - "enabled": true, - "id": "random_sampler", - "params": Object { - "probability": 0.1, - }, - "schema": "bucket", - "type": "random_sampler", - } - `); - }); -}); diff --git a/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.ts b/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.ts deleted file mode 100644 index 0094f94dab151..0000000000000 --- a/src/plugins/data/common/search/aggs/buckets/random_sampler_fn.ts +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { i18n } from '@kbn/i18n'; -import { ExpressionFunctionDefinition } from '@kbn/expressions-plugin/common'; -import { AggExpressionFunctionArgs, AggExpressionType, BUCKET_TYPES } from '..'; -import { RANDOM_SAMPLER_AGG_NAME } from './random_sampler'; - -export const aggRandomSamplerFnName = 'aggRandomSampler'; - -type Input = any; -type Arguments = AggExpressionFunctionArgs; - -type Output = AggExpressionType; -type FunctionDefinition = ExpressionFunctionDefinition< - typeof aggRandomSamplerFnName, - Input, - Arguments, - Output ->; - -export const aggRandomSampler = (): FunctionDefinition => ({ - name: aggRandomSamplerFnName, - help: i18n.translate('data.search.aggs.function.buckets.randomSampler.help', { - defaultMessage: 'Generates a serialized agg config for a Random sampler agg', - }), - type: 'agg_type', - args: { - id: { - types: ['string'], - help: i18n.translate('data.search.aggs.buckets.randomSampler.id.help', { - defaultMessage: 'ID for this aggregation', - }), - }, - enabled: { - types: ['boolean'], - default: true, - help: i18n.translate('data.search.aggs.buckets.randomSampler.enabled.help', { - defaultMessage: 'Specifies whether this aggregation should be enabled', - }), - }, - schema: { - types: ['string'], - help: i18n.translate('data.search.aggs.buckets.randomSampler.schema.help', { - defaultMessage: 'Schema to use for this aggregation', - }), - }, - probability: { - types: ['number'], - help: i18n.translate('data.search.aggs.buckets.randomSampler.probability.help', { - defaultMessage: 'The sampling probability', - }), - }, - seed: { - types: ['number'], - help: i18n.translate('data.search.aggs.buckets.randomSampler.seed.help', { - defaultMessage: 'The seed to generate the random sampling of documents.', - }), - }, - }, - fn: (input, args) => { - const { id, enabled, schema, ...rest } = args; - - return { - type: 'agg_type', - value: { - id, - enabled, - schema, - type: RANDOM_SAMPLER_AGG_NAME, - params: { - ...rest, - }, - }, - }; - }, -}); diff --git a/src/plugins/data/common/search/aggs/metrics/lib/sibling_pipeline_agg_helper.ts b/src/plugins/data/common/search/aggs/metrics/lib/sibling_pipeline_agg_helper.ts index c50eb36468e9f..243a119847a2c 100644 --- a/src/plugins/data/common/search/aggs/metrics/lib/sibling_pipeline_agg_helper.ts +++ b/src/plugins/data/common/search/aggs/metrics/lib/sibling_pipeline_agg_helper.ts @@ -31,13 +31,7 @@ const metricAggFilter: string[] = [ '!filtered_metric', '!single_percentile', ]; -const bucketAggFilter: string[] = [ - '!filter', - '!sampler', - '!diversified_sampler', - '!random_sampler', - '!multi_terms', -]; +const bucketAggFilter: string[] = ['!filter', '!sampler', '!diversified_sampler', '!multi_terms']; export const siblingPipelineType = i18n.translate( 'data.search.aggs.metrics.siblingPipelineAggregationsSubtypeTitle', diff --git a/src/plugins/data/common/search/aggs/types.ts b/src/plugins/data/common/search/aggs/types.ts index 19ecd1eaa866d..bc35e46d8da7a 100644 --- a/src/plugins/data/common/search/aggs/types.ts +++ b/src/plugins/data/common/search/aggs/types.ts @@ -109,10 +109,8 @@ import { AggParamsMovingAvgSerialized, AggParamsSerialDiffSerialized, AggParamsTopHitSerialized, - aggRandomSampler, } from '.'; import { AggParamsSampler } from './buckets/sampler'; -import { AggParamsRandomSampler } from './buckets/random_sampler'; import { AggParamsDiversifiedSampler } from './buckets/diversified_sampler'; import { AggParamsSignificantText } from './buckets/significant_text'; import { aggTopMetrics } from './metrics/top_metrics_fn'; @@ -187,7 +185,6 @@ interface SerializedAggParamsMapping { [BUCKET_TYPES.MULTI_TERMS]: AggParamsMultiTermsSerialized; [BUCKET_TYPES.RARE_TERMS]: AggParamsRareTerms; [BUCKET_TYPES.SAMPLER]: AggParamsSampler; - [BUCKET_TYPES.RANDOM_SAMPLER]: AggParamsRandomSampler; [BUCKET_TYPES.DIVERSIFIED_SAMPLER]: AggParamsDiversifiedSampler; [METRIC_TYPES.AVG]: AggParamsAvg; [METRIC_TYPES.CARDINALITY]: AggParamsCardinality; @@ -233,7 +230,6 @@ export interface AggParamsMapping { [BUCKET_TYPES.MULTI_TERMS]: AggParamsMultiTerms; [BUCKET_TYPES.RARE_TERMS]: AggParamsRareTerms; [BUCKET_TYPES.SAMPLER]: AggParamsSampler; - [BUCKET_TYPES.RANDOM_SAMPLER]: AggParamsRandomSampler; [BUCKET_TYPES.DIVERSIFIED_SAMPLER]: AggParamsDiversifiedSampler; [METRIC_TYPES.AVG]: AggParamsAvg; [METRIC_TYPES.CARDINALITY]: AggParamsCardinality; @@ -269,7 +265,6 @@ export interface AggFunctionsMapping { aggFilter: ReturnType; aggFilters: ReturnType; aggSignificantTerms: ReturnType; - aggRandomSampler: ReturnType; aggIpRange: ReturnType; aggDateRange: ReturnType; aggRange: ReturnType; From 7d0580917bbac873582ba4d7153abe0e1a3e902c Mon Sep 17 00:00:00 2001 From: dej611 Date: Thu, 13 Oct 2022 19:26:46 +0200 Subject: [PATCH 04/18] :wrench: Refactor layer settings panel --- .../data/common/search/aggs/agg_configs.ts | 16 ++- .../data/common/search/aggs/utils/sampler.ts | 29 +++++ .../datasources/form_based/form_based.tsx | 30 ++++- .../form_based/form_based_suggestions.ts | 5 + .../datasources/form_based/layer_settings.tsx | 68 +++++++++++ .../editor_frame/config_panel/layer_panel.tsx | 111 ++++++------------ x-pack/plugins/lens/public/types.ts | 20 ++++ 7 files changed, 193 insertions(+), 86 deletions(-) create mode 100644 src/plugins/data/common/search/aggs/utils/sampler.ts create mode 100644 x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx diff --git a/src/plugins/data/common/search/aggs/agg_configs.ts b/src/plugins/data/common/search/aggs/agg_configs.ts index 1db856893c126..bf3cfde7a9fa8 100644 --- a/src/plugins/data/common/search/aggs/agg_configs.ts +++ b/src/plugins/data/common/search/aggs/agg_configs.ts @@ -26,6 +26,7 @@ import { AggTypesDependencies, GetConfigFn, getUserTimeZone } from '../..'; import { getTime, calculateBounds } from '../..'; import type { IBucketAggConfig } from './buckets'; import { insertTimeShiftSplit, mergeTimeShifts } from './utils/time_splits'; +import { createSamplerAgg, isSamplingEnabled } from './utils/sampler'; function removeParentAggs(obj: any) { for (const prop in obj) { @@ -235,14 +236,11 @@ export class AggConfigs { let dslLvlCursor: Record; let nestedMetrics: Array<{ config: AggConfig; dsl: Record }> | []; - if (this.opts.probability !== 1) { - dslTopLvl.sampling = { - random_sampler: { - probability: this.opts.probability, - seed: this.opts.samplerSeed, - }, - aggs: {}, - }; + if (isSamplingEnabled(this.opts.probability)) { + dslTopLvl.sampling = createSamplerAgg({ + probability: this.opts.probability, + seed: this.opts.samplerSeed, + }); } const timeShifts = this.getTimeShifts(); @@ -275,7 +273,7 @@ export class AggConfigs { // start at the top level dslLvlCursor = dslTopLvl; // when sampling jump directly to the aggs - if (this.opts.probability !== 1) { + if (isSamplingEnabled(this.opts.probability)) { dslLvlCursor = dslLvlCursor.sampling.aggs; } } else { diff --git a/src/plugins/data/common/search/aggs/utils/sampler.ts b/src/plugins/data/common/search/aggs/utils/sampler.ts new file mode 100644 index 0000000000000..fcf9e349d8532 --- /dev/null +++ b/src/plugins/data/common/search/aggs/utils/sampler.ts @@ -0,0 +1,29 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export function createSamplerAgg({ + type = 'random_sampler', + probability, + seed, +}: { + type?: string; + probability: number; + seed?: number; +}) { + return { + [type]: { + probability, + seed, + }, + aggs: {}, + }; +} + +export function isSamplingEnabled(probability: number) { + return probability !== 1; +} diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx index ce19220237dbf..3547dae75f745 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx @@ -36,6 +36,7 @@ import type { IndexPatternField, IndexPattern, IndexPatternRef, + DatasourceLayerSettingsProps, } from '../../types'; import { changeIndexPattern, @@ -91,6 +92,7 @@ import { getStateTimeShiftWarningMessages } from './time_shift_utils'; import { getPrecisionErrorWarningMessages } from './utils'; import { DOCUMENT_FIELD_NAME } from '../../../common/constants'; import { isColumnOfType } from './operations/definitions/helpers'; +import { LayerSettingsPanel } from './layer_settings'; export type { OperationType, GenericIndexPatternColumn } from './operations'; export { deleteColumn } from './operations'; @@ -266,6 +268,32 @@ export function getFormBasedDatasource({ toExpression: (state, layerId, indexPatterns) => toExpression(state, layerId, indexPatterns, uiSettings), + renderLayerSettings( + domElement: Element, + props: DatasourceLayerSettingsProps + ) { + render( + + + + + + + , + domElement + ); + }, + renderDataPanel(domElement: Element, props: DatasourceDataPanelProps) { const { onChangeIndexPattern, ...otherProps } = props; render( @@ -445,7 +473,7 @@ export function getFormBasedDatasource({ }), execute: openLayerSettings, icon: 'gear', - isCompatible: true, + isCompatible: Boolean(state.layers[layerId]), }, ]; }, diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts b/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts index 52008f10bcdeb..7ef3546b44b10 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts @@ -529,6 +529,11 @@ function getEmptyLayerSuggestionsForField( newLayer = createNewLayerWithMetricAggregation(indexPattern, field); } + // copy the sampling rate to the new layer + if (newLayer) { + newLayer.sampling = state.layers[layerId].sampling; + } + const newLayerSuggestions = newLayer ? [ buildSuggestion({ diff --git a/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx b/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx new file mode 100644 index 0000000000000..897b276466018 --- /dev/null +++ b/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx @@ -0,0 +1,68 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { EuiFormRow, EuiToolTip, EuiIcon, EuiRange } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import React from 'react'; +import type { DatasourceLayerSettingsProps } from '../../types'; +import type { FormBasedPrivateState } from './types'; + +const samplingValue = [0.0001, 0.001, 0.01, 0.1, 1]; + +export function LayerSettingsPanel({ + state, + setState, + layerId, +}: DatasourceLayerSettingsProps) { + const samplingIndex = samplingValue.findIndex((v) => v === state.layers[layerId].sampling); + const currentSamplingIndex = samplingIndex > -1 ? samplingIndex : samplingValue.length - 1; + return ( + + + {i18n.translate('xpack.lens.xyChart.randomSampling.label', { + defaultMessage: 'Random Sampling', + })} + + + + } + > + { + setState({ + ...state, + layers: { + ...state.layers, + [layerId]: { + ...state.layers[layerId], + sampling: samplingValue[Number(e.currentTarget.value)], + }, + }, + }); + }} + showInput={false} + showRange={false} + showTicks + step={1} + min={0} + max={samplingValue.length - 1} + ticks={samplingValue.map((v, i) => ({ label: `${v}`, value: i }))} + /> + + ); +} diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index 3a63b24b992d0..29620142d5ba0 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -16,9 +16,6 @@ import { EuiFormRow, EuiText, EuiIconTip, - EuiRange, - EuiIcon, - EuiToolTip, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { LayerActions } from './layer_actions'; @@ -53,8 +50,6 @@ const initialActiveDimensionState = { isNew: false, }; -const samplingValue = [0.0001, 0.001, 0.01, 0.1, 1]; - export function LayerPanel( props: Exclude & { activeVisualization: Visualization; @@ -364,10 +359,6 @@ export function LayerPanel( ] ); - const samplingIndex = samplingValue.findIndex( - (v) => v === layerDatasourceState.layers[layerId].sampling - ); - return ( <>
@@ -674,74 +665,42 @@ export function LayerPanel( })}
- (settingsPanelRef.current = el)} - isOpen={isPanelSettingsOpen} - isFullscreen={false} - groupLabel={i18n.translate('xpack.lens.editorFrame.layerSettingsTitle', { - defaultMessage: 'Layer settings', - })} - handleClose={() => { - // update the current layer settings - setPanelSettingsOpen(false); - return true; - }} - > -
-
- - - {i18n.translate('xpack.lens.xyChart.randomSampling.label', { - defaultMessage: 'Random Sampling', - })} - - - - } - > - -1 ? samplingIndex : samplingValue.length - 1} - onChange={(e) => { - updateDatasource(datasourceId, { - ...layerDatasourceState, - layers: { - ...layerDatasourceState.layers, - [layerId]: { - ...layerDatasourceState.layers[layerId], - sampling: samplingValue[Number(e.target.value)], - }, - }, - }); - }} - showInput={false} - showRange={false} - showTicks - step={1} - min={0} - max={samplingValue.length - 1} - ticks={samplingValue.map((v, i) => ({ label: `${v}`, value: i }))} - /> - + {(layerDatasource?.renderLayerSettings || activeVisualization?.renderLayerSettings) && ( + (settingsPanelRef.current = el)} + isOpen={isPanelSettingsOpen} + isFullscreen={false} + groupLabel={i18n.translate('xpack.lens.editorFrame.layerSettingsTitle', { + defaultMessage: 'Layer settings', + })} + handleClose={() => { + // update the current layer settings + setPanelSettingsOpen(false); + return true; + }} + > +
+
+ {layerDatasource?.renderLayerSettings && ( + + )} + {activeVisualization?.renderLayerSettings && ( + + )} +
-
- - + + )} (panelRef.current = el)} isOpen={isDimensionPanelOpen} diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 84461574d2ab0..499c57b082ec5 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -282,6 +282,10 @@ export interface Datasource { } ) => T; + renderLayerSettings?: ( + domElement: Element, + props: DatasourceLayerSettingsProps + ) => ((cleanupElement: Element) => void) | void; renderDataPanel: ( domElement: Element, props: DatasourceDataPanelProps @@ -496,6 +500,12 @@ export interface DatasourcePublicAPI { getMaxPossibleNumValues: (columnId: string) => number | null; } +export interface DatasourceLayerSettingsProps { + layerId: string; + state: T; + setState: StateSetter; +} + export interface DatasourceDataPanelProps { state: T; dragDropContext: DragContextState; @@ -701,6 +711,11 @@ export interface VisualizationToolbarProps { state: T; } +export type VisualizationLayerSettingsProps = VisualizationConfigProps & { + setState(newState: T | ((currState: T) => T)): void; + panelRef: MutableRefObject; +}; + export type VisualizationDimensionEditorProps = VisualizationConfigProps & { groupId: string; accessor: string; @@ -1060,6 +1075,11 @@ export interface Visualization { dropProps: GetDropPropsArgs ) => { dropTypes: DropType[]; nextLabel?: string } | undefined; + renderLayerSettings?: ( + domElement: Element, + props: VisualizationLayerSettingsProps + ) => ((cleanupElement: Element) => void) | void; + /** * Additional editor that gets rendered inside the dimension popover. * This can be used to configure dimension-specific options From c341c023b79c7a2dba9972fa6757e05dd5dd7860 Mon Sep 17 00:00:00 2001 From: dej611 Date: Fri, 14 Oct 2022 11:42:24 +0200 Subject: [PATCH 05/18] :bug: Fix terms other query and some refactor --- .../data/common/search/aggs/agg_configs.ts | 15 ++++++++++++--- .../aggs/buckets/_terms_other_bucket_helper.ts | 9 ++++++++- .../public/datasources/form_based/form_based.tsx | 2 +- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/plugins/data/common/search/aggs/agg_configs.ts b/src/plugins/data/common/search/aggs/agg_configs.ts index bf3cfde7a9fa8..d036edf7faea4 100644 --- a/src/plugins/data/common/search/aggs/agg_configs.ts +++ b/src/plugins/data/common/search/aggs/agg_configs.ts @@ -114,6 +114,10 @@ export class AggConfigs { return { probability: this.opts.probability, seed: this.opts.samplerSeed }; } + isSamplingEnabled() { + return isSamplingEnabled(this.opts.probability); + } + setTimeFields(timeFields: string[] | undefined) { this.timeFields = timeFields; } @@ -236,7 +240,7 @@ export class AggConfigs { let dslLvlCursor: Record; let nestedMetrics: Array<{ config: AggConfig; dsl: Record }> | []; - if (isSamplingEnabled(this.opts.probability)) { + if (this.isSamplingEnabled()) { dslTopLvl.sampling = createSamplerAgg({ probability: this.opts.probability, seed: this.opts.samplerSeed, @@ -273,7 +277,7 @@ export class AggConfigs { // start at the top level dslLvlCursor = dslTopLvl; // when sampling jump directly to the aggs - if (isSamplingEnabled(this.opts.probability)) { + if (this.isSamplingEnabled()) { dslLvlCursor = dslLvlCursor.sampling.aggs; } } else { @@ -470,7 +474,12 @@ export class AggConfigs { doc_count: response.rawResponse.hits?.total as estypes.AggregationsAggregate, }; } - const aggCursor = transformedRawResponse.aggregations!; + const aggCursor = this.isSamplingEnabled() + ? (transformedRawResponse.aggregations!.sampling! as Record< + string, + estypes.AggregationsAggregate + >) + : transformedRawResponse.aggregations!; mergeTimeShifts(this, aggCursor); return { diff --git a/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts b/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts index f695dc1b1d399..135a5c4a8d55d 100644 --- a/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts +++ b/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts @@ -139,6 +139,7 @@ export const buildOtherBucketAgg = ( const index = bucketAggs.findIndex((agg) => agg.id === aggWithOtherBucket.id); const aggs = aggConfigs.toDsl(); const indexPattern = aggWithOtherBucket.aggConfigs.indexPattern; + const hasSampling = aggConfigs.isSamplingEnabled(); // create filters aggregation const filterAgg = aggConfigs.createAggConfig( @@ -234,7 +235,13 @@ export const buildOtherBucketAgg = ( bool: buildQueryFromFilters(filters, indexPattern), }; }; - walkBucketTree(0, response.aggregations, bucketAggs[0].id, [], ''); + walkBucketTree( + 0, + hasSampling ? response.aggregations.sampling : response.aggregations, + bucketAggs[0].id, + [], + '' + ); // bail if there were no bucket results if (noAggBucketResults || exhaustiveBuckets) { diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx index 3547dae75f745..247a5e9e090d4 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx @@ -828,6 +828,6 @@ function blankLayer(indexPatternId: string) { indexPatternId, columns: {}, columnOrder: [], - sampling: 4, + sampling: 1, }; } From 13609493e13073179e8e8331ee0d85faab812886 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 17 Oct 2022 10:17:36 +0200 Subject: [PATCH 06/18] :label: Fix types issues --- src/plugins/data/common/search/aggs/agg_configs.ts | 6 +++--- src/plugins/data/common/search/aggs/utils/sampler.ts | 6 +++--- .../common/search/expressions/esaggs/esaggs_fn.ts | 6 +++--- .../search/expressions/esaggs/request_handler.ts | 1 - src/plugins/data/common/search/tabify/tabify.ts | 2 +- src/plugins/data/common/search/tabify/types.ts | 1 - .../data/public/search/expressions/esaggs.test.ts | 1 + .../data/server/search/expressions/esaggs.test.ts | 1 + .../lens/public/datasources/form_based/form_based.tsx | 7 +++++-- .../public/datasources/form_based/to_expression.ts | 11 +++++++---- .../editor_frame/expression_helpers.ts | 10 +++++++--- .../editor_frame/workspace_panel/workspace_panel.tsx | 11 +++++++---- x-pack/plugins/lens/public/types.ts | 7 ++++--- 13 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/plugins/data/common/search/aggs/agg_configs.ts b/src/plugins/data/common/search/aggs/agg_configs.ts index d036edf7faea4..67421eb79a0de 100644 --- a/src/plugins/data/common/search/aggs/agg_configs.ts +++ b/src/plugins/data/common/search/aggs/agg_configs.ts @@ -56,8 +56,8 @@ export interface AggConfigsOptions { hierarchical?: boolean; aggExecutionContext?: AggTypesDependencies['aggExecutionContext']; partialRows?: boolean; - probability: number; - samplerSeed?: number; + probability?: number; + samplerSeed?: string; } export type CreateAggConfigParams = Assign; @@ -242,7 +242,7 @@ export class AggConfigs { if (this.isSamplingEnabled()) { dslTopLvl.sampling = createSamplerAgg({ - probability: this.opts.probability, + probability: this.opts.probability!, seed: this.opts.samplerSeed, }); } diff --git a/src/plugins/data/common/search/aggs/utils/sampler.ts b/src/plugins/data/common/search/aggs/utils/sampler.ts index fcf9e349d8532..e16209e5f5113 100644 --- a/src/plugins/data/common/search/aggs/utils/sampler.ts +++ b/src/plugins/data/common/search/aggs/utils/sampler.ts @@ -13,7 +13,7 @@ export function createSamplerAgg({ }: { type?: string; probability: number; - seed?: number; + seed?: string; }) { return { [type]: { @@ -24,6 +24,6 @@ export function createSamplerAgg({ }; } -export function isSamplingEnabled(probability: number) { - return probability !== 1; +export function isSamplingEnabled(probability: number | undefined) { + return probability != null && probability !== 1; } diff --git a/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts b/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts index c659b225aacea..abe5e259d6263 100644 --- a/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts +++ b/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts @@ -32,8 +32,8 @@ interface Arguments { metricsAtAllLevels?: boolean; partialRows?: boolean; timeFields?: string[]; - probability: number; - samplerSeed?: number; + probability?: number; + samplerSeed?: string; } export type EsaggsExpressionFunctionDefinition = ExpressionFunctionDefinition< @@ -105,7 +105,7 @@ export const getEsaggsMeta: () => Omit }), }, samplerSeed: { - types: ['number'], + types: ['string'], help: i18n.translate('data.search.functions.esaggs.samplerSeed.help', { defaultMessage: 'The seed to generate the random sampling of documents. Uses random sampler.', diff --git a/src/plugins/data/common/search/expressions/esaggs/request_handler.ts b/src/plugins/data/common/search/expressions/esaggs/request_handler.ts index 630a50c01adee..03512bcd2e270 100644 --- a/src/plugins/data/common/search/expressions/esaggs/request_handler.ts +++ b/src/plugins/data/common/search/expressions/esaggs/request_handler.ts @@ -133,7 +133,6 @@ export const handleRequest = ({ const tabifyParams = { metricsAtAllLevels: aggs.hierarchical, partialRows: aggs.partialRows, - samplerConfig: aggs.samplerConfig, timeRange: parsedTimeRange ? { from: parsedTimeRange.min, to: parsedTimeRange.max, timeFields: allTimeFields } : undefined, diff --git a/src/plugins/data/common/search/tabify/tabify.ts b/src/plugins/data/common/search/tabify/tabify.ts index 969d530e55fe1..2c332c1ad6a75 100644 --- a/src/plugins/data/common/search/tabify/tabify.ts +++ b/src/plugins/data/common/search/tabify/tabify.ts @@ -147,7 +147,7 @@ export function tabifyAggResponse( const write = new TabbedAggResponseWriter(aggConfigs, respOpts || {}); const topLevelBucket: AggResponseBucket = { - ...(aggConfigs.samplerConfig.probability !== 1 + ...(aggConfigs.isSamplingEnabled() ? esResponse.aggregations.sampling : esResponse.aggregations), doc_count: esResponse.aggregations?.doc_count || esResponse.hits?.total, diff --git a/src/plugins/data/common/search/tabify/types.ts b/src/plugins/data/common/search/tabify/types.ts index cfeb3052d33db..bf0a99725e2ab 100644 --- a/src/plugins/data/common/search/tabify/types.ts +++ b/src/plugins/data/common/search/tabify/types.ts @@ -26,7 +26,6 @@ export interface TabbedResponseWriterOptions { metricsAtAllLevels: boolean; partialRows: boolean; timeRange?: TimeRangeInformation; - samplerConfig: { probability: number; seed?: number }; } /** @internal */ diff --git a/src/plugins/data/public/search/expressions/esaggs.test.ts b/src/plugins/data/public/search/expressions/esaggs.test.ts index aa26ceab9ae6b..01b5872e37d31 100644 --- a/src/plugins/data/public/search/expressions/esaggs.test.ts +++ b/src/plugins/data/public/search/expressions/esaggs.test.ts @@ -50,6 +50,7 @@ describe('esaggs expression function - public', () => { metricsAtAllLevels: true, partialRows: false, timeFields: ['@timestamp', 'utc_time'], + probability: 1, }; beforeEach(() => { diff --git a/src/plugins/data/server/search/expressions/esaggs.test.ts b/src/plugins/data/server/search/expressions/esaggs.test.ts index 166f2719e356d..9954fb2457968 100644 --- a/src/plugins/data/server/search/expressions/esaggs.test.ts +++ b/src/plugins/data/server/search/expressions/esaggs.test.ts @@ -51,6 +51,7 @@ describe('esaggs expression function - server', () => { metricsAtAllLevels: true, partialRows: false, timeFields: ['@timestamp', 'utc_time'], + probability: 1, }; beforeEach(() => { diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx index ed1a237c1fa59..ae9f2eccc8f47 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx @@ -278,8 +278,8 @@ export function getFormBasedDatasource({ return fields; }, - toExpression: (state, layerId, indexPatterns) => - toExpression(state, layerId, indexPatterns, uiSettings), + toExpression: (state, layerId, indexPatterns, searchSessionId) => + toExpression(state, layerId, indexPatterns, uiSettings, searchSessionId), renderLayerSettings( domElement: Element, @@ -482,6 +482,9 @@ export function getFormBasedDatasource({ getDropProps, onDrop, getSupportedActionsForLayer(layerId, state, _, openLayerSettings) { + if (!openLayerSettings) { + return []; + } return [ { displayName: i18n.translate('xpack.lens.indexPattern.layerSettingsAction', { diff --git a/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts b/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts index ff038b0939e6a..9f59dee0aa0cc 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts @@ -52,7 +52,8 @@ const updatePositionIndex = (currentId: string, newIndex: number) => { function getExpressionForLayer( layer: FormBasedLayer, indexPattern: IndexPattern, - uiSettings: IUiSettingsClient + uiSettings: IUiSettingsClient, + searchSessionId?: string ): ExpressionAstExpression | null { const { columnOrder } = layer; if (columnOrder.length === 0 || !indexPattern) { @@ -393,7 +394,7 @@ function getExpressionForLayer( partialRows: false, timeFields: allDateHistogramFields, probability: layer.sampling || 1, - // TODO: add samplerSeed here from search session + samplerSeed: searchSessionId, }).toAst(), { type: 'function', @@ -443,13 +444,15 @@ export function toExpression( state: FormBasedPrivateState, layerId: string, indexPatterns: IndexPatternMap, - uiSettings: IUiSettingsClient + uiSettings: IUiSettingsClient, + searchSessionId?: string ) { if (state.layers[layerId]) { return getExpressionForLayer( state.layers[layerId], indexPatterns[state.layers[layerId].indexPatternId], - uiSettings + uiSettings, + searchSessionId ); } diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts index c9b0358b81a0b..aee10196d5156 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/expression_helpers.ts @@ -11,7 +11,8 @@ import { Visualization, DatasourceMap, DatasourceLayers, IndexPatternMap } from export function getDatasourceExpressionsByLayers( datasourceMap: DatasourceMap, datasourceStates: DatasourceStates, - indexPatterns: IndexPatternMap + indexPatterns: IndexPatternMap, + searchSessionId?: string ): null | Record { const datasourceExpressions: Array<[string, Ast | string]> = []; @@ -24,7 +25,7 @@ export function getDatasourceExpressionsByLayers( const layers = datasource.getLayers(state); layers.forEach((layerId) => { - const result = datasource.toExpression(state, layerId, indexPatterns); + const result = datasource.toExpression(state, layerId, indexPatterns, searchSessionId); if (result) { datasourceExpressions.push([layerId, result]); } @@ -53,6 +54,7 @@ export function buildExpression({ title, description, indexPatterns, + searchSessionId, }: { title?: string; description?: string; @@ -62,6 +64,7 @@ export function buildExpression({ datasourceStates: DatasourceStates; datasourceLayers: DatasourceLayers; indexPatterns: IndexPatternMap; + searchSessionId?: string; }): Ast | null { if (visualization === null) { return null; @@ -70,7 +73,8 @@ export function buildExpression({ const datasourceExpressionsByLayers = getDatasourceExpressionsByLayers( datasourceMap, datasourceStates, - indexPatterns + indexPatterns, + searchSessionId ); const visualizationExpression = visualization.toExpression( diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx index 38f62b6e928b6..2f2003970171a 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx @@ -162,6 +162,7 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ const changesApplied = useLensSelector(selectChangesApplied); const triggerApply = useLensSelector(selectTriggerApplyChanges); const datasourceLayers = useLensSelector((state) => selectDatasourceLayers(state, datasourceMap)); + const searchSessionId = useLensSelector(selectSearchSessionId); const [localState, setLocalState] = useState({ expressionBuildError: undefined, @@ -317,6 +318,7 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ datasourceStates, datasourceLayers, indexPatterns: dataViews.indexPatterns, + searchSessionId, }); if (ast) { @@ -349,16 +351,17 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({ })); } }, [ + configurationValidationError?.length, + missingRefsErrors.length, + unknownVisError, activeVisualization, visualization.state, + visualization.activeId, datasourceMap, datasourceStates, datasourceLayers, - configurationValidationError?.length, - missingRefsErrors.length, - unknownVisError, - visualization.activeId, dataViews.indexPatterns, + searchSessionId, ]); useEffect(() => { diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 1481e9bccf8a9..568bce0fd0f56 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -347,7 +347,8 @@ export interface Datasource { toExpression: ( state: T, layerId: string, - indexPatterns: IndexPatternMap + indexPatterns: IndexPatternMap, + searchSessionId?: string ) => ExpressionAstExpression | string | null; getDatasourceSuggestionsForField: ( @@ -450,7 +451,7 @@ export interface Datasource { layerId: string, state: T, setState: StateSetter, - openLayerSettings: () => void + openLayerSettings?: () => void ) => LayerAction[]; } @@ -1009,7 +1010,7 @@ export interface Visualization { layerId: string, state: T, setState: StateSetter, - openLayerSettings: () => void + openLayerSettings?: () => void ) => LayerAction[]; /** returns the type string of the given layer */ getLayerType: (layerId: string, state?: T) => LayerType | undefined; From f7b9dd50d4d2cd82cc7362b422961514ab789277 Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 17 Oct 2022 11:46:48 +0200 Subject: [PATCH 07/18] :bug: Fix sampling for other terms agg --- .../data/common/search/aggs/agg_configs.ts | 6 +- .../buckets/_terms_other_bucket_helper.ts | 56 +++++++++++++++++-- .../data/common/search/aggs/utils/sampler.ts | 2 +- .../search/expressions/esaggs/esaggs_fn.ts | 4 +- .../datasources/form_based/to_expression.ts | 3 +- 5 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/plugins/data/common/search/aggs/agg_configs.ts b/src/plugins/data/common/search/aggs/agg_configs.ts index 67421eb79a0de..aa0741a5d8d03 100644 --- a/src/plugins/data/common/search/aggs/agg_configs.ts +++ b/src/plugins/data/common/search/aggs/agg_configs.ts @@ -57,7 +57,7 @@ export interface AggConfigsOptions { aggExecutionContext?: AggTypesDependencies['aggExecutionContext']; partialRows?: boolean; probability?: number; - samplerSeed?: string; + samplerSeed?: number; } export type CreateAggConfigParams = Assign; @@ -111,7 +111,7 @@ export class AggConfigs { } public get samplerConfig() { - return { probability: this.opts.probability, seed: this.opts.samplerSeed }; + return { probability: this.opts.probability ?? 1, seed: this.opts.samplerSeed }; } isSamplingEnabled() { @@ -242,7 +242,7 @@ export class AggConfigs { if (this.isSamplingEnabled()) { dslTopLvl.sampling = createSamplerAgg({ - probability: this.opts.probability!, + probability: this.opts.probability ?? 1, seed: this.opts.samplerSeed, }); } diff --git a/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts b/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts index 135a5c4a8d55d..563436f56a4b3 100644 --- a/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts +++ b/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts @@ -20,6 +20,7 @@ import { AggGroupNames } from '../agg_groups'; import { IAggConfigs } from '../agg_configs'; import { IAggType } from '../agg_type'; import { IAggConfig } from '../agg_config'; +import { createSamplerAgg } from '../utils/sampler'; export const OTHER_BUCKET_SEPARATOR = '╰┄►'; @@ -128,6 +129,28 @@ const getOtherAggTerms = (requestAgg: Record, key: string, otherAgg .map((filter: Record) => filter.match_phrase[otherAgg.params.field.name]); }; +/** + * + * @param requestAgg + * @param aggConfigs + * @returns + */ +const getCorrectAggCursorFromRequest = ( + requestAgg: Record, + aggConfigs: IAggConfigs +) => { + return aggConfigs.isSamplingEnabled() ? requestAgg.sampling.aggs : requestAgg; +}; + +const getCorrectAggregationsCursorFromResponse = ( + response: estypes.SearchResponse, + aggConfigs: IAggConfigs +) => { + return aggConfigs.isSamplingEnabled() + ? (response.aggregations?.sampling as Record) + : response.aggregations; +}; + export const buildOtherBucketAgg = ( aggConfigs: IAggConfigs, aggWithOtherBucket: IAggConfig, @@ -139,7 +162,6 @@ export const buildOtherBucketAgg = ( const index = bucketAggs.findIndex((agg) => agg.id === aggWithOtherBucket.id); const aggs = aggConfigs.toDsl(); const indexPattern = aggWithOtherBucket.aggConfigs.indexPattern; - const hasSampling = aggConfigs.isSamplingEnabled(); // create filters aggregation const filterAgg = aggConfigs.createAggConfig( @@ -237,7 +259,7 @@ export const buildOtherBucketAgg = ( }; walkBucketTree( 0, - hasSampling ? response.aggregations.sampling : response.aggregations, + getCorrectAggregationsCursorFromResponse(response, aggConfigs), bucketAggs[0].id, [], '' @@ -249,6 +271,14 @@ export const buildOtherBucketAgg = ( } return () => { + if (aggConfigs.isSamplingEnabled()) { + return { + sampling: { + ...createSamplerAgg(aggConfigs.samplerConfig), + aggs: { 'other-filter': resultAgg }, + }, + }; + } return { 'other-filter': resultAgg, }; @@ -264,16 +294,27 @@ export const mergeOtherBucketAggResponse = ( otherFilterBuilder: (requestAgg: Record, key: string, otherAgg: IAggConfig) => Filter ): estypes.SearchResponse => { const updatedResponse = cloneDeep(response); - each(otherResponse.aggregations['other-filter'].buckets, (bucket, key) => { + const aggregationsRoot = getCorrectAggregationsCursorFromResponse(otherResponse, aggsConfig); + const updatedAggregationsRoot = getCorrectAggregationsCursorFromResponse( + updatedResponse, + aggsConfig + ); + const buckets = + 'buckets' in aggregationsRoot!['other-filter'] ? aggregationsRoot!['other-filter'].buckets : {}; + each(buckets, (bucket, key) => { if (!bucket.doc_count || key === undefined) return; const bucketKey = key.replace(new RegExp(`^${OTHER_BUCKET_SEPARATOR}`), ''); const aggResultBuckets = getAggResultBuckets( aggsConfig, - updatedResponse.aggregations, + updatedAggregationsRoot, otherAgg, bucketKey ); - const otherFilter = otherFilterBuilder(requestAgg, key, otherAgg); + const otherFilter = otherFilterBuilder( + getCorrectAggCursorFromRequest(requestAgg, aggsConfig), + key, + otherAgg + ); bucket.filters = [otherFilter]; bucket.key = '__other__'; @@ -297,7 +338,10 @@ export const updateMissingBucket = ( agg: IAggConfig ) => { const updatedResponse = cloneDeep(response); - const aggResultBuckets = getAggConfigResultMissingBuckets(updatedResponse.aggregations, agg.id); + const aggResultBuckets = getAggConfigResultMissingBuckets( + getCorrectAggregationsCursorFromResponse(updatedResponse, aggConfigs), + agg.id + ); aggResultBuckets.forEach((bucket) => { bucket.key = '__missing__'; }); diff --git a/src/plugins/data/common/search/aggs/utils/sampler.ts b/src/plugins/data/common/search/aggs/utils/sampler.ts index e16209e5f5113..5a6fde63b0a29 100644 --- a/src/plugins/data/common/search/aggs/utils/sampler.ts +++ b/src/plugins/data/common/search/aggs/utils/sampler.ts @@ -13,7 +13,7 @@ export function createSamplerAgg({ }: { type?: string; probability: number; - seed?: string; + seed?: number; }) { return { [type]: { diff --git a/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts b/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts index abe5e259d6263..f0b55de8ffeff 100644 --- a/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts +++ b/src/plugins/data/common/search/expressions/esaggs/esaggs_fn.ts @@ -33,7 +33,7 @@ interface Arguments { partialRows?: boolean; timeFields?: string[]; probability?: number; - samplerSeed?: string; + samplerSeed?: number; } export type EsaggsExpressionFunctionDefinition = ExpressionFunctionDefinition< @@ -105,7 +105,7 @@ export const getEsaggsMeta: () => Omit }), }, samplerSeed: { - types: ['string'], + types: ['number'], help: i18n.translate('data.search.functions.esaggs.samplerSeed.help', { defaultMessage: 'The seed to generate the random sampling of documents. Uses random sampler.', diff --git a/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts b/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts index 9f59dee0aa0cc..365d80a3d8285 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts @@ -7,6 +7,7 @@ import type { IUiSettingsClient } from '@kbn/core/public'; import { partition, uniq } from 'lodash'; +import seedrandom from 'seedrandom'; import { AggFunctionsMapping, EsaggsExpressionFunctionDefinition, @@ -394,7 +395,7 @@ function getExpressionForLayer( partialRows: false, timeFields: allDateHistogramFields, probability: layer.sampling || 1, - samplerSeed: searchSessionId, + samplerSeed: seedrandom(searchSessionId).int32(), }).toAst(), { type: 'function', From 862931b7d097b0c905e6ae8b0fcce1cd418caa3f Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 17 Oct 2022 12:29:33 +0200 Subject: [PATCH 08/18] :bug: Fix issue with count operation --- .../data/common/search/aggs/agg_configs.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/plugins/data/common/search/aggs/agg_configs.ts b/src/plugins/data/common/search/aggs/agg_configs.ts index aa0741a5d8d03..7cab863fba11d 100644 --- a/src/plugins/data/common/search/aggs/agg_configs.ts +++ b/src/plugins/data/common/search/aggs/agg_configs.ts @@ -115,7 +115,10 @@ export class AggConfigs { } isSamplingEnabled() { - return isSamplingEnabled(this.opts.probability); + return ( + isSamplingEnabled(this.opts.probability) && + this.getRequestAggs().filter((agg) => !agg.type.hasNoDsl).length > 0 + ); } setTimeFields(timeFields: string[] | undefined) { @@ -240,13 +243,6 @@ export class AggConfigs { let dslLvlCursor: Record; let nestedMetrics: Array<{ config: AggConfig; dsl: Record }> | []; - if (this.isSamplingEnabled()) { - dslTopLvl.sampling = createSamplerAgg({ - probability: this.opts.probability ?? 1, - seed: this.opts.samplerSeed, - }); - } - const timeShifts = this.getTimeShifts(); const hasMultipleTimeShifts = Object.keys(timeShifts).length > 1; @@ -272,6 +268,13 @@ export class AggConfigs { (config) => 'splitForTimeShift' in config.type && config.type.splitForTimeShift(config, this) ); + if (this.isSamplingEnabled()) { + dslTopLvl.sampling = createSamplerAgg({ + probability: this.opts.probability ?? 1, + seed: this.opts.samplerSeed, + }); + } + requestAggs.forEach((config: AggConfig, i: number, list) => { if (!dslLvlCursor) { // start at the top level From 5eb26e0252c74d65b1280eb0c131991d4d9a2b9e Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 17 Oct 2022 15:57:58 +0200 Subject: [PATCH 09/18] :white_check_mark: Fix jest tests --- .../lens/public/datasources/form_based/form_based.test.ts | 8 ++++++++ .../form_based/form_based_suggestions.test.tsx | 1 + .../datasources/form_based/form_based_suggestions.ts | 3 ++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts b/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts index 6021cdc1e821e..ff3a95e018eaf 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts @@ -487,6 +487,12 @@ describe('IndexPattern Data Source', () => { "partialRows": Array [ false, ], + "probability": Array [ + 1, + ], + "samplerSeed": Array [ + 453225886, + ], "timeFields": Array [ "timestamp", ], @@ -1631,6 +1637,7 @@ describe('IndexPattern Data Source', () => { }, }, currentIndexPatternId: '1', + sampling: 1, }; expect(FormBasedDatasource.insertLayer(state, 'newLayer')).toEqual({ ...state, @@ -1640,6 +1647,7 @@ describe('IndexPattern Data Source', () => { indexPatternId: '1', columnOrder: [], columns: {}, + sampling: 1, }, }, }); diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.test.tsx b/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.test.tsx index d7a544a723e04..06fe3512903b8 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.test.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.test.tsx @@ -176,6 +176,7 @@ function testInitialState(): FormBasedPrivateState { currentIndexPatternId: '1', layers: { first: { + sampling: 1, indexPatternId: '1', columnOrder: ['col1'], columns: { diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts b/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts index 7ef3546b44b10..81ce81bb49053 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.ts @@ -530,8 +530,9 @@ function getEmptyLayerSuggestionsForField( } // copy the sampling rate to the new layer + // or just default to 1 if (newLayer) { - newLayer.sampling = state.layers[layerId].sampling; + newLayer.sampling = state.layers[layerId]?.sampling ?? 1; } const newLayerSuggestions = newLayer From 54c8fc8f08e7d09ed72f5832ee8c55138e5128ec Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 17 Oct 2022 17:54:57 +0200 Subject: [PATCH 10/18] :bug: fix test stability --- .../datasources/form_based/form_based.test.ts | 115 +++++++++++++++--- 1 file changed, 95 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts b/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts index ff3a95e018eaf..767c964918e80 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based.test.ts @@ -315,7 +315,9 @@ describe('IndexPattern Data Source', () => { describe('#toExpression', () => { it('should generate an empty expression when no columns are selected', async () => { const state = FormBasedDatasource.initialize(); - expect(FormBasedDatasource.toExpression(state, 'first', indexPatterns)).toEqual(null); + expect( + FormBasedDatasource.toExpression(state, 'first', indexPatterns, 'testing-seed') + ).toEqual(null); }); it('should create a table when there is a formula without aggs', async () => { @@ -338,7 +340,9 @@ describe('IndexPattern Data Source', () => { }, }, }; - expect(FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns)).toEqual({ + expect( + FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns, 'testing-seed') + ).toEqual({ chain: [ { function: 'createTable', @@ -385,8 +389,9 @@ describe('IndexPattern Data Source', () => { }, }; - expect(FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns)) - .toMatchInlineSnapshot(` + expect( + FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns, 'testing-seed') + ).toMatchInlineSnapshot(` Object { "chain": Array [ Object { @@ -491,7 +496,7 @@ describe('IndexPattern Data Source', () => { 1, ], "samplerSeed": Array [ - 453225886, + 1889181588, ], "timeFields": Array [ "timestamp", @@ -566,7 +571,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; expect(ast.chain[1].arguments.timeFields).toEqual(['timestamp', 'another_datefield']); }); @@ -601,7 +611,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; expect((ast.chain[1].arguments.aggs[1] as Ast).chain[0].arguments.timeShift).toEqual(['1d']); }); @@ -808,7 +823,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; const count = (ast.chain[1].arguments.aggs[1] as Ast).chain[0]; const sum = (ast.chain[1].arguments.aggs[2] as Ast).chain[0]; const average = (ast.chain[1].arguments.aggs[3] as Ast).chain[0]; @@ -872,7 +892,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; expect(ast.chain[1].arguments.aggs[0]).toMatchInlineSnapshot(` Object { "chain": Array [ @@ -996,7 +1021,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; const timeScaleCalls = ast.chain.filter((fn) => fn.function === 'lens_time_scale'); const formatCalls = ast.chain.filter((fn) => fn.function === 'lens_format_column'); expect(timeScaleCalls).toHaveLength(1); @@ -1061,7 +1091,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; const filteredMetricAgg = (ast.chain[1].arguments.aggs[0] as Ast).chain[0].arguments; const metricAgg = (filteredMetricAgg.customMetric[0] as Ast).chain[0].arguments; const bucketAgg = (filteredMetricAgg.customBucket[0] as Ast).chain[0].arguments; @@ -1112,7 +1147,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; const formatIndex = ast.chain.findIndex((fn) => fn.function === 'lens_format_column'); const calculationIndex = ast.chain.findIndex((fn) => fn.function === 'moving_average'); expect(calculationIndex).toBeLessThan(formatIndex); @@ -1160,7 +1200,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; expect(ast.chain[1].arguments.metricsAtAllLevels).toEqual([false]); expect(JSON.parse(ast.chain[2].arguments.idMap[0] as string)).toEqual({ 'col-0-0': [expect.objectContaining({ id: 'bucket1' })], @@ -1199,7 +1244,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; expect(ast.chain[1].arguments.timeFields).toEqual(['timestamp']); expect(ast.chain[1].arguments.timeFields).not.toContain('timefield'); }); @@ -1256,7 +1306,7 @@ describe('IndexPattern Data Source', () => { const optimizeMock = jest.spyOn(operationDefinitionMap.percentile, 'optimizeEsAggs'); - FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns); + FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns, 'testing-seed'); expect(operationDefinitionMap.percentile.optimizeEsAggs).toHaveBeenCalledTimes(1); @@ -1324,7 +1374,12 @@ describe('IndexPattern Data Source', () => { return { aggs: aggs.reverse(), esAggsIdMap }; }); - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; expect(operationDefinitionMap.percentile.optimizeEsAggs).toHaveBeenCalledTimes(1); @@ -1388,7 +1443,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; const idMap = JSON.parse(ast.chain[2].arguments.idMap as unknown as string); @@ -1493,7 +1553,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; // @ts-expect-error we can't isolate just the reference type expect(operationDefinitionMap.testReference.toExpression).toHaveBeenCalled(); expect(ast.chain[3]).toEqual('mock'); @@ -1526,7 +1591,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; expect(JSON.parse(ast.chain[2].arguments.idMap[0] as string)).toEqual({ 'col-0-0': [ @@ -1613,7 +1683,12 @@ describe('IndexPattern Data Source', () => { }, }; - const ast = FormBasedDatasource.toExpression(queryBaseState, 'first', indexPatterns) as Ast; + const ast = FormBasedDatasource.toExpression( + queryBaseState, + 'first', + indexPatterns, + 'testing-seed' + ) as Ast; const chainLength = ast.chain.length; expect(ast.chain[chainLength - 2].arguments.name).toEqual(['math']); expect(ast.chain[chainLength - 1].arguments.id).toEqual(['formula']); From 3039c5dd71b282f1a0fd83ff77af09180a16c54c Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 17 Oct 2022 18:31:49 +0200 Subject: [PATCH 11/18] :bug: fix test with newer params --- src/plugins/data/public/search/expressions/esaggs.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plugins/data/public/search/expressions/esaggs.test.ts b/src/plugins/data/public/search/expressions/esaggs.test.ts index 01b5872e37d31..1a04e4ffeb839 100644 --- a/src/plugins/data/public/search/expressions/esaggs.test.ts +++ b/src/plugins/data/public/search/expressions/esaggs.test.ts @@ -89,7 +89,7 @@ describe('esaggs expression function - public', () => { expect(startDependencies.aggs.createAggConfigs).toHaveBeenCalledWith( {}, args.aggs.map((agg) => agg.value), - { hierarchical: true, partialRows: false } + { hierarchical: true, partialRows: false, probability: 1, samplerSeed: undefined } ); }); @@ -99,6 +99,8 @@ describe('esaggs expression function - public', () => { expect(startDependencies.aggs.createAggConfigs).toHaveBeenCalledWith({}, [], { hierarchical: true, partialRows: false, + probability: 1, + samplerSeed: undefined, }); }); From 89667887a1c67b173538a5de2ac264227e68fee2 Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 18 Oct 2022 10:34:50 +0200 Subject: [PATCH 12/18] :lipstick: Add tech preview label --- .../datasources/form_based/layer_settings.tsx | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx b/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx index 897b276466018..733b299f70aad 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import { EuiFormRow, EuiToolTip, EuiIcon, EuiRange } from '@elastic/eui'; +import { EuiFormRow, EuiToolTip, EuiRange, EuiBetaBadge } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React from 'react'; import type { DatasourceLayerSettingsProps } from '../../types'; @@ -24,21 +24,31 @@ export function LayerSettingsPanel({ - - {i18n.translate('xpack.lens.xyChart.randomSampling.label', { - defaultMessage: 'Random Sampling', + + {i18n.translate('xpack.lens.xyChart.randomSampling.label', { + defaultMessage: 'Sampling', + })}{' '} + - - + delay="long" + position="top" + > + + + } > Date: Tue, 18 Oct 2022 14:55:04 +0200 Subject: [PATCH 13/18] :white_check_mark: Add new tests for sampling --- .../common/search/aggs/agg_configs.test.ts | 155 +++++ .../_terms_other_bucket_helper.test.ts | 576 ++++++++++-------- .../buckets/_terms_other_bucket_helper.ts | 8 +- 3 files changed, 465 insertions(+), 274 deletions(-) diff --git a/src/plugins/data/common/search/aggs/agg_configs.test.ts b/src/plugins/data/common/search/aggs/agg_configs.test.ts index cd0495a3f78c6..c3b7ffe937b35 100644 --- a/src/plugins/data/common/search/aggs/agg_configs.test.ts +++ b/src/plugins/data/common/search/aggs/agg_configs.test.ts @@ -16,6 +16,15 @@ import type { DataView } from '@kbn/data-views-plugin/common'; import { stubIndexPattern } from '../../stubs'; import { IEsSearchResponse } from '..'; +// Mute moment.tz warnings about not finding a mock timezone +jest.mock('../utils', () => { + const original = jest.requireActual('../utils'); + return { + ...original, + getUserTimeZone: jest.fn(() => 'US/Pacific'), + }; +}); + describe('AggConfigs', () => { const indexPattern: DataView = stubIndexPattern; let typesRegistry: AggTypesRegistryStart; @@ -563,6 +572,82 @@ describe('AggConfigs', () => { '1-bucket>_count' ); }); + + it('prepends a sampling agg whenever sampling is enabled', () => { + const configStates = [ + { + enabled: true, + id: '1', + type: 'avg_bucket', + schema: 'metric', + params: { + customBucket: { + id: '1-bucket', + type: 'date_histogram', + schema: 'bucketAgg', + params: { + field: '@timestamp', + interval: '10s', + }, + }, + customMetric: { + id: '1-metric', + type: 'count', + schema: 'metricAgg', + params: {}, + }, + }, + }, + { + enabled: true, + id: '2', + type: 'terms', + schema: 'bucket', + params: { + field: 'clientip', + }, + }, + { + enabled: true, + id: '3', + type: 'terms', + schema: 'bucket', + params: { + field: 'machine.os.raw', + }, + }, + ]; + + const ac = new AggConfigs( + indexPattern, + configStates, + { typesRegistry, hierarchical: true, probability: 0.5 }, + jest.fn() + ); + const topLevelDsl = ac.toDsl(); + + expect(Object.keys(topLevelDsl)).toContain('sampling'); + expect(Object.keys(topLevelDsl.sampling)).toEqual(['random_sampler', 'aggs']); + expect(Object.keys(topLevelDsl.sampling.aggs)).toContain('2'); + expect(Object.keys(topLevelDsl.sampling.aggs['2'].aggs)).toEqual(['1', '3', '1-bucket']); + }); + + it('should not prepend a sampling agg when no nested agg is avaialble', () => { + const ac = new AggConfigs( + indexPattern, + [ + { + enabled: true, + type: 'count', + schema: 'metric', + }, + ], + { typesRegistry, probability: 0.5 }, + jest.fn() + ); + const topLevelDsl = ac.toDsl(); + expect(Object.keys(topLevelDsl)).not.toContain('sampling'); + }); }); describe('#postFlightTransform', () => { @@ -854,4 +939,74 @@ describe('AggConfigs', () => { `); }); }); + + describe('isSamplingEnabled', () => { + it('should return false if probability is 1', () => { + const ac = new AggConfigs( + indexPattern, + [{ enabled: true, type: 'avg', schema: 'metric', params: { field: 'bytes' } }], + { typesRegistry, probability: 1 }, + jest.fn() + ); + + expect(ac.isSamplingEnabled()).toBeFalsy(); + }); + + it('should return true if probability is less than 1', () => { + const ac = new AggConfigs( + indexPattern, + [{ enabled: true, type: 'avg', schema: 'metric', params: { field: 'bytes' } }], + { typesRegistry, probability: 0.1 }, + jest.fn() + ); + + expect(ac.isSamplingEnabled()).toBeTruthy(); + }); + + it('should return false when all aggs have hasNoDsl flag enabled', () => { + const ac = new AggConfigs( + indexPattern, + [ + { + enabled: true, + type: 'count', + schema: 'metric', + }, + ], + { typesRegistry, probability: 1 }, + jest.fn() + ); + + expect(ac.isSamplingEnabled()).toBeFalsy(); + }); + + it('should return false when no nested aggs are avaialble', () => { + const ac = new AggConfigs( + indexPattern, + [{ enabled: false, type: 'avg', schema: 'metric', params: { field: 'bytes' } }], + { typesRegistry, probability: 1 }, + jest.fn() + ); + + expect(ac.isSamplingEnabled()).toBeFalsy(); + }); + + it('should return true if at least one nested agg is available and probability < 1', () => { + const ac = new AggConfigs( + indexPattern, + [ + { + enabled: true, + type: 'count', + schema: 'metric', + }, + { enabled: true, type: 'avg', schema: 'metric', params: { field: 'bytes' } }, + ], + { typesRegistry, probability: 0.1 }, + jest.fn() + ); + + expect(ac.isSamplingEnabled()).toBeTruthy(); + }); + }); }); diff --git a/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.test.ts b/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.test.ts index 6ce588b98fa9c..11f44ec2bbd1e 100644 --- a/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.test.ts +++ b/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.test.ts @@ -18,6 +18,7 @@ import { AggConfigs, CreateAggConfigParams } from '../agg_configs'; import { BUCKET_TYPES } from './bucket_agg_types'; import { IBucketAggConfig } from './bucket_agg_type'; import { mockAggTypesRegistry } from '../test_helpers'; +import { estypes } from '@elastic/elasticsearch'; const indexPattern = { id: '1234', @@ -281,71 +282,56 @@ const nestedOtherResponse = { describe('Terms Agg Other bucket helper', () => { const typesRegistry = mockAggTypesRegistry(); - const getAggConfigs = (aggs: CreateAggConfigParams[] = []) => { - return new AggConfigs(indexPattern, [...aggs], { typesRegistry }, jest.fn()); - }; - - describe('buildOtherBucketAgg', () => { - test('returns a function', () => { - const aggConfigs = getAggConfigs(singleTerm.aggs); - const agg = buildOtherBucketAgg( - aggConfigs, - aggConfigs.aggs[0] as IBucketAggConfig, - singleTermResponse - ); - expect(typeof agg).toBe('function'); - }); - - test('correctly builds query with single terms agg', () => { - const aggConfigs = getAggConfigs(singleTerm.aggs); - const agg = buildOtherBucketAgg( - aggConfigs, - aggConfigs.aggs[0] as IBucketAggConfig, - singleTermResponse - ); - const expectedResponse = { - aggs: undefined, - filters: { - filters: { - '': { - bool: { - must: [], - filter: [{ exists: { field: 'machine.os.raw' } }], - should: [], - must_not: [ - { match_phrase: { 'machine.os.raw': 'ios' } }, - { match_phrase: { 'machine.os.raw': 'win xp' } }, - ], - }, - }, + for (const probability of [1, 0.5]) { + function enrichResponseWithSampling(response: any) { + if (probability === 1) { + return response; + } + return { + ...response, + aggregations: { + sampling: { + ...response.aggregations, }, }, }; - expect(agg).toBeDefined(); - if (agg) { - expect(agg()['other-filter']).toEqual(expectedResponse); - } - }); + } + function getAggConfigs(aggs: CreateAggConfigParams[] = []) { + return new AggConfigs(indexPattern, [...aggs], { typesRegistry, probability }, jest.fn()); + } + + function getTopAggregations(updatedResponse: estypes.SearchResponse) { + return probability === 1 + ? updatedResponse.aggregations! + : (updatedResponse.aggregations!.sampling as Record); + } + + describe(`buildOtherBucketAgg${probability === 1 ? '- with sampling' : ''}`, () => { + test('returns a function', () => { + const aggConfigs = getAggConfigs(singleTerm.aggs); + const agg = buildOtherBucketAgg( + aggConfigs, + aggConfigs.aggs[0] as IBucketAggConfig, + enrichResponseWithSampling(singleTermResponse) + ); + expect(typeof agg).toBe('function'); + }); - test('correctly builds query for nested terms agg', () => { - const aggConfigs = getAggConfigs(nestedTerm.aggs); - const agg = buildOtherBucketAgg( - aggConfigs, - aggConfigs.aggs[1] as IBucketAggConfig, - nestedTermResponse - ); - const expectedResponse = { - 'other-filter': { + test('correctly builds query with single terms agg', () => { + const aggConfigs = getAggConfigs(singleTerm.aggs); + const agg = buildOtherBucketAgg( + aggConfigs, + aggConfigs.aggs[0] as IBucketAggConfig, + enrichResponseWithSampling(singleTermResponse) + ); + const expectedResponse = { aggs: undefined, filters: { filters: { - [`${SEP}IN-with-dash`]: { + '': { bool: { must: [], - filter: [ - { match_phrase: { 'geo.src': 'IN-with-dash' } }, - { exists: { field: 'machine.os.raw' } }, - ], + filter: [{ exists: { field: 'machine.os.raw' } }], should: [], must_not: [ { match_phrase: { 'machine.os.raw': 'ios' } }, @@ -353,272 +339,322 @@ describe('Terms Agg Other bucket helper', () => { ], }, }, - [`${SEP}US-with-dash`]: { - bool: { - must: [], - filter: [ - { match_phrase: { 'geo.src': 'US-with-dash' } }, - { exists: { field: 'machine.os.raw' } }, - ], - should: [], - must_not: [ - { match_phrase: { 'machine.os.raw': 'ios' } }, - { match_phrase: { 'machine.os.raw': 'win xp' } }, - ], + }, + }, + }; + expect(agg).toBeDefined(); + if (agg) { + const resp = agg(); + const topAgg = probability === 1 ? resp : resp.sampling!.aggs; + expect(topAgg['other-filter']).toEqual(expectedResponse); + } + }); + + test('correctly builds query for nested terms agg', () => { + const aggConfigs = getAggConfigs(nestedTerm.aggs); + const agg = buildOtherBucketAgg( + aggConfigs, + aggConfigs.aggs[1] as IBucketAggConfig, + enrichResponseWithSampling(nestedTermResponse) + ); + const expectedResponse = { + 'other-filter': { + aggs: undefined, + filters: { + filters: { + [`${SEP}IN-with-dash`]: { + bool: { + must: [], + filter: [ + { match_phrase: { 'geo.src': 'IN-with-dash' } }, + { exists: { field: 'machine.os.raw' } }, + ], + should: [], + must_not: [ + { match_phrase: { 'machine.os.raw': 'ios' } }, + { match_phrase: { 'machine.os.raw': 'win xp' } }, + ], + }, + }, + [`${SEP}US-with-dash`]: { + bool: { + must: [], + filter: [ + { match_phrase: { 'geo.src': 'US-with-dash' } }, + { exists: { field: 'machine.os.raw' } }, + ], + should: [], + must_not: [ + { match_phrase: { 'machine.os.raw': 'ios' } }, + { match_phrase: { 'machine.os.raw': 'win xp' } }, + ], + }, }, }, }, }, - }, - }; - expect(agg).toBeDefined(); - if (agg) { - expect(agg()).toEqual(expectedResponse); - } - }); + }; + expect(agg).toBeDefined(); + if (agg) { + const resp = agg(); + const topAgg = probability === 1 ? resp : resp.sampling!.aggs; + // console.log({ probability }, JSON.stringify(topAgg, null, 2)); + expect(topAgg).toEqual(expectedResponse); + } + }); - test('correctly builds query for nested terms agg with one disabled', () => { - const oneDisabledNestedTerms = { - aggs: [ - { - id: '2', - type: BUCKET_TYPES.TERMS, - enabled: false, - params: { - field: { - name: 'machine.os.raw', - indexPattern, - filterable: true, + test('correctly builds query for nested terms agg with one disabled', () => { + const oneDisabledNestedTerms = { + aggs: [ + { + id: '2', + type: BUCKET_TYPES.TERMS, + enabled: false, + params: { + field: { + name: 'machine.os.raw', + indexPattern, + filterable: true, + }, + size: 2, + otherBucket: false, + missingBucket: true, }, - size: 2, - otherBucket: false, - missingBucket: true, }, - }, - { - id: '1', - type: BUCKET_TYPES.TERMS, - params: { - field: { - name: 'geo.src', - indexPattern, - filterable: true, + { + id: '1', + type: BUCKET_TYPES.TERMS, + params: { + field: { + name: 'geo.src', + indexPattern, + filterable: true, + }, + size: 2, + otherBucket: true, + missingBucket: false, }, - size: 2, - otherBucket: true, - missingBucket: false, }, - }, - ], - }; - const aggConfigs = getAggConfigs(oneDisabledNestedTerms.aggs); - const agg = buildOtherBucketAgg( - aggConfigs, - aggConfigs.aggs[1] as IBucketAggConfig, - singleTermResponse - ); - const expectedResponse = { - 'other-filter': { - aggs: undefined, - filters: { + ], + }; + const aggConfigs = getAggConfigs(oneDisabledNestedTerms.aggs); + const agg = buildOtherBucketAgg( + aggConfigs, + aggConfigs.aggs[1] as IBucketAggConfig, + enrichResponseWithSampling(singleTermResponse) + ); + const expectedResponse = { + 'other-filter': { + aggs: undefined, filters: { - '': { - bool: { - filter: [ - { - exists: { - field: 'geo.src', + filters: { + '': { + bool: { + filter: [ + { + exists: { + field: 'geo.src', + }, }, - }, - ], - must: [], - must_not: [ - { - match_phrase: { - 'geo.src': 'ios', + ], + must: [], + must_not: [ + { + match_phrase: { + 'geo.src': 'ios', + }, }, - }, - { - match_phrase: { - 'geo.src': 'win xp', + { + match_phrase: { + 'geo.src': 'win xp', + }, }, - }, - ], - should: [], + ], + should: [], + }, }, }, }, }, - }, - }; - expect(agg).toBeDefined(); - if (agg) { - expect(agg()).toEqual(expectedResponse); - } - }); + }; + expect(agg).toBeDefined(); + if (agg) { + const resp = agg(); + const topAgg = probability === 1 ? resp : resp.sampling!.aggs; + expect(topAgg).toEqual(expectedResponse); + } + }); - test('does not build query if sum_other_doc_count is 0 (exhaustive terms)', () => { - const aggConfigs = getAggConfigs(nestedTerm.aggs); - expect( - buildOtherBucketAgg( + test('does not build query if sum_other_doc_count is 0 (exhaustive terms)', () => { + const aggConfigs = getAggConfigs(nestedTerm.aggs); + expect( + buildOtherBucketAgg( + aggConfigs, + aggConfigs.aggs[1] as IBucketAggConfig, + enrichResponseWithSampling(exhaustiveNestedTermResponse) + ) + ).toBeFalsy(); + }); + + test('excludes exists filter for scripted fields', () => { + const aggConfigs = getAggConfigs(nestedTerm.aggs); + aggConfigs.aggs[1].params.field = { + ...aggConfigs.aggs[1].params.field, + scripted: true, + }; + const agg = buildOtherBucketAgg( aggConfigs, aggConfigs.aggs[1] as IBucketAggConfig, - exhaustiveNestedTermResponse - ) - ).toBeFalsy(); - }); - - test('excludes exists filter for scripted fields', () => { - const aggConfigs = getAggConfigs(nestedTerm.aggs); - aggConfigs.aggs[1].params.field.scripted = true; - const agg = buildOtherBucketAgg( - aggConfigs, - aggConfigs.aggs[1] as IBucketAggConfig, - nestedTermResponse - ); - const expectedResponse = { - 'other-filter': { - aggs: undefined, - filters: { + enrichResponseWithSampling(nestedTermResponse) + ); + const expectedResponse = { + 'other-filter': { + aggs: undefined, filters: { - [`${SEP}IN-with-dash`]: { - bool: { - must: [], - filter: [{ match_phrase: { 'geo.src': 'IN-with-dash' } }], - should: [], - must_not: [ - { - script: { + filters: { + [`${SEP}IN-with-dash`]: { + bool: { + must: [], + filter: [{ match_phrase: { 'geo.src': 'IN-with-dash' } }], + should: [], + must_not: [ + { script: { - lang: undefined, - params: { value: 'ios' }, - source: '(undefined) == value', + script: { + lang: undefined, + params: { value: 'ios' }, + source: '(undefined) == value', + }, }, }, - }, - { - script: { + { script: { - lang: undefined, - params: { value: 'win xp' }, - source: '(undefined) == value', + script: { + lang: undefined, + params: { value: 'win xp' }, + source: '(undefined) == value', + }, }, }, - }, - ], + ], + }, }, - }, - [`${SEP}US-with-dash`]: { - bool: { - must: [], - filter: [{ match_phrase: { 'geo.src': 'US-with-dash' } }], - should: [], - must_not: [ - { - script: { + [`${SEP}US-with-dash`]: { + bool: { + must: [], + filter: [{ match_phrase: { 'geo.src': 'US-with-dash' } }], + should: [], + must_not: [ + { script: { - lang: undefined, - params: { value: 'ios' }, - source: '(undefined) == value', + script: { + lang: undefined, + params: { value: 'ios' }, + source: '(undefined) == value', + }, }, }, - }, - { - script: { + { script: { - lang: undefined, - params: { value: 'win xp' }, - source: '(undefined) == value', + script: { + lang: undefined, + params: { value: 'win xp' }, + source: '(undefined) == value', + }, }, }, - }, - ], + ], + }, }, }, }, }, - }, - }; - expect(agg).toBeDefined(); - if (agg) { - expect(agg()).toEqual(expectedResponse); - } - }); + }; + expect(agg).toBeDefined(); + if (agg) { + const resp = agg(); + const topAgg = probability === 1 ? resp : resp.sampling!.aggs; + expect(topAgg).toEqual(expectedResponse); + } + }); - test('returns false when nested terms agg has no buckets', () => { - const aggConfigs = getAggConfigs(nestedTerm.aggs); - const agg = buildOtherBucketAgg( - aggConfigs, - aggConfigs.aggs[1] as IBucketAggConfig, - nestedTermResponseNoResults - ); + test('returns false when nested terms agg has no buckets', () => { + const aggConfigs = getAggConfigs(nestedTerm.aggs); + const agg = buildOtherBucketAgg( + aggConfigs, + aggConfigs.aggs[1] as IBucketAggConfig, + enrichResponseWithSampling(nestedTermResponseNoResults) + ); - expect(agg).toEqual(false); + expect(agg).toEqual(false); + }); }); - }); - describe('mergeOtherBucketAggResponse', () => { - test('correctly merges other bucket with single terms agg', () => { - const aggConfigs = getAggConfigs(singleTerm.aggs); - const otherAggConfig = buildOtherBucketAgg( - aggConfigs, - aggConfigs.aggs[0] as IBucketAggConfig, - singleTermResponse - ); - - expect(otherAggConfig).toBeDefined(); - if (otherAggConfig) { - const mergedResponse = mergeOtherBucketAggResponse( + describe(`mergeOtherBucketAggResponse${probability === 1 ? '- with sampling' : ''}`, () => { + test('correctly merges other bucket with single terms agg', () => { + const aggConfigs = getAggConfigs(singleTerm.aggs); + const otherAggConfig = buildOtherBucketAgg( aggConfigs, - singleTermResponse, - singleOtherResponse, aggConfigs.aggs[0] as IBucketAggConfig, - otherAggConfig(), - constructSingleTermOtherFilter + enrichResponseWithSampling(singleTermResponse) ); - expect((mergedResponse!.aggregations!['1'] as any).buckets[3].key).toEqual('__other__'); - } - }); - test('correctly merges other bucket with nested terms agg', () => { - const aggConfigs = getAggConfigs(nestedTerm.aggs); - const otherAggConfig = buildOtherBucketAgg( - aggConfigs, - aggConfigs.aggs[1] as IBucketAggConfig, - nestedTermResponse - ); + expect(otherAggConfig).toBeDefined(); + if (otherAggConfig) { + const mergedResponse = mergeOtherBucketAggResponse( + aggConfigs, + enrichResponseWithSampling(singleTermResponse), + enrichResponseWithSampling(singleOtherResponse), + aggConfigs.aggs[0] as IBucketAggConfig, + otherAggConfig(), + constructSingleTermOtherFilter + ); - expect(otherAggConfig).toBeDefined(); - if (otherAggConfig) { - const mergedResponse = mergeOtherBucketAggResponse( + const topAgg = getTopAggregations(mergedResponse); + expect((topAgg['1'] as any).buckets[3].key).toEqual('__other__'); + } + }); + + test('correctly merges other bucket with nested terms agg', () => { + const aggConfigs = getAggConfigs(nestedTerm.aggs); + const otherAggConfig = buildOtherBucketAgg( aggConfigs, - nestedTermResponse, - nestedOtherResponse, aggConfigs.aggs[1] as IBucketAggConfig, - otherAggConfig(), - constructSingleTermOtherFilter + enrichResponseWithSampling(nestedTermResponse) ); - expect((mergedResponse!.aggregations!['1'] as any).buckets[1]['2'].buckets[3].key).toEqual( - '__other__' - ); - } + expect(otherAggConfig).toBeDefined(); + if (otherAggConfig) { + const mergedResponse = mergeOtherBucketAggResponse( + aggConfigs, + enrichResponseWithSampling(nestedTermResponse), + enrichResponseWithSampling(nestedOtherResponse), + aggConfigs.aggs[1] as IBucketAggConfig, + otherAggConfig(), + constructSingleTermOtherFilter + ); + + const topAgg = getTopAggregations(mergedResponse); + expect((topAgg['1'] as any).buckets[1]['2'].buckets[3].key).toEqual('__other__'); + } + }); }); - }); - describe('updateMissingBucket', () => { - test('correctly updates missing bucket key', () => { - const aggConfigs = getAggConfigs(nestedTerm.aggs); - const updatedResponse = updateMissingBucket( - singleTermResponse, - aggConfigs, - aggConfigs.aggs[0] as IBucketAggConfig - ); - expect( - (updatedResponse!.aggregations!['1'] as any).buckets.find( - (bucket: Record) => bucket.key === '__missing__' - ) - ).toBeDefined(); + describe(`updateMissingBucket${probability === 1 ? '- with sampling' : ''}`, () => { + test('correctly updates missing bucket key', () => { + const aggConfigs = getAggConfigs(nestedTerm.aggs); + const updatedResponse = updateMissingBucket( + enrichResponseWithSampling(singleTermResponse), + aggConfigs, + aggConfigs.aggs[0] as IBucketAggConfig + ); + const topAgg = getTopAggregations(updatedResponse); + expect( + (topAgg['1'] as any).buckets.find( + (bucket: Record) => bucket.key === '__missing__' + ) + ).toBeDefined(); + }); }); - }); + } }); diff --git a/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts b/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts index 563436f56a4b3..68c64f67ef27f 100644 --- a/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts +++ b/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.ts @@ -130,10 +130,7 @@ const getOtherAggTerms = (requestAgg: Record, key: string, otherAgg }; /** - * - * @param requestAgg - * @param aggConfigs - * @returns + * Helper function to handle sampling case and get the correct cursor agg from a request object */ const getCorrectAggCursorFromRequest = ( requestAgg: Record, @@ -142,6 +139,9 @@ const getCorrectAggCursorFromRequest = ( return aggConfigs.isSamplingEnabled() ? requestAgg.sampling.aggs : requestAgg; }; +/** + * Helper function to handle sampling case and get the correct cursor agg from a response object + */ const getCorrectAggregationsCursorFromResponse = ( response: estypes.SearchResponse, aggConfigs: IAggConfigs From 281b8c92475c5e6ab56479ac3a10e26ea1928b7f Mon Sep 17 00:00:00 2001 From: dej611 Date: Tue, 18 Oct 2022 16:42:50 +0200 Subject: [PATCH 14/18] :white_check_mark: Add more tests --- .../_terms_other_bucket_helper.test.ts | 28 +- .../data/common/search/tabify/tabify.test.ts | 331 ++++++++++-------- 2 files changed, 200 insertions(+), 159 deletions(-) diff --git a/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.test.ts b/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.test.ts index 11f44ec2bbd1e..be2e969f279b1 100644 --- a/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.test.ts +++ b/src/plugins/data/common/search/aggs/buckets/_terms_other_bucket_helper.test.ts @@ -19,6 +19,7 @@ import { BUCKET_TYPES } from './bucket_agg_types'; import { IBucketAggConfig } from './bucket_agg_type'; import { mockAggTypesRegistry } from '../test_helpers'; import { estypes } from '@elastic/elasticsearch'; +import { isSamplingEnabled } from '../utils/sampler'; const indexPattern = { id: '1234', @@ -282,9 +283,15 @@ const nestedOtherResponse = { describe('Terms Agg Other bucket helper', () => { const typesRegistry = mockAggTypesRegistry(); - for (const probability of [1, 0.5]) { + for (const probability of [1, 0.5, undefined]) { + function getTitlePostfix() { + if (!isSamplingEnabled(probability)) { + return ''; + } + return ` - with sampling (probability = ${probability})`; + } function enrichResponseWithSampling(response: any) { - if (probability === 1) { + if (!isSamplingEnabled(probability)) { return response; } return { @@ -296,17 +303,18 @@ describe('Terms Agg Other bucket helper', () => { }, }; } + function getAggConfigs(aggs: CreateAggConfigParams[] = []) { return new AggConfigs(indexPattern, [...aggs], { typesRegistry, probability }, jest.fn()); } function getTopAggregations(updatedResponse: estypes.SearchResponse) { - return probability === 1 + return !isSamplingEnabled(probability) ? updatedResponse.aggregations! : (updatedResponse.aggregations!.sampling as Record); } - describe(`buildOtherBucketAgg${probability === 1 ? '- with sampling' : ''}`, () => { + describe(`buildOtherBucketAgg${getTitlePostfix()}`, () => { test('returns a function', () => { const aggConfigs = getAggConfigs(singleTerm.aggs); const agg = buildOtherBucketAgg( @@ -345,7 +353,7 @@ describe('Terms Agg Other bucket helper', () => { expect(agg).toBeDefined(); if (agg) { const resp = agg(); - const topAgg = probability === 1 ? resp : resp.sampling!.aggs; + const topAgg = !isSamplingEnabled(probability) ? resp : resp.sampling!.aggs; expect(topAgg['other-filter']).toEqual(expectedResponse); } }); @@ -397,7 +405,7 @@ describe('Terms Agg Other bucket helper', () => { expect(agg).toBeDefined(); if (agg) { const resp = agg(); - const topAgg = probability === 1 ? resp : resp.sampling!.aggs; + const topAgg = !isSamplingEnabled(probability) ? resp : resp.sampling!.aggs; // console.log({ probability }, JSON.stringify(topAgg, null, 2)); expect(topAgg).toEqual(expectedResponse); } @@ -480,7 +488,7 @@ describe('Terms Agg Other bucket helper', () => { expect(agg).toBeDefined(); if (agg) { const resp = agg(); - const topAgg = probability === 1 ? resp : resp.sampling!.aggs; + const topAgg = !isSamplingEnabled(probability) ? resp : resp.sampling!.aggs; expect(topAgg).toEqual(expectedResponse); } }); @@ -573,7 +581,7 @@ describe('Terms Agg Other bucket helper', () => { expect(agg).toBeDefined(); if (agg) { const resp = agg(); - const topAgg = probability === 1 ? resp : resp.sampling!.aggs; + const topAgg = !isSamplingEnabled(probability) ? resp : resp.sampling!.aggs; expect(topAgg).toEqual(expectedResponse); } }); @@ -590,7 +598,7 @@ describe('Terms Agg Other bucket helper', () => { }); }); - describe(`mergeOtherBucketAggResponse${probability === 1 ? '- with sampling' : ''}`, () => { + describe(`mergeOtherBucketAggResponse${getTitlePostfix()}`, () => { test('correctly merges other bucket with single terms agg', () => { const aggConfigs = getAggConfigs(singleTerm.aggs); const otherAggConfig = buildOtherBucketAgg( @@ -640,7 +648,7 @@ describe('Terms Agg Other bucket helper', () => { }); }); - describe(`updateMissingBucket${probability === 1 ? '- with sampling' : ''}`, () => { + describe(`updateMissingBucket${getTitlePostfix()}`, () => { test('correctly updates missing bucket key', () => { const aggConfigs = getAggConfigs(nestedTerm.aggs); const updatedResponse = updateMissingBucket( diff --git a/src/plugins/data/common/search/tabify/tabify.test.ts b/src/plugins/data/common/search/tabify/tabify.test.ts index d7d983fabfdaf..90ef53623c298 100644 --- a/src/plugins/data/common/search/tabify/tabify.test.ts +++ b/src/plugins/data/common/search/tabify/tabify.test.ts @@ -11,184 +11,217 @@ import type { DataView } from '@kbn/data-views-plugin/common'; import { AggConfigs, BucketAggParam, IAggConfig, IAggConfigs } from '../aggs'; import { mockAggTypesRegistry } from '../aggs/test_helpers'; import { metricOnly, threeTermBuckets } from './fixtures/fake_hierarchical_data'; +import { isSamplingEnabled } from '../aggs/utils/sampler'; describe('tabifyAggResponse Integration', () => { const typesRegistry = mockAggTypesRegistry(); - const createAggConfigs = (aggs: IAggConfig[] = []) => { - const field = { - name: '@timestamp', - }; - - const indexPattern = { - id: '1234', - title: 'logstash-*', - fields: { - getByName: () => field, - filter: () => [field], - }, - getFormatterForField: () => ({ - toJSON: () => '{}', - }), - } as unknown as DataView; - - return new AggConfigs(indexPattern, aggs, { typesRegistry }, jest.fn()); - }; - - const mockAggConfig = (agg: any): IAggConfig => agg as unknown as IAggConfig; - - test('transforms a simple response properly', () => { - const aggConfigs = createAggConfigs([{ type: 'count' } as any]); - - const resp = tabifyAggResponse(aggConfigs, metricOnly, { - metricsAtAllLevels: true, - }); - - expect(resp).toHaveProperty('rows'); - expect(resp).toHaveProperty('columns'); + for (const probability of [1, 0.5, undefined]) { + function getTitlePostfix() { + if (!isSamplingEnabled(probability)) { + return ''; + } + return ` - with sampling (probability = ${probability})`; + } - expect(resp.rows).toHaveLength(1); - expect(resp.columns).toHaveLength(1); + function enrichResponseWithSampling(response: any) { + if (!isSamplingEnabled(probability)) { + return response; + } + return { + ...response, + aggregations: { + sampling: { + ...response.aggregations, + }, + }, + }; + } - expect(resp.rows[0]).toEqual({ 'col-0-1': 1000 }); - expect(resp.columns[0]).toHaveProperty('name', aggConfigs.aggs[0].makeLabel()); + const createAggConfigs = (aggs: IAggConfig[] = []) => { + const field = { + name: '@timestamp', + }; + + const indexPattern = { + id: '1234', + title: 'logstash-*', + fields: { + getByName: () => field, + filter: () => [field], + }, + getFormatterForField: () => ({ + toJSON: () => '{}', + }), + } as unknown as DataView; + + return new AggConfigs(indexPattern, aggs, { typesRegistry, probability }, jest.fn()); + }; - expect(resp).toHaveProperty('meta.type', 'esaggs'); - expect(resp).toHaveProperty('meta.source', '1234'); - expect(resp).toHaveProperty('meta.statistics.totalCount', 1000); - }); + const mockAggConfig = (agg: any): IAggConfig => agg as unknown as IAggConfig; - describe('scaleMetricValues performance check', () => { - beforeAll(() => { - typesRegistry.get('count').params.push({ - name: 'scaleMetricValues', - default: false, - write: () => {}, - advanced: true, - } as any as BucketAggParam); - }); - test('does not call write if scaleMetricValues is not set', () => { + test(`transforms a simple response properly${getTitlePostfix()}`, () => { const aggConfigs = createAggConfigs([{ type: 'count' } as any]); - const writeMock = jest.fn(); - aggConfigs.getRequestAggs()[0].write = writeMock; - - tabifyAggResponse(aggConfigs, metricOnly, { + const resp = tabifyAggResponse(aggConfigs, enrichResponseWithSampling(metricOnly), { metricsAtAllLevels: true, }); - expect(writeMock).not.toHaveBeenCalled(); - }); - test('does call write if scaleMetricValues is set', () => { - const aggConfigs = createAggConfigs([ - { type: 'count', params: { scaleMetricValues: true } } as any, - ]); + expect(resp).toHaveProperty('rows'); + expect(resp).toHaveProperty('columns'); - const writeMock = jest.fn(() => ({})); - aggConfigs.getRequestAggs()[0].write = writeMock; - - tabifyAggResponse(aggConfigs, metricOnly, { - metricsAtAllLevels: true, - }); - expect(writeMock).toHaveBeenCalled(); - }); - }); - - describe('transforms a complex response', () => { - let esResp: typeof threeTermBuckets; - let aggConfigs: IAggConfigs; - let avg: IAggConfig; - let ext: IAggConfig; - let src: IAggConfig; - let os: IAggConfig; - - beforeEach(() => { - aggConfigs = createAggConfigs([ - mockAggConfig({ type: 'avg', schema: 'metric', params: { field: '@timestamp' } }), - mockAggConfig({ type: 'terms', schema: 'split', params: { field: '@timestamp' } }), - mockAggConfig({ type: 'terms', schema: 'segment', params: { field: '@timestamp' } }), - mockAggConfig({ type: 'terms', schema: 'segment', params: { field: '@timestamp' } }), - ]); - - [avg, ext, src, os] = aggConfigs.aggs; - - esResp = threeTermBuckets; - esResp.aggregations.agg_2.buckets[1].agg_3.buckets[0].agg_4.buckets = []; - }); + expect(resp.rows).toHaveLength(1); + expect(resp.columns).toHaveLength(1); - // check that the columns of a table are formed properly - function expectColumns(table: ReturnType, aggs: IAggConfig[]) { - expect(table.columns).toHaveLength(aggs.length); + expect(resp.rows[0]).toEqual({ 'col-0-1': 1000 }); + expect(resp.columns[0]).toHaveProperty('name', aggConfigs.aggs[0].makeLabel()); - aggs.forEach((agg, i) => { - expect(table.columns[i]).toHaveProperty('name', agg.makeLabel()); - }); - } + expect(resp).toHaveProperty('meta.type', 'esaggs'); + expect(resp).toHaveProperty('meta.source', '1234'); + expect(resp).toHaveProperty('meta.statistics.totalCount', 1000); + }); - // check that a row has expected values - function expectRow( - row: Record, - asserts: Array<(val: string | number) => void> - ) { - expect(typeof row).toBe('object'); - - asserts.forEach((assert, i: number) => { - if (row[`col-${i}`]) { - assert(row[`col-${i}`]); - } + describe(`scaleMetricValues performance check${getTitlePostfix()}`, () => { + beforeAll(() => { + typesRegistry.get('count').params.push({ + name: 'scaleMetricValues', + default: false, + write: () => {}, + advanced: true, + } as any as BucketAggParam); }); - } + test('does not call write if scaleMetricValues is not set', () => { + const aggConfigs = createAggConfigs([{ type: 'count' } as any]); - // check for two character country code - function expectCountry(val: string | number) { - expect(typeof val).toBe('string'); - expect(val).toHaveLength(2); - } + const writeMock = jest.fn(); + aggConfigs.getRequestAggs()[0].write = writeMock; - // check for an OS term - function expectExtension(val: string | number) { - expect(val).toMatch(/^(js|png|html|css|jpg)$/); - } + tabifyAggResponse(aggConfigs, enrichResponseWithSampling(metricOnly), { + metricsAtAllLevels: true, + }); + expect(writeMock).not.toHaveBeenCalled(); + }); - // check for an OS term - function expectOS(val: string | number) { - expect(val).toMatch(/^(win|mac|linux)$/); - } + test('does call write if scaleMetricValues is set', () => { + const aggConfigs = createAggConfigs([ + { type: 'count', params: { scaleMetricValues: true } } as any, + ]); - // check for something like an average bytes result - function expectAvgBytes(val: string | number) { - expect(typeof val).toBe('number'); - expect(val === 0 || val > 1000).toBeDefined(); - } + const writeMock = jest.fn(() => ({})); + aggConfigs.getRequestAggs()[0].write = writeMock; - test('for non-hierarchical vis', () => { - // the default for a non-hierarchical vis is to display - // only complete rows, and only put the metrics at the end. + tabifyAggResponse(aggConfigs, enrichResponseWithSampling(metricOnly), { + metricsAtAllLevels: true, + }); + expect(writeMock).toHaveBeenCalled(); + }); + }); - const tabbed = tabifyAggResponse(aggConfigs, esResp, { metricsAtAllLevels: false }); + describe(`transforms a complex response${getTitlePostfix()}`, () => { + let esResp: typeof threeTermBuckets; + let aggConfigs: IAggConfigs; + let avg: IAggConfig; + let ext: IAggConfig; + let src: IAggConfig; + let os: IAggConfig; + + function getTopAggregations( + rawResp: typeof threeTermBuckets + ): typeof threeTermBuckets['aggregations'] { + return !isSamplingEnabled(probability) + ? rawResp.aggregations! + : // @ts-ignore + rawResp.aggregations!.sampling!; + } + + beforeEach(() => { + aggConfigs = createAggConfigs([ + mockAggConfig({ type: 'avg', schema: 'metric', params: { field: '@timestamp' } }), + mockAggConfig({ type: 'terms', schema: 'split', params: { field: '@timestamp' } }), + mockAggConfig({ type: 'terms', schema: 'segment', params: { field: '@timestamp' } }), + mockAggConfig({ type: 'terms', schema: 'segment', params: { field: '@timestamp' } }), + ]); - expectColumns(tabbed, [ext, src, os, avg]); + [avg, ext, src, os] = aggConfigs.aggs; - tabbed.rows.forEach((row) => { - expectRow(row, [expectExtension, expectCountry, expectOS, expectAvgBytes]); + esResp = enrichResponseWithSampling(threeTermBuckets); + getTopAggregations(esResp).agg_2.buckets[1].agg_3.buckets[0].agg_4.buckets = []; }); - }); - - test('for hierarchical vis', () => { - const tabbed = tabifyAggResponse(aggConfigs, esResp, { metricsAtAllLevels: true }); - expectColumns(tabbed, [ext, avg, src, avg, os, avg]); + // check that the columns of a table are formed properly + function expectColumns(table: ReturnType, aggs: IAggConfig[]) { + expect(table.columns).toHaveLength(aggs.length); + + aggs.forEach((agg, i) => { + expect(table.columns[i]).toHaveProperty('name', agg.makeLabel()); + }); + } + + // check that a row has expected values + function expectRow( + row: Record, + asserts: Array<(val: string | number) => void> + ) { + expect(typeof row).toBe('object'); + + asserts.forEach((assert, i: number) => { + if (row[`col-${i}`]) { + assert(row[`col-${i}`]); + } + }); + } + + // check for two character country code + function expectCountry(val: string | number) { + expect(typeof val).toBe('string'); + expect(val).toHaveLength(2); + } + + // check for an OS term + function expectExtension(val: string | number) { + expect(val).toMatch(/^(js|png|html|css|jpg)$/); + } + + // check for an OS term + function expectOS(val: string | number) { + expect(val).toMatch(/^(win|mac|linux)$/); + } + + // check for something like an average bytes result + function expectAvgBytes(val: string | number) { + expect(typeof val).toBe('number'); + expect(val === 0 || val > 1000).toBeDefined(); + } + + test('for non-hierarchical vis', () => { + // the default for a non-hierarchical vis is to display + // only complete rows, and only put the metrics at the end. + + const tabbed = tabifyAggResponse(aggConfigs, esResp, { metricsAtAllLevels: false }); + + expectColumns(tabbed, [ext, src, os, avg]); + + tabbed.rows.forEach((row) => { + expectRow(row, [expectExtension, expectCountry, expectOS, expectAvgBytes]); + }); + }); - tabbed.rows.forEach((row) => { - expectRow(row, [ - expectExtension, - expectAvgBytes, - expectCountry, - expectAvgBytes, - expectOS, - expectAvgBytes, - ]); + test('for hierarchical vis', () => { + const tabbed = tabifyAggResponse(aggConfigs, esResp, { metricsAtAllLevels: true }); + + expectColumns(tabbed, [ext, avg, src, avg, os, avg]); + + tabbed.rows.forEach((row) => { + expectRow(row, [ + expectExtension, + expectAvgBytes, + expectCountry, + expectAvgBytes, + expectOS, + expectAvgBytes, + ]); + }); }); }); - }); + } }); From 13c8078ae775cb1fb1d04e4276a3d9f506db030e Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 19 Oct 2022 11:53:54 +0200 Subject: [PATCH 15/18] :white_check_mark: Add new test for suggestions --- .../form_based_suggestions.test.tsx | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.test.tsx b/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.test.tsx index 06fe3512903b8..962031a718455 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.test.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based_suggestions.test.tsx @@ -459,7 +459,7 @@ describe('IndexPattern Data Source suggestions', () => { }); describe('with a previous empty layer', () => { - function stateWithEmptyLayer() { + function stateWithEmptyLayer(): FormBasedPrivateState { const state = testInitialState(); return { ...state, @@ -762,6 +762,35 @@ describe('IndexPattern Data Source suggestions', () => { }) ); }); + + it('should inherit the sampling rate when generating new layer, if avaialble', () => { + const state = stateWithEmptyLayer(); + state.layers.previousLayer.sampling = 0.001; + const suggestions = getDatasourceSuggestionsForField( + state, + '1', + { + name: 'bytes', + displayName: 'bytes', + type: 'number', + aggregatable: true, + searchable: true, + }, + expectedIndexPatterns + ); + + expect(suggestions).toContainEqual( + expect.objectContaining({ + state: expect.objectContaining({ + layers: { + previousLayer: expect.objectContaining({ + sampling: 0.001, + }), + }, + }), + }) + ); + }); }); describe('suggesting extensions to non-empty tables', () => { From f9e1bb961fe5a44174d91d7790cda978d527642b Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 19 Oct 2022 14:47:24 +0200 Subject: [PATCH 16/18] :white_check_mark: Add functional tests for layer actions and random sampling --- .../datasources/form_based/form_based.tsx | 1 + .../datasources/form_based/layer_settings.tsx | 2 + .../layer_actions/layer_actions.tsx | 3 + .../test/functional/apps/lens/group1/index.ts | 1 + .../apps/lens/group1/layer_actions.ts | 91 +++++++++++++++++++ .../test/functional/page_objects/lens_page.ts | 17 ++++ 6 files changed, 115 insertions(+) create mode 100644 x-pack/test/functional/apps/lens/group1/layer_actions.ts diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx index 855e046706af7..8eac872b8b068 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx @@ -601,6 +601,7 @@ export function getFormBasedDatasource({ execute: openLayerSettings, icon: 'gear', isCompatible: Boolean(state.layers[layerId]), + 'data-test-subj': 'lnsLayerSettings', }, ]; }, diff --git a/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx b/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx index 733b299f70aad..35d487dcdf56d 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx @@ -23,6 +23,7 @@ export function LayerSettingsPanel({ return ( { setState({ diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx index bc1c41caa650b..9c32a24eac4dd 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_actions/layer_actions.tsx @@ -104,6 +104,9 @@ const InContextMenuActions = (props: LayerActionsProps) => { closePopover={closePopover} panelPaddingSize="none" anchorPosition="downLeft" + panelProps={{ + 'data-test-subj': 'lnsLayerActionsMenu', + }} > { + it('should allow creation of lens xy chart', async () => { + await PageObjects.visualize.navigateToNewVisualization(); + await PageObjects.visualize.clickVisType('lens'); + await PageObjects.lens.goToTimeRange(); + + await PageObjects.lens.openLayerContextMenu(); + + // should be 3 actions available + expect( + (await find.allByCssSelector('[data-test-subj=lnsLayerActionsMenu] button')).length + ).to.eql(3); + }); + + it('should open layer settings for a data layer', async () => { + // click on open layer settings + await testSubjects.click('lnsLayerSettings'); + // random sampling available + await testSubjects.existOrFail('lns-indexPattern-random-sampling-row'); + // tweak the value + await PageObjects.lens.dragRangeInput('lns-indexPattern-random-sampling', 2, 'left'); + + expect(await PageObjects.lens.getRangeInputValue('lns-indexPattern-random-sampling')).to.eql( + 2 // 0.01 + ); + await testSubjects.click('lns-indexPattern-dimensionContainerBack'); + }); + + it('should add an annotation layer and settings shoud not be available', async () => { + // configure a date histogram + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_xDimensionPanel > lns-empty-dimension', + operation: 'date_histogram', + field: '@timestamp', + }); + + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', + operation: 'average', + field: 'bytes', + }); + // add annotation layer + await testSubjects.click('lnsLayerAddButton'); + await testSubjects.click(`lnsLayerAddButton-annotations`); + await PageObjects.lens.openLayerContextMenu(1); + // layer settings not available + await testSubjects.missingOrFail('lnsLayerSettings'); + }); + + it('should switch to pie chart and have layer settings available', async () => { + await PageObjects.lens.switchToVisualization('pie'); + await PageObjects.lens.openLayerContextMenu(); + // layer settings still available + // open the panel + await testSubjects.click('lnsLayerSettings'); + // check the sampling value + expect(await PageObjects.lens.getRangeInputValue('lns-indexPattern-random-sampling')).to.eql( + 2 // 0.01 + ); + await testSubjects.click('lns-indexPattern-dimensionContainerBack'); + }); + + it('should switch to table and still have layer settings', async () => { + await PageObjects.lens.switchToVisualization('lnsDatatable'); + await PageObjects.lens.openLayerContextMenu(); + // layer settings still available + // open the panel + await testSubjects.click('lnsLayerSettings'); + // check the sampling value + expect(await PageObjects.lens.getRangeInputValue('lns-indexPattern-random-sampling')).to.eql( + 2 // 0.01 + ); + await testSubjects.click('lns-indexPattern-dimensionContainerBack'); + }); + }); +} diff --git a/x-pack/test/functional/page_objects/lens_page.ts b/x-pack/test/functional/page_objects/lens_page.ts index c11a9a37de7cf..9465e14f5efbf 100644 --- a/x-pack/test/functional/page_objects/lens_page.ts +++ b/x-pack/test/functional/page_objects/lens_page.ts @@ -600,6 +600,19 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont await this.waitForVisualization(); }, + async dragRangeInput(testId: string, steps: number = 1, direction: 'left' | 'right' = 'right') { + const inputEl = await testSubjects.find(testId); + await inputEl.focus(); + const browserKey = direction === 'left' ? browser.keys.LEFT : browser.keys.RIGHT; + while (steps--) { + await browser.pressKeys(browserKey); + } + }, + + async getRangeInputValue(testId: string) { + return (await testSubjects.find(testId)).getAttribute('value'); + }, + async isTopLevelAggregation() { return await testSubjects.isEuiSwitchChecked('indexPattern-nesting-switch'); }, @@ -1104,6 +1117,10 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont ); }, + async openLayerContextMenu(index: number = 0) { + await testSubjects.click(`lnsLayerSplitButton--${index}`); + }, + async toggleColumnVisibility(dimension: string, no = 1) { await this.openDimensionEditor(dimension); const id = 'lns-table-column-hidden'; From e71845e82e26ba2ad737bc98ee21ed69a6b77f21 Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Mon, 24 Oct 2022 12:49:23 +0200 Subject: [PATCH 17/18] Update x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx Co-authored-by: Michael Marcialis --- .../lens/public/datasources/form_based/layer_settings.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx b/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx index 35d487dcdf56d..9e2ed6d3a86d6 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx @@ -44,7 +44,7 @@ export function LayerSettingsPanel({ label={i18n.translate('xpack.lens.randomSampling.experimentalLabel', { defaultMessage: 'Technical preview', })} - color="subdued" + color="hollow" iconType="beaker" size="s" /> From 45eb2f92a9cc2c8711f0c81ae2d826c15b54c03d Mon Sep 17 00:00:00 2001 From: dej611 Date: Mon, 24 Oct 2022 12:56:13 +0200 Subject: [PATCH 18/18] :ok_hand: Integrated design feedback --- .../datasources/form_based/layer_settings.tsx | 29 ++++++++----------- .../config_panel/dimension_container.scss | 4 --- .../config_panel/flyout_container.tsx | 14 ++++----- 3 files changed, 18 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx b/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx index 9e2ed6d3a86d6..7d02ac98f23a4 100644 --- a/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx +++ b/x-pack/plugins/lens/public/datasources/form_based/layer_settings.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import { EuiFormRow, EuiToolTip, EuiRange, EuiBetaBadge } from '@elastic/eui'; +import { EuiFormRow, EuiRange, EuiBetaBadge } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React from 'react'; import type { DatasourceLayerSettingsProps } from '../../types'; @@ -29,27 +29,22 @@ export function LayerSettingsPanel({ defaultMessage: 'Change the sampling probability to see how your chart is affected', })} label={ - + <> {i18n.translate('xpack.lens.xyChart.randomSampling.label', { defaultMessage: 'Sampling', })}{' '} - - - - + color="hollow" + iconType="beaker" + size="s" + tooltipContent={i18n.translate('xpack.lens.randomSampling.experimentalLabel', { + defaultMessage: 'Technical preview', + })} + /> + } > - - {i18n.translate('xpack.lens.configure.configurePanelTitle', { - defaultMessage: '{groupLabel}', - values: { - groupLabel, - }, - })} - + {i18n.translate('xpack.lens.configure.configurePanelTitle', { + defaultMessage: '{groupLabel}', + values: { + groupLabel, + }, + })}