Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove MeterInterface::isEnabled() and recreate metric streams on config re-enabling #1387

Merged
merged 1 commit into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions src/API/Metrics/LateBindingMeter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
2 changes: 0 additions & 2 deletions src/API/Metrics/MeterInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,4 @@ public function createObservableUpDownCounter(
array|callable $advisory = [],
callable ...$callbacks,
): ObservableUpDownCounterInterface;

public function isEnabled(): bool;
}
5 changes: 0 additions & 5 deletions src/API/Metrics/Noop/NoopMeter.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,4 @@ public function createObservableUpDownCounter(string $name, ?string $unit = null
{
return new NoopObservableUpDownCounter();
}

public function isEnabled(): bool
{
return false;
}
}
8 changes: 0 additions & 8 deletions src/SDK/Metrics/Instrument.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

namespace OpenTelemetry\SDK\Metrics;

use OpenTelemetry\API\Metrics\MeterInterface;

final class Instrument
{
public function __construct(
Expand All @@ -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;
}
}
140 changes: 91 additions & 49 deletions src/SDK/Metrics/Meter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
{
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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}>
*/
Expand Down
4 changes: 2 additions & 2 deletions src/SDK/Metrics/MeterInstruments.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
}
2 changes: 1 addition & 1 deletion src/SDK/Metrics/MeterProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions src/SDK/Metrics/MetricFactory/StreamMetricSource.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,4 @@ public function __destruct()
{
$this->provider->stream->unregister($this->reader);
}

public function isEnabled(): bool
{
return $this->provider->instrument->isEnabled();
}
}
Loading
Loading