Remove MeterInterface::isEnabled() and fix meter config re-enabling (…

…#1387)
open-telemetry · Sep 24, 2024 · 6a2242d · 6a2242d
1 parent 8ed62b1
commit 6a2242d
Show file tree

Hide file tree

Showing 22 changed files with 209 additions and 213 deletions.
diff --git a/src/API/Metrics/LateBindingMeter.php b/src/API/Metrics/LateBindingMeter.php
@@ -58,9 +58,4 @@ public function createObservableUpDownCounter(string $name, ?string $unit = null
     {
         return ($this->meter ??= ($this->factory)())->createObservableUpDownCounter($name, $unit, $description, $advisory, $callbacks);
     }
-
-    public function isEnabled(): bool
-    {
-        return true;
-    }
 }
diff --git a/src/API/Metrics/MeterInterface.php b/src/API/Metrics/MeterInterface.php
@@ -178,6 +178,4 @@ public function createObservableUpDownCounter(
         array|callable $advisory = [],
         callable ...$callbacks,
     ): ObservableUpDownCounterInterface;
-
-    public function isEnabled(): bool;
 }
diff --git a/src/API/Metrics/Noop/NoopMeter.php b/src/API/Metrics/Noop/NoopMeter.php
@@ -56,9 +56,4 @@ public function createObservableUpDownCounter(string $name, ?string $unit = null
     {
         return new NoopObservableUpDownCounter();
     }
-
-    public function isEnabled(): bool
-    {
-        return false;
-    }
 }
diff --git a/src/SDK/Metrics/Instrument.php b/src/SDK/Metrics/Instrument.php
@@ -4,8 +4,6 @@
 
 namespace OpenTelemetry\SDK\Metrics;
 
-use OpenTelemetry\API\Metrics\MeterInterface;
-
 final class Instrument
 {
     public function __construct(
@@ -14,12 +12,6 @@ public function __construct(
         public readonly ?string $unit,
         public readonly ?string $description,
         public readonly array $advisory = [],
-        public readonly ?MeterInterface $meter = null,
     ) {
     }
-
-    public function isEnabled(): bool
-    {
-        return $this->meter?->isEnabled() ?? true;
-    }
 }
diff --git a/src/SDK/Metrics/Meter.php b/src/SDK/Metrics/Meter.php
@@ -61,9 +61,9 @@ public function __construct(
         private readonly MetricRegistryInterface $registry,
         private readonly MetricWriterInterface $writer,
         private readonly ArrayAccess $destructors,
-        private ?Configurator $configurator = null,
+        ?Configurator $configurator = null,
     ) {
-        $this->config = $this->configurator?->resolve($this->instrumentationScope) ?? MeterConfig::default();
+        $this->config = $configurator?->resolve($this->instrumentationScope) ?? MeterConfig::default();
     }
 
     private static function dummyInstrument(): Instrument
@@ -78,8 +78,44 @@ private static function dummyInstrument(): Instrument
      */
     public function updateConfigurator(Configurator $configurator): void
     {
-        $this->configurator = $configurator;
         $this->config = $configurator->resolve($this->instrumentationScope);
+
+        $startTimestamp = $this->clock->now();
+        foreach ($this->instruments->observers[self::instrumentationScopeId($this->instrumentationScope)] as [$instrument, $stalenessHandler, $r]) {
+            if ($this->config->isEnabled() && $r->dormant) {
+                $this->metricFactory->createAsynchronousObserver(
+                    $this->registry,
+                    $this->resource,
+                    $this->instrumentationScope,
+                    $instrument,
+                    $startTimestamp,
+                    $this->viewRegistrationRequests($instrument, $stalenessHandler),
+                );
+                $r->dormant = false;
+            }
+            if (!$this->config->isEnabled() && !$r->dormant) {
+                $this->releaseStreams($instrument);
+                $r->dormant = true;
+            }
+        }
+        foreach ($this->instruments->writers[self::instrumentationScopeId($this->instrumentationScope)] as [$instrument, $stalenessHandler, $r]) {
+            if ($this->config->isEnabled() && $r->dormant) {
+                $this->metricFactory->createSynchronousWriter(
+                    $this->registry,
+                    $this->resource,
+                    $this->instrumentationScope,
+                    $instrument,
+                    $startTimestamp,
+                    $this->viewRegistrationRequests($instrument, $stalenessHandler),
+                    $this->exemplarFilter,
+                );
+                $r->dormant = false;
+            }
+            if (!$this->config->isEnabled() && !$r->dormant) {
+                $this->releaseStreams($instrument);
+                $r->dormant = true;
+            }
+        }
     }
 
     public function batchObserve(callable $callback, AsynchronousInstrument $instrument, AsynchronousInstrument ...$instruments): ObservableCallbackInterface
@@ -131,7 +167,7 @@ public function createCounter(string $name, ?string $unit = null, ?string $descr
             $advisory,
         );
 
-        return new Counter($this->writer, $instrument, $referenceCounter, $this);
+        return new Counter($this->writer, $instrument, $referenceCounter);
     }
 
     public function createObservableCounter(string $name, ?string $unit = null, ?string $description = null, $advisory = [], callable ...$callbacks): ObservableCounterInterface
