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

Issue:
MetricReader.ForceFlush is not part of the spec
From https://github.com/open-telemetry/community/issues/1536#issuecomment-1621988512

> MetricReader.ForceFlush is not part of the spec.
* https://github.com/open-telemetry/opentelemetry-php/blob/cc92a07eac5294055b0d477fb7e47d6480381505/src/SDK/Metrics/MetricReaderInterface.php#L13.
* https://github.com/open-telemetry/opentelemetry-specification/pull/3563

update: issue has been replaced by https://github.com/open-telemetry/opentelemetry-specification/pull/3609

Pull request:
adding PushMetricExporterInterface
https://github.com/open-telemetry/opentelemetry-specification/pull/3563 clarifies the behaviour of forceFlush with push/non-push metric exporters.
Break forceFlush out into a PushMetricExporterInterface, and update ExportingReader to only collect/flush if exporter is a push metric exporter.

Fixes #1086

Patch:
diff --git a/examples/metrics/getting_started.php b/examples/metrics/getting_started.php
index 1d534d58..6ba39ec0 100644
--- a/examples/metrics/getting_started.php
+++ b/examples/metrics/getting_started.php
@@ -5,7 +5,7 @@ declare(strict_types=1);
 use OpenTelemetry\API\Metrics\ObserverInterface;
 use OpenTelemetry\SDK\Metrics\Data\Temporality;
 use OpenTelemetry\SDK\Metrics\MeterProvider;
-use OpenTelemetry\SDK\Metrics\MetricExporter\ConsoleMetricsExporter;
+use OpenTelemetry\SDK\Metrics\MetricExporter\ConsoleMetricExporter;
 use OpenTelemetry\SDK\Metrics\MetricReader\ExportingReader;
 use OpenTelemetry\SDK\Resource\ResourceInfoFactory;
 
@@ -18,7 +18,7 @@ require 'vendor/autoload.php';
  */
 
 $reader = new ExportingReader(
-    new ConsoleMetricsExporter(Temporality::DELTA)
+    new ConsoleMetricExporter(Temporality::DELTA)
 );
 
 $meterProvider = MeterProvider::builder()
diff --git a/examples/metrics/weak-reference-observables.php b/examples/metrics/weak-reference-observables.php
index b44da012..f53428f7 100644
--- a/examples/metrics/weak-reference-observables.php
+++ b/examples/metrics/weak-reference-observables.php
@@ -5,7 +5,7 @@ declare(strict_types=1);
 use OpenTelemetry\API\Metrics\ObserverInterface;
 use OpenTelemetry\SDK\Metrics\Data\Temporality;
 use OpenTelemetry\SDK\Metrics\MeterProvider;
-use OpenTelemetry\SDK\Metrics\MetricExporter\ConsoleMetricsExporter;
+use OpenTelemetry\SDK\Metrics\MetricExporter\ConsoleMetricExporter;
 use OpenTelemetry\SDK\Metrics\MetricReader\ExportingReader;
 use OpenTelemetry\SDK\Resource\ResourceInfoFactory;
 
@@ -19,7 +19,7 @@ require 'vendor/autoload.php';
  */
 
 $reader = new ExportingReader(
-    new ConsoleMetricsExporter(Temporality::DELTA)
+    new ConsoleMetricExporter(Temporality::DELTA)
 );
 
 $meterProvider = MeterProvider::builder()
diff --git a/src/Contrib/Otlp/MetricExporter.php b/src/Contrib/Otlp/MetricExporter.php
index d9f26fd5..d2ca9c27 100644
--- a/src/Contrib/Otlp/MetricExporter.php
+++ b/src/Contrib/Otlp/MetricExporter.php
@@ -8,8 +8,8 @@ use OpenTelemetry\API\Behavior\LogsMessagesTrait;
 use Opentelemetry\Proto\Collector\Metrics\V1\ExportMetricsServiceResponse;
 use OpenTelemetry\SDK\Common\Export\TransportInterface;
 use OpenTelemetry\SDK\Metrics\Data\Temporality;
-use OpenTelemetry\SDK\Metrics\MetricExporterInterface;
 use OpenTelemetry\SDK\Metrics\MetricMetadataInterface;
+use OpenTelemetry\SDK\Metrics\PushMetricExporterInterface;
 use RuntimeException;
 use Throwable;
 
