refactor(otel/exporter): use Zenstruck\Dsn&Uri for ExporterDsn

FriendsOfOpenTelemetry · Jan 4, 2024 · cc27a72 · cc27a72
1 parent a662315
commit cc27a72
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 70 deletions.
diff --git a/composer.json b/composer.json
@@ -46,7 +46,8 @@
         "open-telemetry/sem-conv": "^1.23",
         "symfony/config": "^6.4 || ^7.0",
         "symfony/dependency-injection": "^6.4 || ^7.0",
-        "symfony/http-client": "^6.4 || ^7.0"
+        "symfony/http-client": "^6.4 || ^7.0",
+        "zenstruck/dsn": "^0.2"
     },
     "require-dev": {
         "ext-ffi": "*",

diff --git a/src/OpenTelemetry/Exporter/ExporterDsn.php b/src/OpenTelemetry/Exporter/ExporterDsn.php
@@ -2,87 +2,75 @@
 
 namespace FriendsOfOpenTelemetry\OpenTelemetryBundle\OpenTelemetry\Exporter;
 
+use Zenstruck\Dsn;
+use Zenstruck\Uri;
+
 final class ExporterDsn
 {
-    public function __construct(
-        private readonly string $scheme,
-        private readonly string $host,
-        private readonly ?string $user = null,
-        #[\SensitiveParameter]
-        private readonly ?string $password = null,
-        private readonly ?int $port = null,
-        private readonly ?string $path = null,
-        /**
-         * @var array<string, mixed>
-         */
-        private readonly array $options = []
+    private function __construct(
+        private readonly Uri $uri,
     ) {
     }
 
     public static function fromString(#[\SensitiveParameter] string $dsn): self
     {
-        if (false === $parsedDsn = parse_url($dsn)) {
-            throw new \InvalidArgumentException('The DSN is invalid.');
+        try {
+            $parsedDsn = Dsn::parse($dsn);
+        } catch (Dsn\Exception\UnableToParse $exception) {
+            throw new \InvalidArgumentException('The DSN is invalid.', previous: $exception);
+        }
+
+        if (false === $parsedDsn instanceof Uri) {
+            throw new \InvalidArgumentException('The DSN is not an Uri.');
         }
 
-        if (!isset($parsedDsn['scheme'])) {
+        if (true === $parsedDsn->scheme()->isEmpty()) {
             throw new \InvalidArgumentException('The DSN must contain a scheme.');
         }
 
-        if (!isset($parsedDsn['host'])) {
+        if (true === $parsedDsn->host()->isEmpty()) {
             throw new \InvalidArgumentException('The DSN must contain a host (use "default" by default).');
         }
 
-        $user = '' !== ($parsedDsn['user'] ?? '') ? urldecode($parsedDsn['user']) : null;
-        $password = '' !== ($parsedDsn['pass'] ?? '') ? urldecode($parsedDsn['pass']) : null;
-        $port = $parsedDsn['port'] ?? null;
-        $path = $parsedDsn['path'] ?? null;
-        parse_str($parsedDsn['query'] ?? '', $query);
-
-        return new self($parsedDsn['scheme'], $parsedDsn['host'], $user, $password, $port, $path, $query);
+        return new self($parsedDsn);
     }
 
     public function getScheme(): string
     {
-        return $this->scheme;
+        return $this->uri->scheme()->toString();
     }
 
     public function getHost(): string
     {
-        return $this->host;
+        return $this->uri->host()->toString();
     }
 
     public function getUser(): ?string
     {
-        return $this->user;
+        return $this->uri->username();
     }
 
     public function getPassword(): ?string
     {
-        return $this->password;
+        return $this->uri->password();
     }
 
     public function getPath(): ?string
     {
-        return $this->path;
+        return $this->uri->path()->isEmpty() ? null : $this->uri->path()->toString();
     }
 
     public function getPort(int $default = null): ?int
     {
-        return $this->port ?? $default;
-    }
-
-    public function getOption(string $key, mixed $default = null): mixed
-    {
-        return $this->options[$key] ?? $default;
+        return $this->uri->port() ?? $default;
     }
 
     /**
      * @return string[]
      */
     private function parseScheme(): array
     {
-        return explode('+', $this->getScheme(), 2);
+        return $this->uri->scheme()->segments();
     }
 
     public function getExporter(): string

diff --git a/tests/Unit/OpenTelemetry/Exporter/ExporterDsnTest.php b/tests/Unit/OpenTelemetry/Exporter/ExporterDsnTest.php
@@ -10,62 +10,56 @@
  */
 class ExporterDsnTest extends TestCase
 {
-    public function testGetOption(): void
-    {
-        $options = ['format' => 'json', 'nullable' => null];
-        $dsn = new ExporterDsn(scheme: 'http+otlp', host: 'localhost', options: $options);
-
-        self::assertSame('json', $dsn->getOption('format'));
-        self::assertSame('default', $dsn->getOption('nullable', 'default'));
-        self::assertSame('default', $dsn->getOption('not_existent_property', 'default'));
-    }
-
     /**
      * @dataProvider fromStringProvider
      *
-     * @param array{transport?: string, exporter?: string} $explodedScheme
+     * @param array{transport?: string, exporter?: string} $expected
      */
-    public function testFromString(string $string, ExporterDsn $dsn, array $explodedScheme): void
+    public function testFromString(ExporterDsn $dsn, array $expected): void
     {
-        self::assertEquals($dsn, ExporterDsn::fromString($string));
-        self::assertEquals($explodedScheme, [
+        self::assertEquals($expected, [
             'transport' => $dsn->getTransport(),
             'exporter' => $dsn->getExporter(),
+            'user' => $dsn->getUser(),
+            'password' => $dsn->getPassword(),
+            'host' => $dsn->getHost(),
+            'port' => $dsn->getPort(),
+            'path' => $dsn->getPath(),
         ]);
     }
 
     /**
-     * @return iterable<string, array{0: string, 1: ExporterDsn, 2: array{transport: ?string, exporter: string}}>
+     * @return iterable<string, array{0: ExporterDsn, 1: array{
+     *     transport: ?string,
+     *     exporter: string,
+     *     user: ?string,
+     *     password: ?string,
+     *     host: string,
+     *     port: ?int,
+     *     path: ?string
+     * }}>
      */
     public static function fromStringProvider(): iterable
     {
         yield 'gRPC Transport, OTLP Exporter' => [
-            'grpc+otlp://localhost:4317',
-            new ExporterDsn('grpc+otlp', 'localhost', null, null, 4317),
-            ['transport' => 'grpc', 'exporter' => 'otlp'],
+            ExporterDsn::fromString('grpc+otlp://localhost:4317'),
+            ['transport' => 'grpc', 'exporter' => 'otlp', 'user' => null, 'password' => null, 'host' => 'localhost', 'port' => 4317, 'path' => null],
         ];
-        yield 'HTTP Transport, OTLP Exporter with JSON Content-Type, Compression GZIP' => [
-            'http+otlp://localhost:4318/v1/traces?content-type=application/json&compression=gzip',
-            new ExporterDsn('http+otlp', 'localhost', null, null, 4318, '/v1/traces', [
-                'content-type' => 'application/json',
-                'compression' => 'gzip',
-            ]),
-            ['transport' => 'http', 'exporter' => 'otlp'],
+        yield 'HTTP Transport, OTLP Exporter' => [
+            ExporterDsn::fromString('http+otlp://localhost:4318/v1/traces'),
+            ['transport' => 'http', 'exporter' => 'otlp', 'user' => null, 'password' => null, 'host' => 'localhost', 'port' => 4318, 'path' => '/v1/traces'],
         ];
         yield 'HTTP Transport, Zipkin Exporter with basic authentication' => [
-            'http+zipkin://user:password@localhost:9411/api/v2/spans',
-            new ExporterDsn('http+zipkin', 'localhost', 'user', 'password', 9411, '/api/v2/spans'),
-            ['transport' => 'http', 'exporter' => 'zipkin'],
+            ExporterDsn::fromString('http+zipkin://user:password@localhost:9411/api/v2/spans'),
+            ['transport' => 'http', 'exporter' => 'zipkin', 'user' => 'user', 'password' => 'password', 'host' => 'localhost', 'port' => 9411, 'path' => '/api/v2/spans'],
         ];
         yield 'Stream Transport, Console Exporter' => [
-            'stream+console://default',
-            new ExporterDsn('stream+console', 'default'),
-            ['transport' => 'stream', 'exporter' => 'console'],
+            ExporterDsn::fromString('stream+console://default'),
+            ['transport' => 'stream', 'exporter' => 'console', 'user' => null, 'password' => null, 'host' => 'default', 'port' => null, 'path' => null],
         ];
         yield 'Memory Exporter' => [
-            'memory://default',
-            new ExporterDsn('memory', 'default'),
-            ['transport' => null, 'exporter' => 'memory'],
+            ExporterDsn::fromString('memory://default'),
+            ['transport' => null, 'exporter' => 'memory', 'user' => null, 'password' => null, 'host' => 'default', 'port' => null, 'path' => null],
         ];
     }