Skip to content

Commit

Permalink
Correct trace name resolution (jaegertracing#541)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* Year fix

Fixed a year in a copyright text

Signed-off-by: Valerii Varankin <[email protected]>

* Variable rename

Renamed variable from "remoteSpanId" to "missingSpanId"

Signed-off-by: Valerii Varankin <[email protected]>

* Timestamp constant

Made a constant with base span's timestamp in a tests

Signed-off-by: Valerii Varankin <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* Update Copyright text in packages/jaeger-ui/src/model/trace-viewer.tsx

Co-Authored-By: Yuri Shkuro <[email protected]>
Signed-off-by: Valerii Varankin <[email protected]>

* Update Copyright text in packages/jaeger-ui/src/model/find-trace-name.test.js

Co-Authored-By: Yuri Shkuro <[email protected]>
Signed-off-by: Valerii Varankin <[email protected]>

* More detailed comment for root span

Co-Authored-By: Yuri Shkuro <[email protected]>
Signed-off-by: Valerii Varankin <[email protected]>

* Note about a loop

Co-Authored-By: Yuri Shkuro <[email protected]>
Signed-off-by: Valerii Varankin <[email protected]>

* Change of parents definition logic

Method of parents definition changed to comparing traceIDs of spans

Signed-off-by: Valerii Varankin <[email protected]>

* Tests correction according to changed logic

Tests traces data was changed to fit parents definition method by a traceIDs

Signed-off-by: Valerii Varankin <[email protected]>

* Checking of parent span presence

Added a checking of parent span presence among all trace's spans

Signed-off-by: Valerii Varankin <[email protected]>

* Tests correction according to changed logic

Tests traces data was changed according to changed logic

Signed-off-by: Valerii Varankin <[email protected]>

Co-authored-by: Yuri Shkuro <[email protected]>
Signed-off-by: vvvprabhakar <[email protected]>
  • Loading branch information
swapster and yurishkuro authored Mar 12, 2020
1 parent be650a0 commit 9072f31
Show file tree
Hide file tree
Showing 4 changed files with 284 additions and 20 deletions.
244 changes: 244 additions & 0 deletions packages/jaeger-ui/src/model/find-trace-name.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
// 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.
// 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 missingSpanId = 'missingSpanId';

const currentTraceId = 'currentTraceId';

const serviceName = 'serviceName';
const operationName = 'operationName';

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,
traceID: currentTraceId,
startTime: t + 200,
process: {},
references: [
{
spanID: secondSpanId,
traceID: currentTraceId,
},
],
},
{
spanID: secondSpanId,
traceID: currentTraceId,
startTime: t + 100,
process: {},
references: [
{
spanID: thirdSpanId,
traceID: currentTraceId,
},
],
},
{
spanID: thirdSpanId,
traceID: currentTraceId,
startTime: t,
process: {},
references: [
{
spanID: firstSpanId,
traceID: currentTraceId,
},
],
},
];
const spansWithMultipleRootsDifferentByStartTime = [
{
spanID: firstSpanId,
traceID: currentTraceId,
startTime: t + 200,
process: {},
references: [
{
spanID: thirdSpanId,
traceID: currentTraceId,
},
],
},
{
spanID: secondSpanId, // may be a root span
traceID: currentTraceId,
startTime: t + 100,
process: {},
references: [
{
spanID: missingSpanId,
traceID: currentTraceId,
},
],
},
{
spanID: thirdSpanId, // root span (as the earliest)
traceID: currentTraceId,
startTime: t,
operationName,
process: {
serviceName,
},
references: [
{
spanID: missingSpanId,
traceID: currentTraceId,
},
],
},
];
const spansWithMultipleRootsWithOneWithoutRefs = [
{
spanID: firstSpanId,
traceID: currentTraceId,
startTime: t + 200,
process: {},
references: [
{
spanID: thirdSpanId,
traceID: currentTraceId,
},
],
},
{
spanID: secondSpanId, // root span (as a span without any refs)
traceID: currentTraceId,
startTime: t + 100,
operationName,
process: {
serviceName,
},
},
{
spanID: thirdSpanId, // may be a root span
traceID: currentTraceId,
startTime: t,
process: {},
references: [
{
spanID: missingSpanId,
traceID: currentTraceId,
},
],
},
];
const spansWithOneRootWithRemoteRef = [
{
spanID: firstSpanId,
traceID: currentTraceId,
startTime: t + 200,
process: {},
references: [
{
spanID: secondSpanId,
traceID: currentTraceId,
},
],
},
{
spanID: secondSpanId,
traceID: currentTraceId,
startTime: t + 100,
process: {},
references: [
{
spanID: thirdSpanId,
traceID: currentTraceId,
},
],
},
{
spanID: thirdSpanId, // effective root span, since its parent is missing
traceID: currentTraceId,
startTime: t,
operationName,
process: {
serviceName,
},
references: [
{
spanID: missingSpanId,
traceID: currentTraceId,
},
],
},
];
const spansWithOneRootWithNoRefs = [
{
spanID: firstSpanId,
traceID: currentTraceId,
startTime: t + 200,
process: {},
references: [
{
spanID: thirdSpanId,
traceID: currentTraceId,
},
],
},
{
spanID: secondSpanId, // root span
traceID: currentTraceId,
startTime: t + 100,
operationName,
process: {
serviceName,
},
},
{
spanID: thirdSpanId,
traceID: currentTraceId,
startTime: t,
process: {},
references: [
{
spanID: secondSpanId,
traceID: currentTraceId,
},
],
},
];

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(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', () => {
expect(getTraceName(spansWithOneRootWithRemoteRef)).toEqual(fullTraceName);
});

it('returns an id of root span with no refs', () => {
expect(getTraceName(spansWithOneRootWithNoRefs)).toEqual(fullTraceName);
});
});
19 changes: 0 additions & 19 deletions packages/jaeger-ui/src/model/trace-viewer.js

This file was deleted.

40 changes: 40 additions & 0 deletions packages/jaeger-ui/src/model/trace-viewer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// 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.
// 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 { 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 !parentIDs.some(pID => Boolean(allTraceSpans[pID]));
})
.sort((sp1, sp2) => {
const sp1ParentsNum = sp1.references ? sp1.references.length : 0;
const sp2ParentsNum = sp2.references ? sp2.references.length : 0;

return sp1ParentsNum - sp2ParentsNum || sp1.startTime - sp2.startTime;
})[0];

return rootSpan ? `${rootSpan.process.serviceName}: ${rootSpan.operationName}` : '';
}
1 change: 0 additions & 1 deletion packages/jaeger-ui/tsconfig.lint.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 9072f31

Please sign in to comment.