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
jaegertracing · Sep 5, 2017 · 6059fed · 6059fed
1 parent 3c8057e
commit 6059fed
Show file tree

Hide file tree

Showing 24 changed files with 244 additions and 294 deletions.
diff --git a/src/components/TracePage/TraceSpanGraph.js b/src/components/TracePage/TraceSpanGraph.js
@@ -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,
     };
@@ -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 <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
@@ -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,

diff --git 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('<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: () => {},
     },
   };
@@ -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');
@@ -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');
@@ -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,
@@ -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);
     });
 
@@ -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 };
@@ -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 };

diff --git 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;