Skip to content

Commit bfafc4b

Browse files
committed
[APM] Show errors on the timeline instead of under the transaction (#53756)
* creating error marker and refactoring some stuff * styling popover * adding agent marks and errors to waterfall items * adding agent marks and errors to waterfall items * adding agent marks and errors to waterfall items * fixing tests and typescript checking * refactoring helper * changing transaction error badge style * adding unit test * fixing agent marker position * fixing offset when error is registered before its parent * refactoring error marker * refactoring error marker * refactoring error marker * refactoring error marker * refactoring error marker * refactoring waterfall helper * refactoring waterfall helper * refactoring waterfall helper api * refactoring waterfall helper * removing unused code * refactoring waterfall helper * changing unit test * removing comment * refactoring marker component and waterfall helper * removing servicecolor from waterfall item and adding it to errormark * fixing trace order
1 parent 3f26145 commit bfafc4b

48 files changed

Lines changed: 2575 additions & 1339 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { EuiText, EuiTextColor } from '@elastic/eui';
8+
import { i18n } from '@kbn/i18n';
9+
import React from 'react';
10+
11+
interface Props {
12+
count: number;
13+
}
14+
15+
export const ErrorCount = ({ count }: Props) => (
16+
<EuiText size="xs">
17+
<h4>
18+
<EuiTextColor
19+
color="danger"
20+
onClick={(e: React.MouseEvent) => {
21+
e.stopPropagation();
22+
}}
23+
>
24+
{i18n.translate('xpack.apm.transactionDetails.errorCount', {
25+
defaultMessage:
26+
'{errorCount, number} {errorCount, plural, one {Error} other {Errors}}',
27+
values: { errorCount: count }
28+
})}
29+
</EuiTextColor>
30+
</h4>
31+
</EuiText>
32+
);

x-pack/legacy/plugins/apm/public/components/app/TransactionDetails/WaterfallWithSummmary/ErrorCountBadge.tsx

Lines changed: 0 additions & 17 deletions
This file was deleted.

x-pack/legacy/plugins/apm/public/components/app/TransactionDetails/WaterfallWithSummmary/MaybeViewTraceLink.tsx

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,9 @@ export const MaybeViewTraceLink = ({
2525
}
2626
);
2727

28+
const { rootTransaction } = waterfall;
2829
// the traceroot cannot be found, so we cannot link to it
29-
if (!waterfall.traceRoot) {
30+
if (!rootTransaction) {
3031
return (
3132
<EuiFlexItem grow={false}>
3233
<EuiToolTip
@@ -45,8 +46,7 @@ export const MaybeViewTraceLink = ({
4546
);
4647
}
4748

48-
const isRoot =
49-
transaction.transaction.id === waterfall.traceRoot.transaction.id;
49+
const isRoot = transaction.transaction.id === rootTransaction.transaction.id;
5050

5151
// the user is already viewing the full trace, so don't link to it
5252
if (isRoot) {
@@ -69,15 +69,14 @@ export const MaybeViewTraceLink = ({
6969

7070
// the user is viewing a zoomed in version of the trace. Link to the full trace
7171
} else {
72-
const traceRoot = waterfall.traceRoot;
7372
return (
7473
<EuiFlexItem grow={false}>
7574
<TransactionDetailLink
76-
serviceName={traceRoot.service.name}
77-
transactionId={traceRoot.transaction.id}
78-
traceId={traceRoot.trace.id}
79-
transactionName={traceRoot.transaction.name}
80-
transactionType={traceRoot.transaction.type}
75+
serviceName={rootTransaction.service.name}
76+
transactionId={rootTransaction.transaction.id}
77+
traceId={rootTransaction.trace.id}
78+
transactionName={rootTransaction.transaction.name}
79+
transactionType={rootTransaction.transaction.type}
8180
>
8281
<EuiButton iconType="apmTrace">{viewFullTraceButtonLabel}</EuiButton>
8382
</TransactionDetailLink>

x-pack/legacy/plugins/apm/public/components/app/TransactionDetails/WaterfallWithSummmary/TransactionTabs.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ export function TransactionTabs({
7777

7878
{currentTab.key === timelineTab.key ? (
7979
<WaterfallContainer
80-
transaction={transaction}
8180
location={location}
8281
urlParams={urlParams}
8382
waterfall={waterfall}

x-pack/legacy/plugins/apm/public/components/app/TransactionDetails/WaterfallWithSummmary/WaterfallContainer/get_agent_marks.test.ts renamed to x-pack/legacy/plugins/apm/public/components/app/TransactionDetails/WaterfallWithSummmary/WaterfallContainer/Marks/__test__/get_agent_marks.test.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import { Transaction } from '../../../../../../typings/es_schemas/ui/Transaction';
8-
import { getAgentMarks } from './get_agent_marks';
7+
import { Transaction } from '../../../../../../../../typings/es_schemas/ui/Transaction';
8+
import { getAgentMarks } from '../get_agent_marks';
99

1010
describe('getAgentMarks', () => {
1111
it('should sort the marks by time', () => {
@@ -21,9 +21,24 @@ describe('getAgentMarks', () => {
2121
}
2222
} as any;
2323
expect(getAgentMarks(transaction)).toEqual([
24-
{ name: 'timeToFirstByte', us: 10000 },
25-
{ name: 'domInteractive', us: 117000 },
26-
{ name: 'domComplete', us: 118000 }
24+
{
25+
id: 'timeToFirstByte',
26+
offset: 10000,
27+
type: 'agentMark',
28+
verticalLine: true
29+
},
30+
{
31+
id: 'domInteractive',
32+
offset: 117000,
33+
type: 'agentMark',
34+
verticalLine: true
35+
},
36+
{
37+
id: 'domComplete',
38+
offset: 118000,
39+
type: 'agentMark',
40+
verticalLine: true
41+
}
2742
]);
2843
});
2944

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { IWaterfallItem } from '../../Waterfall/waterfall_helpers/waterfall_helpers';
8+
import { getErrorMarks } from '../get_error_marks';
9+
10+
describe('getErrorMarks', () => {
11+
describe('returns empty array', () => {
12+
it('when items are missing', () => {
13+
expect(getErrorMarks([], {})).toEqual([]);
14+
});
15+
it('when any error is available', () => {
16+
const items = [
17+
{ docType: 'span' },
18+
{ docType: 'transaction' }
19+
] as IWaterfallItem[];
20+
expect(getErrorMarks(items, {})).toEqual([]);
21+
});
22+
});
23+
24+
it('returns error marks', () => {
25+
const items = [
26+
{
27+
docType: 'error',
28+
offset: 10,
29+
skew: 5,
30+
doc: { error: { id: 1 }, service: { name: 'opbeans-java' } }
31+
} as unknown,
32+
{ docType: 'transaction' },
33+
{
34+
docType: 'error',
35+
offset: 50,
36+
skew: 0,
37+
doc: { error: { id: 2 }, service: { name: 'opbeans-node' } }
38+
} as unknown
39+
] as IWaterfallItem[];
40+
expect(
41+
getErrorMarks(items, { 'opbeans-java': 'red', 'opbeans-node': 'blue' })
42+
).toEqual([
43+
{
44+
type: 'errorMark',
45+
offset: 15,
46+
verticalLine: false,
47+
id: 1,
48+
error: { error: { id: 1 }, service: { name: 'opbeans-java' } },
49+
serviceColor: 'red'
50+
},
51+
{
52+
type: 'errorMark',
53+
offset: 50,
54+
verticalLine: false,
55+
id: 2,
56+
error: { error: { id: 2 }, service: { name: 'opbeans-node' } },
57+
serviceColor: 'blue'
58+
}
59+
]);
60+
});
61+
62+
it('returns error marks without service color', () => {
63+
const items = [
64+
{
65+
docType: 'error',
66+
offset: 10,
67+
skew: 5,
68+
doc: { error: { id: 1 }, service: { name: 'opbeans-java' } }
69+
} as unknown,
70+
{ docType: 'transaction' },
71+
{
72+
docType: 'error',
73+
offset: 50,
74+
skew: 0,
75+
doc: { error: { id: 2 }, service: { name: 'opbeans-node' } }
76+
} as unknown
77+
] as IWaterfallItem[];
78+
expect(getErrorMarks(items, {})).toEqual([
79+
{
80+
type: 'errorMark',
81+
offset: 15,
82+
verticalLine: false,
83+
id: 1,
84+
error: { error: { id: 1 }, service: { name: 'opbeans-java' } },
85+
serviceColor: undefined
86+
},
87+
{
88+
type: 'errorMark',
89+
offset: 50,
90+
verticalLine: false,
91+
id: 2,
92+
error: { error: { id: 2 }, service: { name: 'opbeans-node' } },
93+
serviceColor: undefined
94+
}
95+
]);
96+
});
97+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { sortBy } from 'lodash';
8+
import { Transaction } from '../../../../../../../typings/es_schemas/ui/Transaction';
9+
import { Mark } from '.';
10+
11+
// Extends Mark without adding new properties to it.
12+
export interface AgentMark extends Mark {
13+
type: 'agentMark';
14+
}
15+
16+
export function getAgentMarks(transaction?: Transaction): AgentMark[] {
17+
const agent = transaction?.transaction.marks?.agent;
18+
if (!agent) {
19+
return [];
20+
}
21+
22+
return sortBy(
23+
Object.entries(agent).map(([name, ms]) => ({
24+
type: 'agentMark',
25+
id: name,
26+
offset: ms * 1000,
27+
verticalLine: true
28+
})),
29+
'offset'
30+
);
31+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
import { isEmpty } from 'lodash';
7+
import { ErrorRaw } from '../../../../../../../typings/es_schemas/raw/ErrorRaw';
8+
import {
9+
IWaterfallItem,
10+
IWaterfallError,
11+
IServiceColors
12+
} from '../Waterfall/waterfall_helpers/waterfall_helpers';
13+
import { Mark } from '.';
14+
15+
export interface ErrorMark extends Mark {
16+
type: 'errorMark';
17+
error: ErrorRaw;
18+
serviceColor?: string;
19+
}
20+
21+
export const getErrorMarks = (
22+
items: IWaterfallItem[],
23+
serviceColors: IServiceColors
24+
): ErrorMark[] => {
25+
if (isEmpty(items)) {
26+
return [];
27+
}
28+
29+
return (items.filter(
30+
item => item.docType === 'error'
31+
) as IWaterfallError[]).map(error => ({
32+
type: 'errorMark',
33+
offset: error.offset + error.skew,
34+
verticalLine: false,
35+
id: error.doc.error.id,
36+
error: error.doc,
37+
serviceColor: serviceColors[error.doc.service.name]
38+
}));
39+
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
export interface Mark {
8+
type: string;
9+
offset: number;
10+
verticalLine: boolean;
11+
id: string;
12+
}

x-pack/legacy/plugins/apm/public/components/app/TransactionDetails/WaterfallWithSummmary/WaterfallContainer/ServiceLegends.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import React from 'react';
1010
import styled from 'styled-components';
1111
import { px, unit } from '../../../../../style/variables';
1212
// @ts-ignore
13-
import Legend from '../../../../shared/charts/Legend';
13+
import { Legend } from '../../../../shared/charts/Legend';
1414
import { IServiceColors } from './Waterfall/waterfall_helpers/waterfall_helpers';
1515

1616
const Legends = styled.div`

0 commit comments

Comments
 (0)