search

openzipkin / openzipkin.github.io

content for https://zipkin.io
https://zipkin.io
Apache License 2.0
39 stars 64 forks source link

Adds simplest example of set Span.timestamp and duration #56

Closed codefromthecrypt closed 7 years ago

codefromthecrypt commented 7 years ago

I made some insights when collaborating with @paolochiodi. This change represents the simplest pseudocode I can think of for how to understand when you should set Span.timestamp and duration.

https://github.com/paolochiodi/zipkin-simple/pull/2#issuecomment-250770128

cc @basvanbeek

paolochiodi commented 7 years ago

I think it would also be helpful to describe what MUST be provided to zipkin, what is optional and what zipkin is able to calculate and/or aggregate.

A couple of examples: 1- duration: I was initially under the assumption that zipkin would calculate that using the timestamps on the annotations (i.e. cs - cr) 2- My implementation sends multiple requests for the same span (i.e. one on cs and one on cr) and does not merge them in a single request. If I understand correctly this isn't the preferred way (although it seems to work and zipkin seems able to merge them into a single one). Is this supported or just a byproduct of the implementation? are there any downsides?

codefromthecrypt commented 7 years ago

hi @paolochiodi previous pull requests answered most of your questions. including a "why" section written mostly for you. this pull request is just a small clarification.

http://zipkin.io/pages/instrumenting.html

2) is a separate issue, but yes it is an anti-pattern to send multiple times when there's no reason to. imagine the overhead you would create if you sent a "cs" as an http request, right?

codefromthecrypt commented 7 years ago

by why, I mean the section at the bottom of http://zipkin.io/pages/instrumenting.html called "What happens when Span.timestamp and duration are not set?"

paolochiodi commented 7 years ago

@adriancole I totally appreciate that. Also, thanks for your continuos help to make my implementation better!

My comment was intended to just highlight that some of the details of the api at the moment need to be discovered by trial and error, or looking directly at the code of another implementation. The two examples I've made have already been addressed (and were probably caused by wrong assumptions on my side in the first time), but I'm wondering if maybe there some other unknowns or wrong assumptions on my side.

codefromthecrypt commented 7 years ago

First, I'll go ahead and say out loud that it is easier and more sensible for us to help people collaborate with a project like zipkin-js vs try and catalog how to re-invent it step by step. There's a couple reasons for this. Firstly, there are already experienced people in zipkin-js. It is easier to learn how to do something within a group. Learning from others by contributing to a library is a fair exchange. Learning by re-inventing in isolation is an interesting personal journey, but has higher burden and also higher on those who try to help you. I'd really prefer if people work together vs in separate directions, as for example, my time is much more highly leveraged on zipkin-js as it has many users and contributors.

All that said, I do want to help you as much as I can afford to. In fact, I've chatted recently with @basvanbeek that we need more information for some practices, at least to mutually save time for those who want to roll-their-own, and us who are asked questions along the way. I'll later make a more elaborate update to docs about reporting (please read if I do!). Here's a summary for now.

First question is why would you do something like this? For example, there's little value reporting from a feedback point of view as most spans complete relatively quickly. Ie you wouldn't click Zipkin ferociously every 50 milliseconds :) The main reason I think people might do this is just a side-effect of evolving their code. Ex they haven't designed a span recorder, yet.

There's lots of reasons sending annotation at a time isn't great. Here are some.

Imagine you are sending a span annotation. It is maybe 100 bytes. To get this to Zipkin, you need to put it onto the network. We don't have any transport optimized for message sizes that small. The overhead of an http 1.1 request or similar to send an amount that small would be silly. Remember, even if you don't care about the overhead of the zipkin transport, each network request is overhead in your app. Many libraries actually buffer multiple spans at a time to optimize for overhead. You have to at least record a complete span before this could happen.

