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 support for datadog headers #187

Closed bg451 closed 4 years ago

bg451 commented 4 years ago

This change is Reviewable

bg451 commented 4 years ago

Defaults to true

On Thu, Nov 14, 2019 at 4:10 PM Stephanie Baum notifications@github.com wrote:

@sbaum1994 commented on this pull request.

In src/imp/propagator_dd.js https://github.com/lightstep/lightstep-tracer-javascript/pull/187#discussion_r346547172 :

  • }

  • let suffix = key.substr(this._carrierPrefix.length);

  • switch (suffix) {

  • case 'trace-id':

  • foundFields++;

  • traceGUID = parseInt(value, 10).toString(16);

  • break;

  • case 'parent-id':

  • foundFields++;

  • spanGUID = parseInt(value, 10).toString(16);

  • break;

  • case 'sampling-priority':

  • // TODO: support sampling priority somehow. This is a float64 that does not

  • // necessarily represent the sampling decision

  • if (value === 0) {

Should we also set sampled to true if the value is 1? 🤔

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lightstep/lightstep-tracer-javascript/pull/187?email_source=notifications&email_token=AAKJX7PRXZVT46XQOHOJVFDQTW5EDA5CNFSM4JNBN6IKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCLUQP2Y#pullrequestreview-317261803, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKJX7NLFOOUCX2RLTXFXR3QTW5EDANCNFSM4JNBN6IA .

sbaum1994 commented 4 years ago

^^ missed that 🤦‍♀ Otherwise lgtm