Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

refactor: address b3 propagation TODOs #48

Merged
merged 2 commits into from
Jun 20, 2018

Conversation

eduardoemery
Copy link
Contributor

The TODO left was:

TODO: Review this logic, maybe make it more robust. Question: are this logic valid if any of the getHeader operations returns a string[].

I checked and string[] has the same behavior in this logic. getHeader() returns a string | string[], the verification isNaN(Number()) works for both.

Copy link
Contributor

@kjin kjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation in the PR abstract. Does this mean that if we have multiple headers under the X_B3_SAMPLED key, that none of them will end up being used as the options value? Would it make more sense to always use the first value?

@eduardoemery
Copy link
Contributor Author

Added a few lines to make sure we get the first header if it comes as an Array.

@kjin kjin merged commit 2ddf84f into census-instrumentation:master Jun 20, 2018
kjin pushed a commit to kjin/opencensus-node that referenced this pull request Aug 24, 2018
* refactor: address b3 propagation TODOs

* refactor(fix): changes to address review comments
kjin pushed a commit to kjin/opencensus-node that referenced this pull request Aug 24, 2018
* refactor: address b3 propagation TODOs

* refactor(fix): changes to address review comments
kjin pushed a commit to kjin/opencensus-node that referenced this pull request Aug 24, 2018
* refactor: address b3 propagation TODOs

* refactor(fix): changes to address review comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants