From 355a96f2cd6b09e788cc0ecab044cd89291b6fa7 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Thu, 15 Mar 2018 19:23:30 +0800 Subject: [PATCH] Merges client server spans imported via View Saved Trace screen --- zipkin-ui/js/component_ui/uploadTrace.js | 17 +- zipkin-ui/js/spanConverter.js | 70 +++++- zipkin-ui/package.json | 1 + zipkin-ui/test/spanConverter.test.js | 308 ++++++++++++++--------- 4 files changed, 264 insertions(+), 132 deletions(-) diff --git a/zipkin-ui/js/component_ui/uploadTrace.js b/zipkin-ui/js/component_ui/uploadTrace.js index 82f4c62501b..c5e98c97758 100644 --- a/zipkin-ui/js/component_ui/uploadTrace.js +++ b/zipkin-ui/js/component_ui/uploadTrace.js @@ -1,6 +1,7 @@ import {component} from 'flightjs'; import FullPageSpinnerUI from '../component_ui/fullPageSpinner'; import traceToMustache from '../../js/component_ui/traceToMustache'; +import _ from 'lodash'; import {SPAN_V1} from '../spanConverter'; function rootToFrontComparator(span1/* , span2*/) { @@ -19,11 +20,17 @@ function ensureV1(trace) { return trace; } - const newTrace = []; - for (let i = 0; i < trace.length; i++) { - newTrace.push(SPAN_V1.convert(trace[i])); - } - + const groupedById = _(trace).map(SPAN_V1.convert).groupBy('id').value(); + const newTrace = _(groupedById).map((spans) => { + if (spans.length === 1) return spans[0]; + let merged = spans[0]; + for (let i = 1; i < spans.length; i++) { + merged = SPAN_V1.merge(merged, spans[i]); + } + return merged; + }) + .sort((l, r) => l.timestamp || 0 - r.timestamp || 0) + .value(); return newTrace; } diff --git a/zipkin-ui/js/spanConverter.js b/zipkin-ui/js/spanConverter.js index 0ee52ef9e12..776b8a948bb 100644 --- a/zipkin-ui/js/spanConverter.js +++ b/zipkin-ui/js/spanConverter.js @@ -27,15 +27,17 @@ function toV1Annotation(ann, endpoint) { // that 'beginAnnotation' comes first timestamp/duration should always be copied over function convertV1(span) { const res = { - traceId: span.traceId + traceId: span.traceId, }; if (span.parentId) { // instead of writing "parentId": NULL res.parentId = span.parentId; } res.id = span.id; res.name = span.name || ''; // undefined is not allowed in v1 - res.timestamp = span.timestamp; - res.duration = span.duration; + if (!span.shared) { + res.timestamp = span.timestamp; + res.duration = span.duration; + } const jsonEndpoint = toV1Endpoint(span.localEndpoint); @@ -107,8 +109,70 @@ function convertV1(span) { return res; } +function merge(left, right) { + const res = { + traceId: left.traceId, + parentId: left.parentId, + id: left.id + }; + if (right.parentId) { + res.parentId = right.parentId; + } else if (!res.parentId) { + delete(res.parentId); + } + + let leftClientSpan = false; + for (let i = 0; i < left.annotations.length; i++) { + if (left.annotations[i].value === 'cs') { + leftClientSpan = true; + break; + } + } + let rightServerSpan = false; + for (let i = 0; i < right.annotations.length; i++) { + if (right.annotations[i].value === 'sr') { + rightServerSpan = true; + break; + } + } + + if (left.name === '' || left.name === 'unknown') { + res.name = right.name; + } else if (right.name === '' || right.name === 'unknown') { + res.name = left.name; + } else if (leftClientSpan && rightServerSpan) { + res.name = right.name; // prefer the server's span name + } else { + res.name = left.name; + } + + if (right.timestamp) { + res.timestamp = right.timestamp; + } else { + delete(res.timestamp); + } + if (right.duration) { + res.duration = right.duration; + } else { + delete(res.duration); + } + res.annotations = left.annotations + .concat(right.annotations) + .sort((l, r) => l.timestamp - r.timestamp); + res.binaryAnnotations = left.binaryAnnotations + .concat(right.binaryAnnotations); + + if (right.debug) { // instead of writing "debug": false + res.debug = true; + } + return res; +} + module.exports.SPAN_V1 = { convert(span) { return convertV1(span); + }, + merge(left, right) { + return merge(left, right); } }; diff --git a/zipkin-ui/package.json b/zipkin-ui/package.json index dbdc52df05f..d91efc49753 100644 --- a/zipkin-ui/package.json +++ b/zipkin-ui/package.json @@ -27,6 +27,7 @@ "js-cookie": "^2.1.0", "lodash": "^4.5.0", "moment": "^2.11.2", + "npm": "^5.7.1", "query-string": "^3.0.0", "timeago": "^1.5.1" }, diff --git a/zipkin-ui/test/spanConverter.test.js b/zipkin-ui/test/spanConverter.test.js index f98e9d88a40..d87fe57246f 100644 --- a/zipkin-ui/test/spanConverter.test.js +++ b/zipkin-ui/test/spanConverter.test.js @@ -1,4 +1,5 @@ const {SPAN_V1} = require('../js/spanConverter'); +const should = require('chai').should(); describe('SPAN v1 Conversion', () => { it('should transform correctly from v2 to v1', () => { @@ -82,36 +83,27 @@ describe('SPAN v1 Conversion', () => { } }; - const expected = { - traceId: 'a', - name: 'get', - id: 'a', - timestamp: 1, - duration: 2, - annotations: [ - { - endpoint: { - serviceName: 'portalservice', - ipv4: '10.57.50.83', - port: 8080 - }, - timestamp: 1, - value: 'cs' - }, - { - endpoint: { - serviceName: 'portalservice', - ipv4: '10.57.50.83', - port: 8080, - }, - timestamp: 3, - value: 'cr' - } - ] - }; - const spanV1 = SPAN_V1.convert(spanV2); - expect(spanV1.annotations).to.deep.equal(expected.annotations); + expect(spanV1.annotations).to.deep.equal([ + { + endpoint: { + serviceName: 'portalservice', + ipv4: '10.57.50.83', + port: 8080 + }, + timestamp: 1, + value: 'cs' + }, + { + endpoint: { + serviceName: 'portalservice', + ipv4: '10.57.50.83', + port: 8080, + }, + timestamp: 3, + value: 'cr' + } + ]); }); it('should maintain CS/CR annotation order', () => { @@ -135,60 +127,15 @@ describe('SPAN v1 Conversion', () => { ] }; - const expected = { - traceId: 'a', - name: 'get', - id: 'a', - timestamp: 1, - duration: 2, - annotations: [ - { - endpoint: { - serviceName: 'portalservice', - ipv4: '10.57.50.83', - port: 8080 - }, - timestamp: 1, - value: 'cs' - }, - { - endpoint: { - serviceName: 'portalservice', - ipv4: '10.57.50.83', - port: 8080, - }, - timestamp: 2, - value: 'middle' - }, - { - endpoint: { - serviceName: 'portalservice', - ipv4: '10.57.50.83', - port: 8080, - }, - timestamp: 3, - value: 'cr' - } - ] - }; - const spanV1 = SPAN_V1.convert(spanV2); - expect(spanV1.annotations).to.deep.equal(expected.annotations); + expect(spanV1.annotations.map(s => s.timestamp)).to.deep.equal([1, 2, 3]); }); it('should set SA annotation on client span', () => { const spanV2 = { traceId: 'a', - name: 'get', id: 'a', kind: 'CLIENT', - timestamp: 1, - duration: 1, - localEndpoint: { - serviceName: 'portalservice', - ipv4: '10.57.50.83', - port: 8080 - }, remoteEndpoint: { serviceName: 'there', ipv4: '10.57.50.84', @@ -196,49 +143,21 @@ describe('SPAN v1 Conversion', () => { } }; - const expected = { - traceId: 'a', - name: 'get', - id: 'a', - annotations: [ - { - endpoint: { - serviceName: 'portalservice', - ipv4: '10.57.50.83', - port: 8080 - }, - timestamp: 1, - value: 'cs' - }, - { - endpoint: { - serviceName: 'portalservice', - ipv4: '10.57.50.83', - port: 8080, - }, - timestamp: 2, - value: 'cr' - } - ], - binaryAnnotations: [ - { - key: 'sa', - value: true, - endpoint: { - serviceName: 'there', - ipv4: '10.57.50.84', - port: 80 - } - } - ] - }; - const spanV1 = SPAN_V1.convert(spanV2); - expect(spanV1.annotations).to.deep.equal(expected.annotations); - expect(spanV1.binaryAnnotations).to.deep.equal(expected.binaryAnnotations); + expect(spanV1.binaryAnnotations).to.deep.equal([ + { + key: 'sa', + value: true, + endpoint: { + serviceName: 'there', + ipv4: '10.57.50.84', + port: 80 + } + } + ]); }); - it('should write timestamps for shared span', () => { + it('should not write timestamps for shared span', () => { const spanV2 = { traceId: 'a', name: 'get', @@ -254,16 +173,157 @@ describe('SPAN v1 Conversion', () => { } }; - const expected = { + const spanV1 = SPAN_V1.convert(spanV2); + should.equal(spanV1.timestamp, undefined); + should.equal(spanV1.duration, undefined); + }); +}); +describe('SPAN v1 Merge', () => { + const clientSpan = { + traceId: 'a', + name: 'get', + id: 'c', + parentId: 'b', + annotations: [ + { + endpoint: { + serviceName: 'baloonservice', + ipv4: '10.57.50.70', + port: 80 + }, + timestamp: 1, + value: 'cs' + }, + { + endpoint: { + serviceName: 'baloonservice', + ipv4: '10.57.50.70', + port: 80, + }, + timestamp: 4, + value: 'cr' + } + ], + binaryAnnotations: [] + }; + const serverSpan = { + traceId: 'a', + name: '', + id: 'c', + parentId: 'b', + annotations: [ + { + endpoint: { + serviceName: 'portalservice', + ipv4: '10.57.50.83', + port: 8080 + }, + timestamp: 2, + value: 'sr' + }, + { + endpoint: { + serviceName: 'portalservice', + ipv4: '10.57.50.83', + port: 8080, + }, + timestamp: 3, + value: 'ss' + } + ], + binaryAnnotations: [] + }; + const mergedSpan = { + traceId: 'a', + name: 'get', + id: 'c', + parentId: 'b', + annotations: [ + { + endpoint: { + serviceName: 'baloonservice', + ipv4: '10.57.50.70', + port: 80 + }, + timestamp: 1, + value: 'cs' + }, + { + endpoint: { + serviceName: 'portalservice', + ipv4: '10.57.50.83', + port: 8080 + }, + timestamp: 2, + value: 'sr' + }, + { + endpoint: { + serviceName: 'portalservice', + ipv4: '10.57.50.83', + port: 8080, + }, + timestamp: 3, + value: 'ss' + }, + { + endpoint: { + serviceName: 'baloonservice', + ipv4: '10.57.50.70', + port: 80, + }, + timestamp: 4, + value: 'cr' + } + ], + binaryAnnotations: [] + }; + + it('should merge server and client span', () => { + const merged = SPAN_V1.merge(serverSpan, clientSpan); + + expect(merged).to.deep.equal(mergedSpan); + }); + + it('should merge client and server span', () => { + const merged = SPAN_V1.merge(clientSpan, serverSpan); + + expect(merged).to.deep.equal(mergedSpan); + }); + + it('should overwrite client name with server name', () => { + const merged = SPAN_V1.merge(clientSpan, { traceId: 'a', - name: 'get', - id: 'a', - timestamp: 1, - duration: 2 - }; + id: 'c', + name: 'get /users/:userId', + annotations: [{timestamp: 2, value: 'sr'}], + binaryAnnotations: [] + }); - const spanV1 = SPAN_V1.convert(spanV2); - expect(spanV1.timestamp).to.deep.equal(expected.timestamp); - expect(spanV1.duration).to.deep.equal(expected.duration); + expect(merged.name).to.equal('get /users/:userId'); + }); + + it('should not overwrite client name with empty', () => { + const merged = SPAN_V1.merge(clientSpan, { + traceId: 'a', + id: 'c', + name: '', + annotations: [{timestamp: 2, value: 'sr'}], + binaryAnnotations: [] + }); + + expect(merged.name).to.equal(clientSpan.name); + }); + + it('should not overwrite client name with unknown', () => { + const merged = SPAN_V1.merge(clientSpan, { + traceId: 'a', + id: 'c', + name: 'unknown', + annotations: [{timestamp: 2, value: 'sr'}], + binaryAnnotations: [] + }); + + expect(merged.name).to.equal(clientSpan.name); }); });