@@ -18,7 +18,7 @@ use Throwable;
  * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/experimental/serialization/json.md#json-file-serialization
  * @psalm-import-type SUPPORTED_CONTENT_TYPES from ProtobufSerializer
  */
-final class MetricExporter implements MetricExporterInterface
+final class MetricExporter implements PushMetricExporterInterface
 {
     use LogsMessagesTrait;
 
diff --git a/src/SDK/Metrics/MetricExporter/ConsoleMetricsExporter.php b/src/SDK/Metrics/MetricExporter/ConsoleMetricExporter.php
similarity index 95%
rename from src/SDK/Metrics/MetricExporter/ConsoleMetricsExporter.php
rename to src/SDK/Metrics/MetricExporter/ConsoleMetricExporter.php
index 6afdd4f2..b9330a11 100644
--- a/src/SDK/Metrics/MetricExporter/ConsoleMetricsExporter.php
+++ b/src/SDK/Metrics/MetricExporter/ConsoleMetricExporter.php
@@ -7,15 +7,15 @@ namespace OpenTelemetry\SDK\Metrics\MetricExporter;
 use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface;
 use OpenTelemetry\SDK\Metrics\Data\Metric;
 use OpenTelemetry\SDK\Metrics\Data\Temporality;
-use OpenTelemetry\SDK\Metrics\MetricExporterInterface;
 use OpenTelemetry\SDK\Metrics\MetricMetadataInterface;
+use OpenTelemetry\SDK\Metrics\PushMetricExporterInterface;
 use OpenTelemetry\SDK\Resource\ResourceInfo;
 
 /**
  * Console metrics exporter.
  * Note that the output is human-readable JSON, not compatible with OTLP.
  */
-class ConsoleMetricsExporter implements MetricExporterInterface
+class ConsoleMetricExporter implements PushMetricExporterInterface
 {
     /**
      * @var string|Temporality|null
diff --git a/src/SDK/Metrics/MetricExporter/ConsoleMetricExporterFactory.php b/src/SDK/Metrics/MetricExporter/ConsoleMetricExporterFactory.php
index b83a7757..19088738 100644
--- a/src/SDK/Metrics/MetricExporter/ConsoleMetricExporterFactory.php
+++ b/src/SDK/Metrics/MetricExporter/ConsoleMetricExporterFactory.php
@@ -11,6 +11,6 @@ class ConsoleMetricExporterFactory implements MetricExporterFactoryInterface
 {
     public function create(): MetricExporterInterface
     {
-        return new ConsoleMetricsExporter();
+        return new ConsoleMetricExporter();
     }
 }
diff --git a/src/SDK/Metrics/MetricExporter/InMemoryExporter.php b/src/SDK/Metrics/MetricExporter/InMemoryExporter.php
index cd81ba39..208e956f 100644
--- a/src/SDK/Metrics/MetricExporter/InMemoryExporter.php
+++ b/src/SDK/Metrics/MetricExporter/InMemoryExporter.php
@@ -74,9 +74,4 @@ final class InMemoryExporter implements MetricExporterInterface
 
         return true;
     }
-
-    public function forceFlush(): bool
-    {
-        return !$this->closed;
-    }
 }
diff --git a/src/SDK/Metrics/MetricExporter/NoopMetricExporter.php b/src/SDK/Metrics/MetricExporter/NoopMetricExporter.php
index d1a1594a..4d3c070a 100644
--- a/src/SDK/Metrics/MetricExporter/NoopMetricExporter.php
+++ b/src/SDK/Metrics/MetricExporter/NoopMetricExporter.php
@@ -30,9 +30,4 @@ class NoopMetricExporter implements MetricExporterInterface
     {
         return true;
     }
-
-    public function forceFlush(): bool
-    {
-        return true;
-    }
 }
diff --git a/src/SDK/Metrics/MetricExporterInterface.php b/src/SDK/Metrics/MetricExporterInterface.php
index 6b4618b5..f5964a9f 100644
--- a/src/SDK/Metrics/MetricExporterInterface.php
+++ b/src/SDK/Metrics/MetricExporterInterface.php
@@ -26,6 +26,4 @@ interface MetricExporterInterface
     public function export(iterable $batch): bool;
 
     public function shutdown(): bool;
-
-    public function forceFlush(): bool;
 }
diff --git a/src/SDK/Metrics/MetricReader/ExportingReader.php b/src/SDK/Metrics/MetricReader/ExportingReader.php
index ad1f860a..561cfd40 100644
--- a/src/SDK/Metrics/MetricReader/ExportingReader.php
+++ b/src/SDK/Metrics/MetricReader/ExportingReader.php
@@ -16,6 +16,7 @@ use OpenTelemetry\SDK\Metrics\MetricRegistry\MetricCollectorInterface;
 use OpenTelemetry\SDK\Metrics\MetricSourceInterface;
 use OpenTelemetry\SDK\Metrics\MetricSourceProviderInterface;
 use OpenTelemetry\SDK\Metrics\MetricSourceRegistryInterface;
+use OpenTelemetry\SDK\Metrics\PushMetricExporterInterface;
 use OpenTelemetry\SDK\Metrics\StalenessHandlerInterface;
 use function spl_object_id;
 
@@ -139,10 +140,13 @@ final class ExportingReader implements MetricReaderInterface, MetricSourceRegist
         if ($this->closed) {
             return false;
         }
+        if ($this->exporter instanceof PushMetricExporterInterface) {
+            $collect = $this->doCollect();
+            $forceFlush = $this->exporter->forceFlush();
 
-        $collect = $this->doCollect();
-        $forceFlush = $this->exporter->forceFlush();
+            return $collect && $forceFlush;
+        }
 
-        return $collect && $forceFlush;
+        return true;
     }
 }
diff --git a/src/SDK/Metrics/PushMetricExporterInterface.php b/src/SDK/Metrics/PushMetricExporterInterface.php
new file mode 100644
index 00000000..d24b0e39
--- /dev/null
+++ b/src/SDK/Metrics/PushMetricExporterInterface.php
@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\SDK\Metrics;
+
+use OpenTelemetry\SDK\Metrics;
+
+interface PushMetricExporterInterface extends Metrics\MetricExporterInterface
+{
+    public function forceFlush(): bool;
+}


Test patch:
diff --git a/tests/Unit/SDK/Metrics/MetricReader/ExportingReaderTest.php b/tests/Unit/SDK/Metrics/MetricReader/ExportingReaderTest.php
index 6bbc91c6..061e45e6 100644
--- a/tests/Unit/SDK/Metrics/MetricReader/ExportingReaderTest.php
+++ b/tests/Unit/SDK/Metrics/MetricReader/ExportingReaderTest.php
@@ -19,6 +19,7 @@ use OpenTelemetry\SDK\Metrics\MetricMetadataInterface;
 use OpenTelemetry\SDK\Metrics\MetricReader\ExportingReader;
 use OpenTelemetry\SDK\Metrics\MetricSourceInterface;
 use OpenTelemetry\SDK\Metrics\MetricSourceProviderInterface;
+use OpenTelemetry\SDK\Metrics\PushMetricExporterInterface;
 use OpenTelemetry\SDK\Metrics\StalenessHandler\ImmediateStalenessHandler;
 use OpenTelemetry\SDK\Metrics\StalenessHandlerInterface;
 use OpenTelemetry\SDK\Resource\ResourceInfo;
@@ -186,20 +187,28 @@ final class ExportingReaderTest extends TestCase
         $this->assertTrue($reader->shutdown());
     }
 
-    public function test_force_flush_calls_exporter_force_flush(): void
+    public function test_force_flush_calls_push_exporter_force_flush(): void
     {
-        $exporter = $this->createMock(MetricExporterInterface::class);
+        $exporter = $this->createMock(PushMetricExporterInterface::class);
         $exporter->expects($this->once())->method('forceFlush')->willReturn(true);
         $reader = new ExportingReader($exporter);
 
         $this->assertTrue($reader->forceFlush());
     }
 
-    public function test_closed_reader_does_not_call_exporter_methods(): void
+    public function test_force_flush_with_non_push_exporter(): void
     {
         $exporter = $this->createMock(MetricExporterInterface::class);
         $reader = new ExportingReader($exporter);
 
+        $this->assertTrue($reader->forceFlush());
+    }
+
+    public function test_closed_reader_does_not_call_exporter_methods(): void
+    {
+        $exporter = $this->createMock(PushMetricExporterInterface::class);
+        $reader = new ExportingReader($exporter);
+
         $reader->shutdown();
 
         $exporter->expects($this->never())->method('export');