Skip to content

Commit

Permalink
Virtualized scrolling for trace detail view (jaegertracing#68)
Browse files Browse the repository at this point in the history 
* Update prettier and wrap at 110 instead of 80

* WIP commit for refactoring trace detail view

TODO
- Test are currently in a bad state
- Finish shifting to transformed trace, exclusively, instead of raw
  trace
- Misc cleanup (unused / ordering of vars, imports, etc) is outstanding
  while changes are WIP
- Continued refinement of trace-detail components, selectors and utils
- Many styles will be moved to CSS
- Will likely remove `model/trace-viewer.js`

SpanGraph

- Fix #49 - Span position in graph doesn not match its position in the
  detail
- Ticks in span graph made to match trace detail (in number and
  formatting)
- Span graph refactored to trim down files and DOM elements

TracePageHeader

- `trace` prop removed
- Added props for various title values instead of deriving them
  from `trace`

Trace Detail

- Several components split out into separate files
- `transformTrace` to use alread span tree to determine span depth

Span Detail

- Fix uber/jaeger jaegertracing#326: extraneous scrollbars in trace views
- Fix: Some tags were not being rendered due to clashing keys (observed
  in a log message)
- Tall content scrolls via entire table instead of single table cell
- Horizontal scrolling for wide content (e.g. long log values)
- Full width of the header is clickable for tags, process, and logs
  headers (instead of header text, only)
- Service and endpoint are shown on mouseover anywhere in span bar row

Misc
- Several TraceTimelineViewer / utils removed
- `TreeNode` `.walk()` method can now be used to calculate the depth,
  avoiding use of less efficient `.getPath()`
- Removed several `console.error` warnings caused by React key issues

* WIP commit for refactoring trace detail view

TODO
- Test are currently in a bad state
- Finish shifting to transformed trace, exclusively, instead of raw
  trace
- Misc cleanup (unused / ordering of vars, imports, etc) is outstanding
  while changes are WIP
- Continued refinement of trace-detail components, selectors and utils
- Many styles will be moved to CSS
- Will likely remove `model/trace-viewer.js`

Span Bar / Detail
- Label on span bars no longer off-screen
- Clip or hide span bars when zoomed in (insted of flush left)
- Add shadow to left / right boundary when span bar view is clipped
- Darkened span name column to differentiate from span bar section
- Span detail left column color coded by service
- Clicking span detail left column collapses detail
- Clicking anywhere left of parent span name toggles children
  visibility

* WIP refactor trace detail, split out css, start fixing tests

* WIP refactor trace detail, tests working again

* WIP refactor trace, yarn upgrade

Sub-page scrolling used for trace detail (TODO: revert this). This lays
the ground work for using react-virtualized, but unfortunately has major
perf issues, hence the TODO: revert.

yarn upgrade --latest. Notable changes:

- Removed react-sticky
- react-router v4
- react-vis CSS needed to be included via a sym-link

Misc tweaks:

- Styling adjusted on trace mini-map
- Scatterplot dots are sized based on number of spans
- Scatterplot dots mouseover shows trace name

* WIP refactor trace, test maintenance

* remove unused import

* add license to css files

* Revert sub-page scrolling for trace detail

* Support URL prefix via homepage in package.json

* Prevent collision of logs in log entries table

* Add comments to new utils

* WIP Use react-virtualized in trace detail view

* Fix jaegertracing#59 - "Span Name" to "Service & Operation"

* Fix unreleased regression - ellipsis on span name

Add back the styling that adds an ellipsis to truncated span name text.

* Address PR comment on search results scatter plot

https://github.com/uber/jaeger-ui/pull/53#discussion_r134313013

* Misc cleanup from PR comment

https://github.com/uber/jaeger-ui/pull/53#discussion_r134314020

* PR comment - Remove ms and use nano seconds

https://github.com/uber/jaeger-ui/pull/53#discussion_r134316418

* PR comment - Adjust export, relates to recompose

https://github.com/uber/jaeger-ui/pull/53#discussion_r134318188

* Reorganize span detail components

* PR comment - Comment getTraceSpanIdsAsTree()

https://github.com/uber/jaeger-ui/pull/53#discussion_r134321990

* PR comment - Add "SpanID:" label

https://github.com/uber/jaeger-ui/pull/64#issuecomment-324080675

* WIP react-virtualized layout progress

Outstanding:

- Window scroller used
  https://bvaughn.github.io/react-virtualized/#/components/WindowScroller
- Expanding span details
- Collapsing / expanding children

* WIP react-virtualized layout progress

Switching to use a props oriented ui state management for span details.
This is necessary because the virtualized scroller removes and adds span
details as it scrolls. The span detail components cannot retain their
own state (e.g. tags table is expanded / collapsed, etc.) because they
are not long-lived.

* PR comment - Flow typing for props

https://github.com/uber/jaeger-ui/pull/64#discussion_r134792924

* WIP Switching react-virtualized to custom solution

* fix file-header licenses

* WIP Switching react-virtualized to custom solution

* Add flow to ListView, comment ListView, Positions

* Positions tests, fix edge-case bug in Positions

* Fix comment issue from prettier

* Unit tests for ListView, remove unused propType/*

* Minor changes to a comment and a test case description

* Unit tests for TraceTimelineViewer/duck

* remove react-virtualized from package.json

* PR feedback - JSDoc, move trace transform

- JSDoc tweaks - documentation.js can infer types automatically
- Move trace transform - Move trace tranform out of
  src/components/TracePage/TraceTimelineViewer/transforms and into
  src/model/transform-trace-data.js
- Apply trace transform to data from HTTP response so the trace data
  stored in the state is always the enriched form

* Use top-level redux store for traceTimeline state

Signed-off-by: vvvprabhakar <[email protected]>
  • Loading branch information
@tiffon
tiffon authored Sep 8, 2017
1 parent 7d53851 commit e28ba7c
Show file tree
Hide file tree
Showing 51 changed files with 2,948 additions and 705 deletions.
22 changes: 19 additions & 3 deletions .eslintrc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@
],
"rules": {
/* general */
"comma-dangle": 0,
"arrow-parens": [1, "as-needed"],
"no-restricted-syntax": 0,
"comma-dangle": 0,
"no-continue": 0,
"no-plusplus": 0,
"no-restricted-syntax": 0,
"no-self-compare": 0,
"no-underscore-dangle": 0,

/* jsx */
"jsx-a11y/no-static-element-interactions": 1,
Expand All @@ -29,7 +32,20 @@
"react/forbid-prop-types": 1,
"react/require-default-props": 1,
"react/no-array-index-key": 1,

"react/sort-comp": [2, {
"order": [
"type-annotations",
"statics",
"state",
"propTypes",
"static-methods",
"constructor",
"lifecycle",
"everything-else",
"/^on.+$/",
"render"
]
}],

/* import */
"import/prefer-default-export": 1,
Expand Down
3 changes: 3 additions & 0 deletions .flowconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@
[libs]

[options]

[version]
0.53.1
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"homepage": null,
"devDependencies": {
"babel-eslint": "^7.2.3",
"bluebird": "^3.5.0",
"enzyme": "^2.9.1",
"eslint": "^4.5.0",
"eslint-config-airbnb": "^15.1.0",
Expand Down
64 changes: 28 additions & 36 deletions src/components/TracePage/TraceSpanGraph.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@ 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;

export default class TraceSpanGraph extends Component {
static get propTypes() {
return {
xformedTrace: PropTypes.object,
trace: PropTypes.object,
height: PropTypes.number.isRequired,
};
Expand Down Expand Up @@ -71,13 +69,31 @@ 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
);
}

startDragging(boundName, { clientX }) {
this.setState({ currentlyDragging: boundName, prevX: clientX });

const mouseMoveHandler = (...args) => this.onMouseMove(...args);
const mouseUpHandler = () => {
this.stopDragging();
window.removeEventListener('mouseup', mouseUpHandler);
window.removeEventListener('mousemove', mouseMoveHandler);
};

window.addEventListener('mouseup', mouseUpHandler);
window.addEventListener('mousemove', mouseMoveHandler);
}

stopDragging() {
this.setState({ currentlyDragging: null, prevX: null });
}

onMouseMove({ clientX }) {
const { trace } = this.props;
const { prevX, currentlyDragging } = this.state;
Expand All @@ -91,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;
Expand All @@ -115,26 +128,8 @@ export default class TraceSpanGraph extends Component {
}
}

startDragging(boundName, { clientX }) {
this.setState({ currentlyDragging: boundName, prevX: clientX });

const mouseMoveHandler = (...args) => this.onMouseMove(...args);
const mouseUpHandler = () => {
this.stopDragging();
window.removeEventListener('mouseup', mouseUpHandler);
window.removeEventListener('mousemove', mouseMoveHandler);
};

window.addEventListener('mouseup', mouseUpHandler);
window.addEventListener('mousemove', mouseMoveHandler);
}

stopDragging() {
this.setState({ currentlyDragging: null, prevX: null });
}

render() {
const { trace, xformedTrace, height } = this.props;
const { trace, height } = this.props;
const { currentlyDragging } = this.state;
const { timeRangeFilter } = this.context;
const leftBound = timeRangeFilter[0];
Expand All @@ -144,23 +139,20 @@ export default class TraceSpanGraph extends Component {
return <div />;
}

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 (
<div>
<div className="trace-page-timeline--tick-container">
<SpanGraphTickHeader numTicks={TIMELINE_TICK_INTERVAL} duration={traceDuration} />
<SpanGraphTickHeader numTicks={TIMELINE_TICK_INTERVAL} duration={trace.duration} />
</div>
<div>
<svg
Expand All @@ -187,9 +179,9 @@ export default class TraceSpanGraph extends Component {
className="trace-page-timeline__graph--inactive"
/>}
<SpanGraph
valueWidth={xformedTrace.duration}
valueWidth={trace.duration}
numTicks={TIMELINE_TICK_INTERVAL}
items={xformedTrace.spans.map(span => ({
items={trace.spans.map(span => ({
valueOffset: span.relativeStartTime,
valueWidth: span.duration,
serviceName: span.process.serviceName,
Expand Down
30 changes: 11 additions & 19 deletions src/components/TracePage/TraceSpanGraph.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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('<TraceSpanGraph>', () => {
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: () => {},
},
};
Expand All @@ -68,7 +61,7 @@ describe('<TraceSpanGraph>', () => {
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(<TraceSpanGraph {...props} />, { ...options, context });
const leftBox = wrapper.find('.trace-page-timeline__graph--inactive');
Expand All @@ -82,7 +75,7 @@ describe('<TraceSpanGraph>', () => {
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(<TraceSpanGraph {...props} />, { ...options, context });
const rightBox = wrapper.find('.trace-page-timeline__graph--inactive');
Expand Down Expand Up @@ -132,7 +125,7 @@ describe('<TraceSpanGraph>', () => {

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,
Expand All @@ -151,9 +144,8 @@ describe('<TraceSpanGraph>', () => {
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);
});

Expand Down Expand Up @@ -190,7 +182,7 @@ describe('<TraceSpanGraph>', () => {
});

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 };
Expand All @@ -204,7 +196,7 @@ describe('<TraceSpanGraph>', () => {
});

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 };
Expand Down
Loading

0 comments on commit e28ba7c

Please sign in to comment.