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

[TSVB] Add support for histogram type #68837

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
8528662
[TSVB] Add support for histogram type
simianhacker Jun 10, 2020
ba4373e
Merge branch 'master' of github.com:elastic/kibana into issue-52426-t…
simianhacker Jun 15, 2020
42bf37c
Merge branch 'master' of github.com:elastic/kibana into issue-52426-t…
simianhacker Jun 15, 2020
7b4878b
Merge branch 'master' into issue-52426-tsvb-support-for-histograms
elasticmachine Jun 17, 2020
3f422a9
Merge branch 'master' into issue-52426-tsvb-support-for-histograms
elasticmachine Jun 23, 2020
2b1a647
Merge branch 'master' of github.com:elastic/kibana into issue-52426-t…
simianhacker Jun 25, 2020
dcee7e3
Merge branch 'master' into issue-52426-tsvb-support-for-histograms
elasticmachine Jun 26, 2020
49c7181
Merge branch 'master' into issue-52426-tsvb-support-for-histograms
elasticmachine Jul 3, 2020
cae36d5
Merge branch 'master' into issue-52426-tsvb-support-for-histograms
elasticmachine Jul 7, 2020
016ae3b
Adding support to filter ratio; updating test
simianhacker Jul 7, 2020
dd92c95
Merge branch 'master' of github.com:elastic/kibana into issue-52426-t…
simianhacker Jul 7, 2020
62d2b6a
Limist aggs for filter_ratio and histogram fields; add test for AggSe…
simianhacker Jul 8, 2020
3519cfa
Merge branch 'master' of github.com:elastic/kibana into issue-52426-t…
simianhacker Jul 8, 2020
cfca4ab
Ensure only compatible fields are displayed for filter ratio metric aggs
simianhacker Jul 9, 2020
57edebc
Merge branch 'master' of github.com:elastic/kibana into issue-52426-t…
simianhacker Jul 9, 2020
2307544
Merge branch 'master' into issue-52426-tsvb-support-for-histograms
elasticmachine Jul 13, 2020
f7d6647
Merge branch 'master' into issue-52426-tsvb-support-for-histograms
elasticmachine Jul 13, 2020
c1a81fe
Merge branch 'master' into issue-52426-tsvb-support-for-histograms
elasticmachine Jul 13, 2020
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
32 changes: 32 additions & 0 deletions src/plugins/vis_type_timeseries/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,35 @@
*/

export const MAX_BUCKETS_SETTING = 'metrics:max_buckets';

// Tests Constants
simianhacker marked this conversation as resolved.
Show resolved Hide resolved
export const UI_RESTRICTIONS = { '*': true };
export const INDEX_PATTERN = 'some-pattern';
export const FIELDS = {
[INDEX_PATTERN]: [
{
type: 'date',
name: '@timestamp',
},
{
type: 'number',
name: 'system.cpu.user.pct',
},
{
type: 'histogram',
name: 'histogram_value',
},
],
};
export const METRIC = {
type: 'avg',
field: 'system.cpu.user.pct',
};
export const SERIES = {
metrics: [METRIC],
};
export const PANEL = {
type: 'timeseries',
index_pattern: INDEX_PATTERN,
series: SERIES,
};
2 changes: 2 additions & 0 deletions src/plugins/vis_type_timeseries/common/metric_types.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export const METRIC_TYPES = {
VARIANCE: 'variance',
SUM_OF_SQUARES: 'sum_of_squares',
CARDINALITY: 'cardinality',
VALUE_COUNT: 'value_count',
AVERAGE: 'avg',
};

