Skip to content

Commit

Permalink
[TSVB] Remove metrics:max_buckets setting because it is redundant to …
Browse files Browse the repository at this point in the history
…histogram:maxBars

Closes: elastic#94212
  • Loading branch information
alexwizp committed Mar 29, 2021
1 parent ca9ff9e commit 23f7d6f
Show file tree
Hide file tree
Showing 20 changed files with 142 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -55,10 +52,6 @@ export class DefaultSearchCapabilities {
};
}

public get searchTimezone() {
return getTimezoneFromRequest(this.request);
}

createUiRestriction(restrictionsObject?: Record<string, any>) {
return {
'*': !restrictionsObject,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -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,
Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ describe('AbstractSearchStrategy', () => {
asCurrentUser: jest.fn(),
},
},
uiSettings: {
client: jest.fn(),
},
},
search: {
search: jest.fn().mockReturnValue(from(Promise.resolve({}))),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand All @@ -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,
}
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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('*');
Expand Down Expand Up @@ -62,14 +63,21 @@ 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;

if (isViable) {
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
);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down Expand Up @@ -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;
}
}
Expand Down
Loading

0 comments on commit 23f7d6f

Please sign in to comment.