From 76aa568c3c8ef864942b9c43f3901e0bf5aea852 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Mon, 9 Mar 2020 20:56:54 +0300 Subject: [PATCH 01/18] Trace name resolution by root span with remote ref 1. Find all spans without parent reference or with a parent reference that is not found in the trace 2. Sort all found spans by start time (in case when there are more than one span found) 3. Take first of the found spans and get a trace name Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/trace-viewer.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/jaeger-ui/src/model/trace-viewer.js b/packages/jaeger-ui/src/model/trace-viewer.js index cbc756014a..98c4593a65 100644 --- a/packages/jaeger-ui/src/model/trace-viewer.js +++ b/packages/jaeger-ui/src/model/trace-viewer.js @@ -14,6 +14,18 @@ // eslint-disable-next-line import/prefer-default-export export function getTraceName(spans) { - const span = spans.filter(sp => !sp.references || !sp.references.length)[0]; - return span ? `${span.process.serviceName}: ${span.operationName}` : ''; + const allSpanIDs = spans.map(sp => sp.spanID); + const rootSpan = spans + .filter(sp => { + if (!sp.references || !sp.references.length) { + return true; + } + const parentIDs = sp.references.filter(r => r.refType === 'CHILD_OF').map(r => r.spanID); + + // no parent from trace + return !parentIDs.some(pID => allSpanIDs.includes(pID)); + }) + .sort((sp1, sp2) => sp1.startTime - sp2.startTime)[0]; + + return rootSpan ? `${rootSpan.process.serviceName}: ${rootSpan.operationName}` : ''; } From 2e5d77099bc7a71f4720c5f39d5ac01f1bff9442 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Mon, 9 Mar 2020 20:59:44 +0300 Subject: [PATCH 02/18] Unit tests for getTraceName() method * Test for trace without root span * Test for trace with exactly one root span (with remote ref) * Test for trace with exactly one root span (with no refs) * Test for trace with more than one root span Signed-off-by: Valerii Varankin --- .../src/model/find-trace-name.test.js | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 packages/jaeger-ui/src/model/find-trace-name.test.js diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js new file mode 100644 index 0000000000..5a5616f694 --- /dev/null +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -0,0 +1,172 @@ +// Copyright (c) 2018 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { getTraceName } from './trace-viewer'; + +describe('getTraceName', () => { + const firstSpanId = 'firstSpanId'; + const secondSpanId = 'secondSpanId'; + const thirdSpanId = 'thirdSpanId'; + const remoteSpanId = 'remoteSpanId'; + + const serviceName = 'serviceName'; + const operationName = 'operationName'; + + const spansWithNoRoots = [ + { + spanID: firstSpanId, + startTime: 1583758690000, + references: [ + { + spanID: secondSpanId, + refType: 'CHILD_OF', + }, + ], + }, + { + spanID: secondSpanId, + startTime: 1583758680000, + references: [ + { + spanID: thirdSpanId, + refType: 'CHILD_OF', + }, + ], + }, + { + spanID: thirdSpanId, + startTime: 1583758670000, + references: [ + { + spanID: firstSpanId, + refType: 'CHILD_OF', + }, + ], + }, + ]; + const spansWithMultipleRoots = [ + { + spanID: firstSpanId, + startTime: 1583758690000, + references: [ + { + spanID: thirdSpanId, + refType: 'CHILD_OF', + }, + ], + }, + { + spanID: secondSpanId, // root span + startTime: 1583758680000, + }, + { + spanID: thirdSpanId, // root span + startTime: 1583758670000, + operationName, + process: { + serviceName, + }, + references: [ + { + spanID: remoteSpanId, + refType: 'CHILD_OF', + }, + ], + }, + ]; + const spansWithOneRootWithRemoteRef = [ + { + spanID: firstSpanId, + startTime: 1583758690000, + references: [ + { + spanID: secondSpanId, + refType: 'CHILD_OF', + }, + ], + }, + { + spanID: secondSpanId, + startTime: 1583758680000, + references: [ + { + spanID: thirdSpanId, + refType: 'CHILD_OF', + }, + ], + }, + { + spanID: thirdSpanId, // root span + startTime: 1583758670000, + operationName, + process: { + serviceName, + }, + references: [ + { + spanID: remoteSpanId, + refType: 'CHILD_OF', + }, + ], + }, + ]; + const spansWithOneRootWithNoRefs = [ + { + spanID: firstSpanId, + startTime: 1583758690000, + references: [ + { + spanID: thirdSpanId, + refType: 'CHILD_OF', + }, + ], + }, + { + spanID: secondSpanId, // root span + startTime: 1583758680000, + operationName, + process: { + serviceName, + }, + }, + { + spanID: thirdSpanId, + startTime: 1583758670000, + references: [ + { + spanID: secondSpanId, + refType: 'CHILD_OF', + }, + ], + }, + ]; + + const fullTraceName = `${serviceName}: ${operationName}`; + + it('returns an empty string if given spans with no root among them', () => { + expect(getTraceName(spansWithNoRoots)).toEqual(''); + }); + + it('returns an id of root span with the earliest startTime', () => { + expect(getTraceName(spansWithMultipleRoots)).toEqual(fullTraceName); + }); + + it('returns an id of root span with remote ref', () => { + expect(getTraceName(spansWithOneRootWithRemoteRef)).toEqual(fullTraceName); + }); + + it('returns an id of root span with no refs', () => { + expect(getTraceName(spansWithOneRootWithNoRefs)).toEqual(fullTraceName); + }); +}); From e968cdb07c61dedaaf3e1dfda888d8ef596b0491 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Mon, 9 Mar 2020 21:01:50 +0300 Subject: [PATCH 03/18] trace-viewer to .tsx * File extension changed to .tsx * Path to changed file removed from tsconfig.lint.json * Added type for spans argument in getTraceName() Signed-off-by: Valerii Varankin --- .../jaeger-ui/src/model/{trace-viewer.js => trace-viewer.tsx} | 4 +++- packages/jaeger-ui/tsconfig.lint.json | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) rename packages/jaeger-ui/src/model/{trace-viewer.js => trace-viewer.tsx} (93%) diff --git a/packages/jaeger-ui/src/model/trace-viewer.js b/packages/jaeger-ui/src/model/trace-viewer.tsx similarity index 93% rename from packages/jaeger-ui/src/model/trace-viewer.js rename to packages/jaeger-ui/src/model/trace-viewer.tsx index 98c4593a65..8f417198e3 100644 --- a/packages/jaeger-ui/src/model/trace-viewer.js +++ b/packages/jaeger-ui/src/model/trace-viewer.tsx @@ -12,8 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +import { Span } from '../types/trace'; + // eslint-disable-next-line import/prefer-default-export -export function getTraceName(spans) { +export function getTraceName(spans: Span[]) { const allSpanIDs = spans.map(sp => sp.spanID); const rootSpan = spans .filter(sp => { diff --git a/packages/jaeger-ui/tsconfig.lint.json b/packages/jaeger-ui/tsconfig.lint.json index 738684b0ab..562e1172d2 100644 --- a/packages/jaeger-ui/tsconfig.lint.json +++ b/packages/jaeger-ui/tsconfig.lint.json @@ -30,7 +30,6 @@ "src/api/jaeger.js", "src/components/SearchTracePage/SearchResults/ResultItem.markers.js", "src/model/order-by.js", - "src/model/trace-viewer.js", "src/selectors/process.js", "src/selectors/span.js", "src/selectors/trace.js", From 5c633c2699233e572330c5cc1ca77fcbf2e3113e Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Tue, 10 Mar 2020 12:28:09 +0300 Subject: [PATCH 04/18] Year fix Fixed a year in a copyright text Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/find-trace-name.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js index 5a5616f694..78bf16aece 100644 --- a/packages/jaeger-ui/src/model/find-trace-name.test.js +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -1,4 +1,4 @@ -// Copyright (c) 2018 Uber Technologies, Inc. +// Copyright (c) 2020 Uber Technologies, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From 08d8dbb7e98f7cf739d1d291d8e507f42e2d5b71 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Tue, 10 Mar 2020 12:30:43 +0300 Subject: [PATCH 05/18] Variable rename Renamed variable from "remoteSpanId" to "missingSpanId" Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/find-trace-name.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js index 78bf16aece..b3e51438ed 100644 --- a/packages/jaeger-ui/src/model/find-trace-name.test.js +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -18,7 +18,7 @@ describe('getTraceName', () => { const firstSpanId = 'firstSpanId'; const secondSpanId = 'secondSpanId'; const thirdSpanId = 'thirdSpanId'; - const remoteSpanId = 'remoteSpanId'; + const missingSpanId = 'missingSpanId'; const serviceName = 'serviceName'; const operationName = 'operationName'; @@ -79,7 +79,7 @@ describe('getTraceName', () => { }, references: [ { - spanID: remoteSpanId, + spanID: missingSpanId, refType: 'CHILD_OF', }, ], @@ -115,7 +115,7 @@ describe('getTraceName', () => { }, references: [ { - spanID: remoteSpanId, + spanID: missingSpanId, refType: 'CHILD_OF', }, ], From 579300519e0a8d2f73ab2f09bcf1187439b38da1 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Tue, 10 Mar 2020 12:32:33 +0300 Subject: [PATCH 06/18] Timestamp constant Made a constant with base span's timestamp in a tests Signed-off-by: Valerii Varankin --- .../src/model/find-trace-name.test.js | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js index b3e51438ed..ee9e128cd0 100644 --- a/packages/jaeger-ui/src/model/find-trace-name.test.js +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -23,10 +23,12 @@ describe('getTraceName', () => { const serviceName = 'serviceName'; const operationName = 'operationName'; + const t = 1583758670000; + const spansWithNoRoots = [ { spanID: firstSpanId, - startTime: 1583758690000, + startTime: t + 200, references: [ { spanID: secondSpanId, @@ -36,7 +38,7 @@ describe('getTraceName', () => { }, { spanID: secondSpanId, - startTime: 1583758680000, + startTime: t + 100, references: [ { spanID: thirdSpanId, @@ -46,7 +48,7 @@ describe('getTraceName', () => { }, { spanID: thirdSpanId, - startTime: 1583758670000, + startTime: t, references: [ { spanID: firstSpanId, @@ -58,7 +60,7 @@ describe('getTraceName', () => { const spansWithMultipleRoots = [ { spanID: firstSpanId, - startTime: 1583758690000, + startTime: t + 200, references: [ { spanID: thirdSpanId, @@ -68,11 +70,11 @@ describe('getTraceName', () => { }, { spanID: secondSpanId, // root span - startTime: 1583758680000, + startTime: t + 100, }, { spanID: thirdSpanId, // root span - startTime: 1583758670000, + startTime: t, operationName, process: { serviceName, @@ -88,7 +90,7 @@ describe('getTraceName', () => { const spansWithOneRootWithRemoteRef = [ { spanID: firstSpanId, - startTime: 1583758690000, + startTime: t + 200, references: [ { spanID: secondSpanId, @@ -98,7 +100,7 @@ describe('getTraceName', () => { }, { spanID: secondSpanId, - startTime: 1583758680000, + startTime: t + 100, references: [ { spanID: thirdSpanId, @@ -108,7 +110,7 @@ describe('getTraceName', () => { }, { spanID: thirdSpanId, // root span - startTime: 1583758670000, + startTime: t, operationName, process: { serviceName, @@ -124,7 +126,7 @@ describe('getTraceName', () => { const spansWithOneRootWithNoRefs = [ { spanID: firstSpanId, - startTime: 1583758690000, + startTime: t + 200, references: [ { spanID: thirdSpanId, @@ -134,7 +136,7 @@ describe('getTraceName', () => { }, { spanID: secondSpanId, // root span - startTime: 1583758680000, + startTime: t + 100, operationName, process: { serviceName, @@ -142,7 +144,7 @@ describe('getTraceName', () => { }, { spanID: thirdSpanId, - startTime: 1583758670000, + startTime: t, references: [ { spanID: secondSpanId, From 52e3794141f2978fbacd9955404f8f587db822f7 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Tue, 10 Mar 2020 12:37:15 +0300 Subject: [PATCH 07/18] Spans id array to spans dictionary Usage of dictionary of all spans instead of id's array to check an inclusion Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/trace-viewer.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/jaeger-ui/src/model/trace-viewer.tsx b/packages/jaeger-ui/src/model/trace-viewer.tsx index 8f417198e3..5daa216f00 100644 --- a/packages/jaeger-ui/src/model/trace-viewer.tsx +++ b/packages/jaeger-ui/src/model/trace-viewer.tsx @@ -14,9 +14,11 @@ import { Span } from '../types/trace'; +type spansDict = { [index: string]: Span }; + // eslint-disable-next-line import/prefer-default-export export function getTraceName(spans: Span[]) { - const allSpanIDs = spans.map(sp => sp.spanID); + const allTraceSpans: spansDict = spans.reduce((dict, span) => ({ ...dict, [span.spanID]: span }), {}); const rootSpan = spans .filter(sp => { if (!sp.references || !sp.references.length) { @@ -25,7 +27,7 @@ export function getTraceName(spans: Span[]) { const parentIDs = sp.references.filter(r => r.refType === 'CHILD_OF').map(r => r.spanID); // no parent from trace - return !parentIDs.some(pID => allSpanIDs.includes(pID)); + return !parentIDs.some(pID => Boolean(allTraceSpans[pID])); }) .sort((sp1, sp2) => sp1.startTime - sp2.startTime)[0]; From c914bcfcb199c0c4433e03081777b31d88b92717 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Tue, 10 Mar 2020 12:50:38 +0300 Subject: [PATCH 08/18] Correct sort logic Correction of a spans sorting to such order: 1. First, sort spans by a presence of a parent 2. Second, sort spans by a start time Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/trace-viewer.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/jaeger-ui/src/model/trace-viewer.tsx b/packages/jaeger-ui/src/model/trace-viewer.tsx index 5daa216f00..65b1644026 100644 --- a/packages/jaeger-ui/src/model/trace-viewer.tsx +++ b/packages/jaeger-ui/src/model/trace-viewer.tsx @@ -29,7 +29,12 @@ export function getTraceName(spans: Span[]) { // no parent from trace return !parentIDs.some(pID => Boolean(allTraceSpans[pID])); }) - .sort((sp1, sp2) => sp1.startTime - sp2.startTime)[0]; + .sort((sp1, sp2) => { + const sp1ParentsNum = sp1.references ? sp1.references.filter(r => r.refType === 'CHILD_OF').length : 0; + const sp2ParentsNum = sp2.references ? sp2.references.filter(r => r.refType === 'CHILD_OF').length : 0; + + return sp1ParentsNum - sp2ParentsNum || sp1.startTime - sp2.startTime; + })[0]; return rootSpan ? `${rootSpan.process.serviceName}: ${rootSpan.operationName}` : ''; } From a6f67ba80ada6ef9f65dfe9201bd2952e4790a48 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Tue, 10 Mar 2020 12:54:38 +0300 Subject: [PATCH 09/18] Tests for a new sorting logic * Test for a trace with multiple root spans different by parents * Test for a trace with multiple root spans different by start time Signed-off-by: Valerii Varankin --- .../src/model/find-trace-name.test.js | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js index ee9e128cd0..44562925c8 100644 --- a/packages/jaeger-ui/src/model/find-trace-name.test.js +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -57,7 +57,7 @@ describe('getTraceName', () => { ], }, ]; - const spansWithMultipleRoots = [ + const spansWithMultipleRootsDifferentByStartTime = [ { spanID: firstSpanId, startTime: t + 200, @@ -69,11 +69,17 @@ describe('getTraceName', () => { ], }, { - spanID: secondSpanId, // root span + spanID: secondSpanId, // may be a root span startTime: t + 100, + references: [ + { + spanID: missingSpanId, + refType: 'CHILD_OF', + }, + ], }, { - spanID: thirdSpanId, // root span + spanID: thirdSpanId, // root span (as the earliest) startTime: t, operationName, process: { @@ -87,6 +93,36 @@ describe('getTraceName', () => { ], }, ]; + const spansWithMultipleRootsWithOneWithoutRefs = [ + { + spanID: firstSpanId, + startTime: t + 200, + references: [ + { + spanID: thirdSpanId, + refType: 'CHILD_OF', + }, + ], + }, + { + spanID: secondSpanId, // root span (as a span without any refs) + startTime: t + 100, + operationName, + process: { + serviceName, + }, + }, + { + spanID: thirdSpanId, // may be a root span + startTime: t, + references: [ + { + spanID: missingSpanId, + refType: 'CHILD_OF', + }, + ], + }, + ]; const spansWithOneRootWithRemoteRef = [ { spanID: firstSpanId, @@ -161,7 +197,11 @@ describe('getTraceName', () => { }); it('returns an id of root span with the earliest startTime', () => { - expect(getTraceName(spansWithMultipleRoots)).toEqual(fullTraceName); + expect(getTraceName(spansWithMultipleRootsDifferentByStartTime)).toEqual(fullTraceName); + }); + + it('returns an id of root span without any refs', () => { + expect(getTraceName(spansWithMultipleRootsWithOneWithoutRefs)).toEqual(fullTraceName); }); it('returns an id of root span with remote ref', () => { From 270a762d354f29b3b8d9f3d2eae49fea3d0c3447 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Tue, 10 Mar 2020 13:01:45 +0300 Subject: [PATCH 10/18] Empty process object to all test spans Added a process key with an empty object to all test spans to avoid reading property of undefined Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/find-trace-name.test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js index 44562925c8..675a2a2c5e 100644 --- a/packages/jaeger-ui/src/model/find-trace-name.test.js +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -29,6 +29,7 @@ describe('getTraceName', () => { { spanID: firstSpanId, startTime: t + 200, + process: {}, references: [ { spanID: secondSpanId, @@ -39,6 +40,7 @@ describe('getTraceName', () => { { spanID: secondSpanId, startTime: t + 100, + process: {}, references: [ { spanID: thirdSpanId, @@ -49,6 +51,7 @@ describe('getTraceName', () => { { spanID: thirdSpanId, startTime: t, + process: {}, references: [ { spanID: firstSpanId, @@ -61,6 +64,7 @@ describe('getTraceName', () => { { spanID: firstSpanId, startTime: t + 200, + process: {}, references: [ { spanID: thirdSpanId, @@ -71,6 +75,7 @@ describe('getTraceName', () => { { spanID: secondSpanId, // may be a root span startTime: t + 100, + process: {}, references: [ { spanID: missingSpanId, @@ -97,6 +102,7 @@ describe('getTraceName', () => { { spanID: firstSpanId, startTime: t + 200, + process: {}, references: [ { spanID: thirdSpanId, @@ -115,6 +121,7 @@ describe('getTraceName', () => { { spanID: thirdSpanId, // may be a root span startTime: t, + process: {}, references: [ { spanID: missingSpanId, @@ -127,6 +134,7 @@ describe('getTraceName', () => { { spanID: firstSpanId, startTime: t + 200, + process: {}, references: [ { spanID: secondSpanId, @@ -137,6 +145,7 @@ describe('getTraceName', () => { { spanID: secondSpanId, startTime: t + 100, + process: {}, references: [ { spanID: thirdSpanId, @@ -163,6 +172,7 @@ describe('getTraceName', () => { { spanID: firstSpanId, startTime: t + 200, + process: {}, references: [ { spanID: thirdSpanId, @@ -181,6 +191,7 @@ describe('getTraceName', () => { { spanID: thirdSpanId, startTime: t, + process: {}, references: [ { spanID: secondSpanId, From 3944f0504c4256bc7013c24ec03ce89e5846f5a6 Mon Sep 17 00:00:00 2001 From: Valeriy Varankin Date: Wed, 11 Mar 2020 10:29:19 +0300 Subject: [PATCH 11/18] Update Copyright text in packages/jaeger-ui/src/model/trace-viewer.tsx Co-Authored-By: Yuri Shkuro Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/trace-viewer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jaeger-ui/src/model/trace-viewer.tsx b/packages/jaeger-ui/src/model/trace-viewer.tsx index 65b1644026..d326aaa449 100644 --- a/packages/jaeger-ui/src/model/trace-viewer.tsx +++ b/packages/jaeger-ui/src/model/trace-viewer.tsx @@ -1,4 +1,4 @@ -// Copyright (c) 2017 Uber Technologies, Inc. +// Copyright (c) 2020 The Jaeger Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From f65375f09bcdfc50f039b9369c73d3c34761dc42 Mon Sep 17 00:00:00 2001 From: Valeriy Varankin Date: Wed, 11 Mar 2020 10:29:36 +0300 Subject: [PATCH 12/18] Update Copyright text in packages/jaeger-ui/src/model/find-trace-name.test.js Co-Authored-By: Yuri Shkuro Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/find-trace-name.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js index 675a2a2c5e..54f7c7cb71 100644 --- a/packages/jaeger-ui/src/model/find-trace-name.test.js +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -1,4 +1,4 @@ -// Copyright (c) 2020 Uber Technologies, Inc. +// Copyright (c) 2020 The Jaeger Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From 98b7eb74958a2b8328092d16e1bddb3976178e7e Mon Sep 17 00:00:00 2001 From: Valeriy Varankin Date: Wed, 11 Mar 2020 10:30:30 +0300 Subject: [PATCH 13/18] More detailed comment for root span Co-Authored-By: Yuri Shkuro Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/find-trace-name.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js index 54f7c7cb71..db7cfe06b1 100644 --- a/packages/jaeger-ui/src/model/find-trace-name.test.js +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -154,7 +154,7 @@ describe('getTraceName', () => { ], }, { - spanID: thirdSpanId, // root span + spanID: thirdSpanId, // effective root span, since its parent is missing startTime: t, operationName, process: { From bfbce5b9f27a7ab63fe85376a68bbf9ee4deb1bf Mon Sep 17 00:00:00 2001 From: Valeriy Varankin Date: Wed, 11 Mar 2020 10:30:54 +0300 Subject: [PATCH 14/18] Note about a loop Co-Authored-By: Yuri Shkuro Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/find-trace-name.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js index db7cfe06b1..a5c9fe7428 100644 --- a/packages/jaeger-ui/src/model/find-trace-name.test.js +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -25,6 +25,8 @@ describe('getTraceName', () => { const t = 1583758670000; + // Note: this trace has a loop S1 <- S2 <- S3 <- S1, which is the only way + // to make the algorithm return an empty string as trace name. const spansWithNoRoots = [ { spanID: firstSpanId, From 9aa5b4bd17cd30b0ce655483aafc8e0c98e63038 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Wed, 11 Mar 2020 12:31:54 +0300 Subject: [PATCH 15/18] Change of parents definition logic Method of parents definition changed to comparing traceIDs of spans Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/trace-viewer.tsx | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/jaeger-ui/src/model/trace-viewer.tsx b/packages/jaeger-ui/src/model/trace-viewer.tsx index d326aaa449..4db781ead9 100644 --- a/packages/jaeger-ui/src/model/trace-viewer.tsx +++ b/packages/jaeger-ui/src/model/trace-viewer.tsx @@ -14,24 +14,19 @@ import { Span } from '../types/trace'; -type spansDict = { [index: string]: Span }; - // eslint-disable-next-line import/prefer-default-export export function getTraceName(spans: Span[]) { - const allTraceSpans: spansDict = spans.reduce((dict, span) => ({ ...dict, [span.spanID]: span }), {}); const rootSpan = spans .filter(sp => { if (!sp.references || !sp.references.length) { return true; } - const parentIDs = sp.references.filter(r => r.refType === 'CHILD_OF').map(r => r.spanID); - - // no parent from trace - return !parentIDs.some(pID => Boolean(allTraceSpans[pID])); + // returns true if no parent from this trace found + return !sp.references.some(r => r.traceID === sp.traceID); }) .sort((sp1, sp2) => { - const sp1ParentsNum = sp1.references ? sp1.references.filter(r => r.refType === 'CHILD_OF').length : 0; - const sp2ParentsNum = sp2.references ? sp2.references.filter(r => r.refType === 'CHILD_OF').length : 0; + const sp1ParentsNum = sp1.references ? sp1.references.length : 0; + const sp2ParentsNum = sp2.references ? sp2.references.length : 0; return sp1ParentsNum - sp2ParentsNum || sp1.startTime - sp2.startTime; })[0]; From 0c49d43a804b800f939c2b8aa3aef5eb427e0ad2 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Wed, 11 Mar 2020 12:37:13 +0300 Subject: [PATCH 16/18] Tests correction according to changed logic Tests traces data was changed to fit parents definition method by a traceIDs Signed-off-by: Valerii Varankin --- .../src/model/find-trace-name.test.js | 44 +++++++++++++------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js index a5c9fe7428..bdff06dfc9 100644 --- a/packages/jaeger-ui/src/model/find-trace-name.test.js +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -20,6 +20,9 @@ describe('getTraceName', () => { const thirdSpanId = 'thirdSpanId'; const missingSpanId = 'missingSpanId'; + const currentTraceId = 'currentTraceId'; + const anotherTraceId = 'anotherTraceId'; + const serviceName = 'serviceName'; const operationName = 'operationName'; @@ -30,34 +33,37 @@ describe('getTraceName', () => { const spansWithNoRoots = [ { spanID: firstSpanId, + traceID: currentTraceId, startTime: t + 200, process: {}, references: [ { spanID: secondSpanId, - refType: 'CHILD_OF', + traceID: currentTraceId, }, ], }, { spanID: secondSpanId, + traceID: currentTraceId, startTime: t + 100, process: {}, references: [ { spanID: thirdSpanId, - refType: 'CHILD_OF', + traceID: currentTraceId, }, ], }, { spanID: thirdSpanId, + traceID: currentTraceId, startTime: t, process: {}, references: [ { spanID: firstSpanId, - refType: 'CHILD_OF', + traceID: currentTraceId, }, ], }, @@ -65,28 +71,31 @@ describe('getTraceName', () => { const spansWithMultipleRootsDifferentByStartTime = [ { spanID: firstSpanId, + traceID: currentTraceId, startTime: t + 200, process: {}, references: [ { spanID: thirdSpanId, - refType: 'CHILD_OF', + traceID: currentTraceId, }, ], }, { spanID: secondSpanId, // may be a root span + traceID: currentTraceId, startTime: t + 100, process: {}, references: [ { spanID: missingSpanId, - refType: 'CHILD_OF', + traceID: anotherTraceId, }, ], }, { spanID: thirdSpanId, // root span (as the earliest) + traceID: currentTraceId, startTime: t, operationName, process: { @@ -95,7 +104,7 @@ describe('getTraceName', () => { references: [ { spanID: missingSpanId, - refType: 'CHILD_OF', + traceID: anotherTraceId, }, ], }, @@ -103,17 +112,19 @@ describe('getTraceName', () => { const spansWithMultipleRootsWithOneWithoutRefs = [ { spanID: firstSpanId, + traceID: currentTraceId, startTime: t + 200, process: {}, references: [ { spanID: thirdSpanId, - refType: 'CHILD_OF', + traceID: currentTraceId, }, ], }, { spanID: secondSpanId, // root span (as a span without any refs) + traceID: currentTraceId, startTime: t + 100, operationName, process: { @@ -122,12 +133,13 @@ describe('getTraceName', () => { }, { spanID: thirdSpanId, // may be a root span + traceID: currentTraceId, startTime: t, process: {}, references: [ { spanID: missingSpanId, - refType: 'CHILD_OF', + traceID: anotherTraceId, }, ], }, @@ -135,28 +147,31 @@ describe('getTraceName', () => { const spansWithOneRootWithRemoteRef = [ { spanID: firstSpanId, + traceID: currentTraceId, startTime: t + 200, process: {}, references: [ { spanID: secondSpanId, - refType: 'CHILD_OF', + traceID: currentTraceId, }, ], }, { spanID: secondSpanId, + traceID: currentTraceId, startTime: t + 100, process: {}, references: [ { spanID: thirdSpanId, - refType: 'CHILD_OF', + traceID: currentTraceId, }, ], }, { spanID: thirdSpanId, // effective root span, since its parent is missing + traceID: currentTraceId, startTime: t, operationName, process: { @@ -165,7 +180,7 @@ describe('getTraceName', () => { references: [ { spanID: missingSpanId, - refType: 'CHILD_OF', + traceID: anotherTraceId, }, ], }, @@ -173,17 +188,19 @@ describe('getTraceName', () => { const spansWithOneRootWithNoRefs = [ { spanID: firstSpanId, + traceID: currentTraceId, startTime: t + 200, process: {}, references: [ { spanID: thirdSpanId, - refType: 'CHILD_OF', + traceID: currentTraceId, }, ], }, { spanID: secondSpanId, // root span + traceID: currentTraceId, startTime: t + 100, operationName, process: { @@ -192,12 +209,13 @@ describe('getTraceName', () => { }, { spanID: thirdSpanId, + traceID: currentTraceId, startTime: t, process: {}, references: [ { spanID: secondSpanId, - refType: 'CHILD_OF', + traceID: currentTraceId, }, ], }, From 8d3eb81a6d3b1d9f0dda577dde8d671326f821e5 Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Wed, 11 Mar 2020 14:19:01 +0300 Subject: [PATCH 17/18] Checking of parent span presence Added a checking of parent span presence among all trace's spans Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/trace-viewer.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/jaeger-ui/src/model/trace-viewer.tsx b/packages/jaeger-ui/src/model/trace-viewer.tsx index 4db781ead9..049d6dcf6a 100644 --- a/packages/jaeger-ui/src/model/trace-viewer.tsx +++ b/packages/jaeger-ui/src/model/trace-viewer.tsx @@ -14,15 +14,20 @@ import { Span } from '../types/trace'; +type spansDict = { [index: string]: Span }; + // eslint-disable-next-line import/prefer-default-export export function getTraceName(spans: Span[]) { + const allTraceSpans: spansDict = spans.reduce((dict, span) => ({ ...dict, [span.spanID]: span }), {}); const rootSpan = spans .filter(sp => { if (!sp.references || !sp.references.length) { return true; } + const parentIDs = sp.references.filter(r => r.traceID === sp.traceID).map(r => r.spanID); + // returns true if no parent from this trace found - return !sp.references.some(r => r.traceID === sp.traceID); + return !parentIDs.some(pID => Boolean(allTraceSpans[pID])); }) .sort((sp1, sp2) => { const sp1ParentsNum = sp1.references ? sp1.references.length : 0; From 3b2cbae16293e78ef7ee8c04b3bfc80cc48e91ce Mon Sep 17 00:00:00 2001 From: Valerii Varankin Date: Wed, 11 Mar 2020 14:20:24 +0300 Subject: [PATCH 18/18] Tests correction according to changed logic Tests traces data was changed according to changed logic Signed-off-by: Valerii Varankin --- packages/jaeger-ui/src/model/find-trace-name.test.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/jaeger-ui/src/model/find-trace-name.test.js b/packages/jaeger-ui/src/model/find-trace-name.test.js index bdff06dfc9..b9963555dd 100644 --- a/packages/jaeger-ui/src/model/find-trace-name.test.js +++ b/packages/jaeger-ui/src/model/find-trace-name.test.js @@ -21,7 +21,6 @@ describe('getTraceName', () => { const missingSpanId = 'missingSpanId'; const currentTraceId = 'currentTraceId'; - const anotherTraceId = 'anotherTraceId'; const serviceName = 'serviceName'; const operationName = 'operationName'; @@ -89,7 +88,7 @@ describe('getTraceName', () => { references: [ { spanID: missingSpanId, - traceID: anotherTraceId, + traceID: currentTraceId, }, ], }, @@ -104,7 +103,7 @@ describe('getTraceName', () => { references: [ { spanID: missingSpanId, - traceID: anotherTraceId, + traceID: currentTraceId, }, ], }, @@ -139,7 +138,7 @@ describe('getTraceName', () => { references: [ { spanID: missingSpanId, - traceID: anotherTraceId, + traceID: currentTraceId, }, ], }, @@ -180,7 +179,7 @@ describe('getTraceName', () => { references: [ { spanID: missingSpanId, - traceID: anotherTraceId, + traceID: currentTraceId, }, ], },