Reverts skew "uncorrection" in index as it affects service percentages (

openzipkin#2229)

In openzipkin#2228 my assumption was incorrect as @tacigar highlighted. This makes
a test to ensure the logic stays the same for now.

zipkin-ui/js/component_data/default.js

@@ -4,6 +4,7 @@ import $ from 'jquery';
 import queryString from 'query-string';
 import {traceSummary, traceSummariesToMustache} from '../component_ui/traceSummary';
 import {SPAN_V1} from '../spanConverter';
+import {correctForClockSkew} from '../skew';
 
 export function convertToApiQuery(source) {
   const query = Object.assign({}, source);
@@ -37,6 +38,13 @@ export function convertToApiQuery(source) {
   return query;
 }
 
+export function rawTraceToSummary(raw) {
+  const v1Trace = raw.map(SPAN_V1.convert);
+  const mergedTrace = SPAN_V1.mergeById(v1Trace);
+  const clockSkewCorrectedTrace = correctForClockSkew(mergedTrace);
+  return traceSummary(clockSkewCorrectedTrace);
+}
+
 export default component(function DefaultData() {
   this.after('initialize', function() {
     const query = queryString.parse(window.location.search);
@@ -50,12 +58,7 @@ export default component(function DefaultData() {
       type: 'GET',
       dataType: 'json'
     }).done(traces => {
-      const summaries = traces.map(raw => {
-        const v1Trace = raw.map(SPAN_V1.convert);
-        const mergedTrace = SPAN_V1.mergeById(v1Trace);
-        return traceSummary(mergedTrace);
-      });
-
+      const summaries = traces.map(raw => rawTraceToSummary(raw));
       const modelview = {
         traces: traceSummariesToMustache(apiQuery.serviceName, summaries),
         apiURL,

zipkin-ui/js/skew.js

@@ -214,6 +214,7 @@ function adjustTimestamps(span, skew) {
       result.timestamp = annotationTimestamp - skew.skew;
     }
     result.annotations = annotations;
+    result.annotations.sort((a, b) => a.timestamp - b.timestamp);
     return result;
   }
   // Search for a local span on the skewed endpoint

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

@@ -1,6 +1,111 @@
-import {convertToApiQuery} from '../../js/component_data/default';
+import {convertToApiQuery, rawTraceToSummary} from '../../js/component_data/default';
 import queryString from 'query-string';
 
+describe('rawTraceToSummary', () => {
+  it('should create trace summary from skewed trace', () => {
+    // from ../tracedata/skew.json as we can't figure out how to read file with headless chrome env
+    const skewedTrace = [
+      {
+        traceId: '1e223ff1f80f1c69',
+        parentId: '74280ae0c10d8062',
+        id: '43210ae0c10d1234',
+        name: 'async',
+        timestamp: 1470150004008762,
+        duration: 65000,
+        localEndpoint: {
+          serviceName: 'serviceb',
+          ipv4: '192.0.0.0'
+        }
+      },
+      {
+        traceId: '1e223ff1f80f1c69',
+        parentId: 'bf396325699c84bf',
+        id: '74280ae0c10d8062',
+        kind: 'SERVER',
+        name: 'post',
+        timestamp: 1470150004008761,
+        duration: 93577,
+        localEndpoint: {
+          serviceName: 'serviceb',
+          ipv4: '192.0.0.0'
+        },
+        shared: true
+      },
+      {
+        traceId: '1e223ff1f80f1c69',
+        id: 'bf396325699c84bf',
+        kind: 'SERVER',
+        name: 'get',
+        timestamp: 1470150004071068,
+        duration: 99411,
+        localEndpoint: {
+          serviceName: 'servicea',
+          ipv4: '127.0.0.0'
+        },
+        shared: true
+      },
+      {
+        traceId: '1e223ff1f80f1c69',
+        parentId: 'bf396325699c84bf',
+        id: '74280ae0c10d8062',
+        kind: 'CLIENT',
+        name: 'post',
+        timestamp: 1470150004074202,
+        duration: 94539,
+        localEndpoint: {
+          serviceName: 'servicea',
+          ipv4: '127.0.0.0'
+        }
+      }
+    ];
+
+    // TODO: This test only ensures the behavior of the existing code. For example, if we don't
+    // clock skew correct, the result will be a different duration. We haven't verified the logic
+    // is correct.. for example, the calculated duration here may be wrong.
+    const expectedTraceSummary = {
+      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
+        }
+      ],
+      endpoints: [
+        {
+          serviceName: 'servicea',
+          ipv4: '127.0.0.0'
+        },
+        {
+          serviceName: 'serviceb',
+          ipv4: '192.0.0.0'
+        }
+      ],
+      errorType: 'none',
+      totalSpans: 3
+    };
+
+    rawTraceToSummary(skewedTrace).should.deep.equal(expectedTraceSummary);
+  });
+});
+
 describe('convertToApiQuery', () => {
   const should = require('chai').should();
   it('should clear spanName all', () => {

zipkin-ui/test/skew.test.js

@@ -174,7 +174,7 @@ function createRootSpan(endpoint, begin, duration) {
 function childSpan(parent, to, begin, duration, skew) {
   const spanId = parent.id + 1;
   const from = parent.annotations[0].endpoint;
-  return {
+  const res = {
     traceId: parent.traceId,
     parentId: parent.id,
     id: spanId,
@@ -186,6 +186,8 @@ function childSpan(parent, to, begin, duration, skew) {
       {timestamp: begin + duration, value: 'cr', endpoint: from}
     ]
   };
+  res.annotations.sort((a, b) => a.timestamp - b.timestamp);
+  return res;
 }
 
 function localSpan(parent, endpoint, begin, duration) {
@@ -428,6 +430,9 @@ describe('correctForClockSkew', () => {
     const adjustedRpcSpan = adjustedSpans.find((s) => s.id === rpcSpan.id);
     expect(annotationTimestamp(adjustedRpcSpan, 'sr')).to.equal(rpcClientSendTs + networkLatency);
     expect(annotationTimestamp(adjustedRpcSpan, 'cs')).to.equal(adjustedRpcSpan.timestamp);
+    // ensure annotations are sorted after skew adjustment
+    expect(adjustedRpcSpan.annotations.map(a => a.value)).to.deep.equal(['cs', 'sr', 'ss', 'cr']);
+
 
     const adjustedTierSpan = adjustedSpans.find((s) => s.id === tierSpan.id);
     expect(annotationTimestamp(adjustedTierSpan, 'cs')).to.equal(adjustedTierSpan.timestamp);

zipkin-ui/testdata/skew.json

@@ -1,4 +1,16 @@
 [
+  {
+    "traceId": "1e223ff1f80f1c69",
+    "parentId": "74280ae0c10d8062",
+    "id": "43210ae0c10d1234",
+    "name": "async",
+    "timestamp": 1470150004008762,
+    "duration": 65000,
+    "localEndpoint": {
+      "serviceName": "serviceb",
+      "ipv4": "192.0.0.0"
+    }
+  },
   {
     "traceId": "1e223ff1f80f1c69",
     "parentId": "bf396325699c84bf",