Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DDG: Remove kind.server filter and validate the case of service calling itself #557

Merged
merged 7 commits into from
Apr 24, 2020
Merged

DDG: Remove kind.server filter and validate the case of service calling itself #557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cd773f5
DDG: Remove kind.server filter and handle service calling itself
rubenvp8510 Apr 4, 2020
f6a686a
Improve tests on transformTracesToPath
rubenvp8510 Apr 15, 2020
341953a
Preserve (do not replace) client ingress spans
rubenvp8510 Apr 17, 2020
82821d8
Add test for root client span
rubenvp8510 Apr 17, 2020
5c094b6
Improve DDG paths algorithm
rubenvp8510 Apr 20, 2020
5d1e3e4
Use TREE_ROOT_ID on transformTracesToPaths function
rubenvp8510 Apr 23, 2020
0b3bf0c
Change makeTrace to set service name to [spanID]-name
rubenvp8510 Apr 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 71 additions & 27 deletions packages/jaeger-ui/src/model/ddg/transformTracesToPaths.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,18 @@ describe('transform traces to ddg paths', () => {
})),
attributes: [{ key: 'exemplar_trace_id', value: trace.data.traceID }],
});
const makeSpan = (spanName, parent, kind) => ({

const addExemplarTraceIDs = (path, traces) => {
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
traces.forEach(trace => {
path.attributes.push({ key: 'exemplar_trace_id', value: trace.data.traceID });
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
});
return path;
};

const makeSpan = (spanName, parent, kind, operationName, processID) => ({
hasChildren: true,
operationName: `${spanName} operation`,
processID: `${spanName} processID`,
operationName: operationName || `${spanName} operation`,
processID: processID || `${spanName} processID`,
references: parent
? [
{
Expand All @@ -52,7 +60,7 @@ describe('transform traces to ddg paths', () => {
(result, span) => ({
...result,
[span.processID]: {
serviceName: `${span.spanID.split(' ')[0]} service`,
serviceName: `${span.processID}-name`,
},
}),
{}
Expand Down Expand Up @@ -137,11 +145,6 @@ describe('transform traces to ddg paths', () => {
);
});

it('errors if span has ancestor id not in trace data', () => {
const traces = makeTraces(makeTrace([rootSpan, followsFocalSpan], missTraceID));
expect(() => transformTracesToPaths(traces, focalSvc)).toThrowError(/Ancestor spanID.*not found/);
});

it('skips trace without data', () => {
const traces = {
...makeTraces(shortTrace),
Expand All @@ -152,23 +155,63 @@ describe('transform traces to ddg paths', () => {
expect(result.length).toBe(1);
});

it("omits span if tags does not have span.kind === 'server'", () => {
const badSpanName = 'test bad span name';

const clientSpan = makeSpan(badSpanName, focalSpan, 'client');
clientSpan.hasChildren = false;
const clientTrace = makeTrace([rootSpan, focalSpan, clientSpan], 'clientTraceID');

const kindlessSpan = makeSpan(badSpanName, focalSpan, false);
kindlessSpan.hasChildren = false;
const kindlessTrace = makeTrace([rootSpan, focalSpan, kindlessSpan], 'kindlessTraceID');

const { dependencies: result } = transformTracesToPaths(makeTraces(clientTrace, kindlessTrace), focalSvc);
it("omits span if tags does not have span.kind === 'server' and is followed by the same service", () => {
const spanServiceAServer = makeSpan('SpanA1', focalSpan, 'server', 'opA', 'serviceA');
const otherSpanServiceAServer = makeSpan('SpanA2', spanServiceAServer, 'server', 'opB', 'serviceA');
otherSpanServiceAServer.hasChildren = false;
const spanServiceAClient = makeSpan('SpanA3', spanServiceAServer, 'client', 'opA', 'serviceA');
spanServiceAClient.hasChildren = false;
const spanServiceAKindless = makeSpan('SpanA4', spanServiceAServer, false, 'opA', 'serviceA');
spanServiceAKindless.hasChildren = false;

const spanServiceBClient = makeSpan('SpanB1', focalSpan, 'client', 'opA', 'serviceB');
const spanServiceBServer = makeSpan('SpanB2', spanServiceBClient, 'server', 'opB', 'serviceB');
spanServiceBServer.hasChildren = false;

const serverClientTrace = makeTrace(
[rootSpan, focalSpan, spanServiceAServer, spanServiceAClient],
'serverClientTraceID'
);
const clientServerTrace = makeTrace(
[rootSpan, focalSpan, spanServiceBClient, spanServiceBServer],
'clientServerTraceID'
);
const kindlessTrace = makeTrace(
[rootSpan, focalSpan, spanServiceAServer, spanServiceAKindless],
'kindlessTraceID'
);
const twoServersTrace = makeTrace(
[rootSpan, focalSpan, spanServiceAServer, otherSpanServiceAServer],
'twoServersTraceID'
);
const { dependencies: result } = transformTracesToPaths(
makeTraces(serverClientTrace, kindlessTrace, twoServersTrace, clientServerTrace),
focalSvc
);

const path = makeExpectedPath([rootSpan, focalSpan], clientTrace);
path.attributes.push({ key: 'exemplar_trace_id', value: kindlessTrace.data.traceID });
expect(new Set(result)).toEqual(
new Set([
addExemplarTraceIDs(makeExpectedPath([rootSpan, focalSpan, spanServiceAServer], serverClientTrace), [
kindlessTrace,
]),
makeExpectedPath([rootSpan, focalSpan, spanServiceAServer, otherSpanServiceAServer], twoServersTrace),
makeExpectedPath([rootSpan, focalSpan, spanServiceBClient, spanServiceBServer], clientServerTrace),
])
);
});

expect(new Set(result)).toEqual(new Set([path]));
it('support client span root', () => {
const spanServiceAClient = makeSpan('SpanA2', undefined, 'client', 'opA', 'serviceA');
const spanServiceAServer = makeSpan('SpanA1', spanServiceAClient, 'server', 'opA', 'serviceA');
const clientServerTrace = makeTrace([spanServiceAClient, spanServiceAServer], 'clientServerTraceID');
spanServiceAServer.hasChildren = false;
const { dependencies: result } = transformTracesToPaths(
makeTraces(clientServerTrace),
clientServerTrace.data.processes[spanServiceAClient.processID].serviceName
);
expect(new Set(result)).toEqual(
new Set([makeExpectedPath([spanServiceAClient, spanServiceAServer], clientServerTrace)])
);
});

it('dedupled paths', () => {
Expand All @@ -177,8 +220,9 @@ describe('transform traces to ddg paths', () => {
const trace1 = makeTrace([rootSpan, focalSpan, otherSpan], 'trace1');
const trace2 = makeTrace([rootSpan, focalSpan, otherSpan], 'trace2');
const { dependencies: result } = transformTracesToPaths(makeTraces(trace1, trace2), focalSvc);
const path = makeExpectedPath([rootSpan, focalSpan, otherSpan], trace1);
path.attributes.push({ key: 'exemplar_trace_id', value: trace2.data.traceID });
expect(new Set(result)).toEqual(new Set([path]));

expect(new Set(result)).toEqual(
new Set([addExemplarTraceIDs(makeExpectedPath([rootSpan, focalSpan, otherSpan], trace1), [trace2])])
);
});
});
98 changes: 51 additions & 47 deletions packages/jaeger-ui/src/model/ddg/transformTracesToPaths.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@
// limitations under the License.

import memoizeOne from 'memoize-one';

import spanAncestorIds from '../../utils/span-ancestor-ids';
import { getTraceSpanIdsAsTree, TREE_ROOT_ID } from '../../selectors/trace';

import { TDdgPayloadEntry, TDdgPayloadPath, TDdgPayload } from './types';
import { FetchedTrace } from '../../types';
import { Span } from '../../types/trace';

const isKindServer = (span: Span) =>
span.tags.find(({ key, value }) => key === 'span.kind' && value === 'server');

function transformTracesToPaths(
traces: Record<string, FetchedTrace>,
focalService: string,
Expand All @@ -30,53 +32,55 @@ function transformTracesToPaths(
if (data) {
const spanMap: Map<string, Span> = new Map();
const { traceID } = data;
data.spans
.filter(span => {
spanMap.set(span.spanID, span);
return !span.hasChildren;
})
.forEach(leaf => {
const spans = spanAncestorIds(leaf).map(id => {
const span = spanMap.get(id);
if (!span) throw new Error(`Ancestor spanID ${id} not found in trace ${traceID}`);
return span;
});
spans.reverse();
spans.push(leaf);

const path: TDdgPayloadEntry[] = spans
.filter(span => span.tags.find(({ key, value }) => key === 'span.kind' && value === 'server'))
.map(({ processID, operationName: operation }) => ({
service: data.processes[processID].serviceName,
operation,
}));

if (
path.some(
({ service, operation }) =>
service === focalService && (!focalOperation || operation === focalOperation)
)
data.spans.forEach(span => spanMap.set(span.spanID, span));
const tree = getTraceSpanIdsAsTree(data);
tree.paths((pathIds: string[]) => {
const paths = pathIds.reduce((reducedSpans: Span[], id: string): Span[] => {
if (id === TREE_ROOT_ID) {
return reducedSpans;
}
const span = spanMap.get(id);
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
if (!span) throw new Error(`Ancestor spanID ${id} not found in trace ${traceID}`);
if (reducedSpans.length === 0) {
reducedSpans.push(span);
} else if (
reducedSpans[reducedSpans.length - 1].processID !== span.processID ||
isKindServer(span)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole algorithm expressed in these two lines, awesome!

cc @vprithvi

) {
const pathKey = path.map(value => `${value.operation}:${value.service}`).join('/');
const dependency = dependenciesMap.get(pathKey);
if (!dependency) {
dependenciesMap.set(pathKey, {
path,
attributes: [
{
key: 'exemplar_trace_id',
value: traceID,
},
],
});
} else {
dependency.attributes.push({
key: 'exemplar_trace_id',
value: traceID,
});
}
reducedSpans.push(span);
}
return reducedSpans;
}, []);
const path: TDdgPayloadEntry[] = paths.map(({ processID, operationName: operation }) => ({
service: data.processes[processID].serviceName,
operation,
}));
if (
path.some(
({ service, operation }) =>
service === focalService && (!focalOperation || operation === focalOperation)
)
) {
const pathKey = path.map(value => `${value.operation}:${value.service}`).join('/');
const dependency = dependenciesMap.get(pathKey);
if (!dependency) {
dependenciesMap.set(pathKey, {
path,
attributes: [
{
key: 'exemplar_trace_id',
value: traceID,
},
],
});
} else {
dependency.attributes.push({
key: 'exemplar_trace_id',
value: traceID,
});
}
});
}
});
}
});
const dependencies = Array.from(dependenciesMap.values());
Expand Down
20 changes: 20 additions & 0 deletions packages/jaeger-ui/src/utils/TreeNode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,24 @@ export default class TreeNode {
}
}
}

paths(fn) {
const stack = [];
stack.push({ node: this, childIndex: 0 });
const paths = [];
while (stack.length) {
const { node, childIndex } = stack[stack.length - 1];
if (node.children.length >= childIndex + 1) {
stack[stack.length - 1].childIndex++;
stack.push({ node: node.children[childIndex], childIndex: 0 });
} else {
if (node.children.length === 0) {
const path = stack.map(item => item.node.value);
fn(path);
}
stack.pop();
}
}
return paths;
}
}