Add instrument advisory parameter (#1186)

* 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

src/API/Metrics/MeterInterface.php

@@ -13,6 +13,7 @@ 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
@@ -20,7 +21,8 @@ interface MeterInterface
     public function createCounter(
         string $name,
         ?string $unit = null,
-        ?string $description = null
+        ?string $description = null,
+        array $advisory = []
     ): CounterInterface;
 
     /**
@@ -29,6 +31,8 @@ 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|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
      *
@@ -38,6 +42,7 @@ interface MeterInterface
         string $name,
         ?string $unit = null,
         ?string $description = null,
+        $advisory = [],
         callable ...$callbacks
     ): ObservableCounterInterface;
 
@@ -47,6 +52,8 @@ 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, 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
@@ -54,7 +61,8 @@ interface MeterInterface
     public function createHistogram(
         string $name,
         ?string $unit = null,
-        ?string $description = null
+        ?string $description = null,
+        array $advisory = []
     ): HistogramInterface;
 
     /**
@@ -63,6 +71,8 @@ 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|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
      *
@@ -72,6 +82,7 @@ interface MeterInterface
         string $name,
         ?string $unit = null,
         ?string $description = null,
+        $advisory = [],
         callable ...$callbacks
     ): ObservableGaugeInterface;
 
@@ -81,6 +92,7 @@ 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 UpDownCounterInterface created instrument
      *
      * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/api.md#updowncounter-creation
@@ -88,7 +100,8 @@ interface MeterInterface
     public function createUpDownCounter(
         string $name,
         ?string $unit = null,
-        ?string $description = null
+        ?string $description = null,
+        array $advisory = []
     ): UpDownCounterInterface;
 
     /**
@@ -97,6 +110,8 @@ 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|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
      *
@@ -106,6 +121,7 @@ interface MeterInterface
         string $name,
         ?string $unit = null,
         ?string $description = null,
+        $advisory = [],
         callable ...$callbacks
     ): ObservableUpDownCounterInterface;
 }

src/API/Metrics/Noop/NoopMeter.php

@@ -14,32 +14,32 @@ use OpenTelemetry\API\Metrics\UpDownCounterInterface;
 
 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();
     }

src/API/composer.json

@@ -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\\": "."

src/SDK/Metrics/DefaultAggregationProviderInterface.php

@@ -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;
 }

src/SDK/Metrics/DefaultAggregationProviderTrait.php

@@ -6,7 +6,7 @@ namespace OpenTelemetry\SDK\Metrics;
 
 trait DefaultAggregationProviderTrait
 {
-    public function defaultAggregation($instrumentType): ?AggregationInterface
+    public function defaultAggregation($instrumentType, array $advisory = []): ?AggregationInterface
     {
         switch ($instrumentType) {
             case InstrumentType::COUNTER:
@@ -16,7 +16,7 @@ trait DefaultAggregationProviderTrait
             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();
         }

src/SDK/Metrics/Instrument.php

@@ -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;
     }
 }

src/SDK/Metrics/Meter.php

@@ -4,7 +4,9 @@ declare(strict_types=1);
 
 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;
@@ -75,25 +77,31 @@ final class Meter implements MeterInterface
         $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) {
@@ -104,25 +112,31 @@ final class Meter implements MeterInterface
         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) {
@@ -133,25 +147,31 @@ final class Meter implements MeterInterface
         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) {
@@ -166,9 +186,9 @@ final class Meter implements MeterInterface
      * @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);
@@ -213,9 +233,9 @@ final class Meter implements MeterInterface
      * @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);
@@ -293,7 +313,8 @@ final class Meter implements MeterInterface
                             $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),
                     ];
@@ -309,6 +330,11 @@ final class Meter implements MeterInterface
 
     private function instrumentId(Instrument $instrument): string
     {
-        return serialize($instrument);
+        return serialize([
+            $instrument->type,
+            $instrument->name,
+            $instrument->unit,
+            $instrument->description,
+        ]);
     }
 }

src/SDK/Metrics/MetricReader/ExportingReader.php

@@ -41,13 +41,14 @@ final class ExportingReader implements MetricReaderInterface, MetricSourceRegist
         $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

tests/Unit/SDK/Metrics/MeterTest.php

@@ -69,6 +69,37 @@ final class MeterTest extends TestCase
         $meter->createHistogram('name', 'unit', 'description');
     }
 
+    public function test_create_histogram_advisory(): void
+    {
+        $metricFactory = $this->createMock(MetricFactoryInterface::class);
+        $metricFactory->expects($this->once())->method('createSynchronousWriter')
+            ->with(
+                $this->anything(),
+                $this->anything(),
+                new InstrumentationScope('test', null, null, Attributes::create([])),
+                new Instrument(
+                    InstrumentType::HISTOGRAM,
+                    'http.server.duration',
+                    '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]],
+                ),
+                $this->anything(),
+                $this->anything(),
+                $this->anything(),
+            )
+            ->willReturn([]);
+
+        $meterProvider = $this->createMeterProviderForMetricFactory($metricFactory);
+        $meter = $meterProvider->getMeter('test');
+        $meter->createHistogram(
+            'http.server.duration',
+            '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]],
+        );
+    }
+
     public function test_create_up_down_counter(): void
     {
         $metricFactory = $this->createMock(MetricFactoryInterface::class);

tests/Unit/SDK/Metrics/MetricReader/ExportingReaderTest.php

@@ -53,6 +53,17 @@ final class ExportingReaderTest extends TestCase
         $this->assertEquals(new LastValueAggregation(), $reader->defaultAggregation(InstrumentType::ASYNCHRONOUS_GAUGE));
     }
 
+    public function test_default_aggregation_returns_histogram_with_advisory_buckets(): void
+    {
+        $exporter = new InMemoryExporter();
+        $reader = new ExportingReader($exporter);
+
+        $this->assertEquals(
+            new ExplicitBucketHistogramAggregation([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]),
+            $reader->defaultAggregation(InstrumentType::HISTOGRAM, ['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]]),
+        );
+    }
+
     public function test_default_aggregation_returns_exporter_aggregation_if_default_aggregation_provider(): void
     {
         $exporter = $this->createMock(DefaultAggregationProviderExporterInterface::class);