Skip to content

Commit

Permalink
Fix TraceTimelineViewer span details render issue (jaegertracing#629)
Browse files Browse the repository at this point in the history 
* Fixes TraceTimelineViewer span details

Signed-off-by: Ruben Vargas <[email protected]>

* perform deep comparision for memoized ViewBoundsFunc and GetCssClasses

Signed-off-by: Ruben Vargas <[email protected]>
Signed-off-by: vvvprabhakar <[email protected]>
  • Loading branch information
@rubenvp8510 @vvvprabhakar
rubenvp8510 authored and vvvprabhakar committed Jul 4, 2021
1 parent 38ed53c commit 41ae1ce
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 49 deletions.
2 changes: 1 addition & 1 deletion packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ describe('<VirtualizedTraceViewImpl>', () => {
expect(
rowWrapper.containsMatchingElement(
<SpanBarRow
className={instance.clippingCssClasses}
className={instance.getClippingCssClasses()}
columnDivision={props.spanNameColumnWidth}
isChildrenExpanded
isDetailExpanded={false}
Expand Down
98 changes: 50 additions & 48 deletions packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/VirtualizedTraceView.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ import cx from 'classnames';
import { connect } from 'react-redux';
import { bindActionCreators, Dispatch } from 'redux';
import { withRouter, RouteComponentProps } from 'react-router-dom';
import _isEqual from 'lodash/isEqual';

// import { History as RouterHistory, Location } from 'history';

import memoizeOne from 'memoize-one';
import { actions } from './duck';
import ListView from './ListView';
import SpanBarRow from './SpanBarRow';
Expand Down Expand Up @@ -130,6 +132,14 @@ function generateRowStates(
return rowStates;
}

function generateRowStatesFromTrace(
trace: Trace | TNil,
childrenHiddenIDs: Set<string>,
detailStates: Map<string, DetailState | TNil>
): RowState[] {
return trace ? generateRowStates(trace.spans, childrenHiddenIDs, detailStates) : [];
}

function getCssClasses(currentViewRange: [number, number]) {
const [zoomStart, zoomEnd] = currentViewRange;
return cx({
Expand All @@ -138,28 +148,16 @@ function getCssClasses(currentViewRange: [number, number]) {
});
}

const memoizedGenerateRowStates = memoizeOne(generateRowStatesFromTrace);
const memoizedViewBoundsFunc = memoizeOne(createViewedBoundsFunc, _isEqual);
const memoizedGetCssClasses = memoizeOne(getCssClasses, _isEqual);

// export from tests
export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceViewProps> {
clippingCssClasses: string;
listView: ListView | TNil;
rowStates: RowState[];
getViewedBounds: ViewedBoundsFunctionType;

constructor(props: VirtualizedTraceViewProps) {
super(props);
// keep "prop derivations" on the instance instead of calculating in
// `.render()` to avoid recalculating in every invocation of `.renderRow()`
const { currentViewRangeTime, childrenHiddenIDs, detailStates, setTrace, trace, uiFind } = props;
this.clippingCssClasses = getCssClasses(currentViewRangeTime);
const [zoomStart, zoomEnd] = currentViewRangeTime;
this.getViewedBounds = createViewedBoundsFunc({
min: trace.startTime,
max: trace.endTime,
viewStart: zoomStart,
viewEnd: zoomEnd,
});
this.rowStates = generateRowStates(trace.spans, childrenHiddenIDs, detailStates);

const { setTrace, trace, uiFind } = props;
setTrace(trace, uiFind);
}

Expand All @@ -180,14 +178,11 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
}

componentDidUpdate(prevProps: Readonly<VirtualizedTraceViewProps>) {
const { childrenHiddenIDs, detailStates, registerAccessors, trace, currentViewRangeTime } = prevProps;
const { registerAccessors, trace } = prevProps;
const {
shouldScrollToFirstUiFindMatch,
clearShouldScrollToFirstUiFindMatch,
scrollToFirstVisibleSpan,
currentViewRangeTime: nextViewRangeTime,
childrenHiddenIDs: nextHiddenIDs,
detailStates: nextDetailStates,
registerAccessors: nextRegisterAccessors,
setTrace,
trace: nextTrace,
Expand All @@ -198,21 +193,6 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
setTrace(nextTrace, uiFind);
}

if (trace !== nextTrace || childrenHiddenIDs !== nextHiddenIDs || detailStates !== nextDetailStates) {
this.rowStates = nextTrace ? generateRowStates(nextTrace.spans, nextHiddenIDs, nextDetailStates) : [];
}

if (currentViewRangeTime !== nextViewRangeTime) {
this.clippingCssClasses = getCssClasses(nextViewRangeTime);
const [zoomStart, zoomEnd] = nextViewRangeTime;
this.getViewedBounds = createViewedBoundsFunc({
min: trace.startTime,
max: trace.endTime,
viewStart: zoomStart,
viewEnd: zoomEnd,
});
}

if (this.listView && registerAccessors !== nextRegisterAccessors) {
nextRegisterAccessors(this.getAccessors());
}
Expand All @@ -223,6 +203,28 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
}
}

getRowStates(): RowState[] {
const { childrenHiddenIDs, detailStates, trace } = this.props;
return memoizedGenerateRowStates(trace, childrenHiddenIDs, detailStates);
}

getClippingCssClasses(): string {
const { currentViewRangeTime } = this.props;
return memoizedGetCssClasses(currentViewRangeTime);
}

getViewedBounds(): ViewedBoundsFunctionType {
const { currentViewRangeTime, trace } = this.props;
const [zoomStart, zoomEnd] = currentViewRangeTime;

return memoizedViewBoundsFunc({
min: trace.startTime,
max: trace.endTime,
viewStart: zoomStart,
viewEnd: zoomEnd,
});
}

focusSpan = (uiFind: string) => {
const { trace, focusUiFindMatches, location, history } = this.props;
if (trace) {
Expand Down Expand Up @@ -259,12 +261,12 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi

getCollapsedChildren = () => this.props.childrenHiddenIDs;

mapRowIndexToSpanIndex = (index: number) => this.rowStates[index].spanIndex;
mapRowIndexToSpanIndex = (index: number) => this.getRowStates()[index].spanIndex;

mapSpanIndexToRowIndex = (index: number) => {
const max = this.rowStates.length;
const max = this.getRowStates().length;
for (let i = 0; i < max; i++) {
const { spanIndex } = this.rowStates[i];
const { spanIndex } = this.getRowStates()[i];
if (spanIndex === index) {
return i;
}
Expand All @@ -283,17 +285,17 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
// use long form syntax to avert flow error
// https://github.com/facebook/flow/issues/3076#issuecomment-290944051
getKeyFromIndex = (index: number) => {
const { isDetail, span } = this.rowStates[index];
const { isDetail, span } = this.getRowStates()[index];
return `${span.spanID}--${isDetail ? 'detail' : 'bar'}`;
};

getIndexFromKey = (key: string) => {
const parts = key.split('--');
const _spanID = parts[0];
const _isDetail = parts[1] === 'detail';
const max = this.rowStates.length;
const max = this.getRowStates().length;
for (let i = 0; i < max; i++) {
const { span, isDetail } = this.rowStates[i];
const { span, isDetail } = this.getRowStates()[i];
if (span.spanID === _spanID && isDetail === _isDetail) {
return i;
}
Expand All @@ -302,7 +304,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
};

getRowHeight = (index: number) => {
const { span, isDetail } = this.rowStates[index];
const { span, isDetail } = this.getRowStates()[index];
if (!isDetail) {
return DEFAULT_HEIGHTS.bar;
}
Expand All @@ -315,7 +317,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
linksGetter = (span: Span, items: KeyValuePair[], itemIndex: number) => getLinks(span, items, itemIndex);

renderRow = (key: string, style: React.CSSProperties, index: number, attrs: {}) => {
const { isDetail, span, spanIndex } = this.rowStates[index];
const { isDetail, span, spanIndex } = this.getRowStates()[index];
return isDetail
? this.renderSpanDetailRow(span, key, style, attrs)
: this.renderSpanBarRow(span, spanIndex, key, style, attrs);
Expand Down Expand Up @@ -348,7 +350,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
if (isCollapsed) {
const rpcSpan = findServerChildSpan(trace.spans.slice(spanIndex));
if (rpcSpan) {
const rpcViewBounds = this.getViewedBounds(rpcSpan.startTime, rpcSpan.startTime + rpcSpan.duration);
const rpcViewBounds = this.getViewedBounds()(rpcSpan.startTime, rpcSpan.startTime + rpcSpan.duration);
rpc = {
color: colorGenerator.getColorByKey(rpcSpan.process.serviceName),
operationName: rpcSpan.operationName,
Expand All @@ -361,7 +363,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
return (
<div className="VirtualizedTraceView--row" key={key} style={style} {...attrs}>
<SpanBarRow
className={this.clippingCssClasses}
className={this.getClippingCssClasses()}
color={color}
columnDivision={spanNameColumnWidth}
isChildrenExpanded={!isCollapsed}
Expand All @@ -372,7 +374,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
onChildrenToggled={childrenToggle}
rpc={rpc}
showErrorIcon={showErrorIcon}
getViewedBounds={this.getViewedBounds}
getViewedBounds={this.getViewedBounds()}
traceStartTime={trace.startTime}
span={span}
focusSpan={this.focusSpan}
Expand Down Expand Up @@ -428,7 +430,7 @@ export class VirtualizedTraceViewImpl extends React.Component<VirtualizedTraceVi
<div className="VirtualizedTraceView--spans">
<ListView
ref={this.setListView}
dataLength={this.rowStates.length}
dataLength={this.getRowStates().length}
itemHeightGetter={this.getRowHeight}
itemRenderer={this.renderRow}
viewBuffer={300}
Expand Down

0 comments on commit 41ae1ce

Please sign in to comment.