From 2e96621b0f47631eb05102dd86c15851e8ebcc8b Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Wed, 30 Nov 2022 14:03:18 +1100 Subject: [PATCH] allow 16 char b3 trace id (#869) b3 spec allows trace id to be 16 or 32 char. other SIGs left-pad with zero to resolve this. --- .../Propagator/B3/B3MultiPropagator.php | 4 + .../Propagator/B3/B3SinglePropagator.php | 4 + .../Propagator/B3/B3PropagatorTest.php | 96 ++++++++++++------- 3 files changed, 72 insertions(+), 32 deletions(-) diff --git a/src/Extension/Propagator/B3/B3MultiPropagator.php b/src/Extension/Propagator/B3/B3MultiPropagator.php index 1e3bde5ae..6b4af8ef1 100644 --- a/src/Extension/Propagator/B3/B3MultiPropagator.php +++ b/src/Extension/Propagator/B3/B3MultiPropagator.php @@ -166,6 +166,10 @@ private static function extractImpl($carrier, PropagationGetterInterface $getter return SpanContext::getInvalid(); } + // B3 trace id may be 16 or 32 hex chars, but otel requires 32 + if (strlen($traceId) === 16) { + $traceId = str_pad($traceId, 32, '0', STR_PAD_LEFT); + } // Validates the traceId and spanId // Returns an invalid spanContext if any of the checks fail if (!SpanContextValidator::isValidTraceId($traceId) || !SpanContextValidator::isValidSpanId($spanId)) { diff --git a/src/Extension/Propagator/B3/B3SinglePropagator.php b/src/Extension/Propagator/B3/B3SinglePropagator.php index 62a747017..2b36b9d91 100644 --- a/src/Extension/Propagator/B3/B3SinglePropagator.php +++ b/src/Extension/Propagator/B3/B3SinglePropagator.php @@ -155,6 +155,10 @@ private static function extractImpl($carrier, PropagationGetterInterface $getter return SpanContext::getInvalid(); } + // B3 trace id may be 16 or 32 hex chars, but otel requires 32 + if (strlen($traceId) === 16) { + $traceId = str_pad($traceId, 32, '0', STR_PAD_LEFT); + } // Validates the traceId and spanId // Returns an invalid spanContext if any of the checks fail if (!SpanContextValidator::isValidTraceId($traceId) || !SpanContextValidator::isValidSpanId($spanId)) { diff --git a/tests/Unit/Extension/Propagator/B3/B3PropagatorTest.php b/tests/Unit/Extension/Propagator/B3/B3PropagatorTest.php index ed4e4af25..e54a06971 100644 --- a/tests/Unit/Extension/Propagator/B3/B3PropagatorTest.php +++ b/tests/Unit/Extension/Propagator/B3/B3PropagatorTest.php @@ -20,11 +20,10 @@ */ class B3PropagatorTest extends TestCase { - private const B3_SINGLE_TRACE_ID_BASE16 = 'ff0000000000051791e0000000000041'; - private const B3_SINGLE_SPAN_ID_BASE16 = 'ff00051791e00041'; - private const B3_SINGLE_HEADER_SAMPLED = self::B3_SINGLE_TRACE_ID_BASE16 . '-' . self::B3_SINGLE_SPAN_ID_BASE16 . '-1'; - private const B3_MULTI_TRACE_ID_BASE16 = 'ff000000000000301710000000000041'; - private const B3_MULTI_SPAN_ID_BASE16 = 'ff00003017100041'; + private const B3_TRACE_ID_16_CHAR = 'ff00051791e00041'; + private const B3_TRACE_ID = 'ff0000000000051791e0000000000041'; + private const B3_SPAN_ID = 'ff00051791e00041'; + private const B3_SINGLE_HEADER_SAMPLED = self::B3_TRACE_ID . '-' . self::B3_SPAN_ID . '-1'; private const IS_SAMPLED = '1'; private const IS_NOT_SAMPLED = '0'; @@ -68,15 +67,15 @@ public function test_b3multi_inject(): void $carrier, null, $this->withSpanContext( - SpanContext::create(self::B3_MULTI_TRACE_ID_BASE16, self::B3_MULTI_SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_SAMPLED), + SpanContext::create(self::B3_TRACE_ID, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_SAMPLED), Context::getCurrent() ) ); $this->assertSame( [ - $this->TRACE_ID => self::B3_MULTI_TRACE_ID_BASE16, - $this->SPAN_ID => self::B3_MULTI_SPAN_ID_BASE16, + $this->TRACE_ID => self::B3_TRACE_ID, + $this->SPAN_ID => self::B3_SPAN_ID, $this->SAMPLED => self::IS_SAMPLED, ], $carrier @@ -91,7 +90,7 @@ public function test_b3single_inject(): void $carrier, null, $this->withSpanContext( - SpanContext::create(self::B3_SINGLE_TRACE_ID_BASE16, self::B3_SINGLE_SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_SAMPLED), + SpanContext::create(self::B3_TRACE_ID, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_SAMPLED), Context::getCurrent() ) ); @@ -115,7 +114,7 @@ public function test_extract_only_b3single_sampled_context_with_b3single_instanc $this->assertNull($context->get(B3DebugFlagContextKey::instance())); $this->assertEquals( - SpanContext::createFromRemoteParent(self::B3_SINGLE_TRACE_ID_BASE16, self::B3_SINGLE_SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_SAMPLED), + SpanContext::createFromRemoteParent(self::B3_TRACE_ID, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_SAMPLED), $this->getSpanContext($context) ); } @@ -133,7 +132,7 @@ public function test_extract_only_b3single_sampled_context_with_b3multi_instance $this->assertNull($context->get(B3DebugFlagContextKey::instance())); $this->assertEquals( - SpanContext::createFromRemoteParent(self::B3_SINGLE_TRACE_ID_BASE16, self::B3_SINGLE_SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_SAMPLED), + SpanContext::createFromRemoteParent(self::B3_TRACE_ID, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_SAMPLED), $this->getSpanContext($context) ); } @@ -141,8 +140,8 @@ public function test_extract_only_b3single_sampled_context_with_b3multi_instance public function test_extract_only_b3multi_sampled_context_with_b3single_instance(): void { $carrier = [ - $this->TRACE_ID => self::B3_MULTI_TRACE_ID_BASE16, - $this->SPAN_ID => self::B3_MULTI_SPAN_ID_BASE16, + $this->TRACE_ID => self::B3_TRACE_ID, + $this->SPAN_ID => self::B3_SPAN_ID, $this->SAMPLED => self::IS_SAMPLED, ]; @@ -151,16 +150,19 @@ public function test_extract_only_b3multi_sampled_context_with_b3single_instance $context = $propagator->extract($carrier); $this->assertEquals( - SpanContext::createFromRemoteParent(self::B3_MULTI_TRACE_ID_BASE16, self::B3_MULTI_SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_SAMPLED), + SpanContext::createFromRemoteParent(self::B3_TRACE_ID, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_SAMPLED), $this->getSpanContext($context) ); } - public function test_extract_only_b3multi_sampled_context_with_b3multi_instance(): void + /** + * @dataProvider validTraceIdProvider + */ + public function test_extract_only_b3multi_sampled_context_with_b3multi_instance(string $traceId, string $expected): void { $carrier = [ - $this->TRACE_ID => self::B3_MULTI_TRACE_ID_BASE16, - $this->SPAN_ID => self::B3_MULTI_SPAN_ID_BASE16, + $this->TRACE_ID => $traceId, + $this->SPAN_ID => self::B3_SPAN_ID, $this->SAMPLED => self::IS_SAMPLED, ]; @@ -169,16 +171,46 @@ public function test_extract_only_b3multi_sampled_context_with_b3multi_instance( $context = $propagator->extract($carrier); $this->assertEquals( - SpanContext::createFromRemoteParent(self::B3_MULTI_TRACE_ID_BASE16, self::B3_MULTI_SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_SAMPLED), + SpanContext::createFromRemoteParent($expected, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_SAMPLED), + $this->getSpanContext($context) + ); + } + + /** + * @dataProvider validTraceIdProvider + */ + public function test_extract_b3_single(string $traceId, string $expected): void + { + $carrier = [ + 'b3' => $traceId . '-' . self::B3_SPAN_ID, + ]; + $context = B3Propagator::getB3SingleHeaderInstance()->extract($carrier); + + $this->assertEquals( + SpanContext::createFromRemoteParent($expected, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_DEFAULT), $this->getSpanContext($context) ); } + public function validTraceIdProvider(): array + { + return [ + '16 char trace id' => [ + self::B3_TRACE_ID_16_CHAR, + str_pad(self::B3_TRACE_ID_16_CHAR, 32, '0', STR_PAD_LEFT), + ], + '32 char trace id' => [ + self::B3_TRACE_ID, + self::B3_TRACE_ID, + ], + ]; + } + public function test_extract_both_sampled_context_with_b3single_instance(): void { $carrier = [ - $this->TRACE_ID => self::B3_MULTI_TRACE_ID_BASE16, - $this->SPAN_ID => self::B3_MULTI_SPAN_ID_BASE16, + $this->TRACE_ID => self::B3_TRACE_ID, + $this->SPAN_ID => self::B3_SPAN_ID, $this->SAMPLED => self::IS_NOT_SAMPLED, $this->B3 => self::B3_SINGLE_HEADER_SAMPLED, ]; @@ -188,7 +220,7 @@ public function test_extract_both_sampled_context_with_b3single_instance(): void $context = $propagator->extract($carrier); $this->assertEquals( - SpanContext::createFromRemoteParent(self::B3_SINGLE_TRACE_ID_BASE16, self::B3_SINGLE_SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_SAMPLED), + SpanContext::createFromRemoteParent(self::B3_TRACE_ID, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_SAMPLED), $this->getSpanContext($context) ); } @@ -196,8 +228,8 @@ public function test_extract_both_sampled_context_with_b3single_instance(): void public function test_extract_both_sampled_context_with_b3multi_instance(): void { $carrier = [ - $this->TRACE_ID => self::B3_MULTI_TRACE_ID_BASE16, - $this->SPAN_ID => self::B3_MULTI_SPAN_ID_BASE16, + $this->TRACE_ID => self::B3_TRACE_ID, + $this->SPAN_ID => self::B3_SPAN_ID, $this->SAMPLED => self::IS_NOT_SAMPLED, $this->B3 => self::B3_SINGLE_HEADER_SAMPLED, ]; @@ -207,7 +239,7 @@ public function test_extract_both_sampled_context_with_b3multi_instance(): void $context = $propagator->extract($carrier); $this->assertEquals( - SpanContext::createFromRemoteParent(self::B3_SINGLE_TRACE_ID_BASE16, self::B3_SINGLE_SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_SAMPLED), + SpanContext::createFromRemoteParent(self::B3_TRACE_ID, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_SAMPLED), $this->getSpanContext($context) ); } @@ -218,8 +250,8 @@ public function test_extract_both_sampled_context_with_b3multi_instance(): void public function test_extract_b3_single_invalid_and_b3_multi_valid_context_with_b3single_instance($headerValue): void { $carrier = [ - $this->TRACE_ID => self::B3_MULTI_TRACE_ID_BASE16, - $this->SPAN_ID => self::B3_MULTI_SPAN_ID_BASE16, + $this->TRACE_ID => self::B3_TRACE_ID, + $this->SPAN_ID => self::B3_SPAN_ID, $this->SAMPLED => self::IS_NOT_SAMPLED, $this->B3 => $headerValue, ]; @@ -229,7 +261,7 @@ public function test_extract_b3_single_invalid_and_b3_multi_valid_context_with_b $context = $propagator->extract($carrier); $this->assertEquals( - SpanContext::createFromRemoteParent(self::B3_MULTI_TRACE_ID_BASE16, self::B3_MULTI_SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_DEFAULT), + SpanContext::createFromRemoteParent(self::B3_TRACE_ID, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_DEFAULT), $this->getSpanContext($context) ); } @@ -240,8 +272,8 @@ public function test_extract_b3_single_invalid_and_b3_multi_valid_context_with_b public function test_extract_b3_single_invalid_and_b3_multi_valid_context_with_b3multi_instance($headerValue): void { $carrier = [ - $this->TRACE_ID => self::B3_MULTI_TRACE_ID_BASE16, - $this->SPAN_ID => self::B3_MULTI_SPAN_ID_BASE16, + $this->TRACE_ID => self::B3_TRACE_ID, + $this->SPAN_ID => self::B3_SPAN_ID, $this->SAMPLED => self::IS_NOT_SAMPLED, $this->B3 => $headerValue, ]; @@ -251,7 +283,7 @@ public function test_extract_b3_single_invalid_and_b3_multi_valid_context_with_b $context = $propagator->extract($carrier); $this->assertEquals( - SpanContext::createFromRemoteParent(self::B3_MULTI_TRACE_ID_BASE16, self::B3_MULTI_SPAN_ID_BASE16, SpanContextInterface::TRACE_FLAG_DEFAULT), + SpanContext::createFromRemoteParent(self::B3_TRACE_ID, self::B3_SPAN_ID, SpanContextInterface::TRACE_FLAG_DEFAULT), $this->getSpanContext($context) ); } @@ -259,8 +291,8 @@ public function test_extract_b3_single_invalid_and_b3_multi_valid_context_with_b public function invalidB3SingleHeaderValueProvider(): array { return [ - 'invalid traceid' => ['abcdefghijklmnopabcdefghijklmnop-' . self::B3_SINGLE_SPAN_ID_BASE16 . '-1'], - 'invalid spanid' => [self::B3_SINGLE_TRACE_ID_BASE16 . '-abcdefghijklmnop-1'], + 'invalid traceid' => ['abcdefghijklmnopabcdefghijklmnop-' . self::B3_SPAN_ID . '-1'], + 'invalid spanid' => [self::B3_TRACE_ID . '-abcdefghijklmnop-1'], ]; }