Skip to content

Commit

Permalink
[APM] Changes to duration formatting (#69039)
Browse files Browse the repository at this point in the history
* [APM] Changes to duration formatting

* Change the threshold for showing microseconds to 1 millisecond instead of 10. This means you now get "900 µs/1.3 ms/20.0 ms" instead of "900 µs/1300 µs/20 ms."
* Change milliseconds formatted with `asDuration` to be `asDecimal` instead of `asInteger`. That means you get "0.0 ms/2.5 ms/3.0 ms" instead of "0 ms/2 ms/3 ms."
* Tables were all using their own module-scoped functions that called `asDuration` to format things as decimal milliseconds. Extract this to an `asMillisecondDuration` function exported from the duration helpers and use it in all the tables.
* Change `getResponseTimeseries` in chart selectors to use `asDuration` to make all chart timeseries units consistent.
* Don't export `convertTo` from the duration helpers as it's now not used anywhere. Always use a more specific exported function for more consistency.
* Change ">=" to "≥" in the ML flyout text.

* Update e2e snapshot

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
smith and elasticmachine committed Jun 15, 2020
1 parent 661fd85 commit be551dd
Show file tree
Hide file tree
Showing 15 changed files with 59 additions and 157 deletions.
16 changes: 8 additions & 8 deletions x-pack/plugins/apm/e2e/cypress/integration/snapshots.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
module.exports = {
"APM": {
"Transaction duration charts": {
"1": "350 ms",
"2": "175 ms",
"3": "0 ms"
}
APM: {
'Transaction duration charts': {
'1': '350.0 ms',
'2': '175.0 ms',
'3': '0.0 ms',
},
},
"__version": "4.5.0"
}
__version: '4.5.0',
};
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('ServiceOverview -> List', () => {
expect(renderedColumns[0]).toMatchSnapshot();
expect(renderedColumns.slice(2)).toEqual([
'python',
'92 ms',
'91.5 ms',
'86.9 tpm',
'12.6 err.',
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import styled from 'styled-components';
import { ServiceListAPIResponse } from '../../../../../server/lib/services/get_services';
import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n';
import { fontSizes, truncate } from '../../../../style/variables';
import { asDecimal, convertTo } from '../../../../utils/formatters';
import { asDecimal, asMillisecondDuration } from '../../../../utils/formatters';
import { ManagedTable } from '../../../shared/ManagedTable';
import { EnvironmentBadge } from '../../../shared/EnvironmentBadge';
import { TransactionOverviewLink } from '../../../shared/Links/apm/TransactionOverviewLink';
Expand Down Expand Up @@ -81,11 +81,7 @@ export const SERVICE_COLUMNS = [
}),
sortable: true,
dataType: 'number',
render: (time: number) =>
convertTo({
unit: 'milliseconds',
microseconds: time,
}).formatted,
render: (time: number) => asMillisecondDuration(time),
},
{
field: 'transactionsPerMinute',
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import styled from 'styled-components';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { ITransactionGroup } from '../../../../server/lib/transaction_groups/transform';
import { fontSizes, truncate } from '../../../style/variables';
import { convertTo } from '../../../utils/formatters';
import { asMillisecondDuration } from '../../../utils/formatters';
import { EmptyMessage } from '../../shared/EmptyMessage';
import { ImpactBar } from '../../shared/ImpactBar';
import { TransactionDetailLink } from '../../shared/Links/apm/TransactionDetailLink';
Expand Down Expand Up @@ -67,11 +67,7 @@ const traceListColumns: Array<ITableColumn<ITransactionGroup>> = [
}),
sortable: true,
dataType: 'number',
render: (time: number) =>
convertTo({
unit: 'milliseconds',
microseconds: time,
}).formatted,
render: (time: number) => asMillisecondDuration(time),
},
{
field: 'transactionsPerMinute',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { NOT_AVAILABLE_LABEL } from '../../../../../common/i18n';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { ITransactionGroup } from '../../../../../server/lib/transaction_groups/transform';
import { fontFamilyCode, truncate } from '../../../../style/variables';
import { asDecimal, convertTo } from '../../../../utils/formatters';
import { asDecimal, asMillisecondDuration } from '../../../../utils/formatters';
import { ImpactBar } from '../../../shared/ImpactBar';
import { ITableColumn, ManagedTable } from '../../../shared/ManagedTable';
import { LoadingStatePrompt } from '../../../shared/LoadingStatePrompt';
Expand All @@ -29,12 +29,6 @@ interface Props {
isLoading: boolean;
}

const toMilliseconds = (time: number) =>
convertTo({
unit: 'milliseconds',
microseconds: time,
}).formatted;

export function TransactionList({ items, isLoading }: Props) {
const columns: Array<ITableColumn<ITransactionGroup>> = useMemo(
() => [
Expand Down Expand Up @@ -74,7 +68,7 @@ export function TransactionList({ items, isLoading }: Props) {
),
sortable: true,
dataType: 'number',
render: (time: number) => toMilliseconds(time),
render: (time: number) => asMillisecondDuration(time),
},
{
field: 'p95',
Expand All @@ -86,7 +80,7 @@ export function TransactionList({ items, isLoading }: Props) {
),
sortable: true,
dataType: 'number',
render: (time: number) => toMilliseconds(time),
render: (time: number) => asMillisecondDuration(time),
},
{
field: 'transactionsPerMinute',
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('Histogram', () => {
const tooltips = wrapper.find('Tooltip');

expect(tooltips.length).toBe(1);
expect(tooltips.prop('header')).toBe('811 - 927 ms');
expect(tooltips.prop('header')).toBe('811.1 - 926.9 ms');
expect(tooltips.prop('tooltipPoints')).toEqual([
{ value: '49.0 occurrences' },
]);
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('ErrorMarker', () => {
act(() => {
fireEvent.click(component.getByTestId('popover'));
});
expectTextsInDocument(component, ['10,000 μs']);
expectTextsInDocument(component, ['10.0 ms']);
return component;
}
function getKueryDecoded(url: string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export class TransactionCharts extends Component<TransactionChartProps> {
'xpack.apm.metrics.transactionChart.machineLearningTooltip',
{
defaultMessage:
'The stream around the average duration shows the expected bounds. An annotation is shown for anomaly scores >= 75.',
'The stream around the average duration shows the expected bounds. An annotation is shown for anomaly scores 75.',
}
)}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('chartSelectors', () => {
{ x: 0, y: 100 },
{ x: 1000, y: 200 },
],
legendValue: '0 ms',
legendValue: '200 μs',
title: 'Avg.',
type: 'linemark',
},
Expand Down
7 changes: 2 additions & 5 deletions x-pack/plugins/apm/public/selectors/chartSelectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
RectCoordinate,
TimeSeries,
} from '../../typings/timeseries';
import { asDecimal, tpmUnit, convertTo } from '../utils/formatters';
import { asDecimal, asDuration, tpmUnit } from '../utils/formatters';
import { IUrlParams } from '../context/UrlParamsContext/types';
import { getEmptySeries } from '../components/shared/charts/CustomPlot/getEmptySeries';
import { httpStatusCodeToColor } from '../utils/httpStatusCodeToColor';
Expand Down Expand Up @@ -72,10 +72,7 @@ export function getResponseTimeSeries({
}: TimeSeriesAPIResponse) {
const { overallAvgDuration } = apmTimeseries;
const { avg, p95, p99 } = apmTimeseries.responseTimes;
const formattedDuration = convertTo({
unit: 'milliseconds',
microseconds: overallAvgDuration,
}).formatted;
const formattedDuration = asDuration(overallAvgDuration);

const series: TimeSeries[] = [
{
Expand Down
Loading

0 comments on commit be551dd

Please sign in to comment.