From 2a0a9e33e6586458977e15da9ba77e958ff177ab Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 16 Feb 2023 10:00:14 +1100 Subject: [PATCH] fixes and tests for MetricExporterFactory (#926) Fixing grpc path and allowing compression from MetricExporerFactory --- src/Contrib/Otlp/MetricExporterFactory.php | 69 +++++--- .../Otlp/MetricExporterFactoryTest.php | 162 ++++++++++++++++++ .../Contrib/Otlp/SpanExporterFactoryTest.php | 4 +- 3 files changed, 211 insertions(+), 24 deletions(-) create mode 100644 tests/Unit/Contrib/Otlp/MetricExporterFactoryTest.php diff --git a/src/Contrib/Otlp/MetricExporterFactory.php b/src/Contrib/Otlp/MetricExporterFactory.php index c48bf84c..c8690fd2 100644 --- a/src/Contrib/Otlp/MetricExporterFactory.php +++ b/src/Contrib/Otlp/MetricExporterFactory.php @@ -6,16 +6,26 @@ namespace OpenTelemetry\Contrib\Otlp; use OpenTelemetry\API\Common\Signal\Signals; use OpenTelemetry\SDK\Common\Configuration\Configuration; -use OpenTelemetry\SDK\Common\Configuration\KnownValues; +use OpenTelemetry\SDK\Common\Configuration\Defaults; use OpenTelemetry\SDK\Common\Configuration\Variables; +use OpenTelemetry\SDK\Common\Export\TransportFactoryInterface; use OpenTelemetry\SDK\Common\Export\TransportInterface; +use OpenTelemetry\SDK\Common\Otlp\HttpEndpointResolver; use OpenTelemetry\SDK\Metrics\MetricExporterFactoryInterface; use OpenTelemetry\SDK\Metrics\MetricExporterInterface; use OpenTelemetry\SDK\Registry; -use UnexpectedValueException; class MetricExporterFactory implements MetricExporterFactoryInterface { + private const DEFAULT_COMPRESSION = 'none'; + + private ?TransportFactoryInterface $transportFactory; + + public function __construct(?TransportFactoryInterface $transportFactory = null) + { + $this->transportFactory = $transportFactory; + } + /** * @psalm-suppress ArgumentTypeCoercion */ @@ -28,6 +38,9 @@ class MetricExporterFactory implements MetricExporterFactoryInterface return new MetricExporter($this->buildTransport($protocol)); } + /** + * @psalm-suppress UndefinedClass + */ private function buildTransport(string $protocol): TransportInterface { /** @@ -35,32 +48,44 @@ class MetricExporterFactory implements MetricExporterFactoryInterface * - OTEL_METRIC_EXPORT_INTERVAL * - OTEL_METRIC_EXPORT_TIMEOUT */ - $endpoint = Configuration::has(Variables::OTEL_EXPORTER_OTLP_METRICS_ENDPOINT) - ? Configuration::getString(Variables::OTEL_EXPORTER_OTLP_METRICS_ENDPOINT) - : Configuration::getString(Variables::OTEL_EXPORTER_OTLP_ENDPOINT); + $endpoint = $this->getEndpoint($protocol); $headers = Configuration::has(Variables::OTEL_EXPORTER_OTLP_METRICS_HEADERS) ? Configuration::getMap(Variables::OTEL_EXPORTER_OTLP_METRICS_HEADERS) : Configuration::getMap(Variables::OTEL_EXPORTER_OTLP_HEADERS); $headers += OtlpUtil::getUserAgentHeader(); + $compression = $this->getCompression(); - $factory = Registry::transportFactory($protocol); - switch ($protocol) { - case KnownValues::VALUE_GRPC: - return $factory->create( - $endpoint . OtlpUtil::method(Signals::METRICS), - ContentTypes::PROTOBUF, - $headers - ); - case KnownValues::VALUE_HTTP_PROTOBUF: - case KnownValues::VALUE_HTTP_JSON: - return $factory->create( - $endpoint, - Protocols::contentType($protocol), - $headers - ); - default: - throw new UnexpectedValueException('Unknown otlp protocol: ' . $protocol); + $factoryClass = Registry::transportFactory($protocol); + $factory = $this->transportFactory ?: new $factoryClass(); + + return $factory->create( + $endpoint, + Protocols::contentType($protocol), + $headers, + $compression, + ); + } + + private function getCompression(): string + { + return Configuration::has(Variables::OTEL_EXPORTER_OTLP_METRICS_COMPRESSION) ? + Configuration::getEnum(Variables::OTEL_EXPORTER_OTLP_METRICS_COMPRESSION) : + Configuration::getEnum(Variables::OTEL_EXPORTER_OTLP_COMPRESSION, self::DEFAULT_COMPRESSION); + } + + private function getEndpoint(string $protocol): string + { + if (Configuration::has(Variables::OTEL_EXPORTER_OTLP_METRICS_ENDPOINT)) { + return Configuration::getString(Variables::OTEL_EXPORTER_OTLP_METRICS_ENDPOINT); } + $endpoint = Configuration::has(Variables::OTEL_EXPORTER_OTLP_ENDPOINT) + ? Configuration::getString(Variables::OTEL_EXPORTER_OTLP_ENDPOINT) + : Defaults::OTEL_EXPORTER_OTLP_ENDPOINT; + if ($protocol === Protocols::GRPC) { + return $endpoint . OtlpUtil::method(Signals::METRICS); + } + + return HttpEndpointResolver::create()->resolveToString($endpoint, Signals::METRICS); } } diff --git a/tests/Unit/Contrib/Otlp/MetricExporterFactoryTest.php b/tests/Unit/Contrib/Otlp/MetricExporterFactoryTest.php new file mode 100644 index 00000000..14f4f192 --- /dev/null +++ b/tests/Unit/Contrib/Otlp/MetricExporterFactoryTest.php @@ -0,0 +1,162 @@ +transportFactory = $this->createMock(TransportFactoryInterface::class); + $this->transport = $this->createMock(TransportInterface::class); + } + + public function tearDown(): void + { + $this->restoreEnvironmentVariables(); + } + + public function test_unknown_protocol_exception(): void + { + $this->expectException(\RuntimeException::class); + $this->setEnvironmentVariable(Variables::OTEL_EXPORTER_OTLP_PROTOCOL, 'foo'); + $factory = new MetricExporterFactory(); + $factory->create(); + } + + /** + * @dataProvider configProvider + */ + public function test_create(array $env, string $endpoint, string $protocol, string $compression, array $headerKeys = []): void + { + foreach ($env as $k => $v) { + $this->setEnvironmentVariable($k, $v); + } + $factory = new MetricExporterFactory($this->transportFactory); + // @phpstan-ignore-next-line + $this->transportFactory + ->expects($this->once()) + ->method('create') + ->with( + $this->equalTo($endpoint), + $this->equalTo($protocol), + $this->callback(function ($headers) use ($headerKeys) { + $this->assertEqualsCanonicalizing($headerKeys, array_keys($headers)); + + return true; + }), + $this->equalTo($compression) + ) + ->willReturn($this->transport); + // @phpstan-ignore-next-line + $this->transport->method('contentType')->willReturn($protocol); + + $factory->create(); + } + + public function configProvider(): array + { + $defaultHeaderKeys = ['User-Agent']; + + return [ + 'signal-specific endpoint unchanged' => [ + 'env' => [ + Variables::OTEL_EXPORTER_OTLP_METRICS_PROTOCOL => KnownValues::VALUE_GRPC, + Variables::OTEL_EXPORTER_OTLP_METRICS_ENDPOINT => 'http://collector:4317/foo/bar', //should not be changed, per spec + ], + 'endpoint' => 'http://collector:4317/foo/bar', + 'protocol' => 'application/x-protobuf', + 'compression' => 'none', + 'headerKeys' => $defaultHeaderKeys, + ], + 'endpoint has path appended' => [ + 'env' => [ + Variables::OTEL_EXPORTER_OTLP_METRICS_PROTOCOL => KnownValues::VALUE_GRPC, + Variables::OTEL_EXPORTER_OTLP_ENDPOINT => 'http://collector:4317', + ], + 'endpoint' => 'http://collector:4317/opentelemetry.proto.collector.metrics.v1.MetricsService/Export', + 'protocol' => 'application/x-protobuf', + 'compression' => 'none', + 'headerKeys' => $defaultHeaderKeys, + ], + 'protocol' => [ + 'env' => [ + Variables::OTEL_EXPORTER_OTLP_PROTOCOL => KnownValues::VALUE_HTTP_NDJSON, + ], + 'endpoint' => 'http://localhost:4318/v1/metrics', + 'protocol' => 'application/x-ndjson', + 'compression' => 'none', + 'headerKeys' => $defaultHeaderKeys, + ], + 'signal-specific protocol' => [ + 'env' => [ + Variables::OTEL_EXPORTER_OTLP_PROTOCOL => KnownValues::VALUE_HTTP_JSON, + ], + 'endpoint' => 'http://localhost:4318/v1/metrics', + 'protocol' => 'application/json', + 'compression' => 'none', + 'headerKeys' => $defaultHeaderKeys, + ], + 'defaults' => [ + 'env' => [], + 'endpoint' => 'http://localhost:4318/v1/metrics', + 'protocol' => 'application/x-protobuf', + 'compression' => 'none', + 'headerKeys' => $defaultHeaderKeys, + ], + 'compression' => [ + 'env' => [ + Variables::OTEL_EXPORTER_OTLP_COMPRESSION => 'gzip', + ], + 'endpoint' => 'http://localhost:4318/v1/metrics', + 'protocol' => 'application/x-protobuf', + 'compression' => 'gzip', + 'headerKeys' => $defaultHeaderKeys, + ], + 'signal-specific compression' => [ + 'env' => [ + Variables::OTEL_EXPORTER_OTLP_METRICS_COMPRESSION => 'gzip', + ], + 'endpoint' => 'http://localhost:4318/v1/metrics', + 'protocol' => 'application/x-protobuf', + 'compression' => 'gzip', + 'headerKeys' => $defaultHeaderKeys, + ], + 'headers' => [ + 'env' => [ + Variables::OTEL_EXPORTER_OTLP_HEADERS => 'key1=foo,key2=bar', + ], + 'endpoint' => 'http://localhost:4318/v1/metrics', + 'protocol' => 'application/x-protobuf', + 'compression' => 'none', + 'headerKeys' => array_merge($defaultHeaderKeys, ['key1', 'key2']), + ], + 'signal-specific headers' => [ + 'env' => [ + Variables::OTEL_EXPORTER_OTLP_METRICS_HEADERS => 'key3=foo,key4=bar', + ], + 'endpoint' => 'http://localhost:4318/v1/metrics', + 'protocol' => 'application/x-protobuf', + 'compression' => 'none', + 'headerKeys' => array_merge($defaultHeaderKeys, ['key3', 'key4']), + ], + ]; + } +} diff --git a/tests/Unit/Contrib/Otlp/SpanExporterFactoryTest.php b/tests/Unit/Contrib/Otlp/SpanExporterFactoryTest.php index a258ac5f..2beb661d 100644 --- a/tests/Unit/Contrib/Otlp/SpanExporterFactoryTest.php +++ b/tests/Unit/Contrib/Otlp/SpanExporterFactoryTest.php @@ -80,9 +80,9 @@ class SpanExporterFactoryTest extends TestCase 'signal-specific endpoint unchanged' => [ 'env' => [ Variables::OTEL_EXPORTER_OTLP_TRACES_PROTOCOL => KnownValues::VALUE_GRPC, - Variables::OTEL_EXPORTER_OTLP_TRACES_ENDPOINT => 'http://collector:4317', //should not be changed, per spec + Variables::OTEL_EXPORTER_OTLP_TRACES_ENDPOINT => 'http://collector:4317/foo/bar', //should not be changed, per spec ], - 'endpoint' => 'http://collector:4317', + 'endpoint' => 'http://collector:4317/foo/bar', 'protocol' => 'application/x-protobuf', 'compression' => 'none', 'headerKeys' => $defaultHeaderKeys,