Add B3 multiple headers propagation #224
Conversation
duxet
changed the title
duxet
changed the title
| $value = $this->formatter->serialize($context); | ||
| if (!headers_sent()) { |
There was a problem hiding this comment.
i am not sure if i should keep it, it shouldn't cause any side effects IMHO
There was a problem hiding this comment.
It seems like a backwards incompatible change if you remove this code. If someone was relying on having the headers written with this library, we'd remove that functionality. However, since the package is in 0.x version, I suppose this is allowed.
| $enabled = null; | ||
|
|
||
| if ($sampled !== null) { | ||
| $enabled = $sampled === '1'; |
There was a problem hiding this comment.
It is possible some propagators pass true aswell according to https://github.com/openzipkin/b3-propagation#sampling-state-1
| $value = $this->formatter->serialize($context); | ||
| if (!headers_sent()) { |
There was a problem hiding this comment.
It seems like a backwards incompatible change if you remove this code. If someone was relying on having the headers written with this library, we'd remove that functionality. However, since the package is in 0.x version, I suppose this is allowed.
|
@chingor13 @basvanbeek Could you please review this PR? |
|
+1 looks 🆗 let's get this merged? |
|
Seems good. |
|
@chingor13 @basvanbeek Could you please review this PR? |
Solves #216.
I had to do some refactoring, as current setup was prepared only for single header propagation.