search

yurishkuro / opentracing-tutorial

A collection of tutorials for the OpenTracing API
Apache License 2.0
1.57k stars 407 forks source link

Edit nodejs tutorial #6

Closed MarckK closed 6 years ago

yurishkuro commented 6 years ago

Hi Kara, what's the status of this - are you blocked or it's ready to merge?

MarckK commented 6 years ago

Hi Yuri, the glitch with the UDPSender is still not fixed. That is to say, it is still necessary to have a timeout of 12 seconds when you call tracer.close to ensure all spans are flushed. The fact that that bothers me is a 'block'. What I've pushed up to the tutorial itself is fine. However, the tutorial is not completed -- I got distracted with fixing the UDPSender (https://github.com/jaegertracing/jaeger-client-node/pull/207). Nonetheless, what I have here so far for the tutorial is legit.

yurishkuro commented 6 years ago

It would be good to decouple these things. The tutorial can have a documented caveat about why the sleep is needed, but there's a lot of stuff in this PR otherwise that's unrelated to the sleep and would be good to merge.

Re 12 seconds - why so long? The tracer should be flushing spans every second.

MarckK commented 6 years ago

You got me, Yuri. I tried shorter time outs, without success; literally, 12 seconds was the shortest time frame I could be sure would flush the spans.

The tutorial as is can be merged and I'll see if I have time to finish it this weekend :)

MarckK commented 6 years ago

Hi Yuri, I have questions on how to complete the 'Propagate the in-process context' section of lesson02. This section for JS is going to differ from the equivalent sections for either Python or Go, and the OpenTracing API docs for JS SpanContext are not helping me. Nor is this. Additionally, when I pass in the additional childOf option to the startSpan function, whether I give it a value of rootSpan or rootSpan.context() doesn't appear to be having an effect in terms of what is logged in the terminal or displayed in UI.

yurishkuro commented 6 years ago

@MarckK I released Node client 3.9.0 with support for callbacks.

yurishkuro commented 6 years ago

Thanks, @MarckK !