@@ -153,7 +189,7 @@ public function createObservableCounter(string $name, ?string $unit = null, ?str
             $referenceCounter->acquire(true);
         }
 
-        return new ObservableCounter($this->writer, $instrument, $referenceCounter, $this->destructors, $this);
+        return new ObservableCounter($this->writer, $instrument, $referenceCounter, $this->destructors);
     }
 
     public function createHistogram(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): HistogramInterface
@@ -166,7 +202,7 @@ public function createHistogram(string $name, ?string $unit = null, ?string $des
             $advisory,
         );
 
-        return new Histogram($this->writer, $instrument, $referenceCounter, $this);
+        return new Histogram($this->writer, $instrument, $referenceCounter);
     }
 
     public function createGauge(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): GaugeInterface
@@ -179,7 +215,7 @@ public function createGauge(string $name, ?string $unit = null, ?string $descrip
             $advisory,
         );
 
-        return new Gauge($this->writer, $instrument, $referenceCounter, $this);
+        return new Gauge($this->writer, $instrument, $referenceCounter);
     }
 
     public function createObservableGauge(string $name, ?string $unit = null, ?string $description = null, $advisory = [], callable ...$callbacks): ObservableGaugeInterface
@@ -201,7 +237,7 @@ public function createObservableGauge(string $name, ?string $unit = null, ?strin
             $referenceCounter->acquire(true);
         }
 
-        return new ObservableGauge($this->writer, $instrument, $referenceCounter, $this->destructors, $this);
+        return new ObservableGauge($this->writer, $instrument, $referenceCounter, $this->destructors);
     }
 
     public function createUpDownCounter(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): UpDownCounterInterface
@@ -214,7 +250,7 @@ public function createUpDownCounter(string $name, ?string $unit = null, ?string
             $advisory,
         );
 
-        return new UpDownCounter($this->writer, $instrument, $referenceCounter, $this);
+        return new UpDownCounter($this->writer, $instrument, $referenceCounter);
     }
 
     public function createObservableUpDownCounter(string $name, ?string $unit = null, ?string $description = null, $advisory = [], callable ...$callbacks): ObservableUpDownCounterInterface
@@ -236,16 +272,11 @@ public function createObservableUpDownCounter(string $name, ?string $unit = null
             $referenceCounter->acquire(true);
         }
 
