Skip to content

Commit

Permalink
[APM] Show errors on the timeline instead of under the transaction (#…
Browse files Browse the repository at this point in the history
…53756) (#54109)

* 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
  • Loading branch information
cauemarcondes authored Jan 7, 2020
1 parent 3f26145 commit 892dd5d
Show file tree
Hide file tree
Showing 48 changed files with 2,575 additions and 1,339 deletions.
Original file line number Diff line number Diff line change
@@ -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 { EuiText, EuiTextColor } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';

interface Props {
count: number;
}

export const ErrorCount = ({ count }: Props) => (
<EuiText size="xs">
<h4>
<EuiTextColor
color="danger"
onClick={(e: React.MouseEvent) => {
e.stopPropagation();
}}
>
{i18n.translate('xpack.apm.transactionDetails.errorCount', {
defaultMessage:
'{errorCount, number} {errorCount, plural, one {Error} other {Errors}}',
values: { errorCount: count }
})}
</EuiTextColor>
</h4>
</EuiText>
);

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ export const MaybeViewTraceLink = ({
}
);

const { rootTransaction } = waterfall;
// the traceroot cannot be found, so we cannot link to it
if (!waterfall.traceRoot) {
if (!rootTransaction) {
return (
<EuiFlexItem grow={false}>
<EuiToolTip
Expand All @@ -45,8 +46,7 @@ export const MaybeViewTraceLink = ({
);
}

const isRoot =
transaction.transaction.id === waterfall.traceRoot.transaction.id;
const isRoot = transaction.transaction.id === rootTransaction.transaction.id;

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

// the user is viewing a zoomed in version of the trace. Link to the full trace
} else {
const traceRoot = waterfall.traceRoot;
return (
<EuiFlexItem grow={false}>
<TransactionDetailLink
serviceName={traceRoot.service.name}
transactionId={traceRoot.transaction.id}
traceId={traceRoot.trace.id}
transactionName={traceRoot.transaction.name}
transactionType={traceRoot.transaction.type}
serviceName={rootTransaction.service.name}
transactionId={rootTransaction.transaction.id}
traceId={rootTransaction.trace.id}
transactionName={rootTransaction.transaction.name}
transactionType={rootTransaction.transaction.type}
>
<EuiButton iconType="apmTrace">{viewFullTraceButtonLabel}</EuiButton>
</TransactionDetailLink>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ export function TransactionTabs({

{currentTab.key === timelineTab.key ? (
<WaterfallContainer
transaction={transaction}
location={location}
urlParams={urlParams}
waterfall={waterfall}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Transaction } from '../../../../../../typings/es_schemas/ui/Transaction';
import { getAgentMarks } from './get_agent_marks';
import { Transaction } from '../../../../../../../../typings/es_schemas/ui/Transaction';
import { getAgentMarks } from '../get_agent_marks';

describe('getAgentMarks', () => {
it('should sort the marks by time', () => {
Expand All @@ -21,9 +21,24 @@ describe('getAgentMarks', () => {
}
} as any;
expect(getAgentMarks(transaction)).toEqual([
{ name: 'timeToFirstByte', us: 10000 },
{ name: 'domInteractive', us: 117000 },
{ name: 'domComplete', us: 118000 }
{
id: 'timeToFirstByte',
offset: 10000,
type: 'agentMark',
verticalLine: true
},
{
id: 'domInteractive',
offset: 117000,
type: 'agentMark',
verticalLine: true
},
{
id: 'domComplete',
offset: 118000,
type: 'agentMark',
verticalLine: true
}
]);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*
* 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 { IWaterfallItem } from '../../Waterfall/waterfall_helpers/waterfall_helpers';
import { getErrorMarks } from '../get_error_marks';

describe('getErrorMarks', () => {
describe('returns empty array', () => {
it('when items are missing', () => {
expect(getErrorMarks([], {})).toEqual([]);
});
it('when any error is available', () => {
const items = [
{ docType: 'span' },
{ docType: 'transaction' }
] as IWaterfallItem[];
expect(getErrorMarks(items, {})).toEqual([]);
});
});

it('returns error marks', () => {
const items = [
{
docType: 'error',
offset: 10,
skew: 5,
doc: { error: { id: 1 }, service: { name: 'opbeans-java' } }
} as unknown,
{ docType: 'transaction' },
{
docType: 'error',
offset: 50,
skew: 0,
doc: { error: { id: 2 }, service: { name: 'opbeans-node' } }
} as unknown
] as IWaterfallItem[];
expect(
getErrorMarks(items, { 'opbeans-java': 'red', 'opbeans-node': 'blue' })
).toEqual([
{
type: 'errorMark',
offset: 15,
verticalLine: false,
id: 1,
error: { error: { id: 1 }, service: { name: 'opbeans-java' } },
serviceColor: 'red'
},
{
type: 'errorMark',
offset: 50,
verticalLine: false,
id: 2,
error: { error: { id: 2 }, service: { name: 'opbeans-node' } },
serviceColor: 'blue'
}
]);
});

it('returns error marks without service color', () => {
const items = [
{
docType: 'error',
offset: 10,
skew: 5,
doc: { error: { id: 1 }, service: { name: 'opbeans-java' } }
} as unknown,
{ docType: 'transaction' },
{
docType: 'error',
offset: 50,
skew: 0,
doc: { error: { id: 2 }, service: { name: 'opbeans-node' } }
} as unknown
] as IWaterfallItem[];
expect(getErrorMarks(items, {})).toEqual([
{
type: 'errorMark',
offset: 15,
verticalLine: false,
id: 1,
error: { error: { id: 1 }, service: { name: 'opbeans-java' } },
serviceColor: undefined
},
{
type: 'errorMark',
offset: 50,
verticalLine: false,
id: 2,
error: { error: { id: 2 }, service: { name: 'opbeans-node' } },
serviceColor: undefined
}
]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* 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 { sortBy } from 'lodash';
import { Transaction } from '../../../../../../../typings/es_schemas/ui/Transaction';
import { Mark } from '.';

// Extends Mark without adding new properties to it.
export interface AgentMark extends Mark {
type: 'agentMark';
}

export function getAgentMarks(transaction?: Transaction): AgentMark[] {
const agent = transaction?.transaction.marks?.agent;
if (!agent) {
return [];
}

return sortBy(
Object.entries(agent).map(([name, ms]) => ({
type: 'agentMark',
id: name,
offset: ms * 1000,
verticalLine: true
})),
'offset'
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* 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 { isEmpty } from 'lodash';
import { ErrorRaw } from '../../../../../../../typings/es_schemas/raw/ErrorRaw';
import {
IWaterfallItem,
IWaterfallError,
IServiceColors
} from '../Waterfall/waterfall_helpers/waterfall_helpers';
import { Mark } from '.';

export interface ErrorMark extends Mark {
type: 'errorMark';
error: ErrorRaw;
serviceColor?: string;
}

export const getErrorMarks = (
items: IWaterfallItem[],
serviceColors: IServiceColors
): ErrorMark[] => {
if (isEmpty(items)) {
return [];
}

return (items.filter(
item => item.docType === 'error'
) as IWaterfallError[]).map(error => ({
type: 'errorMark',
offset: error.offset + error.skew,
verticalLine: false,
id: error.doc.error.id,
error: error.doc,
serviceColor: serviceColors[error.doc.service.name]
}));
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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.
*/

export interface Mark {
type: string;
offset: number;
verticalLine: boolean;
id: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import React from 'react';
import styled from 'styled-components';
import { px, unit } from '../../../../../style/variables';
// @ts-ignore
import Legend from '../../../../shared/charts/Legend';
import { Legend } from '../../../../shared/charts/Legend';
import { IServiceColors } from './Waterfall/waterfall_helpers/waterfall_helpers';

const Legends = styled.div`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export function SpanFlyout({
const dbContext = span.span.db;
const httpContext = span.span.http;
const spanTypes = getSpanTypes(span);
const spanHttpStatusCode = httpContext?.response.status_code;
const spanHttpStatusCode = httpContext?.response?.status_code;
const spanHttpUrl = httpContext?.url?.original;
const spanHttpMethod = httpContext?.method;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import { DroppedSpansWarning } from './DroppedSpansWarning';
interface Props {
onClose: () => void;
transaction?: Transaction;
errorCount: number;
traceRootDuration?: number;
errorCount?: number;
rootTransactionDuration?: number;
}

function TransactionPropertiesTable({
Expand All @@ -49,8 +49,8 @@ function TransactionPropertiesTable({
export function TransactionFlyout({
transaction: transactionDoc,
onClose,
errorCount,
traceRootDuration
errorCount = 0,
rootTransactionDuration
}: Props) {
if (!transactionDoc) {
return null;
Expand Down Expand Up @@ -84,7 +84,7 @@ export function TransactionFlyout({
<EuiSpacer size="m" />
<TransactionSummary
transaction={transactionDoc}
totalDuration={traceRootDuration}
totalDuration={rootTransactionDuration}
errorCount={errorCount}
/>
<EuiHorizontalRule margin="m" />
Expand Down
Loading

0 comments on commit 892dd5d

Please sign in to comment.