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

Export noopSpan type #48

Closed
honghzzhang opened this issue Mar 23, 2018 · 11 comments
Closed

Export noopSpan type #48

honghzzhang opened this issue Mar 23, 2018 · 11 comments

Comments

@honghzzhang
Copy link

Can we export noopSpan type or let me know how else I could check whether a given span is a noopSpan?

@jcchavezs
Copy link
Contributor

I guess what you need is that somehow determine whether a span context is noop or not. I would instead add the Span::IsNoop method instead like in brave: https://github.com/openzipkin/brave/blob/master/brave/src/main/java/brave/Span.java

Thoughts @basvanbeek ?

@basvanbeek
Copy link
Member

Is this needed because having a sampled span implies needing to do additional expensive computation?

I kind of want to make sure we don't add things as long as they are not really needed. I would like to hear about a proper use case for this.

@honghzzhang
Copy link
Author

I am trying to convert a Java project which uses zipkin tracing to Go. In the java project, it calls the brave API Span.isNoop() to determine whether to propagate the span and I was trying to find the equivalent API from the zipkin-go project.

@jcchavezs
Copy link
Contributor

I'd recommend that you don't check the noop and propagate the context (with X-B3-SAMPLED=0) as the recipient will create noop spans instead but I am curious whether there is another use case behind that. @adriancole can you think on a use case for this?

@honghzzhang
Copy link
Author

If I propagate the context (with X-B3-SAMPLED=0), will the recipient always create noop span or it knows to only create noop spans when applicable?

@basvanbeek
Copy link
Member

Ok, so one of the most important aspects to propagation is to always propagate! How would a downstream service otherwise know that the trace was not sampled? It could start a new trace because without propagation it thinks it is the trace root.

@jcchavezs
Copy link
Contributor

True but IMO if context contains sampled=false we should create noop spans. I raised an issue to discuss this: #49

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Mar 24, 2018 via email

@jcchavezs
Copy link
Contributor

jcchavezs commented Mar 24, 2018

I see but isn't firehose mode magic happening when reporting? Can we just use sampling=100% for firehose mode instead of creating real spans for sampled=false?

If the reason why is firehose them we probably need to do the same in other libraries, otherwise if one of our libraries creates a noop object we will miss that piece. Is this something already happening in brave?

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Mar 24, 2018 via email

@basvanbeek
Copy link
Member

There is merit in knowing when a span is recording items or if not (ex. noopSpan). As said above this helps with avoiding expensive computations and unnecessary burning of cpu cycles.

I do want to have this be part of the bigger scope of work on firehose mode. So closing here.

Firehose mode is discussed in #58

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

No branches or pull requests

4 participants