Skip to content

Commit

Permalink
Minor improvements
Browse files Browse the repository at this point in the history
- rename `MetricObserver::weakMap()` to `::destructors()` to better reflect usage`
- move `ReferenceCounter::acquire()` call to corresponding `MetricObserver::observe()` call for consistency
- cache instrumentation scope id in `Meter` to avoid repeated calls to `serialize()`
- remove obsolete instrument type check from `ViewProjection`, leftover from supporting aggregation per type
  • Loading branch information
Nevay committed Jul 16, 2022
1 parent fa2adf4 commit 5c99ad0
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 17 deletions.
16 changes: 9 additions & 7 deletions src/SDK/Metrics/Meter.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ final class Meter implements MeterInterface
private MeterInstruments $instruments;
private InstrumentationScopeInterface $instrumentationScope;

private ?string $instrumentationScopeId = null;

/**
* @param iterable<MetricSourceRegistryInterface&DefaultAggregationProviderInterface> $metricRegistries
*/
Expand Down Expand Up @@ -164,8 +166,8 @@ private function createSynchronousWriter($instrumentType, string $name, ?string
{
$instrument = new Instrument($instrumentType, $name, $unit, $description);

$instrumentationScopeId = self::instrumentationScopeId($this->instrumentationScope);
$instrumentId = self::instrumentId($instrument);
$instrumentationScopeId = $this->instrumentationScopeId($this->instrumentationScope);
$instrumentId = $this->instrumentId($instrument);

$instruments = $this->instruments;
if ($writer = $instruments->writers[$instrumentationScopeId][$instrumentId] ?? null) {
Expand Down Expand Up @@ -207,8 +209,8 @@ private function createAsynchronousObserver($instrumentType, string $name, ?stri
{
$instrument = new Instrument($instrumentType, $name, $unit, $description);

$instrumentationScopeId = self::instrumentationScopeId($this->instrumentationScope);
$instrumentId = self::instrumentId($instrument);
$instrumentationScopeId = $this->instrumentationScopeId($this->instrumentationScope);
$instrumentId = $this->instrumentId($instrument);

$instruments = $this->instruments;
if ($observer = $instruments->observers[$instrumentationScopeId][$instrumentId] ?? null) {
Expand Down Expand Up @@ -277,12 +279,12 @@ private function viewRegistrationRequests(Instrument $instrument, StalenessHandl
}
}

private static function instrumentationScopeId(InstrumentationScopeInterface $instrumentationScope): string
private function instrumentationScopeId(InstrumentationScopeInterface $instrumentationScope): string
{
return serialize($instrumentationScope);
return $this->instrumentationScopeId ??= serialize($instrumentationScope);
}

private static function instrumentId(Instrument $instrument): string
private function instrumentId(Instrument $instrument): string
{
return serialize($instrument);
}
Expand Down
2 changes: 1 addition & 1 deletion src/SDK/Metrics/MetricObserver/MultiObserver.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function cancel(int $token): void
unset($this->callbacks[$token]);
}

public function weakMap(): ArrayAccess
public function destructors(): ArrayAccess
{
return $this->weakMap ??= WeakMap::create();
}
Expand Down
6 changes: 5 additions & 1 deletion src/SDK/Metrics/MetricObserverInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use ArrayAccess;
use Closure;
use OpenTelemetry\API\Metrics\ObserverInterface;
use OpenTelemetry\SDK\Metrics\MetricObserver\CallbackDestructor;

interface MetricObserverInterface
{
Expand All @@ -19,5 +20,8 @@ public function has(int $token): bool;

public function cancel(int $token): void;

public function weakMap(): ArrayAccess;
/**
* @return ArrayAccess<object, CallbackDestructor>
*/
public function destructors(): ArrayAccess;
}
2 changes: 0 additions & 2 deletions src/SDK/Metrics/ObservableCallback.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ public function __construct(MetricObserverInterface $metricObserver, ReferenceCo
$this->referenceCounter = $referenceCounter;
$this->token = $token;
$this->callbackDestructor = $callbackDestructor;

$this->referenceCounter->acquire();
}

public function detach(): void
Expand Down
3 changes: 2 additions & 1 deletion src/SDK/Metrics/ObservableInstrumentTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ public function observe(callable $callback, bool $weaken = false): ObservableCal

/** @psalm-var \Closure(ObserverInterface): void $callback */
$token = $this->metricObserver->observe($callback);
$this->referenceCounter->acquire();

$destructor = null;
if ($object = $target) {
$destructor = $this->metricObserver->weakMap()[$object] ??= new CallbackDestructor($this->metricObserver, $this->referenceCounter);
$destructor = $this->metricObserver->destructors()[$object] ??= new CallbackDestructor($this->metricObserver, $this->referenceCounter);
$destructor->tokens[$token] = $token;
}

Expand Down
5 changes: 0 additions & 5 deletions src/SDK/Metrics/View/ViewTemplate.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

namespace OpenTelemetry\SDK\Metrics\View;

use function assert;
use function is_string;
use OpenTelemetry\SDK\Metrics\AggregationInterface;
use OpenTelemetry\SDK\Metrics\Instrument;
use OpenTelemetry\SDK\Metrics\ViewProjection;
Expand Down Expand Up @@ -68,9 +66,6 @@ public function withAggregation(?AggregationInterface $aggregation): self

public function project(Instrument $instrument): ViewProjection
{
$instrumentType = $instrument->type;
assert(is_string($instrumentType));

return new ViewProjection(
$this->name ?? $instrument->name,
$instrument->unit,
Expand Down

0 comments on commit 5c99ad0

Please sign in to comment.