Let's say you don't care about process overhead or network inefficiency. Once the partial span gets to zipkin, in most storage backends, it is written as a separate row or document on each update. That would mean that instead of 1 document for a span, you'd have N*annotation documents. This would slow down queries and increase storage.

Finally, there are some aspects of a span, such as timestamp and duration, which make sense in a complete span (ideally written only once). If you can record timestamp and duration properly, you probably have enough state to just write the span once. If you can't, then the writeup I made about missing timestamp and duration apply.

codefromthecrypt commented 7 years ago

My comment was intended to just highlight that some of the details of the api at the moment need to be discovered by trial and error, or looking directly at the code of another implementation.

I think you should expect to look at other code and trail/error. Ex after we have improved documentation, you will still need experience as it won't be prescriptive enough to cover libraries that often reach a thousand+ lines of code, right? Looking at other code is a great way to learn how to do something applied to the language you are using. Please do read other code!

The two examples I've made have already been addressed (and were probably caused by wrong assumptions on my side in the first time), but I'm wondering if maybe there some other unknowns or wrong assumptions on my side.

Assume so! expectations are important. When you choose to rewrite a library from scratch, you are taking some responsibility in the process. There's going to be unknowns or wrong assumptions because this isn't your second or third zipkin library (as it is for those on zipkin-js). These corrections will end up with you gaining more experience.

paolochiodi commented 7 years ago

Thank you for your esaustive answer.

First, I'll go ahead and say out loud that it is easier and more sensible for us to help people collaborate with a project like zipkin-js vs try and catalog how to re-invent it step by step. There's a couple reasons for this. Firstly, there are already experienced people in zipkin-js. It is easier to learn how to do something within a group. Learning from others by contributing to a library is a fair exchange. Learning by re-inventing in isolation is an interesting personal journey, but has higher burden and also higher on those who try to help you. I'd really prefer if people work together vs in separate directions, as for example, my time is much more highly leveraged on zipkin-js as it has many users and contributors.

I understand this. I've already received more help than I usually expect on open source, props to you for this 👍

On the specific issues:

Merging

The main reason I think people might do this is just a side-effect of evolving their code. Ex they haven't designed a span recorder, yet.

You're spot on. I don't have a span recorder - and would prefer not to have one, at the moment. I have a buffer tho, so I'm not doing an http request for each annotation.

Once the partial span gets to zipkin, in most storage backends, it is written as a separate row or document on each update. That would mean that instead of 1 document for a span, you'd have N*annotation documents. This would slow down queries and increase storage.

This is an interesting side effect to know, and may actually convince me to introduce a span recorder

Docs and other's code

I just want to share my experience as a newcomer: the docs do a great jobs to describe the concepts, but lack some "formal" description of the details (what is mandatory? which is the path for the http transport? Should I issue a POST? What status code may I get back?...)

Again: this is just me reporting my experience. I may have overlooked something or missed docs pages.

codefromthecrypt commented 7 years ago

Docs and other's code

I just want to share my experience as a newcomer: the docs do a great jobs to describe the concepts, but lack some "formal" description of the details (what is mandatory? which is the path for the http transport? Should I issue a POST? What status code may I get back?...)

Again: this is just me reporting my experience. I may have overlooked something or missed docs pages.

yes, you are overlooking docs, for example http://zipkin.io/zipkin-api/ is a swagger definition for POST including the PATH and details about the json.

Please look at existing issues before raising new ones or asking unrelated questions on pull requests. There are relevant things here including hints for unmerged work: https://github.com/openzipkin/openzipkin.github.io/issues

There are so many libraries doing post including the javascript library you've decided to rewrite. Please look there first. https://github.com/openzipkin/zipkin-js/tree/master/packages/zipkin-transport-http I'm not going to write a doc on how to make an arbitrarily different javascript library, though others might.

codefromthecrypt commented 7 years ago

closing this PR to stop bleeding time.

paolochiodi commented 7 years ago

Sorry for all the confusion and thank for your time!