maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[APM] Fix divide by 0 bug in percentage duration (#24675)

Browse source 
* [APM] Fixes #40165 by consolidating logic where total duration could be 0 and handling it with a fallback value

* [APM] renamed file from .js to .ts and defined a default value for fallback

* [APM] Add type definitions for apm formatters

* [APM] add basic type definition for imported @elastic/numeral module

* [APM] rename getDurationPercent to asPercent

* [APM] Update tests with the renamed formatter function

* [APM] Pr feedback & typescript cleanup

* [APM] fix bucket & chart point type errors

* Fix typescript issue

* [APM] last minute code cleanup
This commit is contained in:
Oliver Gupte 2018-10-31 23:52:23 -07:00 committed by GitHub
parent 8491b2ee8e
commit f05bb942fa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 127 additions and 103 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
x-pack/plugins/apm/public/components/app/TraceOverview/TraceList.tsx
View file

@ -8,7 +8,6 @@ import React from 'react';
import styled from 'styled-components';
import { ITransactionGroup } from '../../../../typings/TransactionGroup';
import { fontSizes, truncate } from '../../../style/variables';
// @ts-ignore
import { asMillisWithDefault } from '../../../utils/formatters';
import { ImpactBar } from '../../shared/ImpactBar';
import { ITableColumn, ManagedTable } from '../../shared/ManagedTable';

27
x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx
View file

@ -10,7 +10,6 @@ import React, { Component } from 'react';
import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams';
import { IBucket } from 'x-pack/plugins/apm/server/lib/transactions/distribution/get_buckets';
import { IDistributionResponse } from 'x-pack/plugins/apm/server/lib/transactions/distribution/get_distribution';
// @ts-ignore
import { getTimeFormatter, timeUnit } from '../../../../utils/formatters';
import { fromQuery, history, toQuery } from '../../../../utils/url';
// @ts-ignore
@ -19,8 +18,8 @@ import EmptyMessage from '../../../shared/EmptyMessage';
interface IChartPoint {
sample?: IBucket['sample'];
x0: string;
x: string;
x0: number;
x: number;
y: number;
style: {
cursor: string;
@ -32,15 +31,17 @@ export function getFormattedBuckets(buckets: IBucket[], bucketSize: number) {
return [];
}
return buckets.map(({ sample, count, key }) => {
return {
sample,
x0: key,
x: key + bucketSize,
y: count,
style: { cursor: count > 0 && sample ? 'pointer' : 'default' }
};
});
return buckets.map(
({ sample, count, key }): IChartPoint => {
return {
sample,
x0: key,
x: key + bucketSize,
y: count,
style: { cursor: count > 0 && sample ? 'pointer' : 'default' }
};
}
);
}
interface Props {
@ -67,7 +68,7 @@ export class Distribution extends Component<Props> {
);
const isEmpty = distribution.totalHits === 0;
const xMax = d3.max(buckets, d => d.x);
const xMax = d3.max(buckets, d => d.x) || 0;
const timeFormatter = getTimeFormatter(xMax);
const unit = timeUnit(xMax);

14
x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/StickyTransactionProperties.tsx
View file

@ -13,23 +13,13 @@ import {
USER_ID
} from '../../../../../common/constants';
import { Transaction } from '../../../../../typings/Transaction';
import { asPercent, asTime } from '../../../../utils/formatters';
// @ts-ignore
import { asTime } from '../../../../utils/formatters';
import {
IStickyProperty,
StickyProperties
} from '../../../shared/StickyProperties';
function getDurationPercent(
transactionDuration: number,
totalDuration?: number
) {
if (!totalDuration) {
return '';
}
return ((transactionDuration / totalDuration) * 100).toFixed(2) + '%';
}
interface Props {
transaction: Transaction;
totalDuration?: number;
@ -65,7 +55,7 @@ export function StickyTransactionProperties({
},
{
label: '% of trace',
val: getDurationPercent(duration, totalDuration),
val: asPercent(duration, totalDuration),
width: '25%'
},
{

6
x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/SpanFlyout/StickySpanProperties.tsx
View file

@ -5,13 +5,12 @@
*/
// tslint:disable-next-line no-var-requires
const numeral = require('@elastic/numeral');
import React from 'react';
import { first } from 'lodash';
import { Span } from '../../../../../../../../typings/Span';
import { asMillis, asPercent } from '../../../../../../../utils/formatters';
// @ts-ignore
import { asMillis } from '../../../../../../../utils/formatters';
import { StickyProperties } from '../../../../../../shared/StickyProperties';
function getSpanLabel(type: string) {
@ -41,7 +40,6 @@ export function StickySpanProperties({ span, totalDuration }: Props) {
const spanName = span.span.name;
const spanDuration = span.span.duration.us;
const relativeDuration = spanDuration / totalDuration;
const spanTypeLabel = getSpanLabel(getPrimaryType(span.span.type));
const stickyProperties = [
@ -67,7 +65,7 @@ export function StickySpanProperties({ span, totalDuration }: Props) {
},
{
label: '% of transaction',
val: numeral(relativeDuration).format('0.00%'),
val: asPercent(spanDuration, totalDuration),
width: '50%'
}
];

16
x-pack/plugins/apm/public/utils/__test__/formatters.test.js
View file

@ -1,16 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { asTime } from '../formatters';
describe('formatters', () => {
it('asTime', () => {
expect(asTime(1000)).toBe('1 ms');
expect(asTime(1000 * 1000)).toBe('1,000 ms');
expect(asTime(1000 * 1000 * 10)).toBe('10,000 ms');
expect(asTime(1000 * 1000 * 20)).toBe('20.0 s');
});
});

32
x-pack/plugins/apm/public/utils/__test__/formatters.test.ts Normal file
View file

@ -0,0 +1,32 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { asPercent, asTime } from '../formatters';
describe('formatters', () => {
it('asTime', () => {
expect(asTime(1000)).toBe('1 ms');
expect(asTime(1000 * 1000)).toBe('1,000 ms');
expect(asTime(1000 * 1000 * 10)).toBe('10,000 ms');
expect(asTime(1000 * 1000 * 20)).toBe('20.0 s');
});
describe('asPercent', () => {
it('should format item as percent', () => {
expect(asPercent(3725, 10000, 'n/a')).toBe('37.25%');
});
it('should return fallback when denominator is 0 ', () => {
expect(asPercent(3725, 0, 'n/a')).toBe('n/a');
expect(asPercent(3725, 0)).toBe('');
});
it('should return fallback when denominator is undefined ', () => {
expect(asPercent(3725, undefined, 'n/a')).toBe('n/a');
expect(asPercent(3725)).toBe('');
});
});
});

54
x-pack/plugins/apm/public/utils/formatters.js
View file

@ -1,54 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { memoize } from 'lodash';
import numeral from '@elastic/numeral';
const UNIT_CUT_OFF = 10 * 1000000; // 10 seconds in microseconds
export function asSeconds(value, withUnit = true) {
const formatted = asDecimal(value / 1000000);
return `${formatted}${withUnit ? ' s' : ''}`;
}
export function asMillis(value, withUnit = true) {
const formatted = asInteger(value / 1000);
return `${formatted}${withUnit ? ' ms' : ''}`;
}
export function asMillisWithDefault(value) {
if (value == null) {
return `N/A`;
}
return asMillis(value);
}
export const getTimeFormatter = memoize(
max => (max > UNIT_CUT_OFF ? asSeconds : asMillis)
);
export function timeUnit(max) {
return max > UNIT_CUT_OFF ? 's' : 'ms';
}
/*
* value: time in microseconds
*/
export function asTime(value) {
return getTimeFormatter(value)(value);
}
export function asDecimal(value) {
return numeral(value).format('0,0.0');
}
export function asInteger(value) {
return numeral(value).format('0,0');
}
export function tpmUnit(type) {
return type === 'request' ? 'rpm' : 'tpm';
}

73
x-pack/plugins/apm/public/utils/formatters.ts Normal file
View file

@ -0,0 +1,73 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { memoize } from 'lodash';
// tslint:disable-next-line no-var-requires
const numeral: (input: number) => Numeral = require('@elastic/numeral');
interface Numeral {
format: (pattern: string) => string;
}
const UNIT_CUT_OFF = 10 * 1000000; // 10 seconds in microseconds
export function asSeconds(value: number, withUnit = true) {
const formatted = asDecimal(value / 1000000);
return `${formatted}${withUnit ? ' s' : ''}`;
}
export function asMillis(value: number, withUnit = true) {
const formatted = asInteger(value / 1000);
return `${formatted}${withUnit ? ' ms' : ''}`;
}
export function asMillisWithDefault(value?: number) {
if (value == null) {
return `N/A`;
}
return asMillis(value);
}
export const getTimeFormatter: (
max: number
) => (value: number, withUnit?: boolean) => string = memoize(
(max: number) => (max > UNIT_CUT_OFF ? asSeconds : asMillis)
);
export function timeUnit(max: number) {
return max > UNIT_CUT_OFF ? 's' : 'ms';
}
/*
* value: time in microseconds
*/
export function asTime(value: number): string {
return getTimeFormatter(value)(value);
}
export function asDecimal(value: number): string {
return numeral(value).format('0,0.0');
}
export function asInteger(value: number): string {
return numeral(value).format('0,0');
}
export function tpmUnit(type: string): string {
return type === 'request' ? 'rpm' : 'tpm';
}
export function asPercent(
numerator: number,
denominator = 0,
fallbackResult = ''
) {
if (denominator === 0) {
return fallbackResult;
}
return numeral(numerator / denominator).format('0.00%');
}

7
x-pack/plugins/apm/server/lib/transactions/distribution/get_buckets.ts
View file

@ -14,12 +14,11 @@ import {
TRANSACTION_NAME,
TRANSACTION_SAMPLED
} from '../../../../common/constants';
import { TermsAggsBucket } from '../../../../typings/elasticsearch';
import { Transaction } from '../../../../typings/Transaction';
import { Setup } from '../../helpers/setup_request';
export interface IBucket {
key: string;
key: number;
count: number;
sample?: IBucketSample;
}
@ -34,7 +33,9 @@ interface IBucketsResponse {
buckets: IBucket[];
}
interface ESBucket extends TermsAggsBucket {
interface ESBucket {
key: number;
doc_count: number;
sample: SearchResponse<{
transaction: Pick<Transaction['transaction'], 'id' | 'sampled'>;
trace: {