[TSVB] metrics:max_buckets setting should limit buckets on server side too. (#95639)

* tmp

* [TSVB] Remove metrics:max_buckets setting because it is redundant to histogram:maxBars

Closes: #94212

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Alexey Antonov 2021-03-31 12:14:56 +03:00 committed by GitHub
parent ddac0e9501
commit ede6b4fd64
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 157 additions and 89 deletions

View file

@ -6,12 +6,20 @@
* Side Public License, v 1.
*/
import { GTE_INTERVAL_RE } from '../../../common/interval_regexp';
import { i18n } from '@kbn/i18n';
import { GTE_INTERVAL_RE } from '../../../common/interval_regexp';
import { search } from '../../../../../plugins/data/public';
import type { TimeRangeBounds } from '../../../../data/common';
import type { TimeseriesVisParams } from '../../types';
const { parseInterval } = search.aggs;
export function validateInterval(bounds, panel, maxBuckets) {
export function validateInterval(
bounds: TimeRangeBounds,
panel: TimeseriesVisParams,
maxBuckets: number
) {
const { interval } = panel;
const { min, max } = bounds;
// No need to check auto it will return around 100
@ -20,8 +28,9 @@ export function validateInterval(bounds, panel, maxBuckets) {
const greaterThanMatch = interval.match(GTE_INTERVAL_RE);
if (greaterThanMatch) return;
const duration = parseInterval(interval);
if (duration) {
const span = max.valueOf() - min.valueOf();
const span = max!.valueOf() - min!.valueOf();
const buckets = Math.floor(span / duration.asMilliseconds());
if (buckets > maxBuckets) {
throw new Error(

View file

@ -28,14 +28,15 @@ export const metricsRequestHandler = async ({
searchSessionId,
}: MetricsRequestHandlerParams): Promise<TimeseriesVisData | {}> => {
const config = getUISettings();
const data = getDataStart();
const timezone = getTimezone(config);
const uiStateObj = uiState[visParams.type] ?? {};
const data = getDataStart();
const dataSearch = getDataStart().search;
const dataSearch = data.search;
const parsedTimeRange = data.query.timefilter.timefilter.calculateBounds(input?.timeRange!);
if (visParams && visParams.id && !visParams.isModelInvalid) {
const maxBuckets = config.get(MAX_BUCKETS_SETTING);
const maxBuckets = config.get<number>(MAX_BUCKETS_SETTING);
validateInterval(parsedTimeRange, visParams, maxBuckets);

View file

@ -7,20 +7,19 @@
*/
import { DefaultSearchCapabilities } from './default_search_capabilities';
import { VisTypeTimeseriesRequest } from '../../../types';
describe('DefaultSearchCapabilities', () => {
let defaultSearchCapabilities: DefaultSearchCapabilities;
let req: VisTypeTimeseriesRequest;
beforeEach(() => {
req = {} as VisTypeTimeseriesRequest;
defaultSearchCapabilities = new DefaultSearchCapabilities(req);
defaultSearchCapabilities = new DefaultSearchCapabilities({
timezone: 'UTC',
maxBucketsLimit: 2000,
});
});
test('should init default search capabilities', () => {
expect(defaultSearchCapabilities.request).toBe(req);
expect(defaultSearchCapabilities.fieldsCapabilities).toEqual({});
expect(defaultSearchCapabilities.timezone).toBe('UTC');
});
test('should return defaultTimeInterval', () => {
@ -35,18 +34,6 @@ describe('DefaultSearchCapabilities', () => {
});
});
test('should return Search Timezone', () => {
defaultSearchCapabilities.request = ({
body: {
timerange: {
timezone: 'UTC',
},
},
} as unknown) as VisTypeTimeseriesRequest;
expect(defaultSearchCapabilities.searchTimezone).toEqual('UTC');
});
test('should return a valid time interval', () => {
expect(defaultSearchCapabilities.getValidTimeInterval('20m')).toBe('20m');
});

View file

@ -13,23 +13,20 @@ import {
getSuitableUnit,
} from '../../vis_data/helpers/unit_to_seconds';
import { RESTRICTIONS_KEYS } from '../../../../common/ui_restrictions';
import { VisTypeTimeseriesRequest, VisTypeTimeseriesVisDataRequest } from '../../../types';
const isVisDataRequest = (
request: VisTypeTimeseriesRequest
): request is VisTypeTimeseriesVisDataRequest => {
return !!(request as VisTypeTimeseriesVisDataRequest).body;
};
const getTimezoneFromRequest = (request: VisTypeTimeseriesRequest) => {
if (isVisDataRequest(request)) return request.body.timerange.timezone;
};
export interface SearchCapabilitiesOptions {
timezone?: string;
maxBucketsLimit: number;
}
export class DefaultSearchCapabilities {
constructor(
public request: VisTypeTimeseriesRequest,
public fieldsCapabilities: Record<string, any> = {}
) {}
public timezone: SearchCapabilitiesOptions['timezone'];
public maxBucketsLimit: SearchCapabilitiesOptions['maxBucketsLimit'];
constructor(options: SearchCapabilitiesOptions) {
this.timezone = options.timezone;
this.maxBucketsLimit = options.maxBucketsLimit;
}
public get defaultTimeInterval() {
return null;
@ -55,10 +52,6 @@ export class DefaultSearchCapabilities {
};
}
public get searchTimezone() {
return getTimezoneFromRequest(this.request);
}
createUiRestriction(restrictionsObject?: Record<string, any>) {
return {
'*': !restrictionsObject,

View file

@ -8,13 +8,11 @@
import { Unit } from '@elastic/datemath';
import { RollupSearchCapabilities } from './rollup_search_capabilities';
import { VisTypeTimeseriesRequest } from '../../../types';
describe('Rollup Search Capabilities', () => {
const testTimeZone = 'time_zone';
const testInterval = '10s';
const rollupIndex = 'rollupIndex';
const request = ({} as unknown) as VisTypeTimeseriesRequest;
let fieldsCapabilities: Record<string, any>;
let rollupSearchCaps: RollupSearchCapabilities;
@ -33,16 +31,19 @@ describe('Rollup Search Capabilities', () => {
},
};
rollupSearchCaps = new RollupSearchCapabilities(request, fieldsCapabilities, rollupIndex);
rollupSearchCaps = new RollupSearchCapabilities(
{ maxBucketsLimit: 2000, timezone: 'UTC' },
fieldsCapabilities,
rollupIndex
);
});
test('should create instance of RollupSearchRequest', () => {
expect(rollupSearchCaps.fieldsCapabilities).toBe(fieldsCapabilities);
expect(rollupSearchCaps.rollupIndex).toBe(rollupIndex);
});
test('should return the "timezone" for the rollup request', () => {
expect(rollupSearchCaps.searchTimezone).toBe(testTimeZone);
expect(rollupSearchCaps.timezone).toBe(testTimeZone);
});
test('should return the default "interval" for the rollup request', () => {

View file

@ -5,26 +5,28 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { get, has } from 'lodash';
import { leastCommonInterval, isCalendarInterval } from '../lib/interval_helper';
import { DefaultSearchCapabilities } from './default_search_capabilities';
import { VisTypeTimeseriesRequest } from '../../../types';
import {
DefaultSearchCapabilities,
SearchCapabilitiesOptions,
} from './default_search_capabilities';
export class RollupSearchCapabilities extends DefaultSearchCapabilities {
rollupIndex: string;
availableMetrics: Record<string, any>;
constructor(
req: VisTypeTimeseriesRequest,
options: SearchCapabilitiesOptions,
fieldsCapabilities: Record<string, any>,
rollupIndex: string
) {
super(req, fieldsCapabilities);
super(options);
this.rollupIndex = rollupIndex;
this.availableMetrics = get(fieldsCapabilities, `${rollupIndex}.aggs`, {});
this.timezone = get(this.dateHistogram, 'time_zone', null);
}
public get dateHistogram() {
@ -46,10 +48,6 @@ export class RollupSearchCapabilities extends DefaultSearchCapabilities {
);
}
public get searchTimezone() {
return get(this.dateHistogram, 'time_zone', null);
}
public get whiteListedMetrics() {
const baseRestrictions = this.createUiRestriction({
count: this.createUiRestriction(),

View file

@ -25,7 +25,15 @@ class MockSearchStrategy extends AbstractSearchStrategy {
}
describe('SearchStrategyRegister', () => {
const requestContext = {} as VisTypeTimeseriesRequestHandlerContext;
const requestContext = ({
core: {
uiSettings: {
client: {
get: jest.fn(),
},
},
},
} as unknown) as VisTypeTimeseriesRequestHandlerContext;
let registry: SearchStrategyRegistry;
beforeAll(() => {
@ -44,7 +52,7 @@ describe('SearchStrategyRegister', () => {
});
test('should return a DefaultSearchStrategy instance', async () => {
const req = {} as VisTypeTimeseriesRequest;
const req = { body: {} } as VisTypeTimeseriesRequest;
const { searchStrategy, capabilities } = (await registry.getViableStrategy(
requestContext,
@ -65,7 +73,7 @@ describe('SearchStrategyRegister', () => {
});
test('should return a MockSearchStrategy instance', async () => {
const req = {} as VisTypeTimeseriesRequest;
const req = { body: {} } as VisTypeTimeseriesRequest;
const anotherSearchStrategy = new MockSearchStrategy();
registry.addStrategy(anotherSearchStrategy);

View file

@ -33,6 +33,9 @@ describe('AbstractSearchStrategy', () => {
asCurrentUser: jest.fn(),
},
},
uiSettings: {
client: jest.fn(),
},
},
search: {
search: jest.fn().mockReturnValue(from(Promise.resolve({}))),

View file

@ -13,12 +13,23 @@ import {
import { DefaultSearchStrategy } from './default_search_strategy';
describe('DefaultSearchStrategy', () => {
const requestContext = {} as VisTypeTimeseriesRequestHandlerContext;
const requestContext = ({
core: {
uiSettings: {
client: {
get: jest.fn(),
},
},
},
} as unknown) as VisTypeTimeseriesRequestHandlerContext;
let defaultSearchStrategy: DefaultSearchStrategy;
let req: VisTypeTimeseriesVisDataRequest;
beforeEach(() => {
req = {} as VisTypeTimeseriesVisDataRequest;
req = {
body: {},
} as VisTypeTimeseriesVisDataRequest;
defaultSearchStrategy = new DefaultSearchStrategy();
});
@ -32,9 +43,11 @@ describe('DefaultSearchStrategy', () => {
const value = await defaultSearchStrategy.checkForViability(requestContext, req);
expect(value.isViable).toBe(true);
expect(value.capabilities).toEqual({
request: req,
fieldsCapabilities: {},
});
expect(value.capabilities).toMatchInlineSnapshot(`
DefaultSearchCapabilities {
"maxBucketsLimit": undefined,
"timezone": undefined,
}
`);
});
});

View file

@ -15,15 +15,21 @@ import type {
VisTypeTimeseriesRequestHandlerContext,
VisTypeTimeseriesRequest,
} from '../../../types';
import { MAX_BUCKETS_SETTING } from '../../../../common/constants';
export class DefaultSearchStrategy extends AbstractSearchStrategy {
async checkForViability(
requestContext: VisTypeTimeseriesRequestHandlerContext,
req: VisTypeTimeseriesRequest
) {
const uiSettings = requestContext.core.uiSettings.client;
return {
isViable: true,
capabilities: new DefaultSearchCapabilities(req),
capabilities: new DefaultSearchCapabilities({
timezone: req.body.timerange?.timezone,
maxBucketsLimit: await uiSettings.get(MAX_BUCKETS_SETTING),
}),
};
}

View file

@ -17,6 +17,7 @@ import type {
VisTypeTimeseriesRequestHandlerContext,
VisTypeTimeseriesVisDataRequest,
} from '../../../types';
import { MAX_BUCKETS_SETTING } from '../../../../common/constants';
const getRollupIndices = (rollupData: { [key: string]: any }) => Object.keys(rollupData);
const isIndexPatternContainsWildcard = (indexPattern: string) => indexPattern.includes('*');
@ -62,6 +63,7 @@ export class RollupSearchStrategy extends AbstractSearchStrategy {
) {
const rollupData = await this.getRollupData(requestContext, indexPatternString);
const rollupIndices = getRollupIndices(rollupData);
const uiSettings = requestContext.core.uiSettings.client;
isViable = rollupIndices.length === 1;
@ -69,7 +71,13 @@ export class RollupSearchStrategy extends AbstractSearchStrategy {
const [rollupIndex] = rollupIndices;
const fieldsCapabilities = getCapabilitiesForRollupIndices(rollupData);
capabilities = new RollupSearchCapabilities(req, fieldsCapabilities, rollupIndex);
capabilities = new RollupSearchCapabilities(
{
maxBucketsLimit: await uiSettings.get(MAX_BUCKETS_SETTING),
},
fieldsCapabilities,
rollupIndex
);
}
}

View file

@ -30,6 +30,10 @@ const calculateBucketData = (timeInterval, capabilities) => {
bucketSize = 1;
}
if (bucketSize > capabilities.maxBucketsLimit) {
bucketSize = capabilities.maxBucketsLimit;
}
// Check decimal
if (parsedInterval && parsedInterval.value % 1 !== 0) {
if (parsedInterval.unit !== 'ms') {
@ -61,16 +65,16 @@ const calculateBucketSizeForAutoInterval = (req, maxBars) => {
return search.aggs.calcAutoIntervalLessThan(maxBars, timerange).asSeconds();
};
export const getBucketSize = (req, interval, capabilities, maxBars) => {
const bucketSize = calculateBucketSizeForAutoInterval(req, maxBars);
let intervalString = `${bucketSize}s`;
export const getBucketSize = (req, interval, capabilities, bars) => {
const defaultBucketSize = calculateBucketSizeForAutoInterval(req, bars);
let intervalString = `${defaultBucketSize}s`;
const gteAutoMatch = Boolean(interval) && interval.match(GTE_INTERVAL_RE);
if (gteAutoMatch) {
const bucketData = calculateBucketData(gteAutoMatch[1], capabilities);
if (bucketData.bucketSize >= bucketSize) {
if (bucketData.bucketSize >= defaultBucketSize) {
return bucketData;
}
}

View file

@ -18,43 +18,49 @@ describe('getBucketSize', () => {
},
};
const capabilities = {
timezone: 'UTC',
maxBucketsLimit: 200000,
getValidTimeInterval: jest.fn((v) => v),
};
test('returns auto calculated buckets', () => {
const result = getBucketSize(req, 'auto', undefined, 100);
const result = getBucketSize(req, 'auto', capabilities, 100);
expect(result).toHaveProperty('bucketSize', 30);
expect(result).toHaveProperty('intervalString', '30s');
});
test('returns overridden buckets (1s)', () => {
const result = getBucketSize(req, '1s', undefined, 100);
const result = getBucketSize(req, '1s', capabilities, 100);
expect(result).toHaveProperty('bucketSize', 1);
expect(result).toHaveProperty('intervalString', '1s');
});
test('returns overridden buckets (10m)', () => {
const result = getBucketSize(req, '10m', undefined, 100);
const result = getBucketSize(req, '10m', capabilities, 100);
expect(result).toHaveProperty('bucketSize', 600);
expect(result).toHaveProperty('intervalString', '10m');
});
test('returns overridden buckets (1d)', () => {
const result = getBucketSize(req, '1d', undefined, 100);
const result = getBucketSize(req, '1d', capabilities, 100);
expect(result).toHaveProperty('bucketSize', 86400);
expect(result).toHaveProperty('intervalString', '1d');
});
test('returns overridden buckets (>=2d)', () => {
const result = getBucketSize(req, '>=2d', undefined, 100);
const result = getBucketSize(req, '>=2d', capabilities, 100);
expect(result).toHaveProperty('bucketSize', 86400 * 2);
expect(result).toHaveProperty('intervalString', '2d');
});
test('returns overridden buckets (>=10s)', () => {
const result = getBucketSize(req, '>=10s', undefined, 100);
const result = getBucketSize(req, '>=10s', capabilities, 100);
expect(result).toHaveProperty('bucketSize', 30);
expect(result).toHaveProperty('intervalString', '30s');

View file

@ -32,7 +32,7 @@ export function dateHistogram(
barTargetUiSettings
);
const { from, to } = getTimerange(req);
const timezone = capabilities.searchTimezone;
const { timezone } = capabilities;
overwrite(doc, `aggs.${annotation.id}.date_histogram`, {
field: timeField,

View file

@ -37,7 +37,7 @@ export function dateHistogram(
const getDateHistogramForLastBucketMode = () => {
const { from, to } = offsetTime(req, series.offset_time);
const timezone = capabilities.searchTimezone;
const { timezone } = capabilities;
overwrite(doc, `aggs.${series.id}.aggs.timeseries.date_histogram`, {
field: timeField,

View file

@ -23,7 +23,6 @@ describe('dateHistogram(req, panel, series)', () => {
req = {
body: {
timerange: {
timezone: 'UTC',
min: '2017-01-01T00:00:00Z',
max: '2017-01-01T01:00:00Z',
},
@ -40,7 +39,7 @@ describe('dateHistogram(req, panel, series)', () => {
queryStringOptions: {},
};
indexPattern = {};
capabilities = new DefaultSearchCapabilities(req);
capabilities = new DefaultSearchCapabilities({ timezone: 'UTC', maxBucketsLimit: 2000 });
uiSettings = {
get: async (key) => (key === UI_SETTINGS.HISTOGRAM_MAX_BARS ? 100 : 50),
};

View file

@ -50,7 +50,7 @@ describe('metricBuckets(req, panel, series)', () => {
},
{},
{},
undefined,
{ maxBucketsLimit: 2000, getValidTimeInterval: jest.fn(() => '1d') },
{
get: async () => 50,
}

View file

@ -46,14 +46,30 @@ describe('positiveRate(req, panel, series)', () => {
test('calls next when finished', async () => {
const next = jest.fn();
await positiveRate(req, panel, series, {}, {}, undefined, uiSettings)(next)({});
await positiveRate(
req,
panel,
series,
{},
{},
{ maxBucketsLimit: 2000, getValidTimeInterval: jest.fn(() => '1d') },
uiSettings
)(next)({});
expect(next.mock.calls.length).toEqual(1);
});
test('returns positive rate aggs', async () => {
const next = (doc) => doc;
const doc = await positiveRate(req, panel, series, {}, {}, undefined, uiSettings)(next)({});
const doc = await positiveRate(
req,
panel,
series,
{},
{},
{ maxBucketsLimit: 2000, getValidTimeInterval: jest.fn(() => '1d') },
uiSettings
)(next)({});
expect(doc).toEqual({
aggs: {

View file

@ -51,13 +51,29 @@ describe('siblingBuckets(req, panel, series)', () => {
test('calls next when finished', async () => {
const next = jest.fn();
await siblingBuckets(req, panel, series, {}, {}, undefined, uiSettings)(next)({});
await siblingBuckets(
req,
panel,
series,
{},
{},
{ maxBucketsLimit: 2000, getValidTimeInterval: jest.fn(() => '1d') },
uiSettings
)(next)({});
expect(next.mock.calls.length).toEqual(1);
});
test('returns sibling aggs', async () => {
const next = (doc) => doc;
const doc = await siblingBuckets(req, panel, series, {}, {}, undefined, uiSettings)(next)({});
const doc = await siblingBuckets(
req,
panel,
series,
{},
{},
{ maxBucketsLimit: 2000, getValidTimeInterval: jest.fn(() => '1d') },
uiSettings
)(next)({});
expect(doc).toEqual({
aggs: {

View file

@ -32,7 +32,7 @@ export function dateHistogram(req, panel, esQueryConfig, indexPattern, capabilit
barTargetUiSettings
);
const { from, to } = getTimerange(req);
const timezone = capabilities.searchTimezone;
const { timezone } = capabilities;
panel.series.forEach((column) => {
const aggRoot = calculateAggRoot(doc, column);

View file

@ -72,7 +72,7 @@ describe('buildRequestBody(req)', () => {
const series = panel.series[0];
const getValidTimeInterval = jest.fn(() => '10s');
const capabilities = {
searchTimezone: 'UTC',
timezone: 'UTC',
getValidTimeInterval,
};
const config = {

View file

@ -23,7 +23,7 @@ export type ConfigObservable = Observable<SharedGlobalConfig>;
export type VisTypeTimeseriesRequestHandlerContext = DataRequestHandlerContext;
export type VisTypeTimeseriesRouter = IRouter<VisTypeTimeseriesRequestHandlerContext>;
export type VisTypeTimeseriesVisDataRequest = KibanaRequest<{}, {}, VisPayload>;
export type VisTypeTimeseriesFieldsRequest = KibanaRequest<{}, { index: string }, {}>;
export type VisTypeTimeseriesFieldsRequest = KibanaRequest<{}, { index: string }, any>;
export type VisTypeTimeseriesRequest =
| VisTypeTimeseriesFieldsRequest
| VisTypeTimeseriesVisDataRequest;