diff --git a/src/components/TracePage/TraceSpanGraph.js b/src/components/TracePage/TraceSpanGraph.js index 74099aba18..c9bbfd9540 100644 --- a/src/components/TracePage/TraceSpanGraph.js +++ b/src/components/TracePage/TraceSpanGraph.js @@ -25,7 +25,6 @@ import PropTypes from 'prop-types'; import SpanGraph from './SpanGraph'; import SpanGraphTickHeader from './SpanGraph/SpanGraphTickHeader'; import TimelineScrubber from './TimelineScrubber'; -import { getTraceId, getTraceTimestamp, getTraceEndTimestamp, getTraceDuration } from '../../selectors/trace'; import { getPercentageOfInterval } from '../../utils/date'; const TIMELINE_TICK_INTERVAL = 4; @@ -33,7 +32,6 @@ const TIMELINE_TICK_INTERVAL = 4; export default class TraceSpanGraph extends Component { static get propTypes() { return { - xformedTrace: PropTypes.object, trace: PropTypes.object, height: PropTypes.number.isRequired, }; @@ -71,7 +69,7 @@ export default class TraceSpanGraph extends Component { const newRightBound = newTimeRangeFilter[1]; return ( - getTraceId(trace) !== getTraceId(newTrace) || + trace.traceID !== newTrace.traceID || leftBound !== newLeftBound || rightBound !== newRightBound || currentlyDragging !== newCurrentlyDragging @@ -109,19 +107,16 @@ export default class TraceSpanGraph extends Component { let rightBound = timeRangeFilter[1]; const deltaX = (clientX - prevX) / this.svg.clientWidth; - const timestamp = getTraceTimestamp(trace); - const endTimestamp = getTraceEndTimestamp(trace); - const duration = getTraceDuration(this.props.trace); const prevValue = { leftBound, rightBound }[currentlyDragging]; - const newValue = prevValue + duration * deltaX; + const newValue = prevValue + trace.duration * deltaX; // enforce the edges of the graph switch (currentlyDragging) { case 'leftBound': - leftBound = Math.max(timestamp, newValue); + leftBound = Math.max(trace.startTime, newValue); break; case 'rightBound': - rightBound = Math.min(endTimestamp, newValue); + rightBound = Math.min(trace.endTime, newValue); break; /* istanbul ignore next */ default: break; @@ -134,7 +129,7 @@ export default class TraceSpanGraph extends Component { } render() { - const { trace, xformedTrace, height } = this.props; + const { trace, height } = this.props; const { currentlyDragging } = this.state; const { timeRangeFilter } = this.context; const leftBound = timeRangeFilter[0]; @@ -144,23 +139,20 @@ export default class TraceSpanGraph extends Component { return
; } - const initialTimestamp = getTraceTimestamp(trace); - const traceDuration = getTraceDuration(trace); - let leftInactive; if (leftBound) { - leftInactive = getPercentageOfInterval(leftBound, initialTimestamp, traceDuration); + leftInactive = getPercentageOfInterval(leftBound, trace.startTime, trace.duration); } let rightInactive; if (rightBound) { - rightInactive = 100 - getPercentageOfInterval(rightBound, initialTimestamp, traceDuration); + rightInactive = 100 - getPercentageOfInterval(rightBound, trace.startTime, trace.duration); } return (
- +
} ({ + items={trace.spans.map(span => ({ valueOffset: span.relativeStartTime, valueWidth: span.duration, serviceName: span.process.serviceName, diff --git a/src/components/TracePage/TraceSpanGraph.test.js b/src/components/TracePage/TraceSpanGraph.test.js index 3e64cbd325..9c9c8e1450 100644 --- a/src/components/TracePage/TraceSpanGraph.test.js +++ b/src/components/TracePage/TraceSpanGraph.test.js @@ -27,21 +27,14 @@ import TraceSpanGraph from './TraceSpanGraph'; import SpanGraphTickHeader from './SpanGraph/SpanGraphTickHeader'; import TimelineScrubber from './TimelineScrubber'; import traceGenerator from '../../../src/demo/trace-generators'; -import { transformTrace } from './TraceTimelineViewer/transforms'; -import { hydrateSpansWithProcesses } from '../../selectors/trace'; +import transformTraceData from '../../model/transform-trace-data'; describe('', () => { - const trace = hydrateSpansWithProcesses(traceGenerator.trace({})); - const xformedTrace = transformTrace(trace); - - const props = { - trace, - xformedTrace, - }; - + const trace = transformTraceData(traceGenerator.trace({})); + const props = { trace }; const options = { context: { - timeRangeFilter: [trace.timestamp, trace.timestamp + trace.duration], + timeRangeFilter: [trace.startTime, trace.startTime + trace.duration], updateTimeRangeFilter: () => {}, }, }; @@ -68,7 +61,7 @@ describe('', () => { it('renders a filtering box if leftBound exists', () => { const context = { ...options.context, - timeRangeFilter: [trace.timestamp + 0.2 * trace.duration, trace.timestamp + trace.duration], + timeRangeFilter: [trace.startTime + 0.2 * trace.duration, trace.startTime + trace.duration], }; wrapper = shallow(, { ...options, context }); const leftBox = wrapper.find('.trace-page-timeline__graph--inactive'); @@ -82,7 +75,7 @@ describe('', () => { it('renders a filtering box if rightBound exists', () => { const context = { ...options.context, - timeRangeFilter: [trace.timestamp, trace.timestamp + 0.8 * trace.duration], + timeRangeFilter: [trace.startTime, trace.startTime + 0.8 * trace.duration], }; wrapper = shallow(, { ...options, context }); const rightBox = wrapper.find('.trace-page-timeline__graph--inactive'); @@ -132,7 +125,7 @@ describe('', () => { it('passes items to SpanGraph', () => { const spanGraph = wrapper.find(SpanGraph).first(); - const items = xformedTrace.spans.map(span => ({ + const items = trace.spans.map(span => ({ valueOffset: span.relativeStartTime, valueWidth: span.duration, serviceName: span.process.serviceName, @@ -151,9 +144,8 @@ describe('', () => { it('returns true for new trace', () => { const state = wrapper.state(); const instance = wrapper.instance(); - const trace2 = hydrateSpansWithProcesses(traceGenerator.trace({})); - const xformedTrace2 = transformTrace(trace2); - const altProps = { trace: trace2, xformedTrace: xformedTrace2 }; + const trace2 = transformTraceData(traceGenerator.trace({})); + const altProps = { trace: trace2 }; expect(instance.shouldComponentUpdate(altProps, state, options.context)).toBe(true); }); @@ -190,7 +182,7 @@ describe('', () => { }); it('updates the timeRangeFilter for the left handle', () => { - const timestamp = trace.timestamp; + const timestamp = trace.startTime; const duration = trace.duration; const updateTimeRangeFilter = sinon.spy(); const context = { ...options.context, updateTimeRangeFilter }; @@ -204,7 +196,7 @@ describe('', () => { }); it('updates the timeRangeFilter for the right handle', () => { - const timestamp = trace.timestamp; + const timestamp = trace.startTime; const duration = trace.duration; const updateTimeRangeFilter = sinon.spy(); const context = { ...options.context, updateTimeRangeFilter }; diff --git a/src/components/TracePage/TraceTimelineViewer/ListView/Positions.js b/src/components/TracePage/TraceTimelineViewer/ListView/Positions.js index 994815b439..bfc630771d 100644 --- a/src/components/TracePage/TraceTimelineViewer/ListView/Positions.js +++ b/src/components/TracePage/TraceTimelineViewer/ListView/Positions.js @@ -33,8 +33,6 @@ export default class Positions { /** * Indicates how far past the explicitly required height or y-values should * checked. - * @type {number} - * @memberof Positions */ bufferLen: number; dataLen: number; @@ -43,8 +41,6 @@ export default class Positions { * `lastI` keeps track of which values have already been visited. In many * scenarios, values do not need to be revisited. But, revisiting is required * when heights have changed, so `lastI` can be forced. - * @type {number} - * @memberof Positions */ lastI: number; ys: number[]; @@ -60,8 +56,6 @@ export default class Positions { /** * Used to make sure the length of y-values and heights is consistent with * the context; in particular `lastI` needs to remain valid. - * - * @param {any[]} data */ profileData(dataLength: number) { if (dataLength !== this.dataLen) { @@ -78,10 +72,7 @@ export default class Positions { * Calculate and save the heights and y-values, based on `heightGetter`, from * `lastI` until the`max` index; the starting point (`lastI`) can be forced * via the `forcedLastI` parameter. - * - * @param {number} max - * @param {number} heightGetter - * @param {number} [forcedLastI] + * @param {number=} forcedLastI */ calcHeights(max: number, heightGetter: number => number, forcedLastI?: number) { if (forcedLastI != null) { @@ -110,9 +101,6 @@ export default class Positions { /** * Verify the height and y-values from `lastI` up to `yValue`. - * - * @param {number} yValue - * @param {number => number} heightGetter */ calcYs(yValue: number, heightGetter: number => number) { while ((this.ys[this.lastI] == null || yValue > this.ys[this.lastI]) && this.lastI < this.dataLen - 1) { @@ -124,10 +112,6 @@ export default class Positions { * Given a target y-value (`yValue`), find the closest index (in the `.ys` * array) that is prior to the y-value; e.g. map from y-value to index in * `.ys`. - * - * @param {number} yValue - * @param {number => number} heightGetter - * @returns {number} */ findFloorIndex(yValue: number, heightGetter: number => number): number { this.calcYs(yValue, heightGetter); @@ -164,8 +148,6 @@ export default class Positions { /** * Get the estimated height of the whole shebang by extrapolating based on * the average known height. - * - * @returns {number} */ getEstimatedHeight(): number { const known = this.ys[this.lastI] + this.heights[this.lastI]; @@ -183,9 +165,6 @@ export default class Positions { * known territory (_i <= lastI) and the height is different than what is * known, recalculate subsequent y values, but don't confirm the heights of * those items, just update based on the difference. - * - * @param {number} _i - * @param {number => number} heightGetter */ confirmHeight(_i: number, heightGetter: number => number) { let i = _i; diff --git a/src/components/TracePage/TraceTimelineViewer/ListView/index.js b/src/components/TracePage/TraceTimelineViewer/ListView/index.js index 8094a8839b..572b375739 100644 --- a/src/components/TracePage/TraceTimelineViewer/ListView/index.js +++ b/src/components/TracePage/TraceTimelineViewer/ListView/index.js @@ -24,44 +24,40 @@ import * as React from 'react'; import Positions from './Positions'; +/** + * @typedef + */ type ListViewProps = { /** * Number of elements in the list. - * @type {number} */ dataLength: number, /** * Convert item index (number) to the key (string). ListView uses both indexes * and keys to handle the addtion of new rows. - * @type {string => number} */ getIndexFromKey: string => number, /** * Convert item key (string) to the index (number). ListView uses both indexes * and keys to handle the addtion of new rows. - * @type {number => string} */ getKeyFromIndex: number => string, /** * Number of items to draw and add to the DOM, initially. - * @type {number} */ initialDraw: number, /** * The parent provides fallback height measurements when there is not a * rendered element to measure. - * @type {(number, string) => number} */ itemHeightGetter: (number, string) => number, /** * Function that renders an item; rendered items are added directly to the * DOM, they are not wrapped in list item wrapper HTMLElement. - * @type {(string, {}, number, {}) => React.Node} */ itemRenderer: (string, {}, number, {}) => React.Node, /** * `className` for the HTMLElement that holds the items. - * @type {string} */ itemsWrapperClassName?: string, /** @@ -70,14 +66,12 @@ type ListViewProps = { * halfway down (so items [46, 55] are in view), then when a new range of * items is rendered, it will render items `46 - viewBuffer` to * `55 + viewBuffer`. - * @type {number} */ viewBuffer: number, /** * The minimum number of items offscreen in either direction; e.g. at least * `viewBuffer` number of items must be off screen above and below the * current view, or more items will be rendered. - * @type {number} */ viewBufferMin: number, /** @@ -86,9 +80,8 @@ type ListViewProps = { * scrolling as a result of the ListView. Similar to react-virtualized * window scroller. * - * Ref: https://bvaughn.github.io/react-virtualized/#/components/WindowScroller - * Ref:https://github.com/bvaughn/react-virtualized/blob/497e2a1942529560681d65a9ef9f5e9c9c9a49ba/docs/WindowScroller.md - * @type {boolean} + * - Ref: https://bvaughn.github.io/react-virtualized/#/components/WindowScroller + * - Ref:https://github.com/bvaughn/react-virtualized/blob/497e2a1942529560681d65a9ef9f5e9c9c9a49ba/docs/WindowScroller.md */ windowScroller?: boolean, }; @@ -107,77 +100,62 @@ type ListViewProps = { * * @export * @class ListView - * @extends {React.PureComponent} */ export default class ListView extends React.Component { props: ListViewProps; /** * Keeps track of the height and y-value of items, by item index, in the * ListView. - * @type {Positions} */ _yPositions: Positions; /** * Keep track of the known / measured heights of the rendered items; populated * with values through observation and keyed on the item key, not the item * index. - * @type {Map} */ _knownHeights: Map; /** * The start index of the items currently drawn. - * @type {number} */ _startIndexDrawn: number; /** * The end index of the items currently drawn. - * @type {number} */ _endIndexDrawn: number; /** * The start index of the items currently in view. - * @type {number} */ _startIndex: number; /** * The end index of the items currently in view. - * @type {number} */ _endIndex: number; /** * Height of the visual window, e.g. height of the scroller element. - * @type {number} */ _viewHeight: number; /** * `scrollTop` of the current scroll position. - * @type {number} */ _scrollTop: number; /** * Used to keep track of whether or not a re-calculation of what should be * drawn / viewable has been scheduled. - * @type {boolean} */ _isScrolledOrResized: boolean; /** * If `windowScroller` is true, this notes how far down the page the scroller * is located. (Note: repositioning and below-the-fold views are untested) - * - * @type {number} - * @memberof ListView */ _htmlTopOffset: number; _windowScrollListenerAdded: boolean; _htmlElm: HTMLElement; /** * HTMLElement holding the scroller. - * @type HTMLElement */ _wrapperElm: ?HTMLElement; /** * HTMLElement holding the rendered items. - * @type HTMLElement */ _itemHolderElm: ?HTMLElement; @@ -253,8 +231,6 @@ export default class ListView extends React.Component { /** * Scroll event listener that schedules a remeasuring of which items should be * rendered. - * - * @memberof ListView */ _onScroll = function _onScroll() { if (!this._isScrolledOrResized) { @@ -317,8 +293,6 @@ export default class ListView extends React.Component { /** * Checked to see if the currently rendered items are sufficient, if not, * force an update to trigger more items to be rendered. - * - * @memberof ListView */ _positionList = function _positionList() { this._isScrolledOrResized = false; @@ -356,8 +330,6 @@ export default class ListView extends React.Component { * Go through all items that are rendered and save their height based on their * item-key (which is on a data-* attribute). If any new or adjusted heights * are found, re-measure the current known y-positions (via .yPositions). - * - * @memberof ListView */ _scanItemHeights = function _scanItemHeights() { const getIndexFromKey = this.props.getIndexFromKey; @@ -411,8 +383,6 @@ export default class ListView extends React.Component { /** * Get the height of the element at index `i`; first check the known heigths, * fallbck to `.props.itemHeightGetter(...)`. - * - * @memberof ListView */ _getHeight = function _getHeight(i: number) { const key = this.props.getKeyFromIndex(i); diff --git a/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.css b/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.css index 25de81043e..9f4f73bb15 100644 --- a/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.css +++ b/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.css @@ -38,14 +38,18 @@ THE SOFTWARE. white-space: nowrap; } -.AccordianKeyValues--header:hover { +.AccordianKeyValues--header:not(.is-empty):hover { background: #eee; } -.AccordianKeyValues--header.is-high-contrast:hover { +.AccordianKeyValues--header.is-high-contrast:not(.is-empty):hover { background: #ddd; } +.AccordianKeyValues--emptyArtifact { + color: #aaa; +} + .AccordianKeyValues--summary { display: inline; list-style: none; diff --git a/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js b/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js index de0c4b4dc3..6bb6e6d43b 100644 --- a/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js +++ b/src/components/TracePage/TraceTimelineViewer/SpanDetail/AccordianKeyValues.js @@ -21,6 +21,7 @@ // THE SOFTWARE. import React from 'react'; +import cx from 'classnames'; import KeyValuesTable from './KeyValuesTable'; @@ -35,7 +36,11 @@ type AccordianKeyValuesProps = { onToggle: () => void, }; -function getKVSummary(data: { key: string, value: any }[]) { +function KeyValuesSummary(props: { data?: { key: string, value: any }[] }) { + const { data } = props; + if (!Array.isArray(data) || !data.length) { + return null; + } return (
    {data.map((item, i) => @@ -53,23 +58,34 @@ function getKVSummary(data: { key: string, value: any }[]) { ); } +KeyValuesSummary.defaultProps = { + data: null, +}; + export default function AccordianKeyValues(props: AccordianKeyValuesProps) { const { compact, data, highContrast, isOpen, label, onToggle } = props; - + const isEmpty = !Array.isArray(data) || !data.length; + const iconCls = cx( + { minus: isOpen, plus: !isOpen, 'AccordianKeyValues--emptyArtifact': isEmpty }, + 'square outline icon' + ); return ( -
    +
    - + {label} : - {!isOpen && getKVSummary(data)} + {!isOpen && }
    {isOpen && }
    diff --git a/src/components/TracePage/TraceTimelineViewer/SpanDetail/DetailState.js b/src/components/TracePage/TraceTimelineViewer/SpanDetail/DetailState.js index 2bd025ac99..69125f4d04 100644 --- a/src/components/TracePage/TraceTimelineViewer/SpanDetail/DetailState.js +++ b/src/components/TracePage/TraceTimelineViewer/SpanDetail/DetailState.js @@ -22,15 +22,9 @@ import type { Log } from '../../../../types'; -// DetailState { -// isTagsOpen: bool, -// isProcessOpen: bool, -// logs: { -// isOpen: bool, -// openItems: Set -// } -// } - +/** + * Which items of a {@link SpanDetail} component are expanded. + */ export default class DetailState { isTagsOpen: boolean; isProcessOpen: boolean; diff --git a/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js b/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js index 56e8d25c33..3ae38caf79 100644 --- a/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js +++ b/src/components/TracePage/TraceTimelineViewer/SpanDetail/index.js @@ -24,8 +24,7 @@ import AccordianKeyValues from './AccordianKeyValues'; import AccordianLogs from './AccordianLogs'; import DetailState from './DetailState'; import { formatDuration } from '../utils'; -import type { XformedSpan, XformedTrace } from '../transforms'; -import type { Log } from '../../../../types'; +import type { Log, Span } from '../../../../types'; import './index.css'; @@ -34,13 +33,13 @@ type SpanDetailProps = { logItemToggle: (string, Log) => void, logsToggle: string => void, processToggle: string => void, - span: XformedSpan, + span: Span, tagsToggle: string => void, - trace: XformedTrace, + traceStartTime: number, }; export default function SpanDetail(props: SpanDetailProps) { - const { detailState, logItemToggle, logsToggle, processToggle, span, tagsToggle, trace } = props; + const { detailState, logItemToggle, logsToggle, processToggle, span, tagsToggle, traceStartTime } = props; const { isTagsOpen, isProcessOpen, logs: logsState } = detailState; const { operationName, process, duration, relativeStartTime, spanID, logs, tags } = span; return ( @@ -97,7 +96,7 @@ export default function SpanDetail(props: SpanDetailProps) { openedItems={logsState.openedItems} onToggle={() => logsToggle(spanID)} onItemToggle={logItem => logItemToggle(spanID, logItem)} - timestamp={trace.startTime} + timestamp={traceStartTime} />} diff --git a/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js b/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js index fa98b656f0..8e77ef788b 100644 --- a/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js +++ b/src/components/TracePage/TraceTimelineViewer/SpanDetailRow.js @@ -26,8 +26,7 @@ import SpanDetail from './SpanDetail'; import DetailState from './SpanDetail/DetailState'; import SpanTreeOffset from './SpanTreeOffset'; import TimelineRow from './TimelineRow'; -import type { XformedTrace, XformedSpan } from './transforms'; -import type { Log } from '../../../types'; +import type { Log, Span } from '../../../types'; import './SpanDetailRow.css'; @@ -39,9 +38,9 @@ type SpanDetailRowProps = { logItemToggle: (string, Log) => void, logsToggle: string => void, processToggle: string => void, - span: XformedSpan, + span: Span, tagsToggle: string => void, - trace: XformedTrace, + traceStartTime: number, }; export default function SpanDetailRow(props: SpanDetailRowProps) { @@ -55,7 +54,7 @@ export default function SpanDetailRow(props: SpanDetailRowProps) { processToggle, span, tagsToggle, - trace, + traceStartTime, } = props; return ( @@ -80,7 +79,7 @@ export default function SpanDetailRow(props: SpanDetailRowProps) { processToggle={processToggle} span={span} tagsToggle={tagsToggle} - trace={trace} + traceStartTime={traceStartTime} />
    diff --git a/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js b/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js index 7ba40d8ac9..67c86e3d51 100644 --- a/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js +++ b/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.js @@ -30,7 +30,6 @@ import DetailState from './SpanDetail/DetailState'; import SpanDetailRow from './SpanDetailRow'; import Ticks from './Ticks'; import TimelineRow from './TimelineRow'; -import type { XformedSpan, XformedTrace } from './transforms'; import { findServerChildSpan, formatDuration, @@ -38,14 +37,14 @@ import { isErrorSpan, spanContainsErredSpan, } from './utils'; -import type { Log } from '../../../types'; +import type { Log, Span, Trace } from '../../../types'; import colorGenerator from '../../../utils/color-generator'; import './VirtualizedTraceView.css'; type RowState = { isDetail: boolean, - span: XformedSpan, + span: Span, spanIndex: number, }; @@ -60,7 +59,7 @@ type VirtualizedTraceViewProps = { detailToggle: string => void, findMatchesIDs: Set, ticks: number[], - trace?: XformedTrace, + trace?: Trace, zoomEnd: number, zoomStart: number, }; @@ -72,7 +71,7 @@ const DEFAULT_HEIGHTS = { }; function generateRowStates( - spans: ?(XformedSpan[]), + spans: ?(Span[]), childrenHiddenIDs: Set, detailStates: Map ): RowState[] { @@ -120,7 +119,7 @@ function getPropDerivations(props: VirtualizedTraceViewProps) { 'clipping-left': zoomStart > 0, 'clipping-right': zoomEnd < 1, }); - let spans: ?(XformedSpan[]); + let spans: ?(Span[]); if (trace) { spans = trace.spans; } @@ -308,7 +307,7 @@ class VirtualizedTraceView extends React.PureComponent
); diff --git a/src/components/TracePage/TraceTimelineViewer/duck.test.js b/src/components/TracePage/TraceTimelineViewer/duck.test.js index 1b7456f649..27be2b869d 100644 --- a/src/components/TracePage/TraceTimelineViewer/duck.test.js +++ b/src/components/TracePage/TraceTimelineViewer/duck.test.js @@ -22,21 +22,21 @@ import { createStore } from 'redux'; import reducer, { actions, newInitialState } from './duck'; import DetailState from './SpanDetail/DetailState'; -import { transformTrace } from './transforms'; +import transformTraceData from '../../../model/transform-trace-data'; import traceGenerator from '../../../demo/trace-generators'; describe('TraceTimelineViewer/duck', () => { - const xformedTrace = transformTrace(traceGenerator.trace({ numberOfSpans: 10 })); + const trace = transformTraceData(traceGenerator.trace({ numberOfSpans: 10 })); let store; beforeEach(() => { - store = createStore(reducer, newInitialState(xformedTrace)); + store = createStore(reducer, newInitialState(trace)); }); describe('initial state', () => { it('retains a provided trace', () => { const state = store.getState(); - expect(state.trace).toBe(xformedTrace); + expect(state.trace).toBe(trace); }); it('has no details, collapsed children or text search', () => { @@ -48,7 +48,7 @@ describe('TraceTimelineViewer/duck', () => { }); describe('toggles children and details', () => { - const parentID = xformedTrace.spans[0].spanID; + const parentID = trace.spans[0].spanID; const tests = [ { msg: 'toggles children', @@ -87,7 +87,7 @@ describe('TraceTimelineViewer/duck', () => { }); describe("toggles a detail's sub-sections", () => { - const id = xformedTrace.spans[0].spanID; + const id = trace.spans[0].spanID; const baseDetail = new DetailState(); const tests = [ { diff --git a/src/components/TracePage/TraceTimelineViewer/index.js b/src/components/TracePage/TraceTimelineViewer/index.js index d1a6e8474a..a9e5708324 100644 --- a/src/components/TracePage/TraceTimelineViewer/index.js +++ b/src/components/TracePage/TraceTimelineViewer/index.js @@ -30,15 +30,13 @@ import { getPositionInRange } from './utils'; import './grid.css'; import './index.css'; -// TODO: Add unit tests - export default class TraceTimelineViewer extends Component { constructor(props) { super(props); - const { textFilter, xformedTrace } = props; + const { textFilter, trace } = props; - this.store = createStore(reducer, newInitialState(xformedTrace)); + this.store = createStore(reducer, newInitialState(trace)); this.actionsCreators = bindActionCreators(actions, this.store.dispatch); if (textFilter) { this.store.dispatch(actions.find(textFilter)); @@ -46,8 +44,8 @@ export default class TraceTimelineViewer extends Component { } componentWillReceiveProps(nextProps) { - const { xformedTrace, textFilter } = nextProps; - if (xformedTrace !== this.props.xformedTrace) { + const { trace, textFilter } = nextProps; + if (trace !== this.props.trace) { throw new Error('Component does not support changing the trace'); } if (textFilter !== this.props.textFilter) { @@ -56,8 +54,8 @@ export default class TraceTimelineViewer extends Component { } render() { - const { timeRangeFilter: zoomRange, xformedTrace } = this.props; - const { startTime, endTime } = xformedTrace; + const { timeRangeFilter: zoomRange, trace } = this.props; + const { startTime, endTime } = trace; return (
@@ -72,7 +70,7 @@ export default class TraceTimelineViewer extends Component { } } TraceTimelineViewer.propTypes = { - xformedTrace: PropTypes.object, + trace: PropTypes.object, timeRangeFilter: PropTypes.array, textFilter: PropTypes.string, }; diff --git a/src/components/TracePage/TraceTimelineViewer/index.test.js b/src/components/TracePage/TraceTimelineViewer/index.test.js index fb7aa3caa5..cfc94c32c7 100644 --- a/src/components/TracePage/TraceTimelineViewer/index.test.js +++ b/src/components/TracePage/TraceTimelineViewer/index.test.js @@ -22,15 +22,15 @@ import React from 'react'; import { shallow } from 'enzyme'; import TraceTimelineViewer from './index'; -import { transformTrace } from './transforms'; import traceGenerator from '../../../demo/trace-generators'; +import transformTraceData from '../../../model/transform-trace-data'; describe('', () => { - const trace = traceGenerator.trace({}); + const trace = transformTraceData(traceGenerator.trace({})); const props = { + trace, textFilter: null, timeRangeFilter: [0, 1], - xformedTrace: transformTrace(trace), }; let wrapper; diff --git a/src/components/TracePage/TraceTimelineViewer/transforms.js b/src/components/TracePage/TraceTimelineViewer/transforms.js deleted file mode 100644 index 37114a7dbe..0000000000 --- a/src/components/TracePage/TraceTimelineViewer/transforms.js +++ /dev/null @@ -1,94 +0,0 @@ -// @flow - -// Copyright (c) 2017 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - -import { - getTraceDuration, - getTraceEndTimestamp, - getTraceSpanIdsAsTree, - getTraceSpansAsMap, - getTraceTimestamp, -} from '../../../selectors/trace'; -import type { Process, Span } from '../../../types'; - -type SpanWithProcess = Span & { process: Process }; - -export type XformedSpan = SpanWithProcess & { - depth: number, - hasChildren: boolean, - relativeStartTime: number, -}; - -type TracePartial = { - processes: { [string]: Process }, - traceID: string, -}; - -export type XformedTrace = TracePartial & { - duration: number, - endTime: number, - spans: XformedSpan[], - startTime: number, -}; - -const cache: Map = new Map(); - -// eslint-disable-next-line import/prefer-default-export -export function transformTrace(trace: TracePartial & { spans: SpanWithProcess[] }): XformedTrace { - const cached = cache.get(trace.traceID); - if (cached) { - return cached; - } - const traceStartTime = getTraceTimestamp(trace); - const traceEndTime = getTraceEndTimestamp(trace); - const spanMap: Map = getTraceSpansAsMap(trace); - const tree = getTraceSpanIdsAsTree(trace); - const spans: XformedSpan[] = []; - - tree.walk((spanID, node, depth) => { - if (spanID === '__root__') { - return; - } - const span: ?SpanWithProcess = spanMap.get(spanID); - if (!span) { - return; - } - spans.push({ - ...span, - relativeStartTime: span.startTime - traceStartTime, - depth: depth - 1, - hasChildren: node.children.length > 0, - }); - }); - const transform = { - spans, - // can't use spread operator for intersection types - // repl: https://goo.gl/4Z23MJ - // issue: https://github.com/facebook/flow/issues/1511 - processes: trace.processes, - traceID: trace.traceID, - duration: getTraceDuration(trace), - startTime: traceStartTime, - endTime: traceEndTime, - }; - cache.set(trace.traceID, transform); - return transform; -} diff --git a/src/components/TracePage/index.js b/src/components/TracePage/index.js index 94cbd260ef..59d76e585c 100644 --- a/src/components/TracePage/index.js +++ b/src/components/TracePage/index.js @@ -28,17 +28,10 @@ import { bindActionCreators } from 'redux'; import TracePageHeader from './TracePageHeader'; import TraceSpanGraph from './TraceSpanGraph'; import TraceTimelineViewer from './TraceTimelineViewer'; -import { transformTrace } from './TraceTimelineViewer/transforms'; import NotFound from '../App/NotFound'; import * as jaegerApiActions from '../../actions/jaeger-api'; import { getTraceName } from '../../model/trace-viewer'; -import { - dropEmptyStartTimeSpans, - hydrateSpansWithProcesses, - getTraceTimestamp, - getTraceEndTimestamp, - getTraceId, -} from '../../selectors/trace'; +import { getTraceTimestamp, getTraceEndTimestamp, getTraceId } from '../../selectors/trace'; import colorGenerator from '../../utils/color-generator'; import './index.css'; @@ -48,7 +41,6 @@ export default class TracePage extends Component { return { fetchTrace: PropTypes.func.isRequired, trace: PropTypes.object, - xformedTrace: PropTypes.object, loading: PropTypes.bool, id: PropTypes.string.isRequired, }; @@ -146,7 +138,7 @@ export default class TracePage extends Component { } render() { - const { id, trace, xformedTrace } = this.props; + const { id, trace } = this.props; const { slimView, headerHeight } = this.state; if (!trace) { @@ -157,7 +149,7 @@ export default class TracePage extends Component { return ; } - const { duration, processes, spans, startTime, traceID } = xformedTrace; + const { duration, processes, spans, startTime, traceID } = trace; const maxSpanDepth = _maxBy(spans, 'depth').depth + 1; const numberOfServices = new Set(_values(processes).map(p => p.serviceName)).size; return ( @@ -174,13 +166,12 @@ export default class TracePage extends Component { traceID={traceID} onSlimViewClicked={this.toggleSlimView} /> - {!slimView && } + {!slimView && } {headerHeight &&
@@ -193,14 +184,8 @@ export default class TracePage extends Component { // export connected component separately function mapStateToProps(state, ownProps) { const { id } = ownProps.match.params; - let trace = state.trace.traces[id]; - let xformedTrace; - if (trace && !(trace instanceof Error)) { - trace = dropEmptyStartTimeSpans(trace); - trace = hydrateSpansWithProcesses(trace); - xformedTrace = transformTrace(trace); - } - return { id, trace, xformedTrace, loading: state.trace.loading }; + const trace = state.trace.traces[id]; + return { id, trace, loading: state.trace.loading }; } function mapDispatchToProps(dispatch) { diff --git a/src/components/TracePage/index.test.js b/src/components/TracePage/index.test.js index 07ad44d1f5..737a4d93c4 100644 --- a/src/components/TracePage/index.test.js +++ b/src/components/TracePage/index.test.js @@ -26,15 +26,14 @@ import traceGenerator from '../../demo/trace-generators'; import TracePage from './'; import TracePageHeader from './TracePageHeader'; import TraceSpanGraph from './TraceSpanGraph'; -import { transformTrace } from './TraceTimelineViewer/transforms'; +import transformTraceData from '../../model/transform-trace-data'; describe('', () => { - const trace = traceGenerator.trace({}); + const trace = transformTraceData(traceGenerator.trace({})); const defaultProps = { trace, fetchTrace() {}, id: trace.traceID, - xformedTrace: transformTrace(trace), }; let wrapper; @@ -48,7 +47,7 @@ describe('', () => { }); it('renders a ', () => { - const props = { trace: defaultProps.trace, xformedTrace: defaultProps.xformedTrace }; + const props = { trace: defaultProps.trace }; expect(wrapper.contains()).toBeTruthy(); }); diff --git a/src/demo/trace-generators.js b/src/demo/trace-generators.js index a998687028..5ac1d9be42 100644 --- a/src/demo/trace-generators.js +++ b/src/demo/trace-generators.js @@ -123,8 +123,6 @@ export default chance.mixin({ return { traceID, spans, - duration, - timestamp, processes, }; }, diff --git a/src/model/search.js b/src/model/search.js index 790f78e6b4..cfbd403b71 100644 --- a/src/model/search.js +++ b/src/model/search.js @@ -33,7 +33,7 @@ const isErrorTag = ({ key, value }) => key === 'error' && (value === true || val * Transforms a trace from the HTTP response to the data structure needed by the search page. Note: exported * for unit tests. * - * @param {Trace} trace Trace data in the format sent over the wire. + * @param trace Trace data in the format sent over the wire. * @return {TraceSummary} Summary of the trace data for use in the search results. */ export function getTraceSummary(trace: Trace): TraceSummary { diff --git a/src/model/search.test.js b/src/model/search.test.js index ec301b7dae..9b2ab1c2ad 100644 --- a/src/model/search.test.js +++ b/src/model/search.test.js @@ -24,20 +24,21 @@ import _minBy from 'lodash/minBy'; import * as orderBy from './order-by'; import { getTraceSummaries, getTraceSummary, sortTraces } from './search'; import traceGenerator from '../demo/trace-generators'; +import transformTraceData from '../model/transform-trace-data'; describe('getTraceSummary()', () => { let trace; let summary; beforeEach(() => { - trace = traceGenerator.trace({ numberOfSpans: 2 }); + trace = transformTraceData(traceGenerator.trace({ numberOfSpans: 2 })); summary = getTraceSummary(trace); }); it('derives duration, timestamp and numberOfSpans', () => { expect(summary.numberOfSpans).toBe(trace.spans.length); expect(summary.duration).toBe(trace.duration / 1000); - expect(summary.timestamp).toBe(Math.floor(trace.timestamp / 1000)); + expect(summary.timestamp).toBe(Math.floor(trace.startTime / 1000)); }); it('handles error spans', () => { @@ -96,7 +97,10 @@ describe('getTraceSummary()', () => { describe('getTraceSummaries()', () => { it('finds the max duration', () => { - const traces = [traceGenerator.trace({}), traceGenerator.trace({})]; + const traces = [ + transformTraceData(traceGenerator.trace({})), + transformTraceData(traceGenerator.trace({})), + ]; const maxDuration = _maxBy(traces, 'duration').duration / 1000; expect(getTraceSummaries(traces).maxDuration).toBe(maxDuration); }); @@ -106,10 +110,10 @@ describe('sortTraces()', () => { const idMinSpans = 4; const idMaxSpans = 2; const rawTraces = [ - { ...traceGenerator.trace({ numberOfSpans: 3 }), traceID: 1 }, - { ...traceGenerator.trace({ numberOfSpans: 100 }), traceID: idMaxSpans }, - { ...traceGenerator.trace({ numberOfSpans: 5 }), traceID: 3 }, - { ...traceGenerator.trace({ numberOfSpans: 1 }), traceID: idMinSpans }, + { ...transformTraceData(traceGenerator.trace({ numberOfSpans: 3 })), traceID: 1 }, + { ...transformTraceData(traceGenerator.trace({ numberOfSpans: 100 })), traceID: idMaxSpans }, + { ...transformTraceData(traceGenerator.trace({ numberOfSpans: 5 })), traceID: 3 }, + { ...transformTraceData(traceGenerator.trace({ numberOfSpans: 1 })), traceID: idMinSpans }, ]; const { traces } = getTraceSummaries(rawTraces); diff --git a/src/model/transform-trace-data.js b/src/model/transform-trace-data.js new file mode 100644 index 0000000000..b2c7e08f97 --- /dev/null +++ b/src/model/transform-trace-data.js @@ -0,0 +1,103 @@ +// @flow + +// Copyright (c) 2017 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +import _isEqual from 'lodash/isEqual'; + +import { getTraceSpanIdsAsTree } from '../selectors/trace'; +import type { Process, Span, SpanData, Trace, TraceData } from '../types'; + +type SpanWithProcess = SpanData & { process: Process }; + +/** + * NOTE: Mutates `data` - Transform the HTTP response data into the form the app + * generally requires. + */ +export default function transfromTraceData(data: TraceData & { spans: SpanWithProcess[] }): Trace { + let traceEndTime = 0; + let traceStartTime = Number.MAX_SAFE_INTEGER; + const spanIdCounts = new Map(); + const spanMap = new Map(); + // filter out spans with empty start times + // eslint-disable-next-line no-param-reassign + data.spans = data.spans.filter(span => Boolean(span.startTime)); + + const max = data.spans.length; + for (let i = 0; i < max; i++) { + const span = data.spans[i]; + const { startTime, duration, processID } = span; + // + let spanID = span.spanID; + // check for start / end time for the trace + if (startTime < traceStartTime) { + traceStartTime = startTime; + } + if (startTime + duration > traceEndTime) { + traceEndTime = startTime + duration; + } + // make sure span IDs are unique + const idCount = spanIdCounts.get(spanID); + if (idCount != null) { + console.warn(`Dupe spanID, ${idCount + 1} x ${spanID}`, span, spanMap.get(spanID)); + if (_isEqual(span, spanMap.get(spanID))) { + console.warn('\t two spans with same ID have `isEqual(...) === true`'); + } + spanIdCounts.set(spanID, idCount + 1); + spanID = `${spanID}_${idCount}`; + span.spanID = spanID; + } else { + spanIdCounts.set(spanID, 1); + } + span.process = data.processes[processID]; + spanMap.set(spanID, span); + } + // tree is necessary to sort the spans, so children follow parents, and + // siblings are sorted by start time + const tree = getTraceSpanIdsAsTree(data); + const spans: Span[] = []; + + tree.walk((spanID, node, depth) => { + if (spanID === '__root__') { + return; + } + const span: ?SpanWithProcess = spanMap.get(spanID); + if (!span) { + return; + } + spans.push({ + ...span, + relativeStartTime: span.startTime - traceStartTime, + depth: depth - 1, + hasChildren: node.children.length > 0, + }); + }); + return { + spans, + // can't use spread operator for intersection types + // repl: https://goo.gl/4Z23MJ + // issue: https://github.com/facebook/flow/issues/1511 + processes: data.processes, + traceID: data.traceID, + duration: traceEndTime - traceStartTime, + startTime: traceStartTime, + endTime: traceEndTime, + }; +} diff --git a/src/reducers/trace.js b/src/reducers/trace.js index d1aa788825..4913a49110 100644 --- a/src/reducers/trace.js +++ b/src/reducers/trace.js @@ -22,7 +22,7 @@ import keyBy from 'lodash/keyBy'; import { handleActions } from 'redux-actions'; import { fetchTrace, searchTraces } from '../actions/jaeger-api'; -import { enforceUniqueSpanIds } from '../selectors/trace'; +import transformTraceData from '../model/transform-trace-data'; const initialState = { traces: {}, @@ -34,9 +34,9 @@ function fetchStarted(state) { return { ...state, loading: true }; } -function fetchTraceDone(state, { meta, payload }) { - const trace = enforceUniqueSpanIds(payload.data[0]); - const traces = { ...state.traces, [meta.id]: trace }; +function fetchTraceDone(state, { payload }) { + const trace = transformTraceData(payload.data[0]); + const traces = { ...state.traces, [trace.traceID]: trace }; return { ...state, traces, loading: false }; } @@ -46,7 +46,8 @@ function fetchTraceErred(state, { meta, payload }) { } function searchDone(state, { payload }) { - const traces = keyBy(payload.data, 'traceID'); + const processed = payload.data.map(transformTraceData); + const traces = keyBy(processed, 'traceID'); return { ...state, traces, error: null, loading: false }; } diff --git a/src/reducers/trace.test.js b/src/reducers/trace.test.js index b2b293556b..191dc56d6d 100644 --- a/src/reducers/trace.test.js +++ b/src/reducers/trace.test.js @@ -21,6 +21,7 @@ import * as jaegerApiActions from '../../src/actions/jaeger-api'; import traceReducer from '../../src/reducers/trace'; import traceGenerator from '../../src/demo/trace-generators'; +import transformTraceData from '../../src/model/transform-trace-data'; const generatedTrace = traceGenerator.trace({ numberOfSpans: 1 }); const { traceID } = generatedTrace; @@ -38,7 +39,7 @@ it('trace reducer should handle a successful FETCH_TRACE', () => { payload: { data: [generatedTrace] }, meta: { id: traceID }, }); - expect(state.traces).toEqual({ [traceID]: generatedTrace }); + expect(state.traces).toEqual({ [traceID]: transformTraceData(generatedTrace) }); expect(state.loading).toBe(false); }); @@ -60,6 +61,6 @@ it('trace reducer should handle a successful SEARCH_TRACES', () => { payload: { data: [generatedTrace] }, meta: { query: 'whatever' }, }); - expect(state.traces).toEqual({ [traceID]: generatedTrace }); + expect(state.traces).toEqual({ [traceID]: transformTraceData(generatedTrace) }); expect(state.loading).toBe(false); }); diff --git a/src/types/index.js b/src/types/index.js index c37faaa7ff..5f2590525c 100644 --- a/src/types/index.js +++ b/src/types/index.js @@ -48,7 +48,7 @@ export type SpanReference = { traceID: string, }; -export type Span = { +export type SpanData = { spanID: string, traceID: string, processID: string, @@ -60,10 +60,21 @@ export type Span = { references: Array, }; -export type Trace = { +export type Span = SpanData & { + depth: number, + hasChildren: boolean, + process: Process, + relativeStartTime: number, +}; + +export type TraceData = { + processes: { [string]: Process }, traceID: string, - spans: Array, - processes: { - [string]: Process, - }, +}; + +export type Trace = TraceData & { + duration: number, + endTime: number, + spans: Span[], + startTime: number, }; diff --git a/src/types/search.js b/src/types/search.js index 1ac54f0116..897b2fbe38 100644 --- a/src/types/search.js +++ b/src/types/search.js @@ -20,15 +20,16 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. +/** + * Type used to summarize traces for the search page. + */ export type TraceSummary = { /** * Duration of trace in milliseconds. - * @type {number} */ duration: number, /** * Start time of trace in milliseconds. - * @type {number} */ timestamp: number, traceName: string, @@ -41,7 +42,6 @@ export type TraceSummary = { export type TraceSummaries = { /** * Duration of longest trace in `traces` in milliseconds. - * @type {[type]} */ maxDuration: number, traces: TraceSummary[],