From 6a2242d46a98e1420f8610bacbddf74b8b8c8b37 Mon Sep 17 00:00:00 2001 From: Tobias Bachert Date: Wed, 25 Sep 2024 01:58:09 +0200 Subject: [PATCH] Remove `MeterInterface::isEnabled()` and fix meter config re-enabling (#1387) --- src/API/Metrics/LateBindingMeter.php | 5 - src/API/Metrics/MeterInterface.php | 2 - src/API/Metrics/Noop/NoopMeter.php | 5 - src/SDK/Metrics/Instrument.php | 8 - src/SDK/Metrics/Meter.php | 140 ++++++++++++------ src/SDK/Metrics/MeterInstruments.php | 4 +- src/SDK/Metrics/MeterProvider.php | 2 +- .../MetricFactory/StreamMetricSource.php | 5 - .../Metrics/MetricReader/ExportingReader.php | 47 +++--- .../Metrics/MetricRegistry/MetricRegistry.php | 36 ++--- .../MetricRegistryInterface.php | 2 +- src/SDK/Metrics/MetricSourceInterface.php | 1 - ...etricSourceRegistryUnregisterInterface.php | 18 +++ src/SDK/Metrics/ObservableInstrumentTrait.php | 6 - src/SDK/Metrics/RegisteredInstrument.php | 17 +++ .../Metrics/SynchronousInstrumentTrait.php | 9 +- .../SDK/Metrics/MeterConfigTest.php | 16 +- tests/Unit/SDK/Metrics/InstrumentTest.php | 43 ++---- tests/Unit/SDK/Metrics/MeterProviderTest.php | 4 +- tests/Unit/SDK/Metrics/MeterTest.php | 37 ++--- .../MetricReader/ExportingReaderTest.php | 2 - .../Metrics/View/SelectionCriteriaTest.php | 13 +- 22 files changed, 209 insertions(+), 213 deletions(-) create mode 100644 src/SDK/Metrics/MetricSourceRegistryUnregisterInterface.php create mode 100644 src/SDK/Metrics/RegisteredInstrument.php diff --git a/src/API/Metrics/LateBindingMeter.php b/src/API/Metrics/LateBindingMeter.php index 18dd1112d..b146bfbbb 100644 --- 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 index f8f0b9c69..fa12e5814 100644 --- 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 index 63671c800..25ae9f864 100644 --- 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 index efacb8614..8d9a5b363 100644 --- 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 index 40eb129ae..0f4ccebc0 100644 --- 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,26 +307,25 @@ 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; }); @@ -303,15 +333,16 @@ private function createSynchronousWriter(string|InstrumentType $instrumentType, 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,25 +353,24 @@ 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; }); @@ -348,9 +378,21 @@ private function createAsynchronousObserver(string|InstrumentType $instrumentTyp 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 */ diff --git a/src/SDK/Metrics/MeterInstruments.php b/src/SDK/Metrics/MeterInstruments.php index 2070f0860..8d97809c9 100644 --- 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> + * @var array> */ public array $observers = []; /** - * @var array> + * @var array> */ public array $writers = []; } diff --git a/src/SDK/Metrics/MeterProvider.php b/src/SDK/Metrics/MeterProvider.php index 543601e7f..efd2b4b7b 100644 --- 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 index a96c0e7e4..5b105a5a5 100644 --- 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(); - } } diff --git a/src/SDK/Metrics/MetricReader/ExportingReader.php b/src/SDK/Metrics/MetricReader/ExportingReader.php index 45585689b..b1230d714 100644 --- a/src/SDK/Metrics/MetricReader/ExportingReader.php +++ b/src/SDK/Metrics/MetricReader/ExportingReader.php @@ -17,11 +17,12 @@ use OpenTelemetry\SDK\Metrics\MetricSourceInterface; use OpenTelemetry\SDK\Metrics\MetricSourceProviderInterface; use OpenTelemetry\SDK\Metrics\MetricSourceRegistryInterface; +use OpenTelemetry\SDK\Metrics\MetricSourceRegistryUnregisterInterface; use OpenTelemetry\SDK\Metrics\PushMetricExporterInterface; use OpenTelemetry\SDK\Metrics\StalenessHandlerInterface; use function spl_object_id; -final class ExportingReader implements MetricReaderInterface, MetricSourceRegistryInterface, DefaultAggregationProviderInterface +final class ExportingReader implements MetricReaderInterface, MetricSourceRegistryInterface, MetricSourceRegistryUnregisterInterface, DefaultAggregationProviderInterface { use DefaultAggregationProviderTrait { defaultAggregation as private _defaultAggregation; } /** @var array */ @@ -29,7 +30,7 @@ final class ExportingReader implements MetricReaderInterface, MetricSourceRegist /** @var array */ private array $registries = []; - /** @var array> */ + /** @var array>> */ private array $streamIds = []; private bool $closed = false; @@ -64,11 +65,11 @@ public function add(MetricSourceProviderInterface $provider, MetricMetadataInter $sourceId = spl_object_id($source); $this->sources[$sourceId] = $source; - $stalenessHandler->onStale(function () use ($sourceId): void { - unset($this->sources[$sourceId]); - }); - if (!$provider instanceof StreamMetricSourceProvider) { + $stalenessHandler->onStale(function () use ($sourceId): void { + unset($this->sources[$sourceId]); + }); + return; } @@ -77,20 +78,22 @@ public function add(MetricSourceProviderInterface $provider, MetricMetadataInter $registryId = spl_object_id($registry); $this->registries[$registryId] = $registry; - $this->streamIds[$registryId][$streamId] ??= 0; - $this->streamIds[$registryId][$streamId]++; - - $stalenessHandler->onStale(function () use ($streamId, $registryId): void { - if (!--$this->streamIds[$registryId][$streamId]) { - unset($this->streamIds[$registryId][$streamId]); - if (!$this->streamIds[$registryId]) { - unset( - $this->registries[$registryId], - $this->streamIds[$registryId], - ); - } - } - }); + $this->streamIds[$registryId][$streamId][] = $sourceId; + } + + public function unregisterStream(MetricCollectorInterface $collector, int $streamId): void + { + $registryId = spl_object_id($collector); + foreach ($this->streamIds[$registryId][$streamId] ?? [] as $sourceId) { + unset($this->sources[$sourceId]); + } + unset($this->streamIds[$registryId][$streamId]); + if (!$this->streamIds[$registryId]) { + unset( + $this->registries[$registryId], + $this->streamIds[$registryId], + ); + } } private function doCollect(): bool @@ -102,9 +105,7 @@ private function doCollect(): bool $metrics = []; foreach ($this->sources as $source) { - if ($source->isEnabled()) { - $metrics[] = $source->collect(); - } + $metrics[] = $source->collect(); } if ($metrics === []) { diff --git a/src/SDK/Metrics/MetricRegistry/MetricRegistry.php b/src/SDK/Metrics/MetricRegistry/MetricRegistry.php index 2c553419e..ea68865a3 100644 --- a/src/SDK/Metrics/MetricRegistry/MetricRegistry.php +++ b/src/SDK/Metrics/MetricRegistry/MetricRegistry.php @@ -38,8 +38,6 @@ final class MetricRegistry implements MetricRegistryInterface, MetricWriterInter private array $asynchronousCallbacks = []; /** @var array> */ private array $asynchronousCallbackArguments = []; - /** @var array */ - private array $instruments = []; public function __construct( private readonly ?ContextStorageInterface $contextStorage, @@ -57,7 +55,6 @@ public function registerSynchronousStream(Instrument $instrument, MetricStreamIn $this->synchronousAggregators[$streamId] = $aggregator; $this->instrumentToStreams[$instrumentId][$streamId] = $streamId; $this->streamToInstrument[$streamId] = $instrumentId; - $this->instruments[$instrumentId] = $instrument; return $streamId; } @@ -71,25 +68,26 @@ public function registerAsynchronousStream(Instrument $instrument, MetricStreamI $this->asynchronousAggregatorFactories[$streamId] = $aggregatorFactory; $this->instrumentToStreams[$instrumentId][$streamId] = $streamId; $this->streamToInstrument[$streamId] = $instrumentId; - $this->instruments[$instrumentId] = $instrument; return $streamId; } - public function unregisterStream(int $streamId): void + public function unregisterStreams(Instrument $instrument): array { - $instrumentId = $this->streamToInstrument[$streamId]; - unset( - $this->streams[$streamId], - $this->synchronousAggregators[$streamId], - $this->asynchronousAggregatorFactories[$streamId], - $this->instrumentToStreams[$instrumentId][$streamId], - $this->streamToInstrument[$streamId], - $this->instruments[$instrumentId], - ); - if (!$this->instrumentToStreams[$instrumentId]) { - unset($this->instrumentToStreams[$instrumentId]); + $instrumentId = spl_object_id($instrument); + $streamIds = $this->instrumentToStreams[$instrumentId] ?? []; + + foreach ($streamIds as $streamId) { + unset( + $this->streams[$streamId], + $this->synchronousAggregators[$streamId], + $this->asynchronousAggregatorFactories[$streamId], + $this->streamToInstrument[$streamId], + ); } + unset($this->instrumentToStreams[$instrumentId]); + + return $streamIds; } public function record(Instrument $instrument, $value, iterable $attributes = [], $context = null): void @@ -145,12 +143,6 @@ public function collectAndPush(iterable $streamIds): void $callbackIds = []; foreach ($streamIds as $streamId) { $instrumentId = $this->streamToInstrument[$streamId]; - if ( - array_key_exists($instrumentId, $this->instruments) - && $this->instruments[$instrumentId]->meter?->isEnabled() === false - ) { - continue; - } if (!$aggregator = $this->synchronousAggregators[$streamId] ?? null) { $aggregator = $this->asynchronousAggregatorFactories[$streamId]->create(); diff --git a/src/SDK/Metrics/MetricRegistry/MetricRegistryInterface.php b/src/SDK/Metrics/MetricRegistry/MetricRegistryInterface.php index e86731138..21b0887f7 100644 --- a/src/SDK/Metrics/MetricRegistry/MetricRegistryInterface.php +++ b/src/SDK/Metrics/MetricRegistry/MetricRegistryInterface.php @@ -18,5 +18,5 @@ public function registerSynchronousStream(Instrument $instrument, MetricStreamIn public function registerAsynchronousStream(Instrument $instrument, MetricStreamInterface $stream, MetricAggregatorFactoryInterface $aggregatorFactory): int; - public function unregisterStream(int $streamId): void; + public function unregisterStreams(Instrument $instrument): array; } diff --git a/src/SDK/Metrics/MetricSourceInterface.php b/src/SDK/Metrics/MetricSourceInterface.php index ba91e776f..5f00a0717 100644 --- a/src/SDK/Metrics/MetricSourceInterface.php +++ b/src/SDK/Metrics/MetricSourceInterface.php @@ -21,5 +21,4 @@ public function collectionTimestamp(): int; * @return Metric collected metric */ public function collect(): Metric; - public function isEnabled(): bool; } diff --git a/src/SDK/Metrics/MetricSourceRegistryUnregisterInterface.php b/src/SDK/Metrics/MetricSourceRegistryUnregisterInterface.php new file mode 100644 index 000000000..543939752 --- /dev/null +++ b/src/SDK/Metrics/MetricSourceRegistryUnregisterInterface.php @@ -0,0 +1,18 @@ +meter->isEnabled()) { - return false; - } - return $this->writer->enabled($this->instrument); } } diff --git a/src/SDK/Metrics/RegisteredInstrument.php b/src/SDK/Metrics/RegisteredInstrument.php new file mode 100644 index 000000000..efd4c00ff --- /dev/null +++ b/src/SDK/Metrics/RegisteredInstrument.php @@ -0,0 +1,17 @@ +writer = $writer; $this->instrument = $instrument; $this->referenceCounter = $referenceCounter; - $this->meter = $meter; $this->referenceCounter->acquire(); } @@ -49,10 +46,6 @@ public function write($amount, iterable $attributes = [], $context = null): void public function isEnabled(): bool { - if (!$this->meter->isEnabled()) { - return false; - } - return $this->writer->enabled($this->instrument); } } diff --git a/tests/Integration/SDK/Metrics/MeterConfigTest.php b/tests/Integration/SDK/Metrics/MeterConfigTest.php index 63ca4e408..439b9fea7 100644 --- a/tests/Integration/SDK/Metrics/MeterConfigTest.php +++ b/tests/Integration/SDK/Metrics/MeterConfigTest.php @@ -7,6 +7,7 @@ use OpenTelemetry\API\Common\Time\TestClock; use OpenTelemetry\API\Metrics\ObserverInterface; use OpenTelemetry\SDK\Common\InstrumentationScope\Configurator; +use OpenTelemetry\SDK\Metrics\Data\Sum; use OpenTelemetry\SDK\Metrics\Data\Temporality; use OpenTelemetry\SDK\Metrics\MeterConfig; use OpenTelemetry\SDK\Metrics\MeterProvider; @@ -52,13 +53,13 @@ public function test_disable_scopes(): void $this->assertFalse($instrument->isEnabled(), sprintf('instrument %s is disabled', $id)); } - $this->assertTrue($meter_one->isEnabled()); - $this->assertFalse($meter_two->isEnabled()); - $this->assertTrue($meter_three->isEnabled()); + $this->assertTrue($meter_one->createCounter('test')->isEnabled()); + $this->assertFalse($meter_two->createCounter('test')->isEnabled()); + $this->assertTrue($meter_three->createCounter('test')->isEnabled()); $meterProvider->updateConfigurator(Configurator::meter()); - $this->assertTrue($meter_two->isEnabled()); + $this->assertTrue($meter_two->createCounter('test')->isEnabled()); foreach ($instruments as $instrument) { $this->assertTrue($instrument->isEnabled()); @@ -81,7 +82,7 @@ public function test_metrics_not_exported_when_disabled(): void ) ->build(); $meter = $meterProvider->getMeter('test'); - $this->assertFalse($meter->isEnabled()); + $this->assertFalse($meter->createCounter('test')->isEnabled()); $counter = $meter->createCounter('a'); $async_counter = $meter->createObservableCounter('b', callbacks: function (ObserverInterface $o) { $this->fail('observer from disabled meter should not have been called'); @@ -101,10 +102,9 @@ public function test_metrics_not_exported_when_disabled(): void */ public function test_streams_recreated_on_enable(): void { - $this->markTestSkipped('TODO implement drop/create streams'); // @phpstan-ignore-next-line $clock = new TestClock(self::T0); $disabledConfigurator = Configurator::meter() - ->with(static fn (MeterConfig $config) => $config->setDisabled(false), name: '*'); + ->with(static fn (MeterConfig $config) => $config->setDisabled(true), name: '*'); $exporter = new InMemoryExporter(Temporality::CUMULATIVE); $reader = new ExportingReader($exporter); $meterProvider = MeterProvider::builder() @@ -132,6 +132,8 @@ public function test_streams_recreated_on_enable(): void $this->assertCount(1, $metrics); $metric = $metrics[0]; + $this->assertInstanceOf(Sum::class, $metric->data); + $this->assertIsArray($metric->data->dataPoints); $this->assertCount(1, $metric->data->dataPoints); $dataPoint = $metric->data->dataPoints[0]; diff --git a/tests/Unit/SDK/Metrics/InstrumentTest.php b/tests/Unit/SDK/Metrics/InstrumentTest.php index a5620fd27..3f1a5acb1 100644 --- a/tests/Unit/SDK/Metrics/InstrumentTest.php +++ b/tests/Unit/SDK/Metrics/InstrumentTest.php @@ -5,7 +5,6 @@ namespace OpenTelemetry\Tests\Unit\SDK\Metrics; use OpenTelemetry\API\Common\Time\TestClock; -use OpenTelemetry\API\Metrics\MeterInterface; use OpenTelemetry\API\Metrics\ObserverInterface; use OpenTelemetry\SDK\Common\Attribute\Attributes; use OpenTelemetry\SDK\Metrics\Aggregation\ExplicitBucketHistogramAggregation; @@ -41,24 +40,16 @@ #[CoversClass(ObservableInstrumentTrait::class)] final class InstrumentTest extends TestCase { - private MeterInterface $meter; - - public function setUp(): void - { - $this->meter = $this->createMock(MeterInterface::class); - $this->meter->method('isEnabled')->willReturn(true); - } - public function test_counter(): void { $a = new MetricAggregator(null, new SumAggregation(true)); $s = new SynchronousMetricStream(new SumAggregation(true), 0); $w = new MetricRegistry(null, Attributes::factory(), new TestClock(1)); - $i = new Instrument(InstrumentType::COUNTER, 'test', null, null, meter: $this->meter); + $i = new Instrument(InstrumentType::COUNTER, 'test', null, null); $n = $w->registerSynchronousStream($i, $s, $a); $r = $s->register(Temporality::DELTA); - $c = new Counter($w, $i, new NoopStalenessHandler(), $this->meter); + $c = new Counter($w, $i, new NoopStalenessHandler()); $c->add(5); $c->add(7); $c->add(3); @@ -83,11 +74,11 @@ public function test_asynchronous_counter(): void $a = new MetricAggregatorFactory(null, new SumAggregation(true)); $s = new SynchronousMetricStream(new SumAggregation(true), 0); $w = new MetricRegistry(null, Attributes::factory(), new TestClock(1)); - $i = new Instrument(InstrumentType::ASYNCHRONOUS_COUNTER, 'test', null, null, meter: $this->meter); + $i = new Instrument(InstrumentType::ASYNCHRONOUS_COUNTER, 'test', null, null); $n = $w->registerAsynchronousStream($i, $s, $a); $r = $s->register(Temporality::CUMULATIVE); - $c = new ObservableCounter($w, $i, new NoopStalenessHandler(), new WeakMap(), $this->meter); + $c = new ObservableCounter($w, $i, new NoopStalenessHandler(), new WeakMap()); $c->observe(static function (ObserverInterface $observer): void { $observer->observe(5); }); @@ -112,7 +103,7 @@ public function test_asynchronous_counter_weaken(): void $a = new MetricAggregatorFactory(null, new SumAggregation(true)); $s = new SynchronousMetricStream(new SumAggregation(true), 0); $w = new MetricRegistry(null, Attributes::factory(), new TestClock(1)); - $i = new Instrument(InstrumentType::ASYNCHRONOUS_COUNTER, 'test', null, null, meter: $this->meter); + $i = new Instrument(InstrumentType::ASYNCHRONOUS_COUNTER, 'test', null, null); $n = $w->registerAsynchronousStream($i, $s, $a); $r = $s->register(Temporality::CUMULATIVE); @@ -123,7 +114,7 @@ public function __invoke(ObserverInterface $observer) } }; - $c = new ObservableCounter($w, $i, new NoopStalenessHandler(), new WeakMap(), $this->meter); + $c = new ObservableCounter($w, $i, new NoopStalenessHandler(), new WeakMap()); $c->observe($instance); $instance = null; @@ -140,11 +131,11 @@ public function test_up_down_counter(): void $a = new MetricAggregator(null, new SumAggregation(false)); $s = new SynchronousMetricStream(new SumAggregation(false), 0); $w = new MetricRegistry(null, Attributes::factory(), new TestClock(1)); - $i = new Instrument(InstrumentType::UP_DOWN_COUNTER, 'test', null, null, meter: $this->meter); + $i = new Instrument(InstrumentType::UP_DOWN_COUNTER, 'test', null, null); $n = $w->registerSynchronousStream($i, $s, $a); $r = $s->register(Temporality::DELTA); - $c = new UpDownCounter($w, $i, new NoopStalenessHandler(), $this->meter); + $c = new UpDownCounter($w, $i, new NoopStalenessHandler()); $c->add(5); $c->add(7); $c->add(-8); @@ -169,11 +160,11 @@ public function test_histogram(): void $a = new MetricAggregator(null, new ExplicitBucketHistogramAggregation([3, 6, 9])); $s = new SynchronousMetricStream(new ExplicitBucketHistogramAggregation([3, 6, 9]), 0); $w = new MetricRegistry(null, Attributes::factory(), new TestClock(1)); - $i = new Instrument(InstrumentType::HISTOGRAM, 'test', null, null, meter: $this->meter); + $i = new Instrument(InstrumentType::HISTOGRAM, 'test', null, null); $n = $w->registerSynchronousStream($i, $s, $a); $r = $s->register(Temporality::DELTA); - $h = new Histogram($w, $i, new NoopStalenessHandler(), $this->meter); + $h = new Histogram($w, $i, new NoopStalenessHandler()); $h->record(1); $h->record(7); $h->record(9); @@ -254,10 +245,9 @@ public function test_synchronous_disabled_if_meter_disabled(): void { $w = $this->createMock(MetricWriterInterface::class); $c = $this->createMock(ReferenceCounterInterface::class); - $i = new Instrument(InstrumentType::UP_DOWN_COUNTER, 'test', null, null, meter: $this->meter); - $meter = $this->createMock(MeterInterface::class); - $meter->expects($this->once())->method('isEnabled')->willReturn(false); - $counter = new Counter($w, $i, $c, $meter); + $i = new Instrument(InstrumentType::UP_DOWN_COUNTER, 'test', null, null); + $w->expects($this->once())->method('enabled')->with($i)->willReturn(false); + $counter = new Counter($w, $i, $c); $this->assertFalse($counter->isEnabled()); } @@ -266,10 +256,9 @@ public function test_asynchronous_disabled_if_meter_disabled(): void { $w = $this->createMock(MetricWriterInterface::class); $c = $this->createMock(ReferenceCounterInterface::class); - $i = new Instrument(InstrumentType::UP_DOWN_COUNTER, 'test', null, null, meter: $this->meter); - $meter = $this->createMock(MeterInterface::class); - $meter->expects($this->once())->method('isEnabled')->willReturn(false); - $counter = new ObservableCounter($w, $i, $c, new WeakMap(), $meter); + $i = new Instrument(InstrumentType::UP_DOWN_COUNTER, 'test', null, null); + $w->expects($this->once())->method('enabled')->with($i)->willReturn(false); + $counter = new ObservableCounter($w, $i, $c, new WeakMap()); $this->assertFalse($counter->isEnabled()); } diff --git a/tests/Unit/SDK/Metrics/MeterProviderTest.php b/tests/Unit/SDK/Metrics/MeterProviderTest.php index 7df3c7802..c72e0d1e4 100644 --- a/tests/Unit/SDK/Metrics/MeterProviderTest.php +++ b/tests/Unit/SDK/Metrics/MeterProviderTest.php @@ -114,11 +114,11 @@ public function test_disable(): void $meterProvider = MeterProvider::builder()->addReader(new ExportingReader(new InMemoryExporter()))->build(); $this->assertInstanceOf(MeterProvider::class, $meterProvider); $meter = $meterProvider->getMeter('one'); - $this->assertTrue($meter->isEnabled()); + $this->assertTrue($meter->createCounter('test')->isEnabled()); $counter = $meter->createCounter('A'); $this->assertTrue($counter->isEnabled()); $meterProvider->updateConfigurator(Configurator::meter()->with(static fn (MeterConfig $config) => $config->setDisabled(true), name: 'one')); - $this->assertFalse($meter->isEnabled()); + $this->assertFalse($meter->createCounter('test')->isEnabled()); $this->assertFalse($counter->isEnabled()); } } diff --git a/tests/Unit/SDK/Metrics/MeterTest.php b/tests/Unit/SDK/Metrics/MeterTest.php index 1558e7ca1..fa6fd83a7 100644 --- a/tests/Unit/SDK/Metrics/MeterTest.php +++ b/tests/Unit/SDK/Metrics/MeterTest.php @@ -9,13 +9,11 @@ use OpenTelemetry\SDK\Common\Attribute\Attributes; use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScope; use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeFactory; -use OpenTelemetry\SDK\Common\InstrumentationScope\Configurator; use OpenTelemetry\SDK\Metrics\AggregationInterface; use OpenTelemetry\SDK\Metrics\DefaultAggregationProviderInterface; use OpenTelemetry\SDK\Metrics\Instrument; use OpenTelemetry\SDK\Metrics\InstrumentType; use OpenTelemetry\SDK\Metrics\Meter; -use OpenTelemetry\SDK\Metrics\MeterConfig; use OpenTelemetry\SDK\Metrics\MeterProvider; use OpenTelemetry\SDK\Metrics\MetricFactoryInterface; use OpenTelemetry\SDK\Metrics\MetricReaderInterface; @@ -41,7 +39,7 @@ public function test_create_counter(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::COUNTER, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::COUNTER, 'name', 'unit', 'description'), $this->anything(), $this->anything(), $this->anything(), @@ -61,7 +59,7 @@ public function test_create_histogram(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::HISTOGRAM, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::HISTOGRAM, 'name', 'unit', 'description'), $this->anything(), $this->anything(), $this->anything(), @@ -87,7 +85,6 @@ public function test_create_histogram_advisory(): void 's', 'Measures the duration of inbound HTTP requests.', ['ExplicitBucketBoundaries' => [0, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10]], - $meter, ), $this->anything(), $this->anything(), @@ -113,7 +110,7 @@ public function test_create_gauge(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::GAUGE, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::GAUGE, 'name', 'unit', 'description'), $this->anything(), $this->anything(), $this->anything(), @@ -133,7 +130,7 @@ public function test_create_up_down_counter(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::UP_DOWN_COUNTER, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::UP_DOWN_COUNTER, 'name', 'unit', 'description'), $this->anything(), $this->anything(), $this->anything(), @@ -153,7 +150,7 @@ public function test_create_observable_counter(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::ASYNCHRONOUS_COUNTER, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::ASYNCHRONOUS_COUNTER, 'name', 'unit', 'description'), $this->anything(), $this->anything(), ) @@ -172,7 +169,7 @@ public function test_create_observable_gauge(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::ASYNCHRONOUS_GAUGE, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::ASYNCHRONOUS_GAUGE, 'name', 'unit', 'description'), $this->anything(), $this->anything(), ) @@ -191,7 +188,7 @@ public function test_create_observable_up_down_counter(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::ASYNCHRONOUS_UP_DOWN_COUNTER, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::ASYNCHRONOUS_UP_DOWN_COUNTER, 'name', 'unit', 'description'), $this->anything(), $this->anything(), ) @@ -211,7 +208,7 @@ public function test_reuses_writer_when_not_stale(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::COUNTER, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::COUNTER, 'name', 'unit', 'description'), $this->anything(), $this->anything(), $this->anything(), @@ -232,7 +229,7 @@ public function test_releases_writer_on_stale(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::COUNTER, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::COUNTER, 'name', 'unit', 'description'), $this->anything(), $this->anything(), $this->anything(), @@ -254,7 +251,7 @@ public function test_reuses_observer_when_not_stale(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::ASYNCHRONOUS_COUNTER, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::ASYNCHRONOUS_COUNTER, 'name', 'unit', 'description'), $this->anything(), $this->anything(), ) @@ -274,7 +271,7 @@ public function test_releases_observer_on_stale(): void $this->anything(), $this->anything(), new InstrumentationScope('test', null, null, Attributes::create([])), - new Instrument(InstrumentType::ASYNCHRONOUS_COUNTER, 'name', 'unit', 'description', meter: $meter), + new Instrument(InstrumentType::ASYNCHRONOUS_COUNTER, 'name', 'unit', 'description'), $this->anything(), $this->anything(), ) @@ -376,18 +373,6 @@ public function test_uses_default_view_if_null_views_returned(): void $meter->createCounter('name'); } - public function test_update_configurator(): void - { - $metricFactory = $this->createMock(MetricFactoryInterface::class); - $metricFactory->method('createSynchronousWriter')->willReturn([]); - - $meterProvider = $this->createMeterProviderForMetricFactory($metricFactory); - $meter = $meterProvider->getMeter('test'); - $this->assertTrue($meter->isEnabled()); - $meterProvider->updateConfigurator(Configurator::meter()->with(static fn (MeterConfig $config) => $config->setDisabled(true), name: 'test')); - $this->assertFalse($meter->isEnabled()); - } - /** * @param iterable $metricReaders */ diff --git a/tests/Unit/SDK/Metrics/MetricReader/ExportingReaderTest.php b/tests/Unit/SDK/Metrics/MetricReader/ExportingReaderTest.php index 2150de98c..d9bb3e05a 100644 --- a/tests/Unit/SDK/Metrics/MetricReader/ExportingReaderTest.php +++ b/tests/Unit/SDK/Metrics/MetricReader/ExportingReaderTest.php @@ -148,7 +148,6 @@ public function test_collect_collects_sources_with_current_timestamp(): void $source = $this->createMock(MetricSourceInterface::class); $source->expects($this->once())->method('collect')->willReturn($metric); - $source->method('isEnabled')->willReturn(true); $provider = $this->createMock(MetricSourceProviderInterface::class); $provider->expects($this->once())->method('create')->willReturn($source); $metricMetadata = $this->createMock(MetricMetadataInterface::class); @@ -184,7 +183,6 @@ public function test_shutdown_exports_metrics(): void $provider = $this->createMock(MetricSourceProviderInterface::class); $source = $this->createMock(MetricSourceInterface::class); $source->method('collect')->willReturn($this->createMock(Metric::class)); - $source->method('isEnabled')->willReturn(true); $provider->method('create')->willReturn($source); $exporter->method('temporality')->willReturn('foo'); $exporter->expects($this->once())->method('export')->willReturn(true); diff --git a/tests/Unit/SDK/Metrics/View/SelectionCriteriaTest.php b/tests/Unit/SDK/Metrics/View/SelectionCriteriaTest.php index f1400fd71..3038483c3 100644 --- a/tests/Unit/SDK/Metrics/View/SelectionCriteriaTest.php +++ b/tests/Unit/SDK/Metrics/View/SelectionCriteriaTest.php @@ -4,7 +4,6 @@ namespace OpenTelemetry\Tests\Unit\SDK\Metrics\View; -use OpenTelemetry\API\Metrics\MeterInterface; use OpenTelemetry\SDK\Common\Attribute\Attributes; use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScope; use OpenTelemetry\SDK\Metrics\Instrument; @@ -31,22 +30,14 @@ final class SelectionCriteriaTest extends TestCase { use ProphecyTrait; - private MeterInterface $meter; - - public function setUp(): void - { - $this->meter = $this->createMock(MeterInterface::class); - $this->meter->method('isEnabled')->willReturn(true); - } - public function test_instrument_scope_name_criteria(): void { $this->assertTrue((new InstrumentationScopeNameCriteria('scopeName'))->accepts( - new Instrument(InstrumentType::COUNTER, 'name', null, null, meter: $this->meter), + new Instrument(InstrumentType::COUNTER, 'name', null, null), new InstrumentationScope('scopeName', null, null, Attributes::create([])), )); $this->assertFalse((new InstrumentationScopeNameCriteria('scopeName'))->accepts( - new Instrument(InstrumentType::COUNTER, 'name', null, null, meter: $this->meter), + new Instrument(InstrumentType::COUNTER, 'name', null, null), new InstrumentationScope('scope-name', null, null, Attributes::create([])), )); }