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.

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

Brings back relative width in trace query screen #2232

Merged
merged 1 commit into from
Nov 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions zipkin-ui/js/component_data/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ export default component(function DefaultData() {
type: 'GET',
dataType: 'json'
}).done(traces => {
const summaries = traces.map(raw => rawTraceToSummary(raw));
const traceSummaries = traces.map(raw => rawTraceToSummary(raw));
const modelview = {
traces: traceSummariesToMustache(apiQuery.serviceName, summaries),
traces: traceSummariesToMustache(apiQuery.serviceName, traceSummaries),
apiURL,
rawResponse: traces
};
Expand Down
68 changes: 32 additions & 36 deletions zipkin-ui/js/component_ui/traceSummary.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,20 @@ export function getServiceName(span) {
return allServiceNames.length === 0 ? null : allServiceNames[0];
}

function getSpanTimestamps(spans) {
return _(spans).flatMap((span) => getServiceNames(span).map((serviceName) => ({
export function getGroupedTimestamps(spans) {
const spanTimestamps = _(spans).flatMap((span) => getServiceNames(span).map((serviceName) => ({
name: serviceName,
timestamp: span.timestamp,
duration: span.duration
}))).value();

const serviceToNameTimestampDurations = _(spanTimestamps).groupBy((sts) => sts.name).value();

// wash out the redundant name. TODO: rewrite this whole method as it seems easier imperatively
return _(serviceToNameTimestampDurations).mapValues((ntds) => ntds.map((ntd) => ({
timestamp: ntd.timestamp,
duration: ntd.duration
}))).value();
}


Expand Down Expand Up @@ -147,14 +155,14 @@ export function traceSummary(spans = []) {
const endpoints = _(spans).flatMap(endpointsForSpan).uniqWith(endpointEquals).value();
const traceId = spans[0].traceId;
const timestamp = spans[0].timestamp;
const spanTimestamps = getSpanTimestamps(spans);
const groupedTimestamps = getGroupedTimestamps(spans);
const errorType = getTraceErrorType(spans);
const totalSpans = spans.length;
return {
traceId,
timestamp,
duration,
spanTimestamps,
groupedTimestamps,
endpoints,
errorType,
totalSpans
Expand Down Expand Up @@ -188,10 +196,6 @@ function formatDate(timestamp, utc) {
return m.format('MM-DD-YYYYTHH:mm:ss.SSSZZ');
}

export function getGroupedTimestamps(summary) {
return _(summary.spanTimestamps).groupBy((sts) => sts.name).value();
}

export function getServiceDurations(groupedTimestamps) {
return _(groupedTimestamps).toPairs().map(([name, sts]) => ({
name,
Expand Down Expand Up @@ -227,40 +231,32 @@ export function traceSummariesToMustache(serviceName = null, traceSummaries, utc
return [];
} else {
const traceSummariesCleaned = removeEmptyFromArray(traceSummaries);
const maxDuration = Math.max(...traceSummariesCleaned.map((s) => s.duration)) / 1000;
Copy link
Member Author

Choose a reason for hiding this comment

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

this was the bug. all the data are in microseconds, yet somehow this line thought it should be millis :P

const maxDuration = Math.max(...traceSummariesCleaned.map((s) => s.duration));

return traceSummariesCleaned.map((t) => {
const duration = t.duration / 1000;
const groupedTimestamps = getGroupedTimestamps(t);
const serviceDurations = getServiceDurations(groupedTimestamps);
let servicePercentage;
let serviceTime;
if (!serviceName || !groupedTimestamps[serviceName]) {
serviceTime = 0;
} else {
serviceTime = totalServiceTime(groupedTimestamps[serviceName]);
servicePercentage = parseInt(
parseFloat(serviceTime) / parseFloat(t.duration) * 100,
10);
}

const startTs = formatDate(t.timestamp, utc);
const durationStr = mkDurationStr(t.duration);
const width = parseInt(parseFloat(duration) / parseFloat(maxDuration) * 100, 10);
const infoClass = t.errorType === 'none' ? '' : `trace-error-${t.errorType}`;
const timestamp = t.timestamp;
const duration = t.duration;
const groupedTimestamps = t.groupedTimestamps;

return {
const res = {
traceId: t.traceId,
startTs,
timestamp: t.timestamp,
duration,
durationStr,
servicePercentage,
startTs: formatDate(timestamp, utc),
timestamp,
duration: duration / 1000,
durationStr: mkDurationStr(duration),
width: parseInt(parseFloat(duration) / parseFloat(maxDuration) * 100, 10),
totalSpans: t.totalSpans,
serviceDurations,
width,
infoClass
serviceDurations: getServiceDurations(groupedTimestamps),
infoClass: t.errorType === 'none' ? '' : `trace-error-${t.errorType}`
};

// Only add a service percentage when there is a duration for it
if (serviceName && groupedTimestamps[serviceName]) {
const serviceTime = totalServiceTime(groupedTimestamps[serviceName]);
res.servicePercentage = parseInt(parseFloat(serviceTime) / parseFloat(duration) * 100, 10);
}

return res;
}).sort((t1, t2) => {
const durationComparison = t2.duration - t1.duration;
if (durationComparison === 0) {
Expand Down
19 changes: 9 additions & 10 deletions zipkin-ui/js/component_ui/traceToMustache.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import _ from 'lodash';
import {
traceSummary,
getGroupedTimestamps,
getServiceDurations,
getServiceNames,
getServiceName,
Expand Down Expand Up @@ -91,11 +90,11 @@ export function formatEndpoint({ipv4, ipv6, port, serviceName}) {
}

export default function traceToMustache(trace, logsUrl = undefined) {
const summary = traceSummary(trace);
const traceId = summary.traceId;
const duration = mkDurationStr(summary.duration);
const groupedTimestamps = getGroupedTimestamps(summary);
const serviceDurations = getServiceDurations(groupedTimestamps);
const t = traceSummary(trace);
const traceId = t.traceId;
const duration = t.duration;
const serviceDurations = getServiceDurations(t.groupedTimestamps);

const services = serviceDurations.length || 0;
const serviceCounts = _(serviceDurations).sortBy('name').value();
const groupByParentId = _(trace).groupBy((s) => s.parentId).value();
Expand All @@ -109,7 +108,7 @@ export default function traceToMustache(trace, logsUrl = undefined) {
(rootSpan) => childrenToList(createSpanTreeEntry(rootSpan, trace))).map((span) => {
const spanStartTs = span.timestamp || traceTimestamp;
const spanDepth = spanDepths[span.id] || 1;
const width = (span.duration || 0) / summary.duration * 100;
const width = (span.duration || 0) / duration * 100;
let errorType = 'none';

const binaryAnnotations = (span.binaryAnnotations || [])
Expand Down Expand Up @@ -157,7 +156,7 @@ export default function traceToMustache(trace, logsUrl = undefined) {
serviceName: getServiceName(span) || '',
duration: span.duration,
durationStr: mkDurationStr(span.duration),
left: parseFloat(spanStartTs - traceTimestamp) / parseFloat(summary.duration) * 100,
left: parseFloat(spanStartTs - traceTimestamp) / parseFloat(duration) * 100,
width: width < 0.1 ? 0.1 : width,
depth: (spanDepth + 1) * 5,
depthClass: (spanDepth - 1) % 6,
Expand All @@ -179,13 +178,13 @@ export default function traceToMustache(trace, logsUrl = undefined) {

const totalSpans = spans.length;
const timeMarkers = [0.0, 0.2, 0.4, 0.6, 0.8, 1.0]
.map((p, index) => ({index, time: mkDurationStr(summary.duration * p)}));
.map((p, index) => ({index, time: mkDurationStr(duration * p)}));
const timeMarkersBackup = timeMarkers;
const spansBackup = spans;

return {
traceId,
duration,
duration: mkDurationStr(duration),
services,
depth,
totalSpans,
Expand Down
44 changes: 22 additions & 22 deletions zipkin-ui/test/component_data/default.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,28 +66,28 @@ describe('rawTraceToSummary', () => {
traceId: '1e223ff1f80f1c69',
timestamp: 1470150004071068,
duration: 99411,
spanTimestamps: [
{
name: 'servicea',
timestamp: 1470150004071068,
duration: 99411
},
{
name: 'servicea',
timestamp: 1470150004074202,
duration: 94539
},
{
name: 'serviceb',
timestamp: 1470150004074202,
duration: 94539
},
{
name: 'serviceb',
timestamp: 1470150004074684,
duration: 65000
}
],
groupedTimestamps: {
servicea: [
{
timestamp: 1470150004071068,
duration: 99411
},
{
timestamp: 1470150004074202,
duration: 94539
}
],
serviceb: [
{
timestamp: 1470150004074202,
duration: 94539
},
{
timestamp: 1470150004074684,
duration: 65000
}
]
},
endpoints: [
{
serviceName: 'servicea',
Expand Down
54 changes: 40 additions & 14 deletions zipkin-ui/test/component_ui/traceSummary.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,19 +312,24 @@ describe('traceSummariesToMustache', () => {
traceId: 'cafedead',
timestamp: start,
duration: 20000,
spanTimestamps: [{
name: 'A',
timestamp: start,
duration: 10000
}, {
name: 'B',
timestamp: start + 1000,
duration: 20000
}, {
name: 'B',
timestamp: start + 1000,
duration: 15000
}],
groupedTimestamps: {
A: [
{
timestamp: start,
duration: 10000
}
],
B: [
{
timestamp: start + 1000,
duration: 20000
},
{
timestamp: start + 1000,
duration: 15000
}
]
},
endpoints: [ep1, ep2]
};

Expand Down Expand Up @@ -371,8 +376,29 @@ describe('traceSummariesToMustache', () => {
});

it('should calculate the width in percent', () => {
const model = traceSummariesToMustache(null, [summary]);
const summary1 = {
traceId: 'cafebaby',
timestamp: start,
duration: 2000,
groupedTimestamps: {
A: [{timestamp: start + 1, duration: 2000}]
},
endpoints: [ep1]
};
const summary2 = {
traceId: 'cafedead',
timestamp: start,
duration: 20000,
groupedTimestamps: {
A: [{timestamp: start, duration: 20000}]
},
endpoints: [ep1]
};

// Model is ordered by duration, and the width should be relative (percentage)
const model = traceSummariesToMustache(null, [summary1, summary2]);
model[0].width.should.equal(100);
model[1].width.should.equal(10);
});

it('should pass on timestamp', () => {
Expand Down