Codecov Report

Merging #691 (00e281e) into main (74eda3d) will increase coverage by 0.38%.
The diff coverage is 99.05%.

@@             Coverage Diff              @@
##               main     #691      +/-   ##
============================================
+ Coverage     80.37%   80.75%   +0.38%     
- Complexity     1766     1805      +39     
============================================
  Files           222      225       +3     
  Lines          4520     4615      +95     
============================================
+ Hits           3633     3727      +94     
- Misses          887      888       +1     

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74eda3d...00e281e. Read the comment docs.

Single and multi-header propagation should be implemented as a single propagator with two configurations.

The debug flag has to be propagated (B3 Extract), the specification PR mentioned storing it in the returned context.

MUST preserve a debug trace flag, if received, and propagate it with subsequent requests.

@Nevay Added B3Propagator Class to handle two configurations and stored debug flag in the returned context. However, since we have implemented a singleton class for all the propagators, it is not possible to test the B3Propagator Class since there is no mechanism to reset the singleton static instance.

PS: @tidal @bobstrecansky @brettmc

We should allow users to choose between the two modes, either by having a singleton for each mode, or by exposing the constructor with an argument that allows choosing between the two modes.

Configuration Interface:

The SDK MUST provide a programmatic interface for all configuration. This interface SHOULD be written in the language of the SDK itself. All other configuration mechanisms SHOULD be built on top of this interface.

@Nevay so having a third B3Propagator class is not required since we have a singleton for each B3 Single and B3 Multi propagator?

No, we still need the combined ::extract() behavior - I meant replacing B3Propagator::getInstance() with a method for each configuration mode, something like ::getB3SingleHeaderInstance() and ::getB3MultiHeaderInstance().

Should be moved out of the API into an extension package - see Propagators Distribution.

@Nevay do we have any extension packages in opentelemetry-php?

do we have any extension packages in opentelemetry-php?

discussed in SIG, please go ahead and create an extensions dir (this is what otel-java does)

do we have any extension packages in opentelemetry-php?

discussed in SIG, please go ahead and create an extensions dir (this is what otel-java does)

@brettmc otel-java has nested directories under extension. Do we follow the same or a simple hierarchy? Also, they have api and sdk on the root folder whereas we have it under src. Should I place extension under the src?

I think src/Extension/Propagator/B3 or something similar...

I think src/Extension/Propagator/B3 or something similar...

Created the new Extension directory and moved the files under it.

@kishannsangani - do we feel this is ready now?

@kishannsangani - do we feel this is ready now?

Yes, I just pushed the final changes for the clarification on specs.

@Nevay @bobstrecansky @brettmc @tidal can you review it?

Needs a composer.json in either src/Extension/Propagator or src/Extension/Propagator/B3.

@Nevay added composer.json in src/Extension/Propagator/B3. Can you help me verify the same?