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

Adds diagram showing trace instrumentation's role in architecture #71

Merged
merged 1 commit into from
Jan 13, 2017

Conversation

codefromthecrypt
Copy link
Member

@codefromthecrypt codefromthecrypt commented Jan 11, 2017

Adds a sequence diagram that shows instrumentation do things in band and out-of-band

See #68

@codefromthecrypt
Copy link
Member Author

@guettli does this help?

@guettli
Copy link
Contributor

guettli commented Jan 11, 2017

Looks much less boring than a sequence diagram .... on the other hand I like it boring. I like it boring since it is often a good, since most obvious, way.

@codefromthecrypt codefromthecrypt changed the title Attempts to show data responsibility at the arch abstraction Adds diagram showing trace instrumentation's role in architecture Jan 12, 2017
@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Jan 12, 2017

@guettli PTAL I hope this is more what you were looking for (also changed PR description)

@codefromthecrypt codefromthecrypt force-pushed the data-ownership branch 2 times, most recently from 2693b10 to 7bb3e1b Compare January 12, 2017 08:51
@@ -33,6 +33,61 @@ Here's a diagram describing this flow:
To see if an instrumentation library already exists for your platform, see the
list of [existing instrumentations]({{ site.github.url}}/pages/existing_instrumentations).

Example flow
Copy link
Member Author

Choose a reason for hiding this comment

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

@openzipkin/core might want to take a look at this, too. The attempt here is to show the relationship of sorts of activity in scope of trace instrumentation. We've had plenty of requests to clarify this and probably does help to do so in the architecture pane vs requiring people to look at the instrumenting section.

IOTW, people simply using a zipkin system probably could do well to have a brief overview of the process.

thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

@adriancole This sequence diagram is easy to understand. Looks good, thank you. BTW, how did you "paint" it? What tool did you use?

Zipkin).

Here's an example sequence of http tracing. Notice timing data is reported to
Zipkin after the application response is completed.
Copy link
Member

Choose a reason for hiding this comment

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

"after the application response" may also be accurate here, but I believe the diagram is trying to show the tracing of the HTTP call GET /foo being made by "Application". Maybe it would be less confusing/more accurate to say "after the HTTP call is completed."

Copy link
Member

Choose a reason for hiding this comment

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

To that end, maybe it is best to explicitly state in the previous line:

Here's an example sequence of http tracing where "Application" is making an http call to GET /foo that will be traced in a single span.

Copy link
Member Author

Choose a reason for hiding this comment

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

good point.. above in the doc we do define the term Span.. good to reinforce

@codefromthecrypt codefromthecrypt force-pushed the data-ownership branch 2 times, most recently from 5b86209 to df70e85 Compare January 13, 2017 00:55
@codefromthecrypt
Copy link
Member Author

did some rewording based on ideas from @shakuzen

Here's an example sequence of http tracing where user code calls the resource
/foo. This results in a single span, sent asynchronously to Zipkin after user
code receives the http response.

then clarifying below the diagram why

Trace instrumentation report spans asynchronously to prevent delays or failures
relating to the tracing system from delaying or breaking user code.

Not sure if this is too much info for this section, but maybe it is ok? I chose "user code" instead of application as I didn't want folks to misunderstand application as something not in the same process. If the choice makes things worse, let me know

@codefromthecrypt codefromthecrypt merged commit 8df450d into master Jan 13, 2017
@codefromthecrypt codefromthecrypt deleted the data-ownership branch January 13, 2017 05:47
@codefromthecrypt
Copy link
Member Author

as we can edit at anytime (in fact anyone can raise a PR), merged as I think this was good progress

@codefromthecrypt
Copy link
Member Author

codefromthecrypt commented Jan 13, 2017 via email

@guettli
Copy link
Contributor

guettli commented Jan 13, 2017

@adriancole off topic: I want to learn zipkin, I have question here at StackO: http://stackoverflow.com/questions/41508180/zipkin-for-profiling-traditional-progamm-one-process-one-thread if you have three minutes ...

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.

3 participants