-        return new ObservableUpDownCounter($this->writer, $instrument, $referenceCounter, $this->destructors, $this);
-    }
-
-    public function isEnabled(): bool
-    {
-        return $this->config->isEnabled();
+        return new ObservableUpDownCounter($this->writer, $instrument, $referenceCounter, $this->destructors);
     }
 
     /**
-     * @return array{Instrument, ReferenceCounterInterface}|null
+     * @return array{Instrument, ReferenceCounterInterface, RegisteredInstrument}|null
      */
     private function getAsynchronousInstrument(Instrument $instrument, InstrumentationScopeInterface $instrumentationScope): ?array
     {
@@ -261,11 +292,11 @@ private function getAsynchronousInstrument(Instrument $instrument, Instrumentati
     }
 
     /**
-     * @return array{Instrument, ReferenceCounterInterface}
+     * @return array{Instrument, ReferenceCounterInterface, RegisteredInstrument}
      */
     private function createSynchronousWriter(string|InstrumentType $instrumentType, string $name, ?string $unit, ?string $description, array $advisory = []): array
     {
-        $instrument = new Instrument($instrumentType, $name, $unit, $description, $advisory, $this);
+        $instrument = new Instrument($instrumentType, $name, $unit, $description, $advisory);
 
         $instrumentationScopeId = $this->instrumentationScopeId($this->instrumentationScope);
         $instrumentId = $this->instrumentId($instrument);
@@ -276,42 +307,42 @@ private function createSynchronousWriter(string|InstrumentType $instrumentType,
         }
 
         $stalenessHandler = $this->stalenessHandlerFactory->create();
-        $instruments->startTimestamp ??= $this->clock->now();
-        $streamIds = $this->metricFactory->createSynchronousWriter(
-            $this->registry,
-            $this->resource,
-            $this->instrumentationScope,
-            $instrument,
-            $instruments->startTimestamp,
-            $this->viewRegistrationRequests($instrument, $stalenessHandler),
-            $this->exemplarFilter,
-        );
+        if ($this->config->isEnabled()) {
+            $instruments->startTimestamp ??= $this->clock->now();
+            $this->metricFactory->createSynchronousWriter(
+                $this->registry,
+                $this->resource,
+                $this->instrumentationScope,
+                $instrument,
+                $instruments->startTimestamp,
+                $this->viewRegistrationRequests($instrument, $stalenessHandler),
+                $this->exemplarFilter,
+            );
+        }
 
-        $registry = $this->registry;
-        $stalenessHandler->onStale(static function () use ($instruments, $instrumentationScopeId, $instrumentId, $registry, $streamIds): void {
+        $stalenessHandler->onStale(fn () => $this->releaseStreams($instrument));
+        $stalenessHandler->onStale(static function () use ($instruments, $instrumentationScopeId, $instrumentId): void {
             unset($instruments->writers[$instrumentationScopeId][$instrumentId]);
             if (!$instruments->writers[$instrumentationScopeId]) {
                 unset($instruments->writers[$instrumentationScopeId]);
             }
-            foreach ($streamIds as $streamId) {
-                $registry->unregisterStream($streamId);
-            }
 
             $instruments->startTimestamp = null;
         });
 
         return $instruments->writers[$instrumentationScopeId][$instrumentId] = [
             $instrument,
             $stalenessHandler,
+            new RegisteredInstrument(!$this->config->isEnabled(), $this),
         ];
     }
 
     /**
-     * @return array{Instrument, ReferenceCounterInterface}
+     * @return array{Instrument, ReferenceCounterInterface, RegisteredInstrument}
      */
     private function createAsynchronousObserver(string|InstrumentType $instrumentType, string $name, ?string $unit, ?string $description, array $advisory): array
     {
-        $instrument = new Instrument($instrumentType, $name, $unit, $description, $advisory, $this);
+        $instrument = new Instrument($instrumentType, $name, $unit, $description, $advisory);
 
         $instrumentationScopeId = $this->instrumentationScopeId($this->instrumentationScope);
         $instrumentId = $this->instrumentId($instrument);
@@ -322,35 +353,46 @@ private function createAsynchronousObserver(string|InstrumentType $instrumentTyp
         }
 
         $stalenessHandler = $this->stalenessHandlerFactory->create();
-        $instruments->startTimestamp ??= $this->clock->now();
-        $streamIds = $this->metricFactory->createAsynchronousObserver(
-            $this->registry,
-            $this->resource,
-            $this->instrumentationScope,
-            $instrument,
-            $instruments->startTimestamp,
-            $this->viewRegistrationRequests($instrument, $stalenessHandler),
-        );
+        if ($this->config->isEnabled()) {
+            $instruments->startTimestamp ??= $this->clock->now();
+            $this->metricFactory->createAsynchronousObserver(
+                $this->registry,
+                $this->resource,
+                $this->instrumentationScope,
+                $instrument,
+                $instruments->startTimestamp,
+                $this->viewRegistrationRequests($instrument, $stalenessHandler),
+            );
+        }
 
-        $registry = $this->registry;
-        $stalenessHandler->onStale(static function () use ($instruments, $instrumentationScopeId, $instrumentId, $registry, $streamIds): void {
+        $stalenessHandler->onStale(fn () => $this->releaseStreams($instrument));
+        $stalenessHandler->onStale(static function () use ($instruments, $instrumentationScopeId, $instrumentId): void {
             unset($instruments->observers[$instrumentationScopeId][$instrumentId]);
             if (!$instruments->observers[$instrumentationScopeId]) {
                 unset($instruments->observers[$instrumentationScopeId]);
             }
-            foreach ($streamIds as $streamId) {
-                $registry->unregisterStream($streamId);
-            }
 
             $instruments->startTimestamp = null;
         });
 
         return $instruments->observers[$instrumentationScopeId][$instrumentId] = [
             $instrument,
             $stalenessHandler,
+            new RegisteredInstrument(!$this->config->isEnabled(), $this),
         ];
     }
 
+    private function releaseStreams(Instrument $instrument): void
+    {
+        foreach ($this->registry->unregisterStreams($instrument) as $streamId) {
+            foreach ($this->metricRegistries as $metricRegistry) {
+                if ($metricRegistry instanceof MetricSourceRegistryUnregisterInterface) {
+                    $metricRegistry->unregisterStream($this->registry, $streamId);
+                }
+            }
+        }
+    }
+
     /**
      * @return iterable<array{ViewProjection, MetricRegistrationInterface}>
      */

diff --git a/src/SDK/Metrics/MeterInstruments.php b/src/SDK/Metrics/MeterInstruments.php
@@ -11,11 +11,11 @@ final class MeterInstruments
 {
     public ?int $startTimestamp = null;
     /**
-     * @var array<string, array<string, array{Instrument, ReferenceCounterInterface}>>
+     * @var array<string, array<string, array{Instrument, StalenessHandlerInterface&ReferenceCounterInterface, RegisteredInstrument}>>
      */
     public array $observers = [];
     /**
-     * @var array<string, array<string, array{Instrument, ReferenceCounterInterface}>>
+     * @var array<string, array<string, array{Instrument, StalenessHandlerInterface&ReferenceCounterInterface, RegisteredInstrument}>>
      */
     public array $writers = [];
 }
diff --git a/src/SDK/Metrics/MeterProvider.php b/src/SDK/Metrics/MeterProvider.php
@@ -128,7 +128,7 @@ public static function builder(): MeterProviderBuilder
     /**
      * Update the {@link Configurator} for a {@link MeterProvider}, which will reconfigure
      *  all meters created from the provider.
-     * @todo enabling a previous-disabled meter does not drop/recreate the underlying metric streams, so previously collected synchronous metrics will still be exported.
+     *
      * @experimental
      */
     public function updateConfigurator(Configurator $configurator): void

diff --git a/src/SDK/Metrics/MetricFactory/StreamMetricSource.php b/src/SDK/Metrics/MetricFactory/StreamMetricSource.php
@@ -39,9 +39,4 @@ public function __destruct()
     {
         $this->provider->stream->unregister($this->reader);
     }
-
-    public function isEnabled(): bool
-    {
-        return $this->provider->instrument->isEnabled();
-    }
 }