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

Issue:
OTLP/GRPC exporter implementation - `Cannot convert '' to string` exception on empty `parent_span_id` with ext-protobuf
**Environment** 
docker image: `php:7.4.20-fpm-alpine`
related php-ext enabled: `grpc-1.38.0`, `protobuf-3.17.3`
library: `open-telemetry/opentelemetry: 0.0.2`,
jaeger metrics collector: `jaegertracing/opentelemetry-all-in-one`

**Steps to reproduce**
```php

use OpenTelemetry\Contrib\OtlpGrpc\Exporter;
use OpenTelemetry\Sdk\Resource\ResourceInfo;
use OpenTelemetry\Sdk\Trace\Clock;
use OpenTelemetry\Sdk\Trace\Sampler\AlwaysOnSampler;
use OpenTelemetry\Sdk\Trace\SpanProcessor\BatchSpanProcessor;
use OpenTelemetry\Sdk\Trace\TracerProvider;
use OpenTelemetry\Trace\SpanKind;
use OpenTelemetry\Trace\SpanStatus;

$exporter = new Exporter('home.docker.internal:55680'); // point to jaegertracing/opentelemetry-all-in-one

$tracer =  (new TracerProvider(
    ResourceInfo::defaultResource(),
    new AlwaysOnSampler())
)->addSpanProcessor(new BatchSpanProcessor($exporter, Clock::get()))->getTracer('tracername', 'doesntmatter');

$span = $tracer->startAndActivateSpan($request->getMethod(), SpanKind::KIND_SERVER);

if ($span->isRecording()) {
    $span->setAttribute('attribute1', 'value1');
}

$span->setSpanStatus(SpanStatus::OK);
$tracer->endActiveSpan();

// To easily catch exception make `\OpenTelemetry\Sdk\Trace\Tracer::$provider` public
$tracer->provider->shutdown();
```

**What is the expected behavior?**
Tracing information appears in jaeger

**What is the actual behavior?**
Exception: **Cannot convert '' to string**
```
Google\Protobuf\Internal\Message::__construct
vendor/open-telemetry/opentelemetry/proto/Opentelemetry/Proto/Trace/V1/Span.php:244
```

**Additional context**
This exception does not appear in next cases (independently of each other):

- Using `google/protobuf` instead of `ext-protobuf`
- Using non-empty `parentId` in context as well as using invalid `parentId` as parent one 
```php
- 'parent_span_id' => $parent_span_id ? hex2bin($parent_span_id) : null,
+ 'parent_span_id' => $parent_span_id ? hex2bin($parent_span_id) : hex2bin(\OpenTelemetry\Sdk\Trace\SpanContext::INVALID_SPAN),
```
during row initialisation in `vendor/open-telemetry/opentelemetry/contrib/OtlpGrpc/SpanConverter.php:91`.

**NB!** This issue can be related to the protobuf extension, not opentelemetry package 


Pull request:
fixing otlp span converter with ext-protobuf
There looks to be a bug in ext-protobuf (but not the protobuf php library), where an array with any empty string top-level values cause an error. A workaround for this is to strip out entries with empty values.
Adding ext-protobuf to the test matrix, and in passing fixing a mis-named test class.
Closes #357 

Patch:
diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml
index f544728a..7b669b53 100644
--- a/.github/workflows/php.yml
+++ b/.github/workflows/php.yml
@@ -17,9 +17,15 @@ jobs:
       matrix:
         php-version: ['7.4', '8.0']
         os: [ubuntu-latest]
+        extensions: ['ast, grpc', 'ast, grpc, protobuf']
         experimental: [false]
         include:
           - php-version: 8.1
+            extensions: 'ast, grpc'
+            os: ubuntu-latest
+            experimental: true
+          - php-version: 8.1
+            extensions: 'ast, grpc, protobuf'
             os: ubuntu-latest
             experimental: true
 
@@ -32,7 +38,7 @@ jobs:
         php-version: ${{ matrix.php-version }}
         coverage: xdebug
         tools: php-cs-fixer
-        extensions: ast, grpc
+        extensions: ${{ matrix.extensions }}
 
     - name: Validate composer.json and composer.lock
       run: composer validate
diff --git a/README.md b/README.md
index e783a39a..53b90b01 100644
--- a/README.md
+++ b/README.md
@@ -84,6 +84,12 @@ be enabled by setting the `OTEL_PHP_FIBERS_ENABLED` environment variable to a tr
 
 Using fibers with non-`CLI` SAPIs may require pre-loading of bindings. One way to achieve this is setting [`ffi.preload`](https://www.php.net/manual/en/ffi.configuration.php#ini.ffi.preload) to `src/Context/fiber/zend_observer_fiber.h` and setting [`opcache.preload`](https://www.php.net/manual/en/opcache.preloading.php) to `vendor/autoload.php`.
 
+#### 5.) Install PHP [ext-protobuf](https://pecl.php.net/package/protobuf)
+
+** The PHP protobuf extension is optional when using either the `OTLPHttp` or `OTLPGrpc` exporters.**
+
+The protobuf extension makes both exporters more performant. _Note: there are some deprecation warnings with protobuf and PHP 8.1_
+
 ---
 
 ## Installation
diff --git a/composer.json b/composer.json
index 5ba27ed8..e5133a45 100644
--- a/composer.json
+++ b/composer.json
@@ -90,6 +90,7 @@
     },
     "suggest": {
         "ext-grpc": "To use the OTLP GRPC Exporter",
+        "ext-protobuf": "For more performant protobuf/grpc exporting",
         "ext-sockets": "To use the Thrift UDP Exporter for the Jaeger Agent"
     }
 }
diff --git a/src/Contrib/Otlp/SpanConverter.php b/src/Contrib/Otlp/SpanConverter.php
index 9a5f70f5..8cb87398 100644
--- a/src/Contrib/Otlp/SpanConverter.php
+++ b/src/Contrib/Otlp/SpanConverter.php
@@ -165,7 +165,7 @@ class SpanConverter implements SpanConverterInterface
 
         $row['status'] = $status;
 
-        return new CollectorSpan($row);
+        return new CollectorSpan(array_filter($row));
     }
 
     // @return KeyValue[]


Test patch:
diff --git a/tests/Unit/SDK/AttributeLimitsTest.php b/tests/Unit/SDK/AttributeLimitsTest.php
index 726269eb..af88f6c2 100644
--- a/tests/Unit/SDK/AttributeLimitsTest.php
+++ b/tests/Unit/SDK/AttributeLimitsTest.php
@@ -10,7 +10,7 @@ use PHPUnit\Framework\TestCase;
 /**
  * @covers OpenTelemetry\SDK\AttributeLimits
  */
-class AttributesLimitsTest extends TestCase
+class AttributeLimitsTest extends TestCase
 {
     public function test_default_limits(): void
     {