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

Changes index page to order services descending by max span duration #2234

Merged
merged 1 commit into from
Nov 4, 2018

Conversation

codefromthecrypt
Copy link
Member

I figured out what "max" was in the index page. It corresponds to the
longest duration span in the trace, as attributed to a specific service.

This orders the search results descending by the most likely culprit,
using relevant time units to prevent minutes from showing as huge counts
of milliseconds, or short traces showing up as zero.

Before:
screen shot 2018-11-03 at 3 18 38 pm

After: (notice the slowest service is the first one listed)
screen shot 2018-11-03 at 3 18 30 pm

@codefromthecrypt
Copy link
Member Author

cc @tacigar

I figured out what "max" was in the index page. It corresponds to the
longest duration span in the trace, as attributed to a specific service.

This orders the search results descending by the most likely culprit,
using relevant time units to prevent minutes from showing as huge counts
of milliseconds, or short traces showing up as zero.
@codefromthecrypt codefromthecrypt force-pushed the order-services-by-max-durations branch from 0ee846b to d26a58e Compare November 3, 2018 07:30
@codefromthecrypt
Copy link
Member Author

ps I asked @narayaruna offline and basically a "don't care" response as this isn't a very important change. I agree that it isn't important for analysis.. the main goal of this specific change is understanding what the code does so that we can complete v2 refactoring (#2217) which will let us do a UI replacement easier as discussed at LINE https://cwiki.apache.org/confluence/display/ZIPKIN/2018-10-29+Zipkin+UI+at+LINE+Tokyo

so yeah, this is in the weeds, likely not much notable help to end users, but it is a part of becoming better in a different sense (the UI revamp by @tacigar)

/me ends blathering

@@ -61,9 +61,9 @@ describe('convertSuccessResponse', () => {
duration: 0.017,
durationStr: '17μ',
width: 100,
totalSpans: 1,
serviceDurations: [
{name: 'backend', count: 1, max: 0} // TODO: figure out what max means
Copy link
Member Author

Choose a reason for hiding this comment

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

most important change was removing this TODO :P

{name: 'frontend', count: 2, max: 168}
spanCount: 2,
serviceNameAndSpanCounts: [
{serviceName: 'backend', spanCount: 1},
Copy link
Member Author

Choose a reason for hiding this comment

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

second most important change (to me) was removing dead data as it helps us understand what is needed by which screen

{index: 1, time: '3.4000000000000004μ'},
{index: 2, time: '6.800000000000001μ'},
{index: 3, time: '10.2μ'},
{index: 4, time: '13.600000000000001μ'},
Copy link
Member Author

Choose a reason for hiding this comment

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

3rd change is the actual UI nicies, like this horrible rendering of fractions of a us :D

@shakuzen
Copy link
Member

shakuzen commented Nov 3, 2018

What that duration even meant was confusing to users and a ticket was opened long ago: #998. I had never really paid much attention to it before, but I agree it isn't clear what it means. This change seems to be an improvement for people who understand what it means in the first place.

@codefromthecrypt codefromthecrypt merged commit 28185cb into master Nov 4, 2018
@codefromthecrypt codefromthecrypt deleted the order-services-by-max-durations branch November 4, 2018 06:07
@codefromthecrypt
Copy link
Member Author

merged for reason tommy mentioned. we can kill the confusion beast with each change like this.

abesto pushed a commit to abesto/zipkin that referenced this pull request Sep 10, 2019
…penzipkin#2234)

I figured out what "max" was in the index page. It corresponds to the
longest duration span in the trace, as attributed to a specific service.

This orders the search results descending by the most likely culprit,
using relevant time units to prevent minutes from showing as huge counts
of milliseconds, or short traces showing up as zero.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants