Propagate b3 parentspanid and debug flag #1346

srjames90 · 2020-07-24T00:44:33Z

Which problem is this PR solving?

fixes x-b3-parentspanid and x-b3-flags don't get propagated #1338

Short description of the changes

Adds parentSpanId and debug to the spanContext

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts

codecov · 2020-07-24T00:48:16Z

Codecov Report

Merging #1346 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1346      +/-   ##
==========================================
- Coverage   93.63%   93.62%   -0.01%     
==========================================
  Files         149      147       -2     
  Lines        4272     4190      -82     
  Branches      871      860      -11     
==========================================
- Hits         4000     3923      -77     
+ Misses        272      267       -5

Impacted Files	Coverage Δ
...metry-core/src/context/propagation/B3Propagator.ts	`100.00% <100.00%> (+2.32%)`	⬆️
packages/opentelemetry-plugin-fetch/src/fetch.ts
...telemetry-plugin-fetch/src/enums/AttributeNames.ts

srjames90 · 2020-07-24T00:50:10Z

Unrelated, but the test 'should gracefully handle an invalid b3 header' doesn't seem to be testing what we want.

packages/opentelemetry-api/src/trace/span_context.ts

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts

packages/opentelemetry-core/test/context/B3Propagator.test.ts

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts

obecny

Is there a situation when those 2 values will be set together ?

assert.strictEqual(context.getValue(DEBUG_FLAG_KEY), '1');
assert.equal(context.getValue(PARENT_SPAN_ID_KEY), 'Something else the undefined');

srjames90 · 2020-07-31T19:21:24Z

Is there a situation when those 2 values will be set together ?
assert.strictEqual(context.getValue(DEBUG_FLAG_KEY), '1');
assert.equal(context.getValue(PARENT_SPAN_ID_KEY), 'Something else the undefined');

Yeah, added a test for both of them being set

packages/opentelemetry-api/src/trace/span_context.ts

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts

dyladan · 2020-07-31T19:41:46Z

Forgot to add the reason I set my review status to "requested changes" is due to the undefined use of the traceflags second bit.

obecny

Thx for changes - the code is easier to read :).

If possible I would like to encourage you yet to rewrite description of unit test a bit. So they are more readable and are written in style of "when -> then". It also helps to document the code and to understand it faster whenever someone will try to fix the problem in future when tests are failing or need to refactor code. I have added 2 examples what I have on my mind, thank you

packages/opentelemetry-core/test/context/B3Propagator.test.ts

obecny

lgtm, thx for changes

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts

mayurkale22

Added one small comment - going to provide LGTM and hope you can address prior to merging.

srjames90 · 2020-08-04T18:05:08Z

Added one small comment - going to provide LGTM and hope you can address prior to merging.

done, thanks