export const EXTENDED_STATS_TYPES = [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we already started migration js -> ts. If possible please refactor tests in ts

Copy link
Member

Choose a reason for hiding this comment

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

@alexwizp are all of the imported files already migrated to TS? Maybe we can hold off here for now if not?

* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import React from 'react';
import { mountWithIntl } from 'test_utils/enzyme_helpers';
import { Agg } from './agg';
import { FieldSelect } from './field_select';
import { FIELDS, METRIC, SERIES, PANEL } from '../../../../common/constants';
const runTest = (aggType, name, test) => {
describe(aggType, () => {
const metric = {
...METRIC,
type: aggType,
field: 'histogram_value',
};
const series = { ...SERIES, metrics: [metric] };
const panel = { ...PANEL, series };

it(name, () => {
const wrapper = mountWithIntl(
<div>
<Agg
onAdd={jest.fn()}
onChange={jest.fn()}
onDelete={jest.fn()}
panel={panel}
fields={FIELDS}
model={metric}
series={series}
siblings={series.metrics}
dragHandleProps={{}}
/>
</div>
);
test(wrapper);
});
});
};

describe('Histogram Types', () => {
describe('supported', () => {
const shouldHaveHistogramSupport = (aggType) => {
runTest(aggType, 'supports', (wrapper) =>
expect(wrapper.find(FieldSelect).at(0).props().restrict).toContain('histogram')
);
};
shouldHaveHistogramSupport('avg');
shouldHaveHistogramSupport('value_count');
shouldHaveHistogramSupport('percentile');
shouldHaveHistogramSupport('percentile_rank');
});
describe('not supported', () => {
const shouldNotHaveHistogramSupport = (aggType) => {
runTest(aggType, 'does not support', (wrapper) =>
expect(wrapper.find(FieldSelect).at(0).props().restrict).not.toContain('histogram')
);
};
shouldNotHaveHistogramSupport('cardinality');
shouldNotHaveHistogramSupport('max');
shouldNotHaveHistogramSupport('min');
shouldNotHaveHistogramSupport('sum');
shouldNotHaveHistogramSupport('variance');
shouldNotHaveHistogramSupport('sum_of_squares');
shouldNotHaveHistogramSupport('std_deviation');
shouldNotHaveHistogramSupport('positive_rate');
shouldNotHaveHistogramSupport('top_hit');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { FormattedMessage } from '@kbn/i18n/react';
import { KBN_FIELD_TYPES } from '../../../../../../plugins/data/public';
import { Percentiles, newPercentile } from './percentile_ui';

const RESTRICT_FIELDS = [KBN_FIELD_TYPES.NUMBER];
const RESTRICT_FIELDS = [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.HISTOGRAM];
Copy link
Contributor

Choose a reason for hiding this comment

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

I think KBN_FIELD_TYPES.HISTOGRAM should be also added for src/plugins/vis_type_timeseries/public/application/components/aggs/filter_ratio.js

Copy link
Member Author

Choose a reason for hiding this comment

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

Filter ratio doesn't have a field dropdown.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

You should change Metric Aggregation ftom Count to Average for example

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops... I forgot about that feature. Adding it now.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jasonrhodes should be fixed before merge


const checkModel = (model) => Array.isArray(model.percentiles);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import { IFieldType, KBN_FIELD_TYPES } from '../../../../../../../plugins/data/p
import { MetricsItemsSchema, PanelSchema, SeriesItemsSchema } from '../../../../../common/types';
import { DragHandleProps } from '../../../../types';

const RESTRICT_FIELDS = [KBN_FIELD_TYPES.NUMBER];
const RESTRICT_FIELDS = [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.HISTOGRAM];

interface PercentileRankAggProps {
disableDelete: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export const PositiveRateAgg = (props) => {
<FieldSelect
fields={props.fields}
type={model.type}
restrict={KBN_FIELD_TYPES.NUMBER}
restrict={[KBN_FIELD_TYPES.NUMBER]}
indexPattern={indexPattern}
value={model.field}
onChange={handleSelectChange('field')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,23 @@ import { FormattedMessage } from '@kbn/i18n/react';
import { KBN_FIELD_TYPES } from '../../../../../../plugins/data/public';
import { METRIC_TYPES } from '../../../../../../plugins/vis_type_timeseries/common/metric_types';

function getSupportedFieldsByMetricType(type) {
switch (type) {
case METRIC_TYPES.CARDINALITY:
return Object.values(KBN_FIELD_TYPES).filter((type) => type !== KBN_FIELD_TYPES.HISTOGRAM);
case METRIC_TYPES.VALUE_COUNT:
case METRIC_TYPES.AVERAGE:
return [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.HISTOGRAM];
default:
return [KBN_FIELD_TYPES.NUMBER];
}
}

export function StandardAgg(props) {
const { model, panel, series, fields, uiRestrictions } = props;
const handleChange = createChangeHandler(props.onChange, model);
const handleSelectChange = createSelectHandler(handleChange);
const restrictFields = model.type === METRIC_TYPES.CARDINALITY ? [] : [KBN_FIELD_TYPES.NUMBER];

const restrictFields = getSupportedFieldsByMetricType(model.type);
const indexPattern =
(series.override_index_pattern && series.series_index_pattern) || panel.index_pattern;
const htmlId = htmlIdGenerator();
Expand Down