Only sampled spans should be exported (#145)

* Only sampled spans should be exported
https://github.com/open-telemetry/opentelemetry-php/issues/140

* BatchSpanProcessor should flush by time limit even if Span is not sampled

api/Trace/Span.php

@@ -65,5 +65,7 @@ interface Span extends SpanStatus, SpanKind
 
     public function isRecording(): bool;
 
+    public function isSampled(): bool;
+
     // TODO: addLazyEvent
 }

api/Trace/SpanContext.php

@@ -12,6 +12,7 @@ interface SpanContext
     public function getSpanId(): string;
     public function getTraceFlags(): int;
     public function getTracestate(): array;
-    public function IsValidContext(): bool;
-    public function IsRemoteContext(): bool;
+    public function isValidContext(): bool;
+    public function isRemoteContext(): bool;
+    public function isSampled(): bool;
 }

sdk/Trace/Span.php

@@ -190,6 +190,11 @@ class Span implements API\Span
         return false;
     }
 
+    public function isSampled(): bool
+    {
+        return $this->spanContext->isSampled();
+    }
+
     public function setLinks(API\Links $links): Span
     {
         $this->links = $links;

sdk/Trace/SpanProcessor/BatchSpanProcessor.php

@@ -85,14 +85,20 @@ class BatchSpanProcessor implements SpanProcessor
      */
     public function onEnd(API\Span $span): void
     {
-        if (null !== $this->exporter && $this->running) {
-            if (count($this->queue) < $this->maxQueueSize) {
-                $this->queue[] = $span;
-            }
+        if (null === $this->exporter) {
+            return;
+        }
 
-            if ($this->bufferReachedExportLimit() || $this->enoughTimeHasPassed()) {
-                $this->forceFlush();
-            }
+        if (!$this->running) {
+            return;
+        }
+
+        if ($span->isSampled() && !$this->queueReachedLimit()) {
+            $this->queue[] = $span;
+        }
+
+        if ($this->bufferReachedExportLimit() || $this->enoughTimeHasPassed()) {
+            $this->forceFlush();
         }
     }
 
@@ -113,6 +119,11 @@ class BatchSpanProcessor implements SpanProcessor
         return count($this->queue) >= $this->maxExportBatchSize;
     }
 
+    protected function queueReachedLimit(): bool
+    {
+        return count($this->queue) >= $this->maxQueueSize;
+    }
+
     protected function enoughTimeHasPassed(): bool
     {
         $now = $this->clock->timestamp();

sdk/Trace/SpanProcessor/SimpleSpanProcessor.php

@@ -37,12 +37,16 @@ class SimpleSpanProcessor implements SpanProcessor
      */
     public function onEnd(API\Span $span): void
     {
-        if ($this->running) {
-            // @todo only spans with SampleFlag === true should be exported according to
-            // https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-tracing.md#sampling
-            if (null !== $this->exporter) {
-                $this->exporter->export([$span]);
-            }
+        if (!$this->running) {
+            return;
+        }
+
+        if (!$span->isSampled()) {
+            return;
+        }
+
+        if (null !== $this->exporter) {
+            $this->exporter->export([$span]);
         }
     }
 

tests/Sdk/Unit/Trace/SpanProcessor/BatchSpanProcessorTest.php

@@ -24,7 +24,7 @@ class BatchSpanProcessorTest extends TestCase
         $timeout = 3000;
 
         for ($i = 0; $i < $batchSize; $i++) {
-            $spans[] = self::createMock(Span::class);
+            $spans[] = $this->createSampledSpanMock();
         }
 
         $exporter = self::createMock(Exporter::class);
@@ -56,7 +56,7 @@ class BatchSpanProcessorTest extends TestCase
         $timeout = 3000;
 
         for ($i = 0; $i < $batchSize - 1; $i++) {
-            $spans[] = self::createMock(Span::class);
+            $spans[] = $this->createSampledSpanMock();
         }
 
         $exporter = self::createMock(Exporter::class);
@@ -101,7 +101,7 @@ class BatchSpanProcessorTest extends TestCase
         $processor = new BatchSpanProcessor($exporter, $clock, $queueSize, $exportDelay, $timeout, $batchSize);
 
         for ($i = 0; $i < $batchSize - 1; $i++) {
-            $mock_span = self::createMock(Span::class);
+            $mock_span = $this->createSampledSpanMock();
             /** @var \OpenTelemetry\Sdk\Trace\Span $mock_span */
             $processor->onEnd($mock_span);
         }
@@ -113,7 +113,7 @@ class BatchSpanProcessorTest extends TestCase
     public function shouldAllowNullExporter()
     {
         $proc = new BatchSpanProcessor(null, self::createMock(Clock::class));
-        $span = self::createMock(Span::class);
+        $span = $this->createSampledSpanMock();
         $proc->onStart($span);
         $proc->onEnd($span);
         $proc->forceFlush();
@@ -141,7 +141,7 @@ class BatchSpanProcessorTest extends TestCase
 
         $spans = [];
         for ($i = 0; $i < $batchSize - 1; $i++) {
-            $span = self::createMock(Span::class);
+            $span = $this->createSampledSpanMock();
             $spans[] = $span;
             /** @var \OpenTelemetry\Sdk\Trace\Span $mock_span */
             $processor->onEnd($span);
@@ -179,11 +179,40 @@ class BatchSpanProcessorTest extends TestCase
         // calling SpanProcessor's shutdown() calls Exporter's shutdown()
         $this->assertEquals(1, $spy->getInvocationCount());
 
-        $span = self::createMock(Span::class);
+        $span = $this->createSampledSpanMock();
         $proc->onStart($span);
         $proc->onEnd($span);
 
         // calling onEnd here does NOT result in another call to shutdown
         $this->assertEquals(1, $spy->getInvocationCount());
     }
+
+    /**
+     * @test
+     */
+    public function exportsOnlySampledSpans()
+    {
+        $sampledSpan = $this->createSampledSpanMock();
+        $nonSampledSpan = $this->createNonSampledSpanMock();
+
+        $exporter = self::createMock(Exporter::class);
+        $exporter->expects($this->exactly(1))->method('export')->with([$sampledSpan]);
+
+        $batchProcessor = new BatchSpanProcessor($exporter, self::createMock(Clock::class));
+        foreach ([$sampledSpan, $nonSampledSpan] as $span) {
+            $batchProcessor->onEnd($span);
+        }
+
+        $batchProcessor->forceFlush();
+    }
+
+    private function createSampledSpanMock()
+    {
+        return self::createConfiguredMock(Span::class, ['isSampled' => true]);
+    }
+
+    private function createNonSampledSpanMock()
+    {
+        return self::createConfiguredMock(Span::class, ['isSampled' => false]);
+    }
 }

tests/Sdk/Unit/Trace/SpanProcessor/SimpleSpanProcessorTest.php

@@ -6,6 +6,7 @@ namespace OpenTelemetry\Tests\Sdk\Unit\Trace\SpanProcessor;
 
 use OpenTelemetry\Sdk\Trace\Exporter;
 use OpenTelemetry\Sdk\Trace\Span;
+use OpenTelemetry\Sdk\Trace\SpanContext;
 use OpenTelemetry\Sdk\Trace\SpanProcessor\SimpleSpanProcessor;
 use PHPUnit\Framework\TestCase;
 
@@ -20,7 +21,7 @@ class SimpleSpanProcessorTest extends TestCase
         $exporter->expects($this->atLeastOnce())->method('export');
 
         (new SimpleSpanProcessor($exporter))->onEnd(
-            self::createMock(Span::class)
+            new Span('sampled_span', new SpanContext('40de9aea7305cced3bb10ed45ba6872d', '277c169397adf2ec', 1))
         );
     }
 
@@ -73,4 +74,17 @@ class SimpleSpanProcessorTest extends TestCase
         // calling onEnd here does NOT result in another call to shutdown
         $this->assertEquals(1, $spy->getInvocationCount());
     }
+
+    /**
+     * @test
+     */
+    public function shouldExportOnlySampledSpans()
+    {
+        $exporter = self::createMock(Exporter::class);
+        $exporter->expects($this->never())->method('export');
+
+        (new SimpleSpanProcessor($exporter))->onEnd(
+            new Span('sampled_span', new SpanContext('40de9aea7305cced3bb10ed45ba6870d', '277c169397adf2ec', 0))
+        );
+    }
 }