From 39b3408dffff0032cf8c8577fef9609c3e2f51a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Mon, 23 Jul 2018 14:15:59 +0200 Subject: [PATCH] [APM] Performance marks for RUM agent (#20931) * [APM] Performance marks for agent * Fixed formatting and design * Update snapshot * Fixed tooltip id --- x-pack/plugins/apm/common/constants.js | 2 + .../app/ErrorGroupDetails/DetailView/index.js | 7 +- .../Transaction/Spans/TimelineHeader.js | 2 +- .../Transaction/Spans/view.js | 10 +- .../Transaction/__jest__/view.test.js | 64 +++++ .../TransactionDetails/Transaction/view.js | 62 ++++- .../__snapshots__/CustomPlot.test.js.snap | 2 +- .../components/shared/charts/Legend/index.js | 10 +- .../shared/charts/Timeline/AgentMarker.js | 53 +++++ .../shared/charts/Timeline/TimelineAxis.js | 22 +- .../shared/charts/Timeline/VerticalLines.js | 21 +- .../__snapshots__/Timeline.test.js.snap | 222 +++++++++++++++--- .../charts/Timeline/__test__/props.json | 9 +- .../shared/charts/Timeline/index.js | 12 +- .../public/utils/__test__/formatters.test.js | 16 ++ x-pack/plugins/apm/public/utils/formatters.js | 5 +- 16 files changed, 454 insertions(+), 65 deletions(-) create mode 100644 x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/__jest__/view.test.js create mode 100644 x-pack/plugins/apm/public/components/shared/charts/Timeline/AgentMarker.js create mode 100644 x-pack/plugins/apm/public/utils/__test__/formatters.test.js diff --git a/x-pack/plugins/apm/common/constants.js b/x-pack/plugins/apm/common/constants.js index ef0fcfdcf71e0..7bb25a4a2813a 100644 --- a/x-pack/plugins/apm/common/constants.js +++ b/x-pack/plugins/apm/common/constants.js @@ -30,3 +30,5 @@ export const ERROR_CULPRIT = 'error.culprit'; export const ERROR_LOG_MESSAGE = 'error.log.message'; export const ERROR_EXC_MESSAGE = 'error.exception.message'; export const ERROR_EXC_HANDLED = 'error.exception.handled'; + +export const USER_ID = 'context.user.id'; diff --git a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/index.js b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/index.js index 7aa01dc20ebe4..ca137ec39a7e9 100644 --- a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/index.js +++ b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/index.js @@ -29,7 +29,8 @@ import { SERVICE_NAME, ERROR_GROUP_ID, SERVICE_AGENT_NAME, - SERVICE_LANGUAGE_NAME + SERVICE_LANGUAGE_NAME, + USER_ID } from '../../../../../common/constants'; import { fromQuery, toQuery, history } from '../../../../utils/url'; @@ -101,8 +102,8 @@ function DetailView({ errorGroup, urlParams, location }) { }, { label: 'User ID', - fieldName: 'context.user.id', - val: get(errorGroup.data, 'error.context.user.id', 'N/A') + fieldName: USER_ID, + val: get(errorGroup.data.error, USER_ID, 'N/A') } ]; diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/Spans/TimelineHeader.js b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/Spans/TimelineHeader.js index 4de924281a41c..4abea517a7a8f 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/Spans/TimelineHeader.js +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/Spans/TimelineHeader.js @@ -50,7 +50,7 @@ export default function TimelineHeader({ legends, transactionName }) { {legends.map(({ color, label }) => ( - + ))} diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/Spans/view.js b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/Spans/view.js index 6cfd086c1da6c..9d7d283043101 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/Spans/view.js +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/Spans/view.js @@ -43,7 +43,13 @@ const TIMELINE_MARGINS = { class Spans extends PureComponent { render() { - const { agentName, urlParams, location, droppedSpans } = this.props; + const { + agentName, + urlParams, + location, + droppedSpans, + agentMarks + } = this.props; return ( } + agentMarks={agentMarks} duration={totalDuration} height={timelineHeight} margins={TIMELINE_MARGINS} @@ -176,6 +183,7 @@ function getPrimaryType(type) { } Spans.propTypes = { + agentMarks: PropTypes.array, agentName: PropTypes.string.isRequired, droppedSpans: PropTypes.number.isRequired, location: PropTypes.object.isRequired, diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/__jest__/view.test.js b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/__jest__/view.test.js new file mode 100644 index 0000000000000..f26588b861615 --- /dev/null +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/__jest__/view.test.js @@ -0,0 +1,64 @@ +/* + * 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 { getAgentMarks } from '../view'; + +describe('TransactionDetailsView', () => { + describe('getAgentMarks', () => { + it('should be sorted', () => { + const transaction = { + transaction: { + marks: { + agent: { + domInteractive: 117, + timeToFirstByte: 10, + domComplete: 118 + } + } + } + }; + expect(getAgentMarks(transaction)).toEqual([ + { name: 'timeToFirstByte', timeLabel: 10000, timeAxis: 10000 }, + { name: 'domInteractive', timeLabel: 117000, timeAxis: 117000 }, + { name: 'domComplete', timeLabel: 118000, timeAxis: 118000 } + ]); + }); + + it('should ensure they are not too close', () => { + const transaction = { + transaction: { + duration: { + us: 1000 * 1000 + }, + marks: { + agent: { + a: 0, + b: 10, + c: 11, + d: 12, + e: 968, + f: 969, + timeToFirstByte: 970, + domInteractive: 980, + domComplete: 990 + } + } + } + }; + expect(getAgentMarks(transaction)).toEqual([ + { timeLabel: 0, name: 'a', timeAxis: 0 }, + { timeLabel: 10000, name: 'b', timeAxis: 20000 }, + { timeLabel: 11000, name: 'c', timeAxis: 40000 }, + { timeLabel: 12000, name: 'd', timeAxis: 60000 }, + { timeLabel: 968000, name: 'e', timeAxis: 910000 }, + { timeLabel: 969000, name: 'f', timeAxis: 930000 }, + { timeLabel: 970000, name: 'timeToFirstByte', timeAxis: 950000 }, + { timeLabel: 980000, name: 'domInteractive', timeAxis: 970000 }, + { timeLabel: 990000, name: 'domComplete', timeAxis: 990000 } + ]); + }); + }); +}); diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/view.js b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/view.js index f9e64ad300df9..1c1f47748a1d4 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/view.js +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/view.js @@ -15,7 +15,7 @@ import { borderRadius } from '../../../../style/variables'; import { Tab, HeaderMedium } from '../../../shared/UIComponents'; -import { isEmpty, capitalize, get } from 'lodash'; +import { isEmpty, capitalize, get, sortBy, last } from 'lodash'; import { ContextProperties } from '../../../shared/ContextProperties'; import { @@ -27,7 +27,10 @@ import DiscoverButton from '../../../shared/DiscoverButton'; import { TRANSACTION_ID, PROCESSOR_EVENT, - SERVICE_AGENT_NAME + SERVICE_AGENT_NAME, + TRANSACTION_DURATION, + TRANSACTION_RESULT, + USER_ID } from '../../../../../common/constants'; import { fromQuery, toQuery, history } from '../../../../utils/url'; import { asTime } from '../../../../utils/formatters'; @@ -62,6 +65,48 @@ const PropertiesTableContainer = styled.div` const DEFAULT_TAB = 'timeline'; +export function getAgentMarks(transaction) { + const duration = get(transaction, TRANSACTION_DURATION); + const threshold = duration / 100 * 2; + + return sortBy( + Object.entries(get(transaction, 'transaction.marks.agent', [])), + '1' + ) + .map(([name, ms]) => ({ + name, + timeLabel: ms * 1000, + timeAxis: ms * 1000 + })) + .reduce((acc, curItem) => { + const prevTime = get(last(acc), 'timeAxis'); + const nextValidTime = prevTime + threshold; + const isTooClose = prevTime != null && nextValidTime > curItem.timeAxis; + const canFit = nextValidTime <= duration; + + if (isTooClose && canFit) { + acc.push({ ...curItem, timeAxis: nextValidTime }); + } else { + acc.push(curItem); + } + return acc; + }, []) + .reduceRight((acc, curItem) => { + const prevTime = get(last(acc), 'timeAxis'); + const nextValidTime = prevTime - threshold; + const isTooClose = prevTime != null && nextValidTime < curItem.timeAxis; + const canFit = nextValidTime >= 0; + + if (isTooClose && canFit) { + acc.push({ ...curItem, timeAxis: nextValidTime }); + } else { + acc.push(curItem); + } + return acc; + }, []) + .reverse(); +} + // Ensure the selected tab exists or use the default function getCurrentTab(tabs = [], detailTab) { return tabs.includes(detailTab) ? detailTab : DEFAULT_TAB; @@ -86,22 +131,22 @@ function Transaction({ transaction, location, urlParams }) { const timestamp = get(transaction, '@timestamp'); const url = get(transaction, 'context.request.url.full', 'N/A'); - const duration = get(transaction, 'transaction.duration.us'); + const duration = get(transaction, TRANSACTION_DURATION); const stickyProperties = [ { label: 'Duration', - fieldName: 'transaction.duration.us', + fieldName: TRANSACTION_DURATION, val: duration ? asTime(duration) : 'N/A' }, { label: 'Result', - fieldName: 'transaction.result', - val: get(transaction, 'transaction.result', 'N/A') + fieldName: TRANSACTION_RESULT, + val: get(transaction, TRANSACTION_RESULT, 'N/A') }, { label: 'User ID', - fieldName: 'context.user.id', - val: get(transaction, 'context.user.id', 'N/A') + fieldName: USER_ID, + val: get(transaction, USER_ID, 'N/A') } ]; @@ -168,6 +213,7 @@ function Transaction({ transaction, location, urlParams }) { {currentTab === DEFAULT_TAB ? ( {text} diff --git a/x-pack/plugins/apm/public/components/shared/charts/Timeline/AgentMarker.js b/x-pack/plugins/apm/public/components/shared/charts/Timeline/AgentMarker.js new file mode 100644 index 0000000000000..a7d2c57151a61 --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/charts/Timeline/AgentMarker.js @@ -0,0 +1,53 @@ +/* + * 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 React from 'react'; +import PropTypes from 'prop-types'; +import { EuiToolTip } from '@elastic/eui'; +import Legend from '../Legend'; +import { colors, units, px } from '../../../../style/variables'; +import styled from 'styled-components'; +import { asTime } from '../../../../utils/formatters'; + +const NameContainer = styled.div` + border-bottom: 1px solid ${colors.gray3}; + padding-bottom: ${px(units.half)}; +`; + +const TimeContainer = styled.div` + color: ${colors.gray3}; + padding-top: ${px(units.half)}; +`; + +export default function AgentMarker({ agentMark, x }) { + const legendWidth = 11; + return ( +
+ + {agentMark.name} + {asTime(agentMark.timeLabel)} +
+ } + > + + + + ); +} + +AgentMarker.propTypes = { + agentMark: PropTypes.object.isRequired, + x: PropTypes.number.isRequired +}; diff --git a/x-pack/plugins/apm/public/components/shared/charts/Timeline/TimelineAxis.js b/x-pack/plugins/apm/public/components/shared/charts/Timeline/TimelineAxis.js index a84e18fb2eb27..32c9db3fe762a 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/Timeline/TimelineAxis.js +++ b/x-pack/plugins/apm/public/components/shared/charts/Timeline/TimelineAxis.js @@ -5,10 +5,12 @@ */ import React from 'react'; +import PropTypes from 'prop-types'; import _ from 'lodash'; import { Sticky } from 'react-sticky'; import { XYPlot, XAxis } from 'react-vis'; import LastTickValue from './LastTickValue'; +import AgentMarker from './AgentMarker'; import { colors, px } from '../../../../style/variables'; import { getTimeFormatter } from '../../../../utils/formatters'; @@ -16,7 +18,7 @@ import { getTimeFormatter } from '../../../../utils/formatters'; const getXAxisTickValues = (tickValues, xMax) => _.last(tickValues) * 1.05 > xMax ? tickValues.slice(0, -1) : tickValues; -function TimelineAxis({ header, plotValues }) { +function TimelineAxis({ header, plotValues, agentMarks }) { const { margins, tickValues, width, xDomain, xMax, xScale } = plotValues; const tickFormat = getTimeFormatter(xMax); const xAxisTickValues = getXAxisTickValues(tickValues, xMax); @@ -60,6 +62,14 @@ function TimelineAxis({ header, plotValues }) { /> + + {agentMarks.map(agentMark => ( + + ))} ); @@ -68,4 +78,14 @@ function TimelineAxis({ header, plotValues }) { ); } +TimelineAxis.propTypes = { + header: PropTypes.node, + plotValues: PropTypes.object.isRequired, + agentMarks: PropTypes.array +}; + +TimelineAxis.defaultProps = { + agentMarks: [] +}; + export default TimelineAxis; diff --git a/x-pack/plugins/apm/public/components/shared/charts/Timeline/VerticalLines.js b/x-pack/plugins/apm/public/components/shared/charts/Timeline/VerticalLines.js index a2f4a017502ff..317f01290f403 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/Timeline/VerticalLines.js +++ b/x-pack/plugins/apm/public/components/shared/charts/Timeline/VerticalLines.js @@ -5,10 +5,11 @@ */ import React, { PureComponent } from 'react'; +import PropTypes from 'prop-types'; import { XYPlot, VerticalGridLines } from 'react-vis'; import { colors } from '../../../../style/variables'; -export default class VerticalLines extends PureComponent { +class VerticalLines extends PureComponent { render() { const { width, @@ -19,6 +20,10 @@ export default class VerticalLines extends PureComponent { xMax } = this.props.plotValues; + const agentMarkTimes = this.props.agentMarks.map( + ({ timeAxis }) => timeAxis + ); + return (
+ @@ -47,3 +53,14 @@ export default class VerticalLines extends PureComponent { ); } } + +VerticalLines.propTypes = { + plotValues: PropTypes.object.isRequired, + agentMarks: PropTypes.array +}; + +VerticalLines.defaultProps = { + agentMarks: [] +}; + +export default VerticalLines; diff --git a/x-pack/plugins/apm/public/components/shared/charts/Timeline/__test__/__snapshots__/Timeline.test.js.snap b/x-pack/plugins/apm/public/components/shared/charts/Timeline/__test__/__snapshots__/Timeline.test.js.snap index 1e3efb26f3971..3ba61791679f8 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/Timeline/__test__/__snapshots__/Timeline.test.js.snap +++ b/x-pack/plugins/apm/public/components/shared/charts/Timeline/__test__/__snapshots__/Timeline.test.js.snap @@ -1,6 +1,33 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`Timeline should render with data 1`] = ` +.c0 { + display: -webkit-box; + display: -webkit-flex; + display: -ms-flexbox; + display: flex; + -webkit-align-items: center; + -webkit-box-align: center; + -ms-flex-align: center; + align-items: center; + font-size: 12px; + color: #666666; + cursor: pointer; + opacity: 1; + -webkit-user-select: none; + -moz-user-select: none; + -ms-user-select: none; + user-select: none; +} + +.c1 { + width: 11px; + height: 11px; + margin-right: 5.5px; + background: #999999; + border-radius: 100%; +} +
@@ -52,7 +79,7 @@ exports[`Timeline should render with data 1`] = ` onMouseLeave={[Function]} onMouseMove={[Function]} onWheel={[Function]} - width={100} + width={1000} > +
+ +
+ +
+ +
+
+ +
+ +
+ +
+
+ +
+ +
+ +
@@ -520,7 +634,7 @@ exports[`Timeline should render with data 1`] = ` style={ Object { "height": "216px", - "width": "100px", + "width": "1000px", } } > @@ -534,7 +648,7 @@ exports[`Timeline should render with data 1`] = ` onMouseLeave={[Function]} onMouseMove={[Function]} onWheel={[Function]} - width={100} + width={1000} > @@ -571,8 +685,8 @@ exports[`Timeline should render with data 1`] = ` "stroke": "#f5f5f5", } } - x1={0} - x2={0} + x1={175.8817293082474} + x2={175.8817293082474} y1={0} y2={116} /> @@ -583,8 +697,8 @@ exports[`Timeline should render with data 1`] = ` "stroke": "#f5f5f5", } } - x1={0} - x2={0} + x1={263.82259396237106} + x2={263.82259396237106} y1={0} y2={116} /> @@ -595,8 +709,8 @@ exports[`Timeline should render with data 1`] = ` "stroke": "#f5f5f5", } } - x1={0} - x2={0} + x1={351.7634586164948} + x2={351.7634586164948} y1={0} y2={116} /> @@ -607,8 +721,8 @@ exports[`Timeline should render with data 1`] = ` "stroke": "#f5f5f5", } } - x1={0} - x2={0} + x1={439.7043232706185} + x2={439.7043232706185} y1={0} y2={116} /> @@ -619,8 +733,8 @@ exports[`Timeline should render with data 1`] = ` "stroke": "#f5f5f5", } } - x1={0} - x2={0} + x1={527.6451879247421} + x2={527.6451879247421} y1={0} y2={116} /> @@ -631,8 +745,8 @@ exports[`Timeline should render with data 1`] = ` "stroke": "#f5f5f5", } } - x1={0} - x2={0} + x1={615.5860525788659} + x2={615.5860525788659} y1={0} y2={116} /> @@ -643,8 +757,8 @@ exports[`Timeline should render with data 1`] = ` "stroke": "#f5f5f5", } } - x1={0} - x2={0} + x1={703.5269172329896} + x2={703.5269172329896} y1={0} y2={116} /> @@ -655,8 +769,8 @@ exports[`Timeline should render with data 1`] = ` "stroke": "#f5f5f5", } } - x1={0} - x2={0} + x1={791.4677818871132} + x2={791.4677818871132} y1={0} y2={116} /> @@ -667,8 +781,8 @@ exports[`Timeline should render with data 1`] = ` "stroke": "#f5f5f5", } } - x1={0} - x2={0} + x1={879.408646541237} + x2={879.408646541237} y1={0} y2={116} /> @@ -684,8 +798,44 @@ exports[`Timeline should render with data 1`] = ` "stroke": "#999999", } } - x1={0} - x2={0} + x1={439.7043232706185} + x2={439.7043232706185} + y1={0} + y2={116} + /> + + + diff --git a/x-pack/plugins/apm/public/components/shared/charts/Timeline/__test__/props.json b/x-pack/plugins/apm/public/components/shared/charts/Timeline/__test__/props.json index b59a4ac6f515e..986a57c730302 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/Timeline/__test__/props.json +++ b/x-pack/plugins/apm/public/components/shared/charts/Timeline/__test__/props.json @@ -1,5 +1,5 @@ { - "width": 100, + "width": 1000, "duration": 204683, "height": 116, "margins": { @@ -8,5 +8,10 @@ "right": 50, "bottom": 0 }, - "animation": null + "animation": null, + "agentMarks": [ + { "timeLabel": 100000, "name": "timeToFirstByte", "timeAxis": 100000 }, + { "timeLabel": 110000, "name": "domInteractive", "timeAxis": 110000 }, + { "timeLabel": 190000, "name": "domComplete", "timeAxis": 190000 } + ] } diff --git a/x-pack/plugins/apm/public/components/shared/charts/Timeline/index.js b/x-pack/plugins/apm/public/components/shared/charts/Timeline/index.js index fd0be03d89d6b..99a88cdd0caf5 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/Timeline/index.js +++ b/x-pack/plugins/apm/public/components/shared/charts/Timeline/index.js @@ -23,8 +23,7 @@ class Timeline extends PureComponent { ); render() { - const { width, duration, header } = this.props; - + const { width, duration, header, agentMarks } = this.props; if (duration == null || !width) { return null; } @@ -33,14 +32,19 @@ class Timeline extends PureComponent { return (
- - + +
); } } Timeline.propTypes = { + agentMarks: PropTypes.array, duration: PropTypes.number.isRequired, height: PropTypes.number.isRequired, header: PropTypes.node, diff --git a/x-pack/plugins/apm/public/utils/__test__/formatters.test.js b/x-pack/plugins/apm/public/utils/__test__/formatters.test.js new file mode 100644 index 0000000000000..1bd7a4a4e493a --- /dev/null +++ b/x-pack/plugins/apm/public/utils/__test__/formatters.test.js @@ -0,0 +1,16 @@ +/* + * 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'); + }); +}); diff --git a/x-pack/plugins/apm/public/utils/formatters.js b/x-pack/plugins/apm/public/utils/formatters.js index 6960ba3218318..36b867c4606e4 100644 --- a/x-pack/plugins/apm/public/utils/formatters.js +++ b/x-pack/plugins/apm/public/utils/formatters.js @@ -7,7 +7,7 @@ import { memoize } from 'lodash'; import numeral from '@elastic/numeral'; -const UNIT_CUT_OFF = 10 * 1000000; +const UNIT_CUT_OFF = 10 * 1000000; // 10 seconds in microseconds export function asSeconds(value, withUnit = true) { const formatted = asDecimal(value / 1000000); @@ -34,6 +34,9 @@ export function timeUnit(max) { return max > UNIT_CUT_OFF ? 's' : 'ms'; } +/* + * value: time in microseconds + */ export function asTime(value) { return getTimeFormatter(value)(value); }