Skip to content

Commit

Permalink
Add instrument advisory parameter (open-telemetry#1186)
Browse files Browse the repository at this point in the history
* Add instrument advisory parameter

* Undo bucket boundaries change to preserve BC

"SDKs SHOULD use the default value when boundaries are not explicitly provided, unless they have good reasons to use something different (e.g. for backward compatibility reasons in a stable SDK release)."

* Add note that passing callback instead of advisory is deprecated
  • Loading branch information
Nevay authored Dec 13, 2023
1 parent a6d15cf commit d376566
Show file tree
Hide file tree
Showing 10 changed files with 126 additions and 28 deletions.
22 changes: 19 additions & 3 deletions src/API/Metrics/MeterInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@ interface MeterInterface
* @param string $name name of the instrument
* @param string|null $unit unit of measure
* @param string|null $description description of the instrument
* @param array $advisory an optional set of recommendations
* @return CounterInterface created instrument
*
* @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#counter-creation
*/
public function createCounter(
string $name,
?string $unit = null,
?string $description = null
?string $description = null,
array $advisory = []
): CounterInterface;

/**
Expand All @@ -29,6 +31,8 @@ public function createCounter(
* @param string $name name of the instrument
* @param string|null $unit unit of measure
* @param string|null $description description of the instrument
* @param array|callable $advisory an optional set of recommendations, or
* deprecated: the first callback to report measurements
* @param callable ...$callbacks responsible for reporting measurements
* @return ObservableCounterInterface created instrument
*
Expand All @@ -38,6 +42,7 @@ public function createObservableCounter(
string $name,
?string $unit = null,
?string $description = null,
$advisory = [],
callable ...$callbacks
): ObservableCounterInterface;

Expand All @@ -47,14 +52,17 @@ public function createObservableCounter(
* @param string $name name of the instrument
* @param string|null $unit unit of measure
* @param string|null $description description of the instrument
* @param array $advisory an optional set of recommendations, e.g.
* <code>['ExplicitBucketBoundaries' => [0.25, 0.5, 1, 5]]</code>
* @return HistogramInterface created instrument
*
* @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#histogram-creation
*/
public function createHistogram(
string $name,
?string $unit = null,
?string $description = null
?string $description = null,
array $advisory = []
): HistogramInterface;

/**
Expand All @@ -63,6 +71,8 @@ public function createHistogram(
* @param string $name name of the instrument
* @param string|null $unit unit of measure
* @param string|null $description description of the instrument
* @param array|callable $advisory an optional set of recommendations, or
* deprecated: the first callback to report measurements
* @param callable ...$callbacks responsible for reporting measurements
* @return ObservableGaugeInterface created instrument
*
Expand All @@ -72,6 +82,7 @@ public function createObservableGauge(
string $name,
?string $unit = null,
?string $description = null,
$advisory = [],
callable ...$callbacks
): ObservableGaugeInterface;

Expand All @@ -81,14 +92,16 @@ public function createObservableGauge(
* @param string $name name of the instrument
* @param string|null $unit unit of measure
* @param string|null $description description of the instrument
* @param array $advisory an optional set of recommendations
* @return UpDownCounterInterface created instrument
*
* @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#updowncounter-creation
*/
public function createUpDownCounter(
string $name,
?string $unit = null,
?string $description = null
?string $description = null,
array $advisory = []
): UpDownCounterInterface;

/**
Expand All @@ -97,6 +110,8 @@ public function createUpDownCounter(
* @param string $name name of the instrument
* @param string|null $unit unit of measure
* @param string|null $description description of the instrument
* @param array|callable $advisory an optional set of recommendations, or
* deprecated: the first callback to report measurements
* @param callable ...$callbacks responsible for reporting measurements
* @return ObservableUpDownCounterInterface created instrument
*
Expand All @@ -106,6 +121,7 @@ public function createObservableUpDownCounter(
string $name,
?string $unit = null,
?string $description = null,
$advisory = [],
callable ...$callbacks
): ObservableUpDownCounterInterface;
}
12 changes: 6 additions & 6 deletions src/API/Metrics/Noop/NoopMeter.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,32 @@

final class NoopMeter implements MeterInterface
{
public function createCounter(string $name, ?string $unit = null, ?string $description = null): CounterInterface
public function createCounter(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): CounterInterface
{
return new NoopCounter();
}

public function createObservableCounter(string $name, ?string $unit = null, ?string $description = null, callable ...$callbacks): ObservableCounterInterface
public function createObservableCounter(string $name, ?string $unit = null, ?string $description = null, $advisory = [], callable ...$callbacks): ObservableCounterInterface
{
return new NoopObservableCounter();
}

public function createHistogram(string $name, ?string $unit = null, ?string $description = null): HistogramInterface
public function createHistogram(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): HistogramInterface
{
return new NoopHistogram();
}

public function createObservableGauge(string $name, ?string $unit = null, ?string $description = null, callable ...$callbacks): ObservableGaugeInterface
public function createObservableGauge(string $name, ?string $unit = null, ?string $description = null, $advisory = [], callable ...$callbacks): ObservableGaugeInterface
{
return new NoopObservableGauge();
}

public function createUpDownCounter(string $name, ?string $unit = null, ?string $description = null): UpDownCounterInterface
public function createUpDownCounter(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): UpDownCounterInterface
{
return new NoopUpDownCounter();
}

public function createObservableUpDownCounter(string $name, ?string $unit = null, ?string $description = null, callable ...$callbacks): ObservableUpDownCounterInterface
public function createObservableUpDownCounter(string $name, ?string $unit = null, ?string $description = null, $advisory = [], callable ...$callbacks): ObservableUpDownCounterInterface
{
return new NoopObservableUpDownCounter();
}
Expand Down
3 changes: 3 additions & 0 deletions src/API/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
"symfony/polyfill-php81": "^1.26",
"symfony/polyfill-php82": "^1.26"
},
"conflict": {
"open-telemetry/sdk": "<=1.0.1"
},
"autoload": {
"psr-4": {
"OpenTelemetry\\API\\": "."
Expand Down
6 changes: 5 additions & 1 deletion src/SDK/Metrics/DefaultAggregationProviderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ interface DefaultAggregationProviderInterface
{
/**
* @param string|InstrumentType $instrumentType
* @param array $advisory optional set of recommendations
*
* @noinspection PhpDocSignatureInspection not added for BC
* @phan-suppress PhanCommentParamWithoutRealParam @phpstan-ignore-next-line
*/
public function defaultAggregation($instrumentType): ?AggregationInterface;
public function defaultAggregation($instrumentType /*, array $advisory = [] */): ?AggregationInterface;
}
4 changes: 2 additions & 2 deletions src/SDK/Metrics/DefaultAggregationProviderTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

