search

lightstep / lightstep-tracer-javascript

Lightstep distributed tracing library for Node.js and the browser
https://lightstep.com
MIT License
77 stars 66 forks source link

Add B3 format #177

Closed bg451 closed 5 years ago

bg451 commented 5 years ago
austinlparker commented 5 years ago

Are we not implementing B3 extract?

bg451 commented 5 years ago

The class inherits from the ls propagator, this is the extract method but with the b3 prefix: https://github.com/lightstep/lightstep-tracer-javascript/pull/177/files#diff-0bd811c70646164f4df5cb606617eef3R33

bg451 commented 5 years ago

I guess there is a good question of whether we should ignore the sampled field for B3?

austinlparker commented 5 years ago

Oh, d'oh. In terms of sampled... I think we do need to propagate that correctly.

ledor473 commented 5 years ago

I'm not sure about the right answer on how to handle the sampled flag, but the Java implementation inject() it to true and don't even parse it on the extract()

https://github.com/lightstep/lightstep-tracer-java-common/blob/0.16.2/common/src/main/java/com/lightstep/tracer/shared/B3Propagator.java#L19

mwear commented 5 years ago

Just to make things interesting. The Ruby implementation, at least as it's currently proposed, is propagating the sampled flag. See: https://github.com/lightstep/lightstep-tracer-ruby/pull/86/commits/5eb1644b1c460b9d08b8dae432844b1425f14398. This is in part due to the comment above. I think we need to consider how propagating a true sampled decision downstream could impact non-LightStep services.

austinlparker commented 5 years ago

Well, we haven't really written down the logic behind this anywhere afaik but historically we've always injected a true value for sampling because the Satellite ignores sampling flags anyway set by the client. I presume that this would potentially be surprising in other circumstances so perhaps we should start propagating it and fix it everywhere to have the same behavior?

bg451 commented 5 years ago

@austinlparker updated to propagate sampled flag if it's there, ptal