Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KQL support in filter ratio in TSVB #75033

Merged
merged 17 commits into from
Aug 27, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/plugins/vis_type_timeseries/common/vis_schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,18 @@ export const metricsItems = schema.object({
field: stringOptionalNullable,
id: stringRequired,
metric_agg: stringOptionalNullable,
numerator: stringOptionalNullable,
denominator: stringOptionalNullable,
numerator: schema.maybe(
schema.object({
language: schema.string(),
query: schema.string(),
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a queryObject variable to replace

),
denominator: schema.maybe(
schema.object({
language: schema.string(),
query: schema.string(),
})
),
sigma: stringOptionalNullable,
unit: stringOptionalNullable,
model_type: stringOptionalNullable,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,19 @@ import { FieldSelect } from './field_select';
import { AggRow } from './agg_row';
import { createChangeHandler } from '../lib/create_change_handler';
import { createSelectHandler } from '../lib/create_select_handler';
import { createTextHandler } from '../lib/create_text_handler';
import {
htmlIdGenerator,
EuiFlexGroup,
EuiFlexItem,
EuiFormLabel,
EuiFieldText,
EuiSpacer,
EuiFormRow,
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { KBN_FIELD_TYPES } from '../../../../../../plugins/data/public';
import { getSupportedFieldsByMetricType } from '../lib/get_supported_fields_by_metric_type';
import { getDefaultQueryLanguage } from '../lib/get_default_query_language';
import { QueryBarWrapper } from '../query_bar_wrapper';

const isFieldHistogram = (fields, indexPattern, field) => {
const indexFields = fields[indexPattern];
Expand All @@ -51,13 +51,13 @@ export const FilterRatioAgg = (props) => {

const handleChange = createChangeHandler(props.onChange, props.model);
const handleSelectChange = createSelectHandler(handleChange);
const handleTextChange = createTextHandler(handleChange);
const handleQueryChange = (name, value) => handleChange?.({ [name]: value });
alexwizp marked this conversation as resolved.
Show resolved Hide resolved
const indexPattern =
(series.override_index_pattern && series.series_index_pattern) || panel.index_pattern;

const defaults = {
numerator: '*',
denominator: '*',
numerator: { query: '', language: getDefaultQueryLanguage() },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use data.query.getDefaultQuery() instead?

denominator: { query: '', language: getDefaultQueryLanguage() },
metric_agg: 'count',
};

Expand Down Expand Up @@ -101,7 +101,14 @@ export const FilterRatioAgg = (props) => {
/>
}
>
<EuiFieldText onChange={handleTextChange('numerator')} value={model.numerator} />
<QueryBarWrapper
query={{
alexwizp marked this conversation as resolved.
Show resolved Hide resolved
language: model.numerator.language,
query: model.numerator.query,
}}
onChange={(query) => handleQueryChange('numerator', query)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used to wrap callbacks into useCallback to avoid extra re-rendering.

  const handleNumeratorQueryChange = useCallback((query) => handleChange({ numerator: query }), [
    handleChange,
  ]);

I know this won't give a lot of effect, since the handleChange is always newly created, but you could also wrap it in useMemo. In such a case we'll make a short step into performance optimization =)

indexPatterns={[indexPattern]}
/>
</EuiFormRow>
</EuiFlexItem>

Expand All @@ -115,7 +122,14 @@ export const FilterRatioAgg = (props) => {
/>
}
>
<EuiFieldText onChange={handleTextChange('denominator')} value={model.denominator} />
<QueryBarWrapper
query={{
language: model.denominator.language,
query: model.denominator.query,
}}
onChange={(query) => handleQueryChange('denominator', query)}
indexPatterns={[indexPattern]}
/>
</EuiFormRow>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ import { FilterRatioAgg } from './filter_ratio';
import { FIELDS, METRIC, SERIES, PANEL } from '../../../test_utils';
import { EuiComboBox } from '@elastic/eui';

jest.mock('../query_bar_wrapper', () => ({
QueryBarWrapper: jest.fn(() => null),
}));

jest.mock('../lib/get_default_query_language', () => ({
getDefaultQueryLanguage: jest.fn(() => 'lucene'),
}));

describe('TSVB Filter Ratio', () => {
const setup = (metric) => {
const series = { ...SERIES, metrics: [metric] };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ import { mountWithIntl } from 'test_utils/enzyme_helpers';
import { Agg } from './agg';
import { FieldSelect } from './field_select';
import { FIELDS, METRIC, SERIES, PANEL } from '../../../test_utils';

jest.mock('../query_bar_wrapper', () => ({
QueryBarWrapper: jest.fn(() => null),
}));

jest.mock('../lib/get_default_query_language', () => ({
getDefaultQueryLanguage: jest.fn(() => 'lucene'),
}));

const runTest = (aggType, name, test, additionalProps = {}) => {
describe(aggType, () => {
const metric = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,22 @@
const filter = (metric) => metric.type === 'filter_ratio';
import { bucketTransform } from '../../helpers/bucket_transform';
import { overwrite } from '../../helpers';
import { esQuery } from '../../../../../../data/server';

export function ratios(req, panel, series) {
export function ratios(req, panel, series, esQueryConfig, indexPatternObject) {
return (next) => (doc) => {
if (series.metrics.some(filter)) {
series.metrics.filter(filter).forEach((metric) => {
overwrite(doc, `aggs.${series.id}.aggs.timeseries.aggs.${metric.id}-numerator.filter`, {
query_string: { query: metric.numerator || '*', analyze_wildcard: true },
});
overwrite(doc, `aggs.${series.id}.aggs.timeseries.aggs.${metric.id}-denominator.filter`, {
query_string: { query: metric.denominator || '*', analyze_wildcard: true },
});
overwrite(
doc,
`aggs.${series.id}.aggs.timeseries.aggs.${metric.id}-numerator.filter`,
esQuery.buildEsQuery(indexPatternObject, metric.numerator, [], esQueryConfig)
);
overwrite(
doc,
`aggs.${series.id}.aggs.timeseries.aggs.${metric.id}-denominator.filter`,
esQuery.buildEsQuery(indexPatternObject, metric.denominator, [], esQueryConfig)
);

let numeratorPath = `${metric.id}-numerator>_count`;
let denominatorPath = `${metric.id}-denominator>_count`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@

import { ratios } from './filter_ratios';

describe('ratios(req, panel, series)', () => {
describe('ratios(req, panel, series, esQueryConfig, indexPatternObject)', () => {
let panel;
let series;
let req;
let esQueryConfig;
let indexPatternObject;
beforeEach(() => {
panel = {
time_field: 'timestamp',
Expand All @@ -36,8 +38,8 @@ describe('ratios(req, panel, series)', () => {
{
id: 'metric-1',
type: 'filter_ratio',
numerator: 'errors',
denominator: '*',
numerator: { query: 'errors', language: 'lucene' },
denominator: { query: 'warnings', language: 'lucene' },
metric_agg: 'avg',
field: 'cpu',
},
Expand All @@ -51,17 +53,23 @@ describe('ratios(req, panel, series)', () => {
},
},
};
esQueryConfig = {
allowLeadingWildcards: true,
queryStringOptions: { analyze_wildcard: true },
ignoreFilterIfFieldNotInIndex: false,
};
indexPatternObject = {};
});

test('calls next when finished', () => {
const next = jest.fn();
ratios(req, panel, series)(next)({});
ratios(req, panel, series, esQueryConfig, indexPatternObject)(next)({});
expect(next.mock.calls.length).toEqual(1);
});

test('returns filter ratio aggs', () => {
const next = (doc) => doc;
const doc = ratios(req, panel, series)(next)({});
const doc = ratios(req, panel, series, esQueryConfig, indexPatternObject)(next)({});
expect(doc).toEqual({
aggs: {
test: {
Expand All @@ -88,9 +96,18 @@ describe('ratios(req, panel, series)', () => {
},
},
filter: {
query_string: {
analyze_wildcard: true,
query: '*',
bool: {
must: [
{
query_string: {
query: 'warnings',
analyze_wildcard: true,
},
},
],
filter: [],
should: [],
must_not: [],
},
},
},
Expand All @@ -103,9 +120,18 @@ describe('ratios(req, panel, series)', () => {
},
},
filter: {
query_string: {
analyze_wildcard: true,
query: 'errors',
bool: {
must: [
{
query_string: {
query: 'errors',
analyze_wildcard: true,
},
},
],
filter: [],
should: [],
must_not: [],
},
},
},
Expand All @@ -120,7 +146,7 @@ describe('ratios(req, panel, series)', () => {
test('returns empty object when field is not set', () => {
delete series.metrics[0].field;
const next = (doc) => doc;
const doc = ratios(req, panel, series)(next)({});
const doc = ratios(req, panel, series, esQueryConfig, indexPatternObject)(next)({});
expect(doc).toEqual({
aggs: {
test: {
Expand All @@ -141,18 +167,36 @@ describe('ratios(req, panel, series)', () => {
'metric-1-denominator': {
aggs: { metric: {} },
filter: {
query_string: {
analyze_wildcard: true,
query: '*',
bool: {
must: [
{
query_string: {
query: 'warnings',
analyze_wildcard: true,
},
},
],
filter: [],
should: [],
must_not: [],
},
},
},
'metric-1-numerator': {
aggs: { metric: {} },
filter: {
query_string: {
analyze_wildcard: true,
query: 'errors',
bool: {
must: [
{
query_string: {
analyze_wildcard: true,
query: 'errors',
},
},
],
filter: [],
should: [],
must_not: [],
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1544,4 +1544,38 @@ describe('migration visualization', () => {
expect(series[0].split_color_mode).toBeUndefined();
});
});

describe('7.10.0 tsvb filter_ratio migration', () => {
const migrate = (doc: any) =>
visualizationSavedObjectTypeMigrations['7.10.0'](
doc as Parameters<SavedObjectMigrationFn>[0],
savedObjectMigrationContext
);

const testDoc1 = {
attributes: {
title: 'My Vis',
description: 'This is my super cool vis.',
visState: `{"type":"metrics","params":{"id":"61ca57f0-469d-11e7-af02-69e470af7417","type":"timeseries",
"series":[{"id":"61ca57f1-469d-11e7-af02-69e470af7417","metrics":[{"id":"61ca57f2-469d-11e7-af02-69e470af7417",
"type":"filter_ratio","numerator":"Filter Bytes Test:>1000","denominator":"Filter Bytes Test:<1000"}]}]}}`,
},
};

it('should replace numerator string with a query object', () => {
const migratedTestDoc1 = migrate(testDoc1);
const metric = JSON.parse(migratedTestDoc1.attributes.visState).params.series[0].metrics[0];

expect(metric.numerator).toHaveProperty('query');
expect(metric.numerator).toHaveProperty('language');
});

it('should replace denominator string with a query object', () => {
const migratedTestDoc1 = migrate(testDoc1);
const metric = JSON.parse(migratedTestDoc1.attributes.visState).params.series[0].metrics[0];

expect(metric.denominator).toHaveProperty('query');
expect(metric.denominator).toHaveProperty('language');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,45 @@ const migratePercentileRankAggregation: SavedObjectMigrationFn<any, any> = (doc)
return doc;
};

// [TSVB] Replace string query with object
const migrateFilterRatioQuery: SavedObjectMigrationFn<any, any> = (doc) => {
const visStateJSON = get(doc, 'attributes.visState');
let visState;

if (visStateJSON) {
try {
visState = JSON.parse(visStateJSON);
} catch (e) {
// Let it go, the data is invalid and we'll leave it as is
}
if (visState && visState.type === 'metrics') {
const series: any[] = get(visState, 'params.series') || [];
Copy link
Contributor

@stratoula stratoula Aug 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we can't avoid these anys here, right? 😢

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not wrong it's SeriesItemsSchema from src/plugins/vis_type_timeseries/common/types.ts. But not sure that SeriesItemsSchema was a good name for that

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... sorry, it's a migration script. I'm ok to see any here cause otherwise we should create a new type for each migration.


series.forEach((part) => {
(part.metrics || []).forEach((metric: any) => {
if (metric.type === 'filter_ratio') {
if (metric.numerator && typeof metric.numerator === 'string') {
alexwizp marked this conversation as resolved.
Show resolved Hide resolved
metric.numerator = { query: metric.numerator, language: 'lucene' };
}
if (metric.denominator && typeof metric.denominator === 'string') {
metric.denominator = { query: metric.denominator, language: 'lucene' };
}
}
});
});

return {
...doc,
attributes: {
...doc.attributes,
visState: JSON.stringify(visState),
},
};
}
}
return doc;
};

// [TSVB] Remove stale opperator key
const migrateOperatorKeyTypo: SavedObjectMigrationFn<any, any> = (doc) => {
const visStateJSON = get(doc, 'attributes.visState');
Expand Down Expand Up @@ -713,4 +752,5 @@ export const visualizationSavedObjectTypeMigrations = {
'7.4.2': flow(transformSplitFiltersStringToQueryObject),
'7.7.0': flow(migrateOperatorKeyTypo, migrateSplitByChartRow),
'7.8.0': flow(migrateTsvbDefaultColorPalettes),
'7.10.0': flow(migrateFilterRatioQuery),
};