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

Issue:
Use Environment constants and accessor in SpanProcessorFactory

Pull request:
Refactored SpanProcessorFactory to use environment constants and accessor
Refactored SpanProcessorFactory and updated corresponding unit tests. The default value for `OTEL_PHP_TRACES_PROCESSOR` is `batch` so removed null span processor test and replaced it with default span processor test.

Fixes #593.

Patch:
diff --git a/src/SDK/Trace/SpanProcessorFactory.php b/src/SDK/Trace/SpanProcessorFactory.php
index 4ff3998e..5622780c 100644
--- a/src/SDK/Trace/SpanProcessorFactory.php
+++ b/src/SDK/Trace/SpanProcessorFactory.php
@@ -5,18 +5,19 @@ declare(strict_types=1);
 namespace OpenTelemetry\SDK\Trace;
 
 use InvalidArgumentException;
+use OpenTelemetry\SDK\Common\Environment\EnvironmentVariablesTrait;
+use OpenTelemetry\SDK\Common\Environment\Variables as Env;
 use OpenTelemetry\SDK\Trace\SpanProcessor\BatchSpanProcessor;
 use OpenTelemetry\SDK\Trace\SpanProcessor\NoopSpanProcessor;
 use OpenTelemetry\SDK\Trace\SpanProcessor\SimpleSpanProcessor;
 
 class SpanProcessorFactory
 {
+    use EnvironmentVariablesTrait;
+
     public function fromEnvironment(?SpanExporterInterface $exporter = null): SpanProcessorInterface
     {
-        $name = getenv('OTEL_PHP_TRACES_PROCESSOR');
-        if (!$name) {
-            throw new InvalidArgumentException('OTEL_PHP_TRACES_PROCESSOR not set');
-        }
+        $name = $this->getEnumFromEnvironment(Env::OTEL_PHP_TRACES_PROCESSOR);
         switch ($name) {
             case 'batch':
                 return new BatchSpanProcessor($exporter);


Test patch:
diff --git a/tests/Unit/SDK/Trace/SpanProcessorFactoryTest.php b/tests/Unit/SDK/Trace/SpanProcessorFactoryTest.php
index 3975cb56..c9c22743 100644
--- a/tests/Unit/SDK/Trace/SpanProcessorFactoryTest.php
+++ b/tests/Unit/SDK/Trace/SpanProcessorFactoryTest.php
@@ -42,25 +42,23 @@ class SpanProcessorFactoryTest extends TestCase
             'batch' => ['batch', BatchSpanProcessor::class],
             'simple' => ['simple', SimpleSpanProcessor::class],
             'noop' => ['noop', NoopSpanProcessor::class],
+            'none' => ['none', NoopSpanProcessor::class],
         ];
     }
-    /**
-     * @dataProvider invalidProcessorProvider
-     */
-    public function test_span_processor_factory_invalid_span_processor(?string $processor): void
+
+    public function test_span_processor_factory_default_span_processor(): void
     {
-        $this->setEnvironmentVariable('OTEL_PHP_TRACES_PROCESSOR', $processor);
         $factory = new SpanProcessorFactory();
         $exporter = $this->createMock(SpanExporterInterface::class);
-        $this->expectException(InvalidArgumentException::class);
-        $factory->fromEnvironment($exporter);
+        $this->assertInstanceOf(BatchSpanProcessor::class, $factory->fromEnvironment($exporter));
     }
 
-    public function invalidProcessorProvider()
+    public function test_span_processor_factory_invalid_span_processor(): void
     {
-        return [
-            'not set' => [null],
-            'invalid processor' => ['foo'],
-        ];
+        $this->setEnvironmentVariable('OTEL_PHP_TRACES_PROCESSOR', 'foo');
+        $factory = new SpanProcessorFactory();
+        $exporter = $this->createMock(SpanExporterInterface::class);
+        $this->expectException(InvalidArgumentException::class);
+        $factory->fromEnvironment($exporter);
     }
 }