Skip to content

Commit 82f0c44

Browse files
authored
move temporality into own interface (#1098)
per TC review, temporality is not part of MetricExporterInterface. Riffing on Java's solution, this moves `temporality` into `AggregationTemporalitySelectorInterface`, and adds that interface to MetricExporter instances that use it (everything except `NoopMetricExporter`).
1 parent 55c08f3 commit 82f0c44

File tree

9 files changed

+39
-27
lines changed

9 files changed

+39
-27
lines changed

src/Contrib/Otlp/MetricExporter.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use OpenTelemetry\API\Behavior\LogsMessagesTrait;
88
use Opentelemetry\Proto\Collector\Metrics\V1\ExportMetricsServiceResponse;
99
use OpenTelemetry\SDK\Common\Export\TransportInterface;
10+
use OpenTelemetry\SDK\Metrics\AggregationTemporalitySelectorInterface;
1011
use OpenTelemetry\SDK\Metrics\Data\Temporality;
1112
use OpenTelemetry\SDK\Metrics\MetricMetadataInterface;
1213
use OpenTelemetry\SDK\Metrics\PushMetricExporterInterface;
@@ -18,7 +19,7 @@
1819
* @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/experimental/serialization/json.md#json-file-serialization
1920
* @psalm-import-type SUPPORTED_CONTENT_TYPES from ProtobufSerializer
2021
*/
21-
final class MetricExporter implements PushMetricExporterInterface
22+
final class MetricExporter implements PushMetricExporterInterface, AggregationTemporalitySelectorInterface
2223
{
2324
use LogsMessagesTrait;
2425

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OpenTelemetry\SDK\Metrics;
6+
7+
use OpenTelemetry\SDK\Metrics\Data\Temporality;
8+
9+
interface AggregationTemporalitySelectorInterface
10+
{
11+
/**
12+
* Returns the temporality to use for the given metric.
13+
*
14+
* It is recommended to return {@see MetricMetadataInterface::temporality()}
15+
* if the exporter does not require a specific temporality.
16+
*
17+
* @return string|Temporality|null temporality to use, or null to signal
18+
* that the given metric should not be exported by this exporter
19+
*/
20+
public function temporality(MetricMetadataInterface $metric);
21+
}

src/SDK/Metrics/MetricExporter/ConsoleMetricExporter.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace OpenTelemetry\SDK\Metrics\MetricExporter;
66

77
use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeInterface;
8+
use OpenTelemetry\SDK\Metrics\AggregationTemporalitySelectorInterface;
89
use OpenTelemetry\SDK\Metrics\Data\Metric;
910
use OpenTelemetry\SDK\Metrics\Data\Temporality;
1011
use OpenTelemetry\SDK\Metrics\MetricMetadataInterface;
@@ -15,7 +16,7 @@
1516
* Console metrics exporter.
1617
* Note that the output is human-readable JSON, not compatible with OTLP.
1718
*/
18-
class ConsoleMetricExporter implements PushMetricExporterInterface
19+
class ConsoleMetricExporter implements PushMetricExporterInterface, AggregationTemporalitySelectorInterface
1920
{
2021
/**
2122
* @var string|Temporality|null

src/SDK/Metrics/MetricExporter/InMemoryExporter.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace OpenTelemetry\SDK\Metrics\MetricExporter;
66

77
use function array_push;
8+
use OpenTelemetry\SDK\Metrics\AggregationTemporalitySelectorInterface;
89
use OpenTelemetry\SDK\Metrics\Data\Metric;
910
use OpenTelemetry\SDK\Metrics\Data\Temporality;
1011
use OpenTelemetry\SDK\Metrics\MetricExporterInterface;
@@ -13,7 +14,7 @@
1314
/**
1415
* @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk_exporters/in-memory.md
1516
*/
16-
final class InMemoryExporter implements MetricExporterInterface
17+
final class InMemoryExporter implements MetricExporterInterface, AggregationTemporalitySelectorInterface
1718
{
1819
/**
1920
* @var list<Metric>

src/SDK/Metrics/MetricExporter/NoopMetricExporter.php

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,9 @@
55
namespace OpenTelemetry\SDK\Metrics\MetricExporter;
66

77
use OpenTelemetry\SDK\Metrics\MetricExporterInterface;
8-
use OpenTelemetry\SDK\Metrics\MetricMetadataInterface;
98

109
class NoopMetricExporter implements MetricExporterInterface
1110
{
12-
13-
/**
14-
* @inheritDoc
15-
*/
16-
public function temporality(MetricMetadataInterface $metric)
17-
{
18-
return $metric->temporality();
19-
}
20-
2111
/**
2212
* @inheritDoc
2313
*/

src/SDK/Metrics/MetricExporterInterface.php

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,9 @@
55
namespace OpenTelemetry\SDK\Metrics;
66

77
use OpenTelemetry\SDK\Metrics\Data\Metric;
8-
use OpenTelemetry\SDK\Metrics\Data\Temporality;
98

109
interface MetricExporterInterface
1110
{
12-
/**
13-
* Returns the temporality to use for the given metric.
14-
*
15-
* It is recommended to return {@see MetricMetadataInterface::temporality()}
16-
* if the exporter does not require a specific temporality.
17-
*
18-
* @return string|Temporality|null temporality to use, or null to signal
19-
* that the given metric should not be exported by this exporter
20-
*/
21-
public function temporality(MetricMetadataInterface $metric);
22-
2311
/**
2412
* @param iterable<int, Metric> $batch
2513
*/

src/SDK/Metrics/MetricReader/ExportingReader.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use function array_keys;
88
use OpenTelemetry\SDK\Metrics\AggregationInterface;
9+
use OpenTelemetry\SDK\Metrics\AggregationTemporalitySelectorInterface;
910
use OpenTelemetry\SDK\Metrics\DefaultAggregationProviderInterface;
1011
use OpenTelemetry\SDK\Metrics\DefaultAggregationProviderTrait;
1112
use OpenTelemetry\SDK\Metrics\MetricExporterInterface;
@@ -54,6 +55,9 @@ public function add(MetricSourceProviderInterface $provider, MetricMetadataInter
5455
if ($this->closed) {
5556
return;
5657
}
58+
if (!$this->exporter instanceof AggregationTemporalitySelectorInterface) {
59+
return;
60+
}
5761
if (!$temporality = $this->exporter->temporality($metadata)) {
5862
return;
5963
}

tests/Unit/Contrib/Otlp/MetricExporterFactoryTest.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OpenTelemetry\SDK\Common\Configuration\Variables;
1111
use OpenTelemetry\SDK\Common\Export\TransportFactoryInterface;
1212
use OpenTelemetry\SDK\Common\Export\TransportInterface;
13+
use OpenTelemetry\SDK\Metrics\AggregationTemporalitySelectorInterface;
1314
use OpenTelemetry\SDK\Metrics\Data\Temporality;
1415
use OpenTelemetry\SDK\Metrics\MetricMetadataInterface;
1516
use PHPUnit\Framework\TestCase;
@@ -59,6 +60,7 @@ public function test_create_with_temporality(array $env, ?string $expected): voi
5960
$factory = new MetricExporterFactory($this->transportFactory);
6061
$exporter = $factory->create();
6162

63+
$this->assertInstanceOf(AggregationTemporalitySelectorInterface::class, $exporter);
6264
$this->assertSame($expected, $exporter->temporality($this->createMock(MetricMetadataInterface::class)));
6365
}
6466

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use OpenTelemetry\SDK\Metrics\Aggregation\ExplicitBucketHistogramAggregation;
99
use OpenTelemetry\SDK\Metrics\Aggregation\LastValueAggregation;
1010
use OpenTelemetry\SDK\Metrics\Aggregation\SumAggregation;
11+
use OpenTelemetry\SDK\Metrics\AggregationTemporalitySelectorInterface;
1112
use OpenTelemetry\SDK\Metrics\Data\DataInterface;
1213
use OpenTelemetry\SDK\Metrics\Data\Metric;
1314
use OpenTelemetry\SDK\Metrics\Data\Temporality;
@@ -78,7 +79,6 @@ public function test_add_creates_metric_source_with_exporter_temporality(): void
7879
public function test_add_does_not_create_metric_source_if_exporter_temporality_null(): void
7980
{
8081
$exporter = $this->createMock(MetricExporterInterface::class);
81-
$exporter->method('temporality')->willReturn(null);
8282
$reader = new ExportingReader($exporter);
8383

8484
$provider = $this->createMock(MetricSourceProviderInterface::class);
@@ -168,7 +168,7 @@ public function test_shutdown_does_not_export_empty_metrics(): void
168168

169169
public function test_shutdown_exports_metrics(): void
170170
{
171-
$exporter = $this->createMock(MetricExporterInterface::class);
171+
$exporter = $this->createMock(MetricExporterWithTemporalityInterface::class);
172172
$provider = $this->createMock(MetricSourceProviderInterface::class);
173173
$source = $this->createMock(MetricSourceInterface::class);
174174
$source->method('collect')->willReturn($this->createMock(Metric::class));
@@ -224,3 +224,7 @@ public function test_closed_reader_does_not_call_exporter_methods(): void
224224
interface DefaultAggregationProviderExporterInterface extends MetricExporterInterface, DefaultAggregationProviderInterface
225225
{
226226
}
227+
228+
interface MetricExporterWithTemporalityInterface extends MetricExporterInterface, AggregationTemporalitySelectorInterface
229+
{
230+
}

0 commit comments

Comments
 (0)