From b88909649d60879b20363285a92611fa8ce5a7c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Moz=CC=87ejko?= Date: Wed, 6 Feb 2019 08:08:27 +0100 Subject: [PATCH 1/3] Add B3 multiple headers propagation --- .../Integrations/Guzzle/EventSubscriber.php | 2 +- src/Trace/Integrations/Guzzle/Middleware.php | 9 ++- src/Trace/Propagator/B3HeadersPropagator.php | 58 ++++++++++++++ src/Trace/Propagator/FormatterInterface.php | 2 +- .../Propagator/GrpcMetadataPropagator.php | 20 ----- src/Trace/Propagator/HttpHeaderPropagator.php | 32 ++------ src/Trace/Propagator/PropagatorInterface.php | 14 ---- src/Trace/SpanContext.php | 6 ++ .../integration/guzzle5/tests/Guzzle5Test.php | 2 +- .../integration/guzzle6/tests/Guzzle6Test.php | 2 +- .../Guzzle/EventSubscriberTest.php | 4 +- .../Integrations/Guzzle/MiddlewareTest.php | 2 +- .../Propagator/B3HeadersPropagatorTest.php | 80 +++++++++++++++++++ .../Propagator/HttpHeaderPropagatorTest.php | 16 ++-- tests/unit/Trace/RequestHandlerTest.php | 6 +- 15 files changed, 172 insertions(+), 83 deletions(-) create mode 100644 src/Trace/Propagator/B3HeadersPropagator.php create mode 100644 tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php diff --git a/src/Trace/Integrations/Guzzle/EventSubscriber.php b/src/Trace/Integrations/Guzzle/EventSubscriber.php index abc0af046..4a8f2e107 100644 --- a/src/Trace/Integrations/Guzzle/EventSubscriber.php +++ b/src/Trace/Integrations/Guzzle/EventSubscriber.php @@ -88,7 +88,7 @@ public function onBefore(BeforeEvent $event) $request = $event->getRequest(); $context = Tracer::spanContext(); if ($context->enabled()) { - $request->setHeader($this->propagator->key(), $this->propagator->formatter()->serialize($context)); + $request->setHeaders($this->propagator->inject($context, [])); } $span = Tracer::startSpan([ 'name' => 'GuzzleHttp::request', diff --git a/src/Trace/Integrations/Guzzle/Middleware.php b/src/Trace/Integrations/Guzzle/Middleware.php index e6b51bbe0..b0470a173 100644 --- a/src/Trace/Integrations/Guzzle/Middleware.php +++ b/src/Trace/Integrations/Guzzle/Middleware.php @@ -70,10 +70,11 @@ public function __invoke(callable $handler) return function (RequestInterface $request, $options) use ($handler) { $context = Tracer::spanContext(); if ($context->enabled()) { - $request = $request->withHeader( - $this->propagator->key(), - $this->propagator->formatter()->serialize($context) - ); + $headers = $this->propagator->inject($context, []) ; + + foreach ($headers as $headerName => $headerValue) { + $request = $request->withHeader($headerName, $headerValue); + } } return Tracer::inSpan([ 'name' => 'GuzzleHttp::request', diff --git a/src/Trace/Propagator/B3HeadersPropagator.php b/src/Trace/Propagator/B3HeadersPropagator.php new file mode 100644 index 000000000..24d0202f1 --- /dev/null +++ b/src/Trace/Propagator/B3HeadersPropagator.php @@ -0,0 +1,58 @@ + $context->traceId(), + self::X_B3_SPAN_ID => $context->spanId(), + self::X_B3_SAMPLED => $context->enabled() ? 1 : 0, + ] + $container; + } +} diff --git a/src/Trace/Propagator/FormatterInterface.php b/src/Trace/Propagator/FormatterInterface.php index b3d1223ef..2ac870d1a 100644 --- a/src/Trace/Propagator/FormatterInterface.php +++ b/src/Trace/Propagator/FormatterInterface.php @@ -27,7 +27,7 @@ interface FormatterInterface /** * Generate a SpanContext object from the Trace Context header * - * @param string $header + * @param string[] $header * @return SpanContext */ public function deserialize($header); diff --git a/src/Trace/Propagator/GrpcMetadataPropagator.php b/src/Trace/Propagator/GrpcMetadataPropagator.php index f897f9cb9..64e703e31 100644 --- a/src/Trace/Propagator/GrpcMetadataPropagator.php +++ b/src/Trace/Propagator/GrpcMetadataPropagator.php @@ -78,24 +78,4 @@ public function inject(SpanContext $context, $metadata) $metadata[$this->key] = $this->formatter->serialize($context); return $metadata; } - - /** - * Fetch the formatter for propagating the SpanContext - * - * @return FormatterInterface - */ - public function formatter() - { - return $this->formatter; - } - - /** - * Return the key used to propagate the SpanContext - * - * @return string - */ - public function key() - { - return $this->key; - } } diff --git a/src/Trace/Propagator/HttpHeaderPropagator.php b/src/Trace/Propagator/HttpHeaderPropagator.php index e83f241e7..8f6e1c610 100644 --- a/src/Trace/Propagator/HttpHeaderPropagator.php +++ b/src/Trace/Propagator/HttpHeaderPropagator.php @@ -25,7 +25,7 @@ */ class HttpHeaderPropagator implements PropagatorInterface { - const DEFAULT_HEADER = 'HTTP_X_CLOUD_TRACE_CONTEXT'; + const DEFAULT_HEADER = 'X-Cloud-Trace-Context'; /** * @var FormatterInterface @@ -44,7 +44,7 @@ class HttpHeaderPropagator implements PropagatorInterface * deserialize SpanContext. **Defaults to** a new * CloudTraceFormatter. * @param string $key [optional] The header key to store/retrieve the - * encoded SpanContext. **Defaults to** `HTTP_X_CLOUD_TRACE_CONTEXT` + * encoded SpanContext. **Defaults to** `X-Cloud-Trace-Context` */ public function __construct(FormatterInterface $formatter = null, $header = null) { @@ -75,33 +75,11 @@ public function extract($headers) */ public function inject(SpanContext $context, $container) { - $header = $this->key(); + $header = $this->header; $value = $this->formatter->serialize($context); - if (!headers_sent()) { - header("$header: $value"); - } - return [ - $header => $value - ]; - } - /** - * Returns the current formatter - * - * @return FormatterInterface - */ - public function formatter() - { - return $this->formatter; - } + $container[$header] = $value; - /** - * Return the key used to propagate the SpanContext - * - * @return string - */ - public function key() - { - return str_replace('_', '-', preg_replace('/^HTTP_/', '', $this->header)); + return $container; } } diff --git a/src/Trace/Propagator/PropagatorInterface.php b/src/Trace/Propagator/PropagatorInterface.php index 1df798531..c9ee8bd06 100644 --- a/src/Trace/Propagator/PropagatorInterface.php +++ b/src/Trace/Propagator/PropagatorInterface.php @@ -42,18 +42,4 @@ public function extract($container); * @return array */ public function inject(SpanContext $context, $container); - - /** - * Fetch the formatter for propagating the SpanContext - * - * @return FormatterInterface - */ - public function formatter(); - - /** - * Return the key used to propagate the SpanContext - * - * @return string - */ - public function key(); } diff --git a/src/Trace/SpanContext.php b/src/Trace/SpanContext.php index 3bd9b9b03..882b723f6 100644 --- a/src/Trace/SpanContext.php +++ b/src/Trace/SpanContext.php @@ -52,6 +52,12 @@ class SpanContext */ private $enabled; + + /** + * @var bool Whether or not this context was detected from a request header. + */ + private $fromHeader; + /** * Creates a new SpanContext instance * diff --git a/tests/integration/guzzle5/tests/Guzzle5Test.php b/tests/integration/guzzle5/tests/Guzzle5Test.php index 4c377fd75..bf4b9293e 100644 --- a/tests/integration/guzzle5/tests/Guzzle5Test.php +++ b/tests/integration/guzzle5/tests/Guzzle5Test.php @@ -95,7 +95,7 @@ function (RequestInterface $request, ResponseInterface &$response) { $tracer = Tracer::start($exporter->reveal(), [ 'skipReporting' => true, 'headers' => [ - 'HTTP_X_CLOUD_TRACE_CONTEXT' => $traceContextHeader + 'X-Cloud-Trace-Context' => $traceContextHeader ] ]); diff --git a/tests/integration/guzzle6/tests/Guzzle6Test.php b/tests/integration/guzzle6/tests/Guzzle6Test.php index bb30e7b0f..07e8c939e 100644 --- a/tests/integration/guzzle6/tests/Guzzle6Test.php +++ b/tests/integration/guzzle6/tests/Guzzle6Test.php @@ -95,7 +95,7 @@ function (RequestInterface $request, ResponseInterface &$response) { $tracer = Tracer::start($exporter->reveal(), [ 'skipReporting' => true, 'headers' => [ - 'HTTP_X_CLOUD_TRACE_CONTEXT' => $traceContextHeader + 'X-Cloud-Trace-Context' => $traceContextHeader ] ]); diff --git a/tests/unit/Trace/Integrations/Guzzle/EventSubscriberTest.php b/tests/unit/Trace/Integrations/Guzzle/EventSubscriberTest.php index 4d68df645..995dddab4 100644 --- a/tests/unit/Trace/Integrations/Guzzle/EventSubscriberTest.php +++ b/tests/unit/Trace/Integrations/Guzzle/EventSubscriberTest.php @@ -72,8 +72,8 @@ public function testAddsSpanContextHeader() $request = $history->getLastRequest(); $headers = $request->getHeaders(); - $this->assertArrayHasKey('X-CLOUD-TRACE-CONTEXT', $headers); - $this->assertRegExp('/[0-9a-f]+\/4660;o=1/', $headers['X-CLOUD-TRACE-CONTEXT'][0]); + $this->assertArrayHasKey('X-Cloud-Trace-Context', $headers); + $this->assertRegExp('/[0-9a-f]+\/4660;o=1/', $headers['X-Cloud-Trace-Context'][0]); $rt->onExit(); } diff --git a/tests/unit/Trace/Integrations/Guzzle/MiddlewareTest.php b/tests/unit/Trace/Integrations/Guzzle/MiddlewareTest.php index 7a2884279..693ef906e 100644 --- a/tests/unit/Trace/Integrations/Guzzle/MiddlewareTest.php +++ b/tests/unit/Trace/Integrations/Guzzle/MiddlewareTest.php @@ -63,7 +63,7 @@ public function testAddsSpanContextHeader() $req = $this->prophesize(RequestInterface::class); $req->getMethod()->willReturn('GET')->shouldBeCalled(); $req->getUri()->willReturn('/')->shouldBeCalled(); - $req->withHeader('X-CLOUD-TRACE-CONTEXT', Argument::that(function ($val) { + $req->withHeader('X-Cloud-Trace-Context', Argument::that(function ($val) { return preg_match('/[0-9a-f]+\/4660;o=1/', $val); }))->willReturn($req->reveal())->shouldBeCalled(); $request = $req->reveal(); diff --git a/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php b/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php new file mode 100644 index 000000000..dc37fb2f1 --- /dev/null +++ b/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php @@ -0,0 +1,80 @@ +extract($headers); + $this->assertEquals($traceId, $context->traceId()); + $this->assertEquals($spanId, $context->spanId()); + $this->assertEquals($enabled, $context->enabled()); + $this->assertTrue($context->fromHeader()); + } + + /** + * @dataProvider traceMetadata + */ + public function testInject($traceId, $spanId, $enabled, $headers) + { + $propagator = new B3HeadersPropagator(); + $context = new SpanContext($traceId, $spanId, $enabled); + $output = $propagator->inject($context, []); + + if (array_key_exists('X-B3-Flags', $headers)) { + $headers['X-B3-Sampled'] = $headers['X-B3-Flags']; + unset($headers['X-B3-Flags']); + } + + $this->assertEquals($headers, $output); + } + + public function traceMetadata() + { + return [ + [ + '463ac35c9f6413ad48485a3953bb6124', + 'a2fb4a1d1a96d312', + true, + [ + 'X-B3-TraceId' => '463ac35c9f6413ad48485a3953bb6124', + 'X-B3-SpanId' => 'a2fb4a1d1a96d312', + 'X-B3-Sampled' => '1', + ], + ], + [ + '463ac35c9f6413ad48485a3953bb6124', + 'a2fb4a1d1a96d312', + false, + [ + 'X-B3-TraceId' => '463ac35c9f6413ad48485a3953bb6124', + 'X-B3-SpanId' => 'a2fb4a1d1a96d312', + 'X-B3-Sampled' => '0', + ], + ], + [ + '463ac35c9f6413ad48485a3953bb6124', + 'a2fb4a1d1a96d312', + true, + [ + 'X-B3-TraceId' => '463ac35c9f6413ad48485a3953bb6124', + 'X-B3-SpanId' => 'a2fb4a1d1a96d312', + 'X-B3-Flags' => '1', + ], + ], + ]; + } +} diff --git a/tests/unit/Trace/Propagator/HttpHeaderPropagatorTest.php b/tests/unit/Trace/Propagator/HttpHeaderPropagatorTest.php index da01989d2..e242b2f5f 100644 --- a/tests/unit/Trace/Propagator/HttpHeaderPropagatorTest.php +++ b/tests/unit/Trace/Propagator/HttpHeaderPropagatorTest.php @@ -33,7 +33,7 @@ class HttpHeaderPropagatorTest extends TestCase public function testExtract($traceId, $spanId, $enabled, $header) { $propagator = new HttpHeaderPropagator(); - $context = $propagator->extract(['HTTP_X_CLOUD_TRACE_CONTEXT' => $header]); + $context = $propagator->extract(['X-Cloud-Trace-Context' => $header]); $this->assertEquals($traceId, $context->traceId()); $this->assertEquals($spanId, $context->spanId()); $this->assertEquals($enabled, $context->enabled()); @@ -45,8 +45,8 @@ public function testExtract($traceId, $spanId, $enabled, $header) */ public function testExtractCustomKey($traceId, $spanId, $enabled, $header) { - $propagator = new HttpHeaderPropagator(new CloudTraceFormatter(), 'HTTP_TRACE_CONTEXT'); - $context = $propagator->extract(['HTTP_TRACE_CONTEXT' => $header]); + $propagator = new HttpHeaderPropagator(new CloudTraceFormatter(), 'Trace-Context'); + $context = $propagator->extract(['Trace-Context' => $header]); $this->assertEquals($traceId, $context->traceId()); $this->assertEquals($spanId, $context->spanId()); $this->assertEquals($enabled, $context->enabled()); @@ -61,8 +61,8 @@ public function testInject($traceId, $spanId, $enabled, $header) $propagator = new HttpHeaderPropagator(); $context = new SpanContext($traceId, $spanId, $enabled); $output = $propagator->inject($context, []); - $this->assertArrayHasKey('X-CLOUD-TRACE-CONTEXT', $output); - $this->assertEquals($header, $output['X-CLOUD-TRACE-CONTEXT']); + $this->assertArrayHasKey('X-Cloud-Trace-Context', $output); + $this->assertEquals($header, $output['X-Cloud-Trace-Context']); } /** @@ -70,11 +70,11 @@ public function testInject($traceId, $spanId, $enabled, $header) */ public function testInjectCustomKey($traceId, $spanId, $enabled, $header) { - $propagator = new HttpHeaderPropagator(new CloudTraceFormatter(), 'HTTP_TRACE_CONTEXT'); + $propagator = new HttpHeaderPropagator(new CloudTraceFormatter(), 'Trace-Context'); $context = new SpanContext($traceId, $spanId, $enabled); $output = $propagator->inject($context, []); - $this->assertArrayHasKey('TRACE-CONTEXT', $output); - $this->assertEquals($header, $output['TRACE-CONTEXT']); + $this->assertArrayHasKey('Trace-Context', $output); + $this->assertEquals($header, $output['Trace-Context']); } /** diff --git a/tests/unit/Trace/RequestHandlerTest.php b/tests/unit/Trace/RequestHandlerTest.php index 9865e4d17..ed9ceab27 100644 --- a/tests/unit/Trace/RequestHandlerTest.php +++ b/tests/unit/Trace/RequestHandlerTest.php @@ -87,7 +87,7 @@ public function testCanParseParentContext() new HttpHeaderPropagator(), [ 'headers' => [ - 'HTTP_X_CLOUD_TRACE_CONTEXT' => '12345678901234567890123456789012/5555;o=1' + 'X-Cloud-Trace-Context' => '12345678901234567890123456789012/5555;o=1' ], 'skipReporting' => true ] @@ -106,7 +106,7 @@ public function testForceEnabledContextHeader() new HttpHeaderPropagator(), [ 'headers' => [ - 'HTTP_X_CLOUD_TRACE_CONTEXT' => '12345678901234567890123456789012;o=1' + 'X-Cloud-Trace-Context' => '12345678901234567890123456789012;o=1' ], 'skipReporting' => true ] @@ -124,7 +124,7 @@ public function testForceDisabledContextHeader() new HttpHeaderPropagator(), [ 'headers' => [ - 'HTTP_X_CLOUD_TRACE_CONTEXT' => '12345678901234567890123456789012;o=0' + 'X-Cloud-Trace-Context' => '12345678901234567890123456789012;o=0' ], 'skipReporting' => true ] From 820236f120d6f4196a58305ce3af743224a8af0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Moz=CC=87ejko?= Date: Wed, 6 Feb 2019 09:49:14 +0100 Subject: [PATCH 2/3] Add support for booleans in X-B3-Sampled header --- src/Trace/Propagator/B3HeadersPropagator.php | 2 +- .../Propagator/B3HeadersPropagatorTest.php | 33 ++++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/Trace/Propagator/B3HeadersPropagator.php b/src/Trace/Propagator/B3HeadersPropagator.php index 24d0202f1..0b2a6ce95 100644 --- a/src/Trace/Propagator/B3HeadersPropagator.php +++ b/src/Trace/Propagator/B3HeadersPropagator.php @@ -30,7 +30,7 @@ public function extract($container) $enabled = null; if ($sampled !== null) { - $enabled = $sampled === '1'; + $enabled = ($sampled === '1' || $sampled === 'true'); } if ($flags === '1') { diff --git a/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php b/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php index dc37fb2f1..d20467946 100644 --- a/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php +++ b/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php @@ -34,12 +34,15 @@ public function testInject($traceId, $spanId, $enabled, $headers) $context = new SpanContext($traceId, $spanId, $enabled); $output = $propagator->inject($context, []); - if (array_key_exists('X-B3-Flags', $headers)) { - $headers['X-B3-Sampled'] = $headers['X-B3-Flags']; - unset($headers['X-B3-Flags']); - } + $this->assertArrayHasKey('X-B3-TraceId', $output); + $this->assertArrayHasKey('X-B3-SpanId', $output); + $this->assertArrayHasKey('X-B3-Sampled', $output); - $this->assertEquals($headers, $output); + $sampled = $enabled ? '1' : '0'; + + $this->assertEquals($traceId, $output['X-B3-TraceId']); + $this->assertEquals($spanId, $output['X-B3-SpanId']); + $this->assertEquals($sampled, $output['X-B3-Sampled']); } public function traceMetadata() @@ -55,6 +58,16 @@ public function traceMetadata() 'X-B3-Sampled' => '1', ], ], + [ + '463ac35c9f6413ad48485a3953bb6124', + 'a2fb4a1d1a96d312', + true, + [ + 'X-B3-TraceId' => '463ac35c9f6413ad48485a3953bb6124', + 'X-B3-SpanId' => 'a2fb4a1d1a96d312', + 'X-B3-Sampled' => 'true', + ], + ], [ '463ac35c9f6413ad48485a3953bb6124', 'a2fb4a1d1a96d312', @@ -65,6 +78,16 @@ public function traceMetadata() 'X-B3-Sampled' => '0', ], ], + [ + '463ac35c9f6413ad48485a3953bb6124', + 'a2fb4a1d1a96d312', + false, + [ + 'X-B3-TraceId' => '463ac35c9f6413ad48485a3953bb6124', + 'X-B3-SpanId' => 'a2fb4a1d1a96d312', + 'X-B3-Sampled' => 'false', + ], + ], [ '463ac35c9f6413ad48485a3953bb6124', 'a2fb4a1d1a96d312', From c71d7a7ce5c878e75ba598a81657f951f334ee0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Moz=CC=87ejko?= Date: Wed, 6 Feb 2019 09:56:20 +0100 Subject: [PATCH 3/3] Set fromHeader to false when headers are missing --- src/Trace/Propagator/B3HeadersPropagator.php | 4 ++++ src/Trace/SpanContext.php | 2 +- tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php | 8 ++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/Trace/Propagator/B3HeadersPropagator.php b/src/Trace/Propagator/B3HeadersPropagator.php index 0b2a6ce95..ae4d69c22 100644 --- a/src/Trace/Propagator/B3HeadersPropagator.php +++ b/src/Trace/Propagator/B3HeadersPropagator.php @@ -27,6 +27,10 @@ public function extract($container) $sampled = $container[self::X_B3_SAMPLED] ?? null; $flags = $container[self::X_B3_FLAGS] ?? null; + if (!$traceId || !$spanId) { + return new SpanContext(); + } + $enabled = null; if ($sampled !== null) { diff --git a/src/Trace/SpanContext.php b/src/Trace/SpanContext.php index 882b723f6..7126381af 100644 --- a/src/Trace/SpanContext.php +++ b/src/Trace/SpanContext.php @@ -110,7 +110,7 @@ public function setSpanId($spanId) /** * Whether or not the request is being traced. * - * @return bool + * @return bool|null */ public function enabled() { diff --git a/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php b/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php index d20467946..a8dc6bf1e 100644 --- a/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php +++ b/tests/unit/Trace/Propagator/B3HeadersPropagatorTest.php @@ -25,6 +25,14 @@ public function testExtract($traceId, $spanId, $enabled, $headers) $this->assertTrue($context->fromHeader()); } + public function testExtractWithoutHeaders() + { + $propagator = new B3HeadersPropagator(); + $context = $propagator->extract([]); + $this->assertNull($context->enabled()); + $this->assertFalse($context->fromHeader()); + } + /** * @dataProvider traceMetadata */