From 83b47c1bc81e9dd5f49b2f093a902f49455a6134 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Thu, 3 Jun 2021 12:37:45 +0300 Subject: [PATCH] [TSVB] Math params._interval is incorrect when using entire timerange mode (#100775) * [TSVB] Math params._interval is incorrect when using entire timerange mode Closes: #100615 * fix jest * rename get -> overwrite * apply fix for "bucket script" * Update date_histogram.js Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../lib/vis_data/helpers/bucket_transform.js | 19 ++++++--- .../series/date_histogram.js | 35 +++++++++------- .../series/date_histogram.test.js | 42 +++++++++++++++---- .../series/metric_buckets.js | 21 ++-------- .../series/sibling_buckets.js | 23 +++------- .../table/date_histogram.js | 25 ++++++----- .../table/metric_buckets.js | 12 ++---- .../table/sibling_buckets.js | 14 +++---- .../response_processors/series/math.js | 6 ++- .../response_processors/series/math.test.js | 21 +++++++++- .../series/build_request_body.test.ts | 1 - 11 files changed, 124 insertions(+), 95 deletions(-) diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/helpers/bucket_transform.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/helpers/bucket_transform.js index 2877373ffba9a..16e7b9d6072cb 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/helpers/bucket_transform.js +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/helpers/bucket_transform.js @@ -7,11 +7,11 @@ */ import { getBucketsPath } from './get_buckets_path'; -import { parseInterval } from './parse_interval'; import { set } from '@elastic/safer-lodash-set'; import { isEmpty } from 'lodash'; import { i18n } from '@kbn/i18n'; import { MODEL_SCRIPTS } from './moving_fn_scripts'; +import { convertIntervalToUnit } from './unit_to_seconds'; function checkMetric(metric, fields) { fields.forEach((field) => { @@ -161,19 +161,24 @@ export const bucketTransform = { }; }, - derivative: (bucket, metrics, bucketSize) => { + derivative: (bucket, metrics, intervalString) => { checkMetric(bucket, ['type', 'field']); + const body = { derivative: { buckets_path: getBucketsPath(bucket.field, metrics), gap_policy: 'skip', // seems sane - unit: bucketSize, + unit: intervalString, }, }; + if (bucket.gap_policy) body.derivative.gap_policy = bucket.gap_policy; if (bucket.unit) { - body.derivative.unit = /^([\d]+)([shmdwMy]|ms)$/.test(bucket.unit) ? bucket.unit : bucketSize; + body.derivative.unit = /^([\d]+)([shmdwMy]|ms)$/.test(bucket.unit) + ? bucket.unit + : intervalString; } + return body; }, @@ -214,8 +219,10 @@ export const bucketTransform = { }; }, - calculation: (bucket, metrics, bucketSize) => { + calculation: (bucket, metrics, intervalString) => { checkMetric(bucket, ['variables', 'script']); + const inMsInterval = convertIntervalToUnit(intervalString, 'ms'); + const body = { bucket_script: { buckets_path: bucket.variables.reduce((acc, row) => { @@ -226,7 +233,7 @@ export const bucketTransform = { source: bucket.script, lang: 'painless', params: { - _interval: parseInterval(bucketSize).asMilliseconds(), + _interval: inMsInterval?.value, }, }, gap_policy: 'skip', // seems sane diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/date_histogram.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/date_histogram.js index f82f332df19fd..253612c0274ad 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/date_histogram.js +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/date_histogram.js @@ -29,17 +29,20 @@ export function dateHistogram( const barTargetUiSettings = await uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET); const { timeField, interval, maxBars } = getIntervalAndTimefield(panel, series, seriesIndex); - const { bucketSize, intervalString } = getBucketSize( - req, - interval, - capabilities, - maxBars ? Math.min(maxBarsUiSettings, maxBars) : barTargetUiSettings - ); + const { from, to } = offsetTime(req, series.offset_time); - const getDateHistogramForLastBucketMode = () => { - const { from, to } = offsetTime(req, series.offset_time); + let bucketInterval; + + const overwriteDateHistogramForLastBucketMode = () => { const { timezone } = capabilities; + const { intervalString } = getBucketSize( + req, + interval, + capabilities, + maxBars ? Math.min(maxBarsUiSettings, maxBars) : barTargetUiSettings + ); + overwrite(doc, `aggs.${series.id}.aggs.timeseries.date_histogram`, { field: timeField, min_doc_count: 0, @@ -50,25 +53,29 @@ export function dateHistogram( }, ...dateHistogramInterval(intervalString), }); + + bucketInterval = intervalString; }; - const getDateHistogramForEntireTimerangeMode = () => + const overwriteDateHistogramForEntireTimerangeMode = () => { overwrite(doc, `aggs.${series.id}.aggs.timeseries.auto_date_histogram`, { field: timeField, buckets: 1, }); + bucketInterval = `${to.valueOf() - from.valueOf()}ms`; + }; + isLastValueTimerangeMode(panel, series) - ? getDateHistogramForLastBucketMode() - : getDateHistogramForEntireTimerangeMode(); + ? overwriteDateHistogramForLastBucketMode() + : overwriteDateHistogramForEntireTimerangeMode(); overwrite(doc, `aggs.${series.id}.meta`, { timeField, - intervalString, - bucketSize, + panelId: panel.id, seriesId: series.id, + intervalString: bucketInterval, index: panel.use_kibana_indexes ? seriesIndex.indexPattern?.id : undefined, - panelId: panel.id, }); return next(doc); diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/date_histogram.test.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/date_histogram.test.js index 741eb93267f4c..2cd7a213b273e 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/date_histogram.test.js +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/date_histogram.test.js @@ -86,7 +86,6 @@ describe('dateHistogram(req, panel, series)', () => { }, }, meta: { - bucketSize: 10, intervalString: '10s', timeField: '@timestamp', seriesId: 'test', @@ -128,7 +127,6 @@ describe('dateHistogram(req, panel, series)', () => { }, }, meta: { - bucketSize: 10, intervalString: '10s', timeField: '@timestamp', seriesId: 'test', @@ -173,7 +171,6 @@ describe('dateHistogram(req, panel, series)', () => { }, }, meta: { - bucketSize: 20, intervalString: '20s', timeField: 'timestamp', seriesId: 'test', @@ -185,8 +182,11 @@ describe('dateHistogram(req, panel, series)', () => { }); describe('dateHistogram for entire time range mode', () => { - test('should ignore entire range mode for timeseries', async () => { + beforeEach(() => { panel.time_range_mode = 'entire_time_range'; + }); + + test('should ignore entire range mode for timeseries', async () => { panel.type = 'timeseries'; const next = (doc) => doc; @@ -204,9 +204,36 @@ describe('dateHistogram(req, panel, series)', () => { expect(doc.aggs.test.aggs.timeseries.date_histogram).toBeDefined(); }); - test('should returns valid date histogram for entire range mode', async () => { - panel.time_range_mode = 'entire_time_range'; + test('should set meta values', async () => { + // set 15 minutes (=== 900000ms) interval; + req.body.timerange = { + min: '2021-01-01T00:00:00Z', + max: '2021-01-01T00:15:00Z', + }; + + const next = (doc) => doc; + const doc = await dateHistogram( + req, + panel, + series, + config, + indexPattern, + capabilities, + uiSettings + )(next)({}); + expect(doc.aggs.test.meta).toMatchInlineSnapshot(` + Object { + "index": undefined, + "intervalString": "900000ms", + "panelId": "panelId", + "seriesId": "test", + "timeField": "@timestamp", + } + `); + }); + + test('should returns valid date histogram for entire range mode', async () => { const next = (doc) => doc; const doc = await dateHistogram( req, @@ -232,8 +259,7 @@ describe('dateHistogram(req, panel, series)', () => { meta: { timeField: '@timestamp', seriesId: 'test', - bucketSize: 10, - intervalString: '10s', + intervalString: '3600000ms', panelId: 'panelId', }, }, diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/metric_buckets.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/metric_buckets.js index 29a11bf163e0b..33c6622f73941 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/metric_buckets.js +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/metric_buckets.js @@ -7,30 +7,17 @@ */ import { overwrite } from '../../helpers'; -import { getBucketSize } from '../../helpers/get_bucket_size'; import { bucketTransform } from '../../helpers/bucket_transform'; -import { getIntervalAndTimefield } from '../../get_interval_and_timefield'; -import { UI_SETTINGS } from '../../../../../../data/common'; +import { get } from 'lodash'; -export function metricBuckets( - req, - panel, - series, - esQueryConfig, - seriesIndex, - capabilities, - uiSettings -) { +export function metricBuckets(req, panel, series) { return (next) => async (doc) => { - const barTargetUiSettings = await uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET); - - const { interval } = getIntervalAndTimefield(panel, series, seriesIndex); - const { intervalString } = getBucketSize(req, interval, capabilities, barTargetUiSettings); - series.metrics .filter((row) => !/_bucket$/.test(row.type) && !/^series/.test(row.type)) .forEach((metric) => { const fn = bucketTransform[metric.type]; + const intervalString = get(doc, `aggs.${series.id}.meta.intervalString`); + if (fn) { try { const bucket = fn(metric, series.metrics, intervalString); diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/sibling_buckets.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/sibling_buckets.js index dbeb3b1393bd5..c3075dd6dcac0 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/sibling_buckets.js +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/series/sibling_buckets.js @@ -6,39 +6,28 @@ * Side Public License, v 1. */ +import { get } from 'lodash'; import { overwrite } from '../../helpers'; -import { getBucketSize } from '../../helpers/get_bucket_size'; import { bucketTransform } from '../../helpers/bucket_transform'; -import { getIntervalAndTimefield } from '../../get_interval_and_timefield'; -import { UI_SETTINGS } from '../../../../../../data/common'; -export function siblingBuckets( - req, - panel, - series, - esQueryConfig, - seriesIndex, - capabilities, - uiSettings -) { +export function siblingBuckets(req, panel, series) { return (next) => async (doc) => { - const barTargetUiSettings = await uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET); - const { interval } = getIntervalAndTimefield(panel, series, seriesIndex); - const { bucketSize } = getBucketSize(req, interval, capabilities, barTargetUiSettings); - series.metrics .filter((row) => /_bucket$/.test(row.type)) .forEach((metric) => { const fn = bucketTransform[metric.type]; + const intervalString = get(doc, `aggs.${series.id}.meta.intervalString`); + if (fn) { try { - const bucket = fn(metric, series.metrics, bucketSize); + const bucket = fn(metric, series.metrics, intervalString); overwrite(doc, `aggs.${series.id}.aggs.${metric.id}`, bucket); } catch (e) { // meh } } }); + return next(doc); }; } diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/date_histogram.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/date_histogram.js index 3e883abc9e5e0..92ac4078a3835 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/date_histogram.js +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/date_histogram.js @@ -20,6 +20,7 @@ export function dateHistogram(req, panel, esQueryConfig, seriesIndex, capabiliti return (next) => async (doc) => { const barTargetUiSettings = await uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET); const { timeField, interval } = getIntervalAndTimefield(panel, {}, seriesIndex); + const { from, to } = getTimerange(req); const meta = { timeField, @@ -27,14 +28,8 @@ export function dateHistogram(req, panel, esQueryConfig, seriesIndex, capabiliti panelId: panel.id, }; - const getDateHistogramForLastBucketMode = () => { - const { bucketSize, intervalString } = getBucketSize( - req, - interval, - capabilities, - barTargetUiSettings - ); - const { from, to } = getTimerange(req); + const overwriteDateHistogramForLastBucketMode = () => { + const { intervalString } = getBucketSize(req, interval, capabilities, barTargetUiSettings); const { timezone } = capabilities; panel.series.forEach((column) => { @@ -54,12 +49,13 @@ export function dateHistogram(req, panel, esQueryConfig, seriesIndex, capabiliti overwrite(doc, aggRoot.replace(/\.aggs$/, '.meta'), { ...meta, intervalString, - bucketSize, }); }); }; - const getDateHistogramForEntireTimerangeMode = () => { + const overwriteDateHistogramForEntireTimerangeMode = () => { + const intervalString = `${to.valueOf() - from.valueOf()}ms`; + panel.series.forEach((column) => { const aggRoot = calculateAggRoot(doc, column); @@ -68,13 +64,16 @@ export function dateHistogram(req, panel, esQueryConfig, seriesIndex, capabiliti buckets: 1, }); - overwrite(doc, aggRoot.replace(/\.aggs$/, '.meta'), meta); + overwrite(doc, aggRoot.replace(/\.aggs$/, '.meta'), { + ...meta, + intervalString, + }); }); }; isLastValueTimerangeMode(panel) - ? getDateHistogramForLastBucketMode() - : getDateHistogramForEntireTimerangeMode(); + ? overwriteDateHistogramForLastBucketMode() + : overwriteDateHistogramForEntireTimerangeMode(); return next(doc); }; diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/metric_buckets.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/metric_buckets.js index 421f9d2d75f0c..8e0d0060225ff 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/metric_buckets.js +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/metric_buckets.js @@ -6,19 +6,13 @@ * Side Public License, v 1. */ +import { get } from 'lodash'; import { overwrite } from '../../helpers'; -import { getBucketSize } from '../../helpers/get_bucket_size'; import { bucketTransform } from '../../helpers/bucket_transform'; -import { getIntervalAndTimefield } from '../../get_interval_and_timefield'; import { calculateAggRoot } from './calculate_agg_root'; -import { UI_SETTINGS } from '../../../../../../data/common'; -export function metricBuckets(req, panel, esQueryConfig, seriesIndex, capabilities, uiSettings) { +export function metricBuckets(req, panel) { return (next) => async (doc) => { - const barTargetUiSettings = await uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET); - const { interval } = getIntervalAndTimefield(panel, {}, seriesIndex); - const { intervalString } = getBucketSize(req, interval, capabilities, barTargetUiSettings); - panel.series.forEach((column) => { const aggRoot = calculateAggRoot(doc, column); column.metrics @@ -27,7 +21,9 @@ export function metricBuckets(req, panel, esQueryConfig, seriesIndex, capabiliti const fn = bucketTransform[metric.type]; if (fn) { try { + const intervalString = get(doc, aggRoot.replace(/\.aggs$/, '.meta.intervalString')); const bucket = fn(metric, column.metrics, intervalString); + overwrite(doc, `${aggRoot}.timeseries.aggs.${metric.id}`, bucket); } catch (e) { // meh diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/sibling_buckets.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/sibling_buckets.js index 9b4b0f244fc2c..6ce956c490900 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/sibling_buckets.js +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/request_processors/table/sibling_buckets.js @@ -7,18 +7,12 @@ */ import { overwrite } from '../../helpers'; -import { getBucketSize } from '../../helpers/get_bucket_size'; import { bucketTransform } from '../../helpers/bucket_transform'; -import { getIntervalAndTimefield } from '../../get_interval_and_timefield'; import { calculateAggRoot } from './calculate_agg_root'; -import { UI_SETTINGS } from '../../../../../../data/common'; +import { get } from 'lodash'; -export function siblingBuckets(req, panel, esQueryConfig, seriesIndex, capabilities, uiSettings) { +export function siblingBuckets(req, panel) { return (next) => async (doc) => { - const barTargetUiSettings = await uiSettings.get(UI_SETTINGS.HISTOGRAM_BAR_TARGET); - const { interval } = getIntervalAndTimefield(panel, {}, seriesIndex); - const { bucketSize } = getBucketSize(req, interval, capabilities, barTargetUiSettings); - panel.series.forEach((column) => { const aggRoot = calculateAggRoot(doc, column); column.metrics @@ -27,7 +21,9 @@ export function siblingBuckets(req, panel, esQueryConfig, seriesIndex, capabilit const fn = bucketTransform[metric.type]; if (fn) { try { - const bucket = fn(metric, column.metrics, bucketSize); + const intervalString = get(doc, aggRoot.replace(/\.aggs$/, '.meta.intervalString')); + const bucket = fn(metric, column.metrics, intervalString); + overwrite(doc, `${aggRoot}.${metric.id}`, bucket); } catch (e) { // meh diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/response_processors/series/math.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/response_processors/series/math.js index 403b486cc4d09..d3cff76524ee3 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/response_processors/series/math.js +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/response_processors/series/math.js @@ -6,6 +6,8 @@ * Side Public License, v 1. */ +import { convertIntervalToUnit } from '../../helpers/unit_to_seconds'; + const percentileValueMatch = /\[([0-9\.]+)\]$/; import { startsWith, flatten, values, first, last } from 'lodash'; import { getDefaultDecoration } from '../../helpers/get_default_decoration'; @@ -82,13 +84,15 @@ export function mathAgg(resp, panel, series, meta, extractFields) { if (someNull) return [ts, null]; try { // calculate the result based on the user's script and return the value + const inMsInterval = convertIntervalToUnit(split.meta?.intervalString || 0, 'ms'); + const result = evaluate(mathMetric.script, { params: { ...params, _index: index, _timestamp: ts, _all: all, - _interval: split.meta.bucketSize * 1000, + _interval: inMsInterval?.value, }, }); // if the result is an object (usually when the user is working with maps and functions) flatten the results and return the last value. diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/response_processors/series/math.test.js b/src/plugins/vis_type_timeseries/server/lib/vis_data/response_processors/series/math.test.js index 1e30720d6e5b2..7b5eb1e029069 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/response_processors/series/math.test.js +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/response_processors/series/math.test.js @@ -54,7 +54,7 @@ describe('math(resp, panel, series)', () => { aggregations: { test: { meta: { - bucketSize: 5, + intervalString: '5s', }, buckets: [ { @@ -124,6 +124,25 @@ describe('math(resp, panel, series)', () => { ); }); + test('should works with predefined variables (params._interval)', async () => { + const expectedInterval = 5000; + + series.metrics[2].script = 'params._interval'; + + const next = await mathAgg(resp, panel, series)((results) => results); + const results = await stdMetric(resp, panel, series)(next)([]); + + expect(results).toHaveLength(1); + expect(results[0]).toEqual( + expect.objectContaining({ + data: [ + [1, expectedInterval], + [2, expectedInterval], + ], + }) + ); + }); + test('throws on actual tinymath expression errors #1', async () => { series.metrics[2].script = 'notExistingFn(params.a)'; diff --git a/src/plugins/vis_type_timeseries/server/lib/vis_data/series/build_request_body.test.ts b/src/plugins/vis_type_timeseries/server/lib/vis_data/series/build_request_body.test.ts index 5b865d451003a..46acbb27e15e1 100644 --- a/src/plugins/vis_type_timeseries/server/lib/vis_data/series/build_request_body.test.ts +++ b/src/plugins/vis_type_timeseries/server/lib/vis_data/series/build_request_body.test.ts @@ -153,7 +153,6 @@ describe('buildRequestBody(req)', () => { time_zone: 'UTC', }, meta: { - bucketSize: 10, intervalString: '10s', seriesId: 'c9b5f9c0-e403-11e6-be91-6f7688e9fac7', timeField: '@timestamp',