Skip to content

Commit

Permalink
[Lens] Avoid to compute over time suggestion for expensive configurat…
Browse files Browse the repository at this point in the history
…ions (#115932) (#123022)

* ⚡ Avoid over time suggestion if too expensive

* ✅ Add unit tests

* 👌 Integrate feedback

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit eb9262d)

Co-authored-by: Marco Liberati <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
3 people authored Jan 14, 2022
1 parent ae2052d commit 2ba1530
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1952,6 +1952,64 @@ describe('IndexPattern Data Source suggestions', () => {
suggestions.forEach((suggestion) => expect(suggestion.table.columns.length).toBe(1));
});

it("should not propose an over time suggestion if there's a top values aggregation with an high size", () => {
const initialState = testInitialState();
(initialState.layers.first.columns.col1 as { params: { size: number } }).params!.size = 6;
const suggestions = getDatasourceSuggestionsFromCurrentState({
...initialState,
indexPatterns: { 1: { ...initialState.indexPatterns['1'], timeFieldName: undefined } },
});
suggestions.forEach((suggestion) => expect(suggestion.table.columns.length).toBe(1));
});

it('should not propose an over time suggestion if there are multiple bucket dimensions', () => {
const initialState = testInitialState();
const state: IndexPatternPrivateState = {
...initialState,
layers: {
first: {
indexPatternId: '1',
columnOrder: ['col1', 'col2', 'col3'],
columns: {
...initialState.layers.first.columns,
col2: {
label: 'My Op',
customLabel: true,
dataType: 'number',
isBucketed: false,
operationType: 'average',
sourceField: 'bytes',
scale: 'ratio',
},
col3: {
label: 'My Op',
customLabel: true,
dataType: 'string',
isBucketed: true,

// Private
operationType: 'terms',
sourceField: 'dest',
params: {
size: 5,
orderBy: { type: 'alphabetical' },
orderDirection: 'asc',
},
},
},
},
},
};
const suggestions = getDatasourceSuggestionsFromCurrentState({
...state,
indexPatterns: { 1: { ...state.indexPatterns['1'], timeFieldName: undefined } },
});
suggestions.forEach((suggestion) => {
const firstBucket = suggestion.table.columns.find(({ columnId }) => columnId === 'col1');
expect(firstBucket?.operation).not.toBe('date');
});
});

it('returns simplified versions of table with more than 2 columns', () => {
const initialState = testInitialState();
const fields = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
getExistingColumnGroups,
isReferenced,
getReferencedColumnIds,
hasTermsWithManyBuckets,
} from './operations';
import { hasField } from './utils';
import type {
Expand Down Expand Up @@ -424,7 +425,7 @@ export function getDatasourceSuggestionsFromCurrentState(
);

if (!references.length && metrics.length && buckets.length === 0) {
if (timeField) {
if (timeField && buckets.length < 1 && !hasTermsWithManyBuckets(layer)) {
// suggest current metric over time if there is a default time field
suggestions.push(createSuggestionWithDefaultDateHistogram(state, layerId, timeField));
}
Expand All @@ -436,7 +437,13 @@ export function getDatasourceSuggestionsFromCurrentState(

// base range intervals are of number dataType.
// Custom range/intervals have a different dataType so they still receive the Over Time suggestion
if (!timeDimension && timeField && !hasNumericDimension) {
if (
!timeDimension &&
timeField &&
buckets.length < 2 &&
!hasNumericDimension &&
!hasTermsWithManyBuckets(layer)
) {
// suggest current configuration over time if there is a default time field
// and no time dimension yet
suggestions.push(createSuggestionWithDefaultDateHistogram(state, layerId, timeField));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
deleteColumn,
updateLayerIndexPattern,
getErrorMessages,
hasTermsWithManyBuckets,
} from './layer_helpers';
import { operationDefinitionMap, OperationType } from '../operations';
import { TermsIndexPatternColumn } from './definitions/terms';
Expand Down Expand Up @@ -3005,4 +3006,79 @@ describe('state_helpers', () => {
);
});
});

describe('hasTermsWithManyBuckets', () => {
it('should return false for a bucketed non terms operation', () => {
const layer: IndexPatternLayer = {
columnOrder: ['col1'],
columns: {
col1: {
dataType: 'date',
isBucketed: true,
label: '',
operationType: 'date_histogram',
sourceField: 'fieldD',
params: {
interval: 'd',
},
},
},
indexPatternId: 'original',
};

expect(hasTermsWithManyBuckets(layer)).toBeFalsy();
});

it('should return false if all terms operation have a lower size', () => {
const layer: IndexPatternLayer = {
columnOrder: ['col1'],
columns: {
col1: {
label: 'My Op',
customLabel: true,
dataType: 'string',
isBucketed: true,

// Private
operationType: 'terms',
sourceField: 'dest',
params: {
size: 5,
orderBy: { type: 'alphabetical' },
orderDirection: 'asc',
},
},
},
indexPatternId: 'original',
};

expect(hasTermsWithManyBuckets(layer)).toBeFalsy();
});

it('should return true if the size is high', () => {
const layer: IndexPatternLayer = {
columnOrder: ['col1'],
columns: {
col1: {
label: 'My Op',
customLabel: true,
dataType: 'string',
isBucketed: true,

// Private
operationType: 'terms',
sourceField: 'dest',
params: {
size: 15,
orderBy: { type: 'alphabetical' },
orderDirection: 'asc',
},
},
},
indexPatternId: 'original',
};

expect(hasTermsWithManyBuckets(layer)).toBeTruthy();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,17 @@ export function getReferencedColumnIds(layer: IndexPatternLayer, columnId: strin
return referencedIds;
}

export function hasTermsWithManyBuckets(layer: IndexPatternLayer): boolean {
return layer.columnOrder.some((columnId) => {
const column = layer.columns[columnId];
if (column) {
return (
column.isBucketed && column.params && 'size' in column.params && column.params.size > 5
);
}
});
}

export function isOperationAllowedAsReference({
operationType,
validation,
Expand Down

0 comments on commit 2ba1530

Please sign in to comment.