* feat: graceful shutdown for tracing and metrics * fix: wording in test case * fix: typo * fix meterprovider config to use bracket notation Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> * fix meterprovider config to use bracket notation Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> * fix: add callbacks to shutdown methods * fix: merge conflict * simplify meter shutdown code Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> * fix: fix one-liner * private function name style fix Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> * fix: naming of private member variables * fix: graceful shutdown now works in browser * fix: window event listener will trigger once * fix: modify global shutdown helper functions * fix: remove callback from remove listener args * fix: change global shutdown function names and simplify functionality * fix: add rest of function refactoring and simplification * fix: remove unintended code snippet * fix: refactor naming of listener cleanup function and fix sandbox issue * fix: make global shutdown cleanup local * fix: change interval of MeterProvider collection to ensure it does not trigger through clock * chore: removing _cleanupGlobalShutdownListeners * fix: remove unnecesary trace provider member function * Removing default span attributes (#1342) * refactor(opentelemetry-tracing): removing default span attributes Signed-off-by: Aravin Sivakumar <aravinarjunn@gmail.com> * refactor(opentelemetry-tracing): removing default span attributed from tracer object Signed-off-by: Aravin Sivakumar <aravinarjunn@gmail.com> * refactor(opentelemetry-tracing): removing accidental add to package.json Signed-off-by: Aravin Sivakumar <aravinarjunn@gmail.com> * refactor(opentelemetry-tracing): removing redundant test and fixing suggestions by Shawn and Daniel Signed-off-by: Aravin Sivakumar <aravinarjunn@gmail.com> * feat: add baggage support to the opentracing shim (#918) Co-authored-by: Mayur Kale <mayurkale@google.com> * Add nodejs sdk package (#1187) Co-authored-by: Naseem <naseemkullah@gmail.com> Co-authored-by: legendecas <legendecas@gmail.com> Co-authored-by: Mark Wolff <mrw00010@gmail.com> Co-authored-by: Matthew Wear <matthew.wear@gmail.com> * feat: add OTEL_LOG_LEVEL env var (#974) * Proto update to latest to support arrays and maps (#1339) * chore: 0.10.0 release proposal (#1345) * fix: add missing grpc-js index (#1358) * chore: 0.10.1 release proposal (#1359) * feat(api/context-base): change compile target to es5 (#1368) * Feat: Make ID generator configurable (#1331) Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> * fix: require grpc-js instead of grpc in grpc-js example (#1364) Co-authored-by: Bartlomiej Obecny <bobecny@gmail.com> * chore(deps): update all non-major dependencies (#1371) * chore: bump metapackage dependencies (#1383) * chore: 0.10.2 proposal (#1382) * fix: remove unnecesary trace provider member function * refactor(metrics): distinguish different aggregator types (#1325) Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> * Propagate b3 parentspanid and debug flag (#1346) * feat: Export MinMaxLastSumCountAggregator metrics to the collector as Summary (#1320) Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> * feat: Collector Metric Exporter for the Web (#1308) Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> * Fix issues in TypeScript getting started example code (#1374) Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> * chore: deploy canary releases (#1384) * fix: protos pull * fix: address marius' feedback * chore: deleting removeAllListeners from prometheus, fixing tests, cleanu of events when using shutdown notifier * fix: add documentation and cleanup code * fix: remove async label from shutdown and cleanup test case * fix: update controller collect to return promise * fix: make downsides of disabling graceful shutdown more apparent Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com> Co-authored-by: Bartlomiej Obecny <bobecny@gmail.com> Co-authored-by: Aravin <34178459+aravinsiva@users.noreply.github.com> Co-authored-by: Ruben Vargas Palma <ruben.vp8510@gmail.com> Co-authored-by: Mayur Kale <mayurkale@google.com> Co-authored-by: Naseem <naseemkullah@gmail.com> Co-authored-by: legendecas <legendecas@gmail.com> Co-authored-by: Mark Wolff <mrw00010@gmail.com> Co-authored-by: Matthew Wear <matthew.wear@gmail.com> Co-authored-by: Naseem <naseem@transit.app> Co-authored-by: Mark Wolff <mark.wolff@microsoft.com> Co-authored-by: Cong Zou <32532612+EdZou@users.noreply.github.com> Co-authored-by: Reginald McDonald <40721169+reggiemcdonald@users.noreply.github.com> Co-authored-by: WhiteSource Renovate <bot@renovateapp.com> Co-authored-by: srjames90 <srjames@lightstep.com> Co-authored-by: David W <dwitt12345@gmail.com> Co-authored-by: Mick Dekkers <mickdekkersnl@gmail.com>

Co-authored-by: Amir Blum <amirgiraffe@gmail.com>

srjames90 requested review from dyladan, legendecas, markwolff, mayurkale22, mwear, naseemkullah, obecny, OlivierAlbertini and vmarchaud as code owners July 24, 2020 00:44

srjames90 commented Jul 24, 2020

View reviewed changes

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts Outdated Show resolved Hide resolved

srjames90 force-pushed the add-b3-parentspanid-and-debug-flag branch from 0492264 to bfaa63c Compare July 24, 2020 00:51

srjames90 commented Jul 24, 2020

View reviewed changes

packages/opentelemetry-api/src/trace/span_context.ts Outdated Show resolved Hide resolved

dyladan requested changes Jul 24, 2020

View reviewed changes

packages/opentelemetry-api/src/trace/span_context.ts Outdated Show resolved Hide resolved

packages/opentelemetry-api/src/trace/span_context.ts Outdated Show resolved Hide resolved

srjames90 force-pushed the add-b3-parentspanid-and-debug-flag branch from 30a8b86 to df4cb29 Compare July 25, 2020 22:54

srjames90 requested a review from dyladan July 25, 2020 22:54

dyladan reviewed Jul 27, 2020

View reviewed changes

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts Outdated Show resolved Hide resolved

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts Outdated Show resolved Hide resolved

srjames90 force-pushed the add-b3-parentspanid-and-debug-flag branch from c8add74 to cdbd1cb Compare July 27, 2020 18:33

srjames90 requested a review from dyladan July 27, 2020 18:33

srjames90 force-pushed the add-b3-parentspanid-and-debug-flag branch from cdbd1cb to 1a13d1d Compare July 27, 2020 18:35

dyladan reviewed Jul 27, 2020

View reviewed changes

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts Outdated Show resolved Hide resolved

obecny reviewed Jul 27, 2020

View reviewed changes

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts Outdated Show resolved Hide resolved

srjames90 requested review from dyladan and obecny July 27, 2020 19:17

srjames90 force-pushed the add-b3-parentspanid-and-debug-flag branch from cc5ede0 to bfd2885 Compare July 28, 2020 18:19

dyladan approved these changes Jul 30, 2020

View reviewed changes

mayurkale22 reviewed Jul 30, 2020

View reviewed changes

obecny reviewed Jul 30, 2020

View reviewed changes

srjames90 force-pushed the add-b3-parentspanid-and-debug-flag branch from 1a52ab7 to 3adff4f Compare July 30, 2020 23:25

srjames90 force-pushed the add-b3-parentspanid-and-debug-flag branch from 12755a7 to dcca659 Compare July 31, 2020 18:14

srjames90 requested review from obecny and mayurkale22 July 31, 2020 18:18

srjames90 force-pushed the add-b3-parentspanid-and-debug-flag branch from dcca659 to 0e2b8d0 Compare July 31, 2020 18:38

style: refactor b3 extract and tests

849f915

srjames90 force-pushed the add-b3-parentspanid-and-debug-flag branch from 0e2b8d0 to 849f915 Compare July 31, 2020 18:50

dyladan reviewed Jul 31, 2020

View reviewed changes

packages/opentelemetry-api/src/trace/span_context.ts Outdated Show resolved Hide resolved

dyladan requested changes Jul 31, 2020

View reviewed changes

fix: remove undefined traceFlags in spanContext

4209039

srjames90 requested a review from dyladan July 31, 2020 21:17

obecny reviewed Jul 31, 2020

View reviewed changes

packages/opentelemetry-core/test/context/B3Propagator.test.ts Show resolved Hide resolved

packages/opentelemetry-core/test/context/B3Propagator.test.ts Show resolved Hide resolved

packages/opentelemetry-core/test/context/B3Propagator.test.ts Outdated Show resolved Hide resolved

style: rename and add describes to some b3 tests

55304c7

srjames90 force-pushed the add-b3-parentspanid-and-debug-flag branch from bdecf88 to 55304c7 Compare August 3, 2020 18:17

obecny approved these changes Aug 3, 2020

View reviewed changes

dyladan approved these changes Aug 3, 2020

View reviewed changes

Merge branch 'master' into add-b3-parentspanid-and-debug-flag

bafbdc5

mayurkale22 reviewed Aug 4, 2020

View reviewed changes

packages/opentelemetry-core/src/context/propagation/B3Propagator.ts Outdated Show resolved Hide resolved

mayurkale22 approved these changes Aug 4, 2020

View reviewed changes

style: combine traceFlags none condition

9e1c337

Merge branch 'master' into add-b3-parentspanid-and-debug-flag

08abd7c

dyladan added the enhancement New feature or request label Aug 4, 2020

dyladan changed the title ~~Add b3 parentspanid and debug flag~~ Propagate b3 parentspanid and debug flag Aug 4, 2020

dyladan merged commit 81d75d2 into open-telemetry:master Aug 4, 2020

srjames90 deleted the add-b3-parentspanid-and-debug-flag branch August 4, 2020 18:22

jonahrosenblum pushed a commit to jonahrosenblum/opentelemetry-js that referenced this pull request Aug 8, 2020

Propagate b3 parentspanid and debug flag (open-telemetry#1346)

d07163a

pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023

chore(deps): update actions/stale action to v7 (open-telemetry#1346)

6e33568

Co-authored-by: Amir Blum <amirgiraffe@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propagate b3 parentspanid and debug flag #1346

Propagate b3 parentspanid and debug flag #1346

srjames90 commented Jul 24, 2020 •

edited

Loading

codecov bot commented Jul 24, 2020 •

edited

Loading

srjames90 commented Jul 24, 2020

obecny left a comment

srjames90 commented Jul 31, 2020

dyladan commented Jul 31, 2020

obecny left a comment

obecny left a comment

mayurkale22 left a comment

srjames90 commented Aug 4, 2020

Propagate b3 parentspanid and debug flag #1346

Propagate b3 parentspanid and debug flag #1346

Conversation

srjames90 commented Jul 24, 2020 • edited Loading

Which problem is this PR solving?

Short description of the changes

codecov bot commented Jul 24, 2020 • edited Loading

Codecov Report

srjames90 commented Jul 24, 2020

obecny left a comment

Choose a reason for hiding this comment

srjames90 commented Jul 31, 2020

dyladan commented Jul 31, 2020

obecny left a comment

Choose a reason for hiding this comment

obecny left a comment

Choose a reason for hiding this comment

mayurkale22 left a comment

Choose a reason for hiding this comment

srjames90 commented Aug 4, 2020

srjames90 commented Jul 24, 2020 •

edited

Loading

codecov bot commented Jul 24, 2020 •

edited

Loading