Repository: open-telemetry/opentelemetry-php (php)

Issue:
Support for 16 character trace IDs
**Is your feature request related to a problem?**

We are using Traefik in front of a PHP application and we want to connect the traces of both. Currently they are not connected, because Traefik sends a 16 char trace ID and opentelemetry-php [expects 32 chars](https://github.com/open-telemetry/opentelemetry-php/blob/0.0.16/src/API/Trace/SpanContextValidator.php#L17).

**Describe the solution you'd like**

According to the B3 specs and to the Open Telemetry specs the trace ID is either 16 or 32 chars long. Therefore opentracing-php should allow both tracing lengths.

> The first position of the b3 value is the 32 or 16 lower-hex character TraceId. Unless propagating only the Sampling State, the TraceId is required. — https://github.com/openzipkin/b3-propagation#traceid-2

> TraceId is the identifier for a trace. It is worldwide unique with practically sufficient probability by being made as 16 randomly generated bytes. — https://opentelemetry.io/docs/reference/specification/overview/#spancontext


**Describe alternatives you've considered**

~We tried making Traefik send 32 char trace IDs, but did not find any config option for this.~

Edit: Traefik offers a [gen128bit](https://doc.traefik.io/traefik/observability/tracing/jaeger/#gen128bit) option that somehow results in 32 chars. While this works for us, requiring 32 chars is still not what the specs say.

**Additional context**

None


Pull request:
allow 16 char b3 trace id
b3 spec allows trace id to be 16 or 32 char. other SIGs left-pad with zero to resolve this.

Closes #863 

Patch:
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 3877862b..e0d2d530 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -2,27 +2,19 @@
 [@open-telemetry/php-maintainers](https://github.com/orgs/open-telemetry/teams/php-maintainers)
 
 - [Bob Strecansky](https://github.com/bobstrecansky)
-- [Timo Michna](https://github.com/tidal/)
 - [Brett McBride](https://github.com/brettmc/)
+- [Przemek Delweski](https://github.com/pdelewski)
 
 Find more about the maintainer role in [community repository](https://github.com/open-telemetry/community/blob/master/community-membership.md#maintainer)
 
-
 ## Approvers
 [@open-telemetry/php-approvers](https://github.com/orgs/open-telemetry/teams/php-approvers)
 
-- [Levi Morrison](https://github.com/morrisonlevi)
-- [Beniamin Calota](https://github.com/beniamin)
-- [Fahmy Mohammed](https://github.com/Fahmy-Mohammed)
-- [Amber Zsistla](https://github.com/zsistla)
-
 Find more information about the approver role in the [community repository](https://github.com/open-telemetry/community/blob/master/community-membership.md#approver)
 
 ## Triagers
 [@open-telemetry/php-triagers](https://github.com/orgs/open-telemetry/teams/php-triagers)
 
-- [Jodee Varney](https://github.com/jodeev)
-
 Find more information about the triager role in the [community repository](https://github.com/open-telemetry/community/blob/master/community-membership.md#triager)
 
 ## Members
@@ -31,6 +23,17 @@ Find more information about the triager role in the [community repository](https
 
 Find more information about the member role in the [community repository](https://github.com/open-telemetry/community/blob/master/community-membership.md#member)
 
+## Emeritus maintainers/approvers/triagers
+
+- [Timo Michna](https://github.com/tidal/)
+- [Beniamin Calota](https://github.com/beniamin)
+- [Fahmy Mohammed](https://github.com/Fahmy-Mohammed)
+- [Levi Morrison](https://github.com/morrisonlevi)
+- [Amber Zsistla](https://github.com/zsistla)
+- [Jodee Varney](https://github.com/jodeev)
+
+Find more about emeritus roles in the [community repository](https://github.com/open-telemetry/community/blob/main/community-membership.md#emeritus-maintainerapprovertriager)
+
 ## Communication
 
 Most of our communication is done on CNCF Slack in the channel [otel-php](https://cloud-native.slack.com/archives/C01NFPCV44V).
diff --git a/src/Extension/Propagator/B3/B3MultiPropagator.php b/src/Extension/Propagator/B3/B3MultiPropagator.php
index 1e3bde5a..6b4af8ef 100644
--- a/src/Extension/Propagator/B3/B3MultiPropagator.php
+++ b/src/Extension/Propagator/B3/B3MultiPropagator.php
@@ -166,6 +166,10 @@ final class B3MultiPropagator implements TextMapPropagatorInterface
             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 62a74701..2b36b9d9 100644
--- a/src/Extension/Propagator/B3/B3SinglePropagator.php
+++ b/src/Extension/Propagator/B3/B3SinglePropagator.php
@@ -155,6 +155,10 @@ final class B3SinglePropagator implements TextMapPropagatorInterface
             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)) {


Test patch:
diff --git a/tests/Unit/Extension/Propagator/B3/B3PropagatorTest.php b/tests/Unit/Extension/Propagator/B3/B3PropagatorTest.php
index ed4e4af2..e54a0697 100644
--- a/tests/Unit/Extension/Propagator/B3/B3PropagatorTest.php
+++ b/tests/Unit/Extension/Propagator/B3/B3PropagatorTest.php
@@ -20,11 +20,10 @@ use PHPUnit\Framework\TestCase;
  */
 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 @@ class B3PropagatorTest extends TestCase
             $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 @@ class B3PropagatorTest extends TestCase
             $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 @@ class B3PropagatorTest extends TestCase
         $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 @@ class B3PropagatorTest extends TestCase
         $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 @@ class B3PropagatorTest extends TestCase
     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 @@ class B3PropagatorTest extends TestCase
         $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 @@ class B3PropagatorTest extends TestCase
         $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 @@ class B3PropagatorTest extends TestCase
         $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 @@ class B3PropagatorTest extends TestCase
     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 @@ class B3PropagatorTest extends TestCase
         $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 @@ class B3PropagatorTest extends TestCase
     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 @@ class B3PropagatorTest extends TestCase
         $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 @@ class B3PropagatorTest extends TestCase
     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 @@ class B3PropagatorTest extends TestCase
         $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 @@ class B3PropagatorTest extends TestCase
     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'],
         ];
     }