Skip to content

Commit

Permalink
Fix counter rate metric and remove unused comments
Browse files Browse the repository at this point in the history
  • Loading branch information
VladLasitsa committed Sep 7, 2022
1 parent b4c214b commit ac69c8a
Show file tree
Hide file tree
Showing 12 changed files with 61 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import { stubLogstashDataView } from '@kbn/data-views-plugin/common/data_view.st
import { TSVB_METRIC_TYPES } from '../../../../common/enums';
import type { Metric } from '../../../../common/types';
import { createSeries } from '../__mocks__';
import { convertToCounterRateFormulaColumn } from './counter_rate';
import { CommonColumnsConverterArgs, FormulaColumn } from './types';
import { convertToCounterRateColumn } from './counter_rate';
import { CommonColumnsConverterArgs, CounterRateColumn, MaxColumn } from './types';

describe('convertToCounterRateFormulaColumn', () => {
const dataView = stubLogstashDataView;
Expand All @@ -21,33 +21,32 @@ describe('convertToCounterRateFormulaColumn', () => {
type: TSVB_METRIC_TYPES.POSITIVE_RATE,
};

test.each<[string, CommonColumnsConverterArgs, Partial<FormulaColumn> | null]>([
test.each<
[string, CommonColumnsConverterArgs, [Partial<MaxColumn>, Partial<CounterRateColumn>] | null]
>([
['null if metric contains empty field param', { series, metrics: [metric], dataView }, null],
[
'formula column if metric contains field param',
'max and cpunter rate columns if metric contains field param',
{ series, metrics: [{ ...metric, field: dataView.fields[0].name }], dataView },
{
operationType: 'formula',
params: { formula: 'pick_max(differences(max(bytes)), 0)' },
meta: { metricId: 'some-id' },
},
],
[
'differences formula with shift if metric contains unit',
{ series, metrics: [{ ...metric, field: dataView.fields[0].name, unit: '1h' }], dataView },
{
operationType: 'formula',
params: {
formula: 'pick_max(differences(max(bytes), shift=1h), 0)',
[
{
operationType: 'max',
sourceField: dataView.fields[0].name,
},
{
operationType: 'counter_rate',
},
meta: { metricId: 'some-id' },
},
],
],
])('should return %s', (_, input, expected) => {
if (expected === null) {
expect(convertToCounterRateFormulaColumn(input)).toBeNull();
expect(convertToCounterRateColumn(input)).toBeNull();
} else if (Array.isArray(expected)) {
const results = convertToCounterRateColumn(input);
expect(results).toEqual(expected.map(expect.objectContaining));
expect(results?.[1].references[0]).toEqual(results?.[0].columnId);
} else {
expect(convertToCounterRateFormulaColumn(input)).toEqual(expect.objectContaining(expected));
expect(convertToCounterRateColumn(input)).toEqual(expect.objectContaining(expected));
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,36 @@
* Side Public License, v 1.
*/

import { buildCounterRateFormula } from '../metrics';
import { createFormulaColumn } from './formula';
import { CommonColumnsConverterArgs, FormulaColumn } from './types';
import { Operations } from '@kbn/visualizations-plugin/common/convert_to_lens';
import { createColumn, getFormat } from './column';
import { CommonColumnsConverterArgs, CounterRateColumn, MaxColumn } from './types';

export const convertToCounterRateFormulaColumn = ({
export const convertToCounterRateColumn = ({
series,
metrics,
dataView,
}: CommonColumnsConverterArgs): FormulaColumn | null => {
}: CommonColumnsConverterArgs): [MaxColumn, CounterRateColumn] | null => {
const metric = metrics[metrics.length - 1];

const field = metric.field ? dataView.getFieldByName(metric.field) : undefined;
if (!field) {
return null;
}

const formula = buildCounterRateFormula(metric, field.name);
return createFormulaColumn(formula, { series, metric, dataView });
const maxColumn = {
operationType: Operations.MAX,
sourceField: field.name,
...createColumn(series, metric, field),
params: { ...getFormat(series) },
};

return [
maxColumn,
{
operationType: Operations.COUNTER_RATE,
references: [maxColumn.columnId],
...createColumn(series, metric, field),
params: { ...getFormat(series) },
},
];
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@
* Side Public License, v 1.
*/

/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { DataView } from '@kbn/data-views-plugin/common';
import uuid from 'uuid';
import { DateHistogramParams, DataType } from '@kbn/visualizations-plugin/common/convert_to_lens';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export { convertToStaticValueColumn } from './static_value';
export { convertToFiltersColumn } from './filters';
export { convertToDateHistogramColumn } from './date_histogram';
export { converToTermsColumn } from './terms';
export { convertToCounterRateFormulaColumn } from './counter_rate';
export { convertToCounterRateColumn } from './counter_rate';
export { convertToStandartDeviationColumn } from './std_deviation';

export * from './types';
Original file line number Diff line number Diff line change
Expand Up @@ -268,26 +268,6 @@ describe('convertMetricAggregationToColumn', () => {
reducedTimeRange: '10m',
},
],
[
'null for counter rate if field in metric is empty',
[
SUPPORTED_METRICS.positive_rate,
{ series, metric: { id, type: TSVB_METRIC_TYPES.POSITIVE_RATE }, dataView },
],
null,
],
[
'formula column for counter rate',
[
SUPPORTED_METRICS.positive_rate,
{ series, metric: { id, field, type: TSVB_METRIC_TYPES.POSITIVE_RATE }, dataView },
],
{
meta: { metricId: 'some-id' },
operationType: 'formula',
params: { formula: 'pick_max(differences(max(bytes)), 0)' },
},
],
[
'null for last value (unsupported)',
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import { createFormulaColumn } from './formula';
import { convertToMovingAverageParams } from './moving_average';
import { convertToPercentileColumn } from './percentile';
import { convertToPercentileRankColumn } from './percentile_rank';
import { convertToCounterRateFormulaColumn } from './counter_rate';

type MetricAggregationWithoutParams =
| typeof Operations.AVERAGE
Expand Down Expand Up @@ -92,6 +91,7 @@ const SUPPORTED_METRICS_AGGS_WITHOUT_PARAMS: MetricAggregationWithoutParams[] =
Operations.MIN,
Operations.SUM,
Operations.STANDARD_DEVIATION,
Operations.COUNTER_RATE,
];

const SUPPORTED_METRIC_AGGS: MetricAggregation[] = [
Expand Down Expand Up @@ -159,9 +159,6 @@ export const convertMetricAggregationToColumn = (
reducedTimeRange,
});
}
if (aggregation.name === Operations.COUNTER_RATE) {
return convertToCounterRateFormulaColumn({ series, metrics: [metric], dataView });
}

if (aggregation.name === Operations.LAST_VALUE) {
return null;
Expand Down Expand Up @@ -237,7 +234,7 @@ const convertMovingAvgOrDerivativeToColumns = (
const [nestedFieldId, _] = subMetricField?.split('[') ?? [];
// support nested aggs with formula
const additionalSubFunction = metrics.find(({ id }) => id === nestedFieldId);
if (additionalSubFunction) {
if (additionalSubFunction || pipelineAgg.name === 'counter_rate') {
const formula = getParentPipelineSeriesFormula(
metrics,
subFunctionMetric,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { stubLogstashDataView } from '@kbn/data-views-plugin/common/data_view.st
import { TSVB_METRIC_TYPES } from '../../../../common/enums';
import { Metric } from '../../../../common/types';
import { buildCounterRateFormula } from './counter_rate_formula';
import { SUPPORTED_METRICS } from './supported_metrics';

describe('buildCounterRateFormula', () => {
test('should return correct formula for counter rate', () => {
Expand All @@ -20,7 +21,10 @@ describe('buildCounterRateFormula', () => {
unit: '1h',
};

const formula = buildCounterRateFormula(metric, dataView.fields[0].name);
expect(formula).toStrictEqual('pick_max(differences(max(bytes), shift=1h), 0)');
const formula = buildCounterRateFormula(
SUPPORTED_METRICS[metric.type]!.name,
dataView.fields[0].name
);
expect(formula).toStrictEqual('counter_rate(max(bytes))');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,17 @@
* Side Public License, v 1.
*/

import type { Metric } from '../../../../common/types';
import { TimeScaleValue, isTimeScaleValue } from './metrics_helpers';

const buildMaxFormula = (selector: string) => {
return `max(${selector})`;
};

const buildPickMaxPositiveFormula = (selector: string) => {
return `pick_max(${selector}, 0)`;
};

const buildDifferencesFormula = (selector: string, shift?: TimeScaleValue) => {
return `differences(${selector}${shift ? `, shift=${shift}` : ''})`;
const buildСounterRateFormula = (aggFormula: string, selector: string) => {
return `${aggFormula}(${selector})`;
};

export const buildCounterRateFormula = (metric: Metric, fieldName: string) => {
export const buildCounterRateFormula = (aggFormula: string, fieldName: string) => {
const maxFormula = buildMaxFormula(fieldName);

const unit = metric.unit && isTimeScaleValue(metric.unit) ? metric.unit : undefined;
const diffOfMaxFormula = buildDifferencesFormula(maxFormula, unit);

const counterRateFormula = buildPickMaxPositiveFormula(diffOfMaxFormula);
const counterRateFormula = buildСounterRateFormula(aggFormula, maxFormula);
return counterRateFormula;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@
* Side Public License, v 1.
*/

/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { TimeRange } from '@kbn/data-plugin/common';
import { METRIC_TYPES } from '@kbn/data-plugin/public';
import type { Metric, Series, Panel } from '../../../../common/types';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export const getFormulaEquivalent = (
);
}
case 'positive_rate': {
return buildCounterRateFormula(currentMetric, currentMetric.field!);
return buildCounterRateFormula(aggFormula, currentMetric.field!);
}
case 'filter_ratio': {
return getFilterRatioFormula(currentMetric, reducedTimeRange);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const mockConvertMathToFormulaColumn = jest.fn();
const mockConvertParentPipelineAggToColumns = jest.fn();
const mockConvertToCumulativeSumColumns = jest.fn();
const mockConvertFilterRatioToFormulaColumn = jest.fn();
const mockConvertToCounterRateFormulaColumn = jest.fn();
const mockConvertToCounterRateColumn = jest.fn();
const mockConvertOtherAggsToFormulaColumn = jest.fn();
const mockConvertToLastValueColumn = jest.fn();
const mockConvertToStaticValueColumn = jest.fn();
Expand All @@ -30,7 +30,7 @@ jest.mock('../convert', () => ({
convertParentPipelineAggToColumns: jest.fn(() => mockConvertParentPipelineAggToColumns()),
convertToCumulativeSumColumns: jest.fn(() => mockConvertToCumulativeSumColumns()),
convertFilterRatioToFormulaColumn: jest.fn(() => mockConvertFilterRatioToFormulaColumn()),
convertToCounterRateFormulaColumn: jest.fn(() => mockConvertToCounterRateFormulaColumn()),
convertToCounterRateColumn: jest.fn(() => mockConvertToCounterRateColumn()),
convertOtherAggsToFormulaColumn: jest.fn(() => mockConvertOtherAggsToFormulaColumn()),
convertToLastValueColumn: jest.fn(() => mockConvertToLastValueColumn()),
convertToStaticValueColumn: jest.fn(() => mockConvertToStaticValueColumn()),
Expand Down Expand Up @@ -111,13 +111,13 @@ describe('getMetricsColumns', () => {
mockConvertFilterRatioToFormulaColumn,
],
[
'call convertToCounterRateFormulaColumn if metric type is positive rate',
'call convertToCounterRateColumn if metric type is positive rate',
[
createSeries({ metrics: [{ type: TSVB_METRIC_TYPES.POSITIVE_RATE, id: '1' }] }),
dataView,
1,
],
mockConvertToCounterRateFormulaColumn,
mockConvertToCounterRateColumn,
],
[
'call convertOtherAggsToFormulaColumn if metric type is positive only',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
convertToLastValueColumn,
convertToStaticValueColumn,
convertMetricAggregationColumnWithoutSpecialParams,
convertToCounterRateFormulaColumn,
convertToCounterRateColumn,
convertToStandartDeviationColumn,
} from '../convert';
import { getValidColumns } from './columns';
Expand Down Expand Up @@ -98,7 +98,7 @@ export const getMetricsColumns = (
return getValidColumns(formulaColumn);
}
case 'positive_rate': {
const formulaColumn = convertToCounterRateFormulaColumn(columnsConverterArgs);
const formulaColumn = convertToCounterRateColumn(columnsConverterArgs);
return getValidColumns(formulaColumn);
}
case 'positive_only':
Expand Down

0 comments on commit ac69c8a

Please sign in to comment.