Brings back relative width in trace query screen (#2232)

This fixes a bug in the trace query screen and reorganizes code to more
easily tell which screens need which data from the trace summary.

zipkin-ui/js/component_data/default.js

@@ -58,9 +58,9 @@ export default component(function DefaultData() {
       type: 'GET',
       dataType: 'json'
     }).done(traces => {
-      const summaries = traces.map(raw => rawTraceToSummary(raw));
+      const traceSummaries = traces.map(raw => rawTraceToSummary(raw));
       const modelview = {
-        traces: traceSummariesToMustache(apiQuery.serviceName, summaries),
+        traces: traceSummariesToMustache(apiQuery.serviceName, traceSummaries),
         apiURL,
         rawResponse: traces
       };

zipkin-ui/js/component_ui/traceSummary.js

@@ -108,12 +108,20 @@ export function getServiceName(span) {
   return allServiceNames.length === 0 ? null : allServiceNames[0];
 }
 
-function getSpanTimestamps(spans) {
-  return _(spans).flatMap((span) => getServiceNames(span).map((serviceName) => ({
+export function getGroupedTimestamps(spans) {
+  const spanTimestamps = _(spans).flatMap((span) => getServiceNames(span).map((serviceName) => ({
     name: serviceName,
     timestamp: span.timestamp,
     duration: span.duration
   }))).value();
+
+  const serviceToNameTimestampDurations = _(spanTimestamps).groupBy((sts) => sts.name).value();
+
+  // wash out the redundant name. TODO: rewrite this whole method as it seems easier imperatively
+  return _(serviceToNameTimestampDurations).mapValues((ntds) => ntds.map((ntd) => ({
+    timestamp: ntd.timestamp,
+    duration: ntd.duration
+  }))).value();
 }
 
 
@@ -147,14 +155,14 @@ export function traceSummary(spans = []) {
     const endpoints = _(spans).flatMap(endpointsForSpan).uniqWith(endpointEquals).value();
     const traceId = spans[0].traceId;
     const timestamp = spans[0].timestamp;
-    const spanTimestamps = getSpanTimestamps(spans);
+    const groupedTimestamps = getGroupedTimestamps(spans);
     const errorType = getTraceErrorType(spans);
     const totalSpans = spans.length;
     return {
       traceId,
       timestamp,
       duration,
-      spanTimestamps,
+      groupedTimestamps,
       endpoints,
       errorType,
       totalSpans
@@ -188,10 +196,6 @@ function formatDate(timestamp, utc) {
   return m.format('MM-DD-YYYYTHH:mm:ss.SSSZZ');
 }
 
-export function getGroupedTimestamps(summary) {
-  return _(summary.spanTimestamps).groupBy((sts) => sts.name).value();
-}
-
 export function getServiceDurations(groupedTimestamps) {
   return _(groupedTimestamps).toPairs().map(([name, sts]) => ({
     name,
@@ -227,40 +231,32 @@ export function traceSummariesToMustache(serviceName = null, traceSummaries, utc
     return [];
   } else {
     const traceSummariesCleaned = removeEmptyFromArray(traceSummaries);
-    const maxDuration = Math.max(...traceSummariesCleaned.map((s) => s.duration)) / 1000;
+    const maxDuration = Math.max(...traceSummariesCleaned.map((s) => s.duration));
 
     return traceSummariesCleaned.map((t) => {
-      const duration = t.duration / 1000;
-      const groupedTimestamps = getGroupedTimestamps(t);
-      const serviceDurations = getServiceDurations(groupedTimestamps);
-      let servicePercentage;
-      let serviceTime;
-      if (!serviceName || !groupedTimestamps[serviceName]) {
-        serviceTime = 0;
-      } else {
-        serviceTime = totalServiceTime(groupedTimestamps[serviceName]);
-        servicePercentage = parseInt(
-          parseFloat(serviceTime) / parseFloat(t.duration) * 100,
-        10);
-      }
-
-      const startTs = formatDate(t.timestamp, utc);
-      const durationStr = mkDurationStr(t.duration);
-      const width = parseInt(parseFloat(duration) / parseFloat(maxDuration) * 100, 10);
-      const infoClass = t.errorType === 'none' ? '' : `trace-error-${t.errorType}`;
+      const timestamp = t.timestamp;
+      const duration = t.duration;
+      const groupedTimestamps = t.groupedTimestamps;
 
-      return {
+      const res = {
         traceId: t.traceId,
-        startTs,
-        timestamp: t.timestamp,
-        duration,
-        durationStr,
-        servicePercentage,
+        startTs: formatDate(timestamp, utc),
+        timestamp,
+        duration: duration / 1000,
+        durationStr: mkDurationStr(duration),
+        width: parseInt(parseFloat(duration) / parseFloat(maxDuration) * 100, 10),
         totalSpans: t.totalSpans,
-        serviceDurations,
-        width,
-        infoClass
+        serviceDurations: getServiceDurations(groupedTimestamps),
+        infoClass: t.errorType === 'none' ? '' : `trace-error-${t.errorType}`
       };
+
+      // Only add a service percentage when there is a duration for it
+      if (serviceName && groupedTimestamps[serviceName]) {
+        const serviceTime = totalServiceTime(groupedTimestamps[serviceName]);
+        res.servicePercentage = parseInt(parseFloat(serviceTime) / parseFloat(duration) * 100, 10);
+      }
+
+      return res;
     }).sort((t1, t2) => {
       const durationComparison = t2.duration - t1.duration;
       if (durationComparison === 0) {

zipkin-ui/js/component_ui/traceToMustache.js

@@ -1,7 +1,6 @@
 import _ from 'lodash';
 import {
   traceSummary,
-  getGroupedTimestamps,
   getServiceDurations,
   getServiceNames,
   getServiceName,
@@ -91,11 +90,11 @@ export function formatEndpoint({ipv4, ipv6, port, serviceName}) {
 }
 
 export default function traceToMustache(trace, logsUrl = undefined) {
-  const summary = traceSummary(trace);
-  const traceId = summary.traceId;
-  const duration = mkDurationStr(summary.duration);
-  const groupedTimestamps = getGroupedTimestamps(summary);
-  const serviceDurations = getServiceDurations(groupedTimestamps);
+  const t = traceSummary(trace);
+  const traceId = t.traceId;
+  const duration = t.duration;
+  const serviceDurations = getServiceDurations(t.groupedTimestamps);
+
   const services = serviceDurations.length || 0;
   const serviceCounts = _(serviceDurations).sortBy('name').value();
   const groupByParentId = _(trace).groupBy((s) => s.parentId).value();
@@ -109,7 +108,7 @@ export default function traceToMustache(trace, logsUrl = undefined) {
     (rootSpan) => childrenToList(createSpanTreeEntry(rootSpan, trace))).map((span) => {
       const spanStartTs = span.timestamp || traceTimestamp;
       const spanDepth = spanDepths[span.id] || 1;
-      const width = (span.duration || 0) / summary.duration * 100;
+      const width = (span.duration || 0) / duration * 100;
       let errorType = 'none';
 
       const binaryAnnotations = (span.binaryAnnotations || [])
@@ -157,7 +156,7 @@ export default function traceToMustache(trace, logsUrl = undefined) {
         serviceName: getServiceName(span) || '',
         duration: span.duration,
         durationStr: mkDurationStr(span.duration),
-        left: parseFloat(spanStartTs - traceTimestamp) / parseFloat(summary.duration) * 100,
+        left: parseFloat(spanStartTs - traceTimestamp) / parseFloat(duration) * 100,
         width: width < 0.1 ? 0.1 : width,
         depth: (spanDepth + 1) * 5,
         depthClass: (spanDepth - 1) % 6,
@@ -179,13 +178,13 @@ export default function traceToMustache(trace, logsUrl = undefined) {
 
   const totalSpans = spans.length;
   const timeMarkers = [0.0, 0.2, 0.4, 0.6, 0.8, 1.0]
-      .map((p, index) => ({index, time: mkDurationStr(summary.duration * p)}));
+      .map((p, index) => ({index, time: mkDurationStr(duration * p)}));
   const timeMarkersBackup = timeMarkers;
   const spansBackup = spans;
 
   return {
     traceId,
-    duration,
+    duration: mkDurationStr(duration),
     services,
     depth,
     totalSpans,

zipkin-ui/test/component_data/default.test.js

@@ -66,28 +66,28 @@ describe('rawTraceToSummary', () => {
       traceId: '1e223ff1f80f1c69',
       timestamp: 1470150004071068,
       duration: 99411,
-      spanTimestamps: [
-        {
-          name: 'servicea',
-          timestamp: 1470150004071068,
-          duration: 99411
-        },
-        {
-          name: 'servicea',
-          timestamp: 1470150004074202,
-          duration: 94539
-        },
-        {
-          name: 'serviceb',
-          timestamp: 1470150004074202,
-          duration: 94539
-        },
-        {
-          name: 'serviceb',
-          timestamp: 1470150004074684,
-          duration: 65000
-        }
-      ],
+      groupedTimestamps: {
+        servicea: [
+          {
+            timestamp: 1470150004071068,
+            duration: 99411
+          },
+          {
+            timestamp: 1470150004074202,
+            duration: 94539
+          }
+        ],
+        serviceb: [
+          {
+            timestamp: 1470150004074202,
+            duration: 94539
+          },
+          {
+            timestamp: 1470150004074684,
+            duration: 65000
+          }
+        ]
+      },
       endpoints: [
         {
           serviceName: 'servicea',

zipkin-ui/test/component_ui/traceSummary.test.js

@@ -312,19 +312,24 @@ describe('traceSummariesToMustache', () => {
     traceId: 'cafedead',
     timestamp: start,
     duration: 20000,
-    spanTimestamps: [{
-      name: 'A',
-      timestamp: start,
-      duration: 10000
-    }, {
-      name: 'B',
-      timestamp: start + 1000,
-      duration: 20000
-    }, {
-      name: 'B',
-      timestamp: start + 1000,
-      duration: 15000
-    }],
+    groupedTimestamps: {
+      A: [
+        {
+          timestamp: start,
+          duration: 10000
+        }
+      ],
+      B: [
+        {
+          timestamp: start + 1000,
+          duration: 20000
+        },
+        {
+          timestamp: start + 1000,
+          duration: 15000
+        }
+      ]
+    },
     endpoints: [ep1, ep2]
   };
 
@@ -371,8 +376,29 @@ describe('traceSummariesToMustache', () => {
   });
 
   it('should calculate the width in percent', () => {
-    const model = traceSummariesToMustache(null, [summary]);
+    const summary1 = {
+      traceId: 'cafebaby',
+      timestamp: start,
+      duration: 2000,
+      groupedTimestamps: {
+        A: [{timestamp: start + 1, duration: 2000}]
+      },
+      endpoints: [ep1]
+    };
+    const summary2 = {
+      traceId: 'cafedead',
+      timestamp: start,
+      duration: 20000,
+      groupedTimestamps: {
+        A: [{timestamp: start, duration: 20000}]
+      },
+      endpoints: [ep1]
+    };
+
+    // Model is ordered by duration, and the width should be relative (percentage)
+    const model = traceSummariesToMustache(null, [summary1, summary2]);
     model[0].width.should.equal(100);
+    model[1].width.should.equal(10);
   });
 
   it('should pass on timestamp', () => {