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

Add support for HDR percentiles in TSVB visualizations #78306

Merged
merged 16 commits into from
Oct 7, 2020
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions src/plugins/vis_type_timeseries/common/vis_schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export const metricsItems = schema.object({
})
)
),
numberOfSignificantValueDigits: numberOptional,
percentiles: schema.maybe(
schema.arrayOf(
schema.object({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +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 { createNumberHandler } from '../lib/create_number_handler';
import {
htmlIdGenerator,
EuiSpacer,
EuiFlexGroup,
EuiFlexGrid,
EuiFlexItem,
EuiFormLabel,
EuiFormRow,
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { KBN_FIELD_TYPES } from '../../../../../../plugins/data/public';
import { Percentiles, newPercentile } from './percentile_ui';
import { PercentileHdr } from './percentile_hdr';

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

Expand All @@ -46,6 +48,8 @@ export function PercentileAgg(props) {

const handleChange = createChangeHandler(props.onChange, model);
const handleSelectChange = createSelectHandler(handleChange);
const handleNumberChange = createNumberHandler(handleChange);

const indexPattern =
(series.override_index_pattern && series.series_index_pattern) || panel.index_pattern;

Expand All @@ -66,7 +70,7 @@ export function PercentileAgg(props) {
siblings={props.siblings}
dragHandleProps={props.dragHandleProps}
>
<EuiFlexGroup gutterSize="s">
<EuiFlexGrid gutterSize="s" columns={2}>
<EuiFlexItem>
<EuiFormLabel htmlFor={htmlId('aggregation')}>
<FormattedMessage
Expand Down Expand Up @@ -103,11 +107,25 @@ export function PercentileAgg(props) {
/>
</EuiFormRow>
</EuiFlexItem>
</EuiFlexGroup>

<EuiSpacer size="m" />

<Percentiles onChange={handleChange} name="percentiles" model={model} panel={panel} />
<EuiFlexItem>
<EuiFormRow
label={
<FormattedMessage
id="visTypeTimeseries.percentile.percents"
defaultMessage="Percents"
/>
}
>
<Percentiles onChange={handleChange} name="percentiles" model={model} panel={panel} />
</EuiFormRow>
</EuiFlexItem>
<EuiFlexItem>
<PercentileHdr
value={model.numberOfSignificantValueDigits}
onChange={handleNumberChange('numberOfSignificantValueDigits')}
/>
</EuiFlexItem>
</EuiFlexGrid>
</AggRow>
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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 { EuiFieldNumber, EuiFormRow, EuiIconTip } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';

export interface PercentileHdrProps {
value: number | undefined;
onChange: () => void;
}

export const PercentileHdr = ({ value, onChange }: PercentileHdrProps) => (
<EuiFormRow
label={
<>
<FormattedMessage
id="visTypeTimeseries.percentileHdr.numberOfSignificantValueDigits"
defaultMessage="Number of significant value digits (HDR histogram)"
/>{' '}
<EuiIconTip
position="right"
content={
<FormattedMessage
id="visTypeTimeseries.percentileHdr.numberOfSignificantValueDigits.hint"
defaultMessage="HDR Histogram (High Dynamic Range Histogram) is an alternative implementation that can be useful when calculating percentile ranks for latency measurements as it can be faster than the t-digest implementation with the trade-off of a larger memory footprint. Number of significant value digits parameter specifies the resolution of values for the histogram in number of significant digits"
/>
}
type="questionInCircle"
/>
</>
}
>
<EuiFieldNumber value={value || ''} onChange={onChange} />
</EuiFormRow>
);
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,7 @@
*/
import React, { ChangeEvent } from 'react';
import { get } from 'lodash';
import { FormattedMessage } from '@kbn/i18n/react';
import {
htmlIdGenerator,
EuiFieldNumber,
EuiFlexGroup,
EuiFlexItem,
EuiFormLabel,
EuiSpacer,
} from '@elastic/eui';
import { EuiFieldNumber, EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';

import { AddDeleteButtons } from '../../add_delete_buttons';

Expand All @@ -50,8 +42,6 @@ export const MultiValueRow = ({
disableAdd,
disableDelete,
}: MultiValueRowProps) => {
const htmlId = htmlIdGenerator();

const onFieldNumberChange = (event: ChangeEvent<HTMLInputElement>) =>
onChange({
...model,
Expand All @@ -60,15 +50,7 @@ export const MultiValueRow = ({

return (
<div className="tvbAggRow__multiValueRow">
<EuiFlexGroup responsive={false} alignItems="center">
<EuiFlexItem grow={false}>
<EuiFormLabel htmlFor={htmlId('value')}>
<FormattedMessage
id="visTypeTimeseries.multivalueRow.valueLabel"
defaultMessage="Value:"
/>
</EuiFormLabel>
</EuiFlexItem>
<EuiFlexGroup responsive={false} alignItems="center" gutterSize="s">
<EuiFlexItem grow={false}>
<EuiFieldNumber
value={model.value === '' ? '' : Number(model.value)}
Expand All @@ -82,10 +64,11 @@ export const MultiValueRow = ({
onDelete={() => onDelete(model)}
disableDelete={disableDelete}
disableAdd={disableAdd}
responsive={false}
/>
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer />
<EuiSpacer size={'s'} />
alexwizp marked this conversation as resolved.
Show resolved Hide resolved
</div>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@
import React from 'react';
import {
htmlIdGenerator,
EuiFlexGroup,
EuiFlexItem,
EuiFormLabel,
EuiFormRow,
EuiSpacer,
EuiFlexGrid,
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { AggSelect } from '../agg_select';
Expand All @@ -34,12 +34,16 @@ import { FieldSelect } from '../field_select';
import { createChangeHandler } from '../../lib/create_change_handler';
// @ts-ignore
import { createSelectHandler } from '../../lib/create_select_handler';
// @ts-ignore
import { createNumberHandler } from '../../lib/create_number_handler';

import { AggRow } from '../agg_row';
import { PercentileRankValues } from './percentile_rank_values';

import { IFieldType, KBN_FIELD_TYPES } from '../../../../../../../plugins/data/public';
import { MetricsItemsSchema, PanelSchema, SeriesItemsSchema } from '../../../../../common/types';
import { DragHandleProps } from '../../../../types';
import { PercentileHdr } from '../percentile_hdr';

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

Expand Down Expand Up @@ -67,6 +71,7 @@ export const PercentileRankAgg = (props: PercentileRankAggProps) => {
const isTablePanel = panel.type === 'table';
const handleChange = createChangeHandler(props.onChange, model);
const handleSelectChange = createSelectHandler(handleChange);
const handleNumberChange = createNumberHandler(handleChange);

const handlePercentileRankValuesChange = (values: MetricsItemsSchema['values']) => {
handleChange({
Expand All @@ -84,7 +89,7 @@ export const PercentileRankAgg = (props: PercentileRankAggProps) => {
siblings={props.siblings}
dragHandleProps={props.dragHandleProps}
>
<EuiFlexGroup gutterSize="s">
<EuiFlexGrid gutterSize="s" columns={2}>
<EuiFlexItem>
<EuiFormLabel htmlFor={htmlId('aggregation')}>
<FormattedMessage
Expand Down Expand Up @@ -121,17 +126,36 @@ export const PercentileRankAgg = (props: PercentileRankAggProps) => {
/>
</EuiFormRow>
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer />
{model.values && (
<PercentileRankValues
disableAdd={isTablePanel}
disableDelete={isTablePanel}
showOnlyLastRow={isTablePanel}
model={model.values}
onChange={handlePercentileRankValuesChange}
/>
)}

<EuiFlexItem>
<EuiFormRow
label={
<FormattedMessage
id="visTypeTimeseries.percentileRank.values"
defaultMessage="Values"
/>
}
>
<>
{model.values && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't you add the whole EuiFormRow under model.values check here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, I just moved it from the other place without thinking about it. I see that model.values are not optional fo this agg and should have a value. I think it was added cause in model it's an optional value....

Fixed

<PercentileRankValues
disableAdd={isTablePanel}
Copy link
Contributor

Choose a reason for hiding this comment

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

I get a warning for the key here when I add multiple Values
Screenshot 2020-09-24 at 4 15 39 PM
I don't reproduce it in master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stratoula you are not right, on master I see the same issue. But it's not a reason to don't solve it. Will be fixed in that PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

My mistake @alexwizp, thank you so much for fixing this

disableDelete={isTablePanel}
showOnlyLastRow={isTablePanel}
model={model.values}
onChange={handlePercentileRankValuesChange}
/>
)}
</>
</EuiFormRow>
</EuiFlexItem>
<EuiFlexItem>
<PercentileHdr
value={model.numberOfSignificantValueDigits}
onChange={handleNumberChange('numberOfSignificantValueDigits')}
/>
</EuiFlexItem>
</EuiFlexGrid>
</AggRow>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class PercentilesUi extends Component {
/>
</EuiFlexItem>

<EuiFlexItem>
<EuiFlexItem grow={false}>
<AddDeleteButtons
onAdd={handleAdd}
onDelete={handleDelete}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ export async function getSeriesData(req, panel) {
(acc, items) => acc.concat(items),
[]
);

const data = await searchStrategy.search(req, searches);

const handleResponseBodyFn = handleResponseBody(panel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ function extendStatsBucket(bucket, metrics) {
return body;
}

function getPercentileHdrParam(bucket) {
return bucket.numberOfSignificantValueDigits
? {
hdr: {
number_of_significant_value_digits: bucket.numberOfSignificantValueDigits,
},
}
: undefined;
}

export const bucketTransform = {
count: () => {
return {
Expand Down Expand Up @@ -139,13 +149,14 @@ export const bucketTransform = {
bucket.percentiles.filter((p) => p.percentile).map((p) => p.percentile)
);
}
const agg = {

return {
percentiles: {
field: bucket.field,
percents,
...getPercentileHdrParam(bucket),
},
};
return agg;
},

percentile_rank: (bucket) => {
Expand All @@ -155,6 +166,7 @@ export const bucketTransform = {
percentile_ranks: {
field: bucket.field,
values: (bucket.values || []).map((value) => (isEmpty(value) ? 0 : value)),
...getPercentileHdrParam(bucket),
},
};
},
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -3854,7 +3854,6 @@
"visTypeTimeseries.movingAverage.period": "期間",
"visTypeTimeseries.movingAverage.windowSizeHint": "ウィンドウは、必ず、期間のサイズの 2 倍以上でなければなりません",
"visTypeTimeseries.movingAverage.windowSizeLabel": "ウィンドウサイズ",
"visTypeTimeseries.multivalueRow.valueLabel": "値:",
"visTypeTimeseries.noButtonLabel": "いいえ",
"visTypeTimeseries.noDataDescription": "選択されたメトリックに表示するデータがありません",
"visTypeTimeseries.percentile.aggregationLabel": "集約",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -3855,7 +3855,6 @@
"visTypeTimeseries.movingAverage.period": "期间",
"visTypeTimeseries.movingAverage.windowSizeHint": "窗口必须始终至少是期间大小的两倍",
"visTypeTimeseries.movingAverage.windowSizeLabel": "窗口大小",
"visTypeTimeseries.multivalueRow.valueLabel": "值:",
"visTypeTimeseries.noButtonLabel": "否",
"visTypeTimeseries.noDataDescription": "所选指标没有可显示的数据",
"visTypeTimeseries.percentile.aggregationLabel": "聚合",
Expand Down