trait DefaultAggregationProviderTrait
{
public function defaultAggregation($instrumentType): ?AggregationInterface
public function defaultAggregation($instrumentType, array $advisory = []): ?AggregationInterface
{
switch ($instrumentType) {
case InstrumentType::COUNTER:
Expand All @@ -16,7 +16,7 @@ public function defaultAggregation($instrumentType): ?AggregationInterface
case InstrumentType::ASYNCHRONOUS_UP_DOWN_COUNTER:
return new Aggregation\SumAggregation();
case InstrumentType::HISTOGRAM:
return new Aggregation\ExplicitBucketHistogramAggregation([0, 5, 10, 25, 50, 75, 100, 250, 500, 1000]);
return new Aggregation\ExplicitBucketHistogramAggregation($advisory['ExplicitBucketBoundaries'] ?? [0, 5, 10, 25, 50, 75, 100, 250, 500, 1000]);
case InstrumentType::ASYNCHRONOUS_GAUGE:
return new Aggregation\LastValueAggregation();
}
Expand Down
8 changes: 7 additions & 1 deletion src/SDK/Metrics/Instrument.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,20 @@ final class Instrument
* @readonly
*/
public ?string $description;
/**
* @readonly
*/
public array $advisory;

/**
* @param string|InstrumentType $type
*/
public function __construct($type, string $name, ?string $unit, ?string $description)
public function __construct($type, string $name, ?string $unit, ?string $description, array $advisory = [])
{
$this->type = $type;
$this->name = $name;
$this->unit = $unit;
$this->description = $description;
$this->advisory = $advisory;
}
}
50 changes: 38 additions & 12 deletions src/SDK/Metrics/Meter.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

namespace OpenTelemetry\SDK\Metrics;

use function array_unshift;
use ArrayAccess;
use function is_callable;
use OpenTelemetry\API\Metrics\CounterInterface;
use OpenTelemetry\API\Metrics\HistogramInterface;
use OpenTelemetry\API\Metrics\MeterInterface;
Expand Down Expand Up @@ -75,25 +77,31 @@ public function __construct(
$this->writer = $writer;
}

public function createCounter(string $name, ?string $unit = null, ?string $description = null): CounterInterface
public function createCounter(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): CounterInterface
{
[$instrument, $referenceCounter] = $this->createSynchronousWriter(
InstrumentType::COUNTER,
$name,
$unit,
$description,
$advisory,
);

return new Counter($this->writer, $instrument, $referenceCounter);
}

public function createObservableCounter(string $name, ?string $unit = null, ?string $description = null, callable ...$callbacks): ObservableCounterInterface
public function createObservableCounter(string $name, ?string $unit = null, ?string $description = null, $advisory = [], callable ...$callbacks): ObservableCounterInterface
{
if (is_callable($advisory)) {
array_unshift($callbacks, $advisory);
$advisory = [];
}
[$instrument, $referenceCounter, $destructors] = $this->createAsynchronousObserver(
InstrumentType::ASYNCHRONOUS_COUNTER,
$name,
$unit,
$description,
$advisory,
);

foreach ($callbacks as $callback) {
Expand All @@ -104,25 +112,31 @@ public function createObservableCounter(string $name, ?string $unit = null, ?str
return new ObservableCounter($this->writer, $instrument, $referenceCounter, $destructors);
}

public function createHistogram(string $name, ?string $unit = null, ?string $description = null): HistogramInterface
public function createHistogram(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): HistogramInterface
{
[$instrument, $referenceCounter] = $this->createSynchronousWriter(
InstrumentType::HISTOGRAM,
$name,
$unit,
$description,
$advisory,
);

return new Histogram($this->writer, $instrument, $referenceCounter);
}

public function createObservableGauge(string $name, ?string $unit = null, ?string $description = null, callable ...$callbacks): ObservableGaugeInterface
public function createObservableGauge(string $name, ?string $unit = null, ?string $description = null, $advisory = [], callable ...$callbacks): ObservableGaugeInterface
{
if (is_callable($advisory)) {
array_unshift($callbacks, $advisory);
$advisory = [];
}
[$instrument, $referenceCounter, $destructors] = $this->createAsynchronousObserver(
InstrumentType::ASYNCHRONOUS_GAUGE,
$name,
$unit,
$description,
$advisory,
);

foreach ($callbacks as $callback) {
Expand All @@ -133,25 +147,31 @@ public function createObservableGauge(string $name, ?string $unit = null, ?strin
return new ObservableGauge($this->writer, $instrument, $referenceCounter, $destructors);
}

public function createUpDownCounter(string $name, ?string $unit = null, ?string $description = null): UpDownCounterInterface
public function createUpDownCounter(string $name, ?string $unit = null, ?string $description = null, array $advisory = []): UpDownCounterInterface
{
[$instrument, $referenceCounter] = $this->createSynchronousWriter(
InstrumentType::UP_DOWN_COUNTER,
$name,
$unit,
$description,
$advisory,
);

return new UpDownCounter($this->writer, $instrument, $referenceCounter);
}

public function createObservableUpDownCounter(string $name, ?string $unit = null, ?string $description = null, callable ...$callbacks): ObservableUpDownCounterInterface
public function createObservableUpDownCounter(string $name, ?string $unit = null, ?string $description = null, $advisory = [], callable ...$callbacks): ObservableUpDownCounterInterface
{
if (is_callable($advisory)) {
array_unshift($callbacks, $advisory);
$advisory = [];
}
[$instrument, $referenceCounter, $destructors] = $this->createAsynchronousObserver(
InstrumentType::ASYNCHRONOUS_UP_DOWN_COUNTER,
$name,
$unit,
$description,
$advisory,
);

foreach ($callbacks as $callback) {
Expand All @@ -166,9 +186,9 @@ public function createObservableUpDownCounter(string $name, ?string $unit = null
* @param string|InstrumentType $instrumentType
* @return array{Instrument, ReferenceCounterInterface}
*/
private function createSynchronousWriter($instrumentType, string $name, ?string $unit, ?string $description): array
private function createSynchronousWriter($instrumentType, string $name, ?string $unit, ?string $description, array $advisory = []): array
{
$instrument = new Instrument($instrumentType, $name, $unit, $description);
$instrument = new Instrument($instrumentType, $name, $unit, $description, $advisory);

$instrumentationScopeId = $this->instrumentationScopeId($this->instrumentationScope);
$instrumentId = $this->instrumentId($instrument);
Expand Down Expand Up @@ -213,9 +233,9 @@ private function createSynchronousWriter($instrumentType, string $name, ?string
* @param string|InstrumentType $instrumentType
* @return array{Instrument, ReferenceCounterInterface, ArrayAccess<object, ObservableCallbackDestructor>}
*/
private function createAsynchronousObserver($instrumentType, string $name, ?string $unit, ?string $description): array
private function createAsynchronousObserver($instrumentType, string $name, ?string $unit, ?string $description, array $advisory): array
{
$instrument = new Instrument($instrumentType, $name, $unit, $description);
$instrument = new Instrument($instrumentType, $name, $unit, $description, $advisory);

$instrumentationScopeId = $this->instrumentationScopeId($this->instrumentationScope);
$instrumentId = $this->instrumentId($instrument);
Expand Down Expand Up @@ -293,7 +313,8 @@ private function viewRegistrationRequests(Instrument $instrument, StalenessHandl
$view->unit,
$view->description,
$view->attributeKeys,
$metricRegistry->defaultAggregation($instrument->type),
/** @phan-suppress-next-line PhanParamTooMany @phpstan-ignore-next-line */
$metricRegistry->defaultAggregation($instrument->type, $instrument->advisory),
),
new RegistryRegistration($metricRegistry, $stalenessHandler),
];
Expand All @@ -309,6 +330,11 @@ private function instrumentationScopeId(InstrumentationScopeInterface $instrumen

private function instrumentId(Instrument $instrument): string
{
return serialize($instrument);
return serialize([
$instrument->type,
$instrument->name,
$instrument->unit,
$instrument->description,
]);
}
}
7 changes: 4 additions & 3 deletions src/SDK/Metrics/MetricReader/ExportingReader.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,14 @@ public function __construct(MetricExporterInterface $exporter)
$this->exporter = $exporter;
}

public function defaultAggregation($instrumentType): ?AggregationInterface
public function defaultAggregation($instrumentType, array $advisory = []): ?AggregationInterface
{
if ($this->exporter instanceof DefaultAggregationProviderInterface) {
return $this->exporter->defaultAggregation($instrumentType);
/** @phan-suppress-next-line PhanParamTooMany @phpstan-ignore-next-line */
return $this->exporter->defaultAggregation($instrumentType, $advisory);
}

return $this->_defaultAggregation($instrumentType);
return $this->_defaultAggregation($instrumentType, $advisory);
}

public function add(MetricSourceProviderInterface $provider, MetricMetadataInterface $metadata, StalenessHandlerInterface $stalenessHandler): void
Expand Down
Loading

0 comments on commit d376566

Please sign in to comment.