diff --git a/zipkin-ui/js/skew.js b/zipkin-ui/js/skew.js index 655cf8c9f6c..a7e3678f1fc 100644 --- a/zipkin-ui/js/skew.js +++ b/zipkin-ui/js/skew.js @@ -241,7 +241,7 @@ function oneWaySkew(serverRecv, clientSend) { } // Uses client/server annotations to determine if there's clock skew. -function getClockSkew(span) { +function getClockSkew(parent, span) { let clientSend; let serverRecv; let serverSend; @@ -265,6 +265,21 @@ function getClockSkew(span) { } }); + // This may be a single-host span. Look for the parent + if (serverRecv && !clientSend && parent) { + (parent.annotations || []).forEach((a) => { + switch (a.value) { + case 'cs': + clientSend = a; + break; + case 'cr': + clientRecv = a; + break; + default: + } + }); + } + let oneWay = false; if (!clientSend || !serverRecv) { return undefined; @@ -351,8 +366,11 @@ function adjust(node, skewFromParent) { node.setValue(adjustTimestamps(node.value, skewFromParent)); } - // Is there any skew in the current span? - let skew = getClockSkew(node.value); + // An RPC span can share an ID (have the same ID) or be split across parent and child + // We only look for skew between a client and the server + const parentVal = node.parent ? node.parent.value : undefined; + let skew = getClockSkew(parentVal, node.value); + if (skew) { // the current span's skew may be a different endpoint than its parent, so adjust again. node.setValue(adjustTimestamps(node.value, skew)); diff --git a/zipkin-ui/test/skew.test.js b/zipkin-ui/test/skew.test.js index f00b06c2c90..ab78d80ac69 100644 --- a/zipkin-ui/test/skew.test.js +++ b/zipkin-ui/test/skew.test.js @@ -283,7 +283,7 @@ describe('correctForClockSkew', () => { {timestamp: 40, value: 'cr', endpoint: frontend} ] }; - should.equal(getClockSkew(skewedSameHost), undefined); + should.equal(getClockSkew(undefined, skewedSameHost), undefined); }); /* @@ -297,7 +297,7 @@ describe('correctForClockSkew', () => { const cr = {timestamp: 40, value: 'cr', endpoint: frontend}; const skewedRpc = {traceId: '1', id: '1', annotations: [cs, sr, ss, cr]}; - const skew = getClockSkew(skewedRpc); + const skew = getClockSkew(undefined, skewedRpc); // Skew correction pushes the server side forward, so the skew endpoint is the server expect(skew.endpoint).to.equal(sr.endpoint); @@ -311,13 +311,26 @@ describe('correctForClockSkew', () => { ); }); + it('corrects skew on single-host spans', () => { + const cs = {timestamp: 20, value: 'cs', endpoint: frontend}; + const sr = {timestamp: 10 /* skew */, value: 'sr', endpoint: backend}; + const ss = {timestamp: 20, value: 'ss', endpoint: backend}; + const cr = {timestamp: 40, value: 'cr', endpoint: frontend}; + + const skewedRpc = {traceId: '1', id: '1', annotations: [cs, sr, ss, cr]}; + const skewedClient = {traceId: '1', id: '1', annotations: [cs, cr]}; + const skewedServer = {traceId: '1', id: '2', annotations: [sr, ss]}; + + expect(getClockSkew(skewedClient, skewedServer)).to.eql(getClockSkew(undefined, skewedRpc)); + }); + // Sets the server to 1us past the client it('skew on one-way spans assumes latency is at least 1us', () => { const cs = {timestamp: 20, value: 'cs', endpoint: frontend}; const sr = {timestamp: 10 /* skew */, value: 'sr', endpoint: backend}; const skewedOneWay = {traceId: '1', id: '1', annotations: [cs, sr]}; - const skew = getClockSkew(skewedOneWay); + const skew = getClockSkew(undefined, skewedOneWay); expect(skew.skew).to.equal( sr.timestamp - cs.timestamp // how much sr is behind @@ -333,7 +346,7 @@ describe('correctForClockSkew', () => { const cr = {timestamp: 25, value: 'cr', endpoint: frontend}; const skewedAsyncRpc = {traceId: '1', id: '1', annotations: [cs, sr, ss, cr]}; - const skew = getClockSkew(skewedAsyncRpc); + const skew = getClockSkew(undefined, skewedAsyncRpc); expect(skew.skew).to.equal( sr.timestamp - cs.timestamp // how much sr is behind @@ -352,7 +365,7 @@ describe('correctForClockSkew', () => { {timestamp: 40, value: 'cr'} ] }; - should.equal(getClockSkew(skewedButNoendpoints), undefined); + should.equal(getClockSkew(undefined, skewedButNoendpoints), undefined); }); it('span with the mixed endpoints is not single-host', () => { @@ -413,6 +426,30 @@ describe('correctForClockSkew', () => { ]); }); + it('should correct single-host one-way RPC spans', () => { + const trace = [ + { + traceId: '1', + id: '1', + annotations: [{timestamp: 20, value: 'cs', endpoint: frontend}] + }, + { + traceId: '1', + parentId: '1', + id: '2', + annotations: [{timestamp: 10 /* skew */, value: 'sr', endpoint: backend}] + } + ]; + + const adjusted = correctForClockSkew(trace); + + expect(adjusted.length).to.equal(2); + expect(adjusted[0]).to.eql(trace[0]); + expect(adjusted[1].annotations).to.deep.equal([ + {timestamp: 21 /* pushed 1us later */, value: 'sr', endpoint: backend} + ]); + }); + function assertClockSkewIsCorrectlyApplied(skew) { const rpcClientSendTs = 50; const dbClientSendTimestamp = 60 + skew;