chore: remove the TaggableCollectorInterface

lyrixx · Mar 1, 2024 · 5e301ff · 5e301ff
1 parent ee76a70
commit 5e301ff
Show file tree

Hide file tree

Showing 10 changed files with 144 additions and 148 deletions.
diff --git a/README.md b/README.md
@@ -86,6 +86,10 @@ beberlei_metrics:
         influxdb:
             type: influxdb
             database: metrics
+            # host: localhost # option
+            # username: username # optional
+            # password: password # optional
+            # port: 8086 # optional
             # If you want to use a custom database service
             # It must be an instance of "InfluxDB\Database"
             # In this case, you can omit de "database" option

diff --git a/src/Metrics/Collector/InfluxDbV1.php b/src/Metrics/Collector/InfluxDbV1.php
@@ -13,7 +13,7 @@
 use InfluxDB\Exception;
 use InfluxDB\Point;
 
-class InfluxDbV1 implements CollectorInterface, TaggableCollectorInterface
+class InfluxDbV1 implements CollectorInterface
 {
     private array $data = [];
 
@@ -61,9 +61,4 @@ public function flush(): void
 
         $this->data = [];
     }
-
-    public function setTags(array $tags): void
-    {
-        $this->tags = $tags;
-    }
 }
diff --git a/src/Metrics/Collector/NullCollector.php b/src/Metrics/Collector/NullCollector.php
@@ -9,7 +9,7 @@
 
 namespace Beberlei\Metrics\Collector;
 
-class NullCollector implements CollectorInterface, GaugeableCollectorInterface, TaggableCollectorInterface
+class NullCollector implements CollectorInterface, GaugeableCollectorInterface
 {
     public function increment(string $variable, array $tags = []): void
     {
@@ -34,8 +34,4 @@ public function gauge(string $variable, string|int $value, array $tags = []): vo
     public function flush(): void
     {
     }
-
-    public function setTags(array $tags): void
-    {
-    }
 }
diff --git a/src/Metrics/Collector/Prometheus.php b/src/Metrics/Collector/Prometheus.php
@@ -11,15 +11,15 @@
 
 use Prometheus\CollectorRegistry;
 
-class Prometheus implements CollectorInterface, GaugeableCollectorInterface, TaggableCollectorInterface
+class Prometheus implements CollectorInterface, GaugeableCollectorInterface
 {
     private array $counters = [];
     private array $gauges = [];
 
     public function __construct(
         private readonly CollectorRegistry $registry,
         private readonly string $namespace = '',
-        private array $tags = [],
+        private readonly array $tags = [],
     ) {
     }
 
@@ -76,11 +76,6 @@ public function flush(): void
         $this->counters = $this->gauges = [];
     }
 
-    public function setTags(array $tags): void
-    {
-        $this->tags = $tags;
-    }
-
     private function normalizeVariable(string $variable): string
     {
         return str_replace(['.', ':'], ['_', '_'], $variable);

diff --git a/src/Metrics/Collector/TaggableCollectorInterface.php b/src/Metrics/Collector/TaggableCollectorInterface.php
diff --git a/src/Metrics/Collector/Telegraf.php b/src/Metrics/Collector/Telegraf.php
@@ -16,16 +16,19 @@
  * ad hoc implementation for the StatsD - Telegraf integration,
  * support tagging.
  */
-class Telegraf implements CollectorInterface, GaugeableCollectorInterface, TaggableCollectorInterface
+class Telegraf implements CollectorInterface, GaugeableCollectorInterface
 {
     private array $data = [];
+    private string $tags;
 
     public function __construct(
         private readonly string $host = 'localhost',
         private readonly int $port = 8125,
         private readonly string $prefix = '',
-        private string $tags = '',
+        array $tags = [],
     ) {
+        $this->tags = http_build_query($tags, '', ',');
+        $this->tags = \strlen($this->tags) > 0 ? ',' . $this->tags : $this->tags;
     }
 
     public function measure(string $variable, int $value, array $tags = []): void
@@ -67,12 +70,6 @@ public function flush(): void
         Box::box($this->doFlush(...));
     }
 
-    public function setTags(array $tags): void
-    {
-        $this->tags = http_build_query($tags, '', ',');
-        $this->tags = \strlen($this->tags) > 0 ? ',' . $this->tags : $this->tags;
-    }
-
     private function doFlush(): void
     {
         $fp = fsockopen('udp://' . $this->host, $this->port, $errno, $errstr, 1.0);

diff --git a/src/MetricsBundle/DependencyInjection/BeberleiMetricsExtension.php b/src/MetricsBundle/DependencyInjection/BeberleiMetricsExtension.php
@@ -77,76 +77,82 @@ private function createCollector(ContainerBuilder $container, string $name, stri
         $definition->addTag(CollectorInterface::class);
         $definition->addTag('kernel.reset', ['method' => 'flush']);
 
-        if ($config['tags'] ?? []) {
-            $definition->addMethodCall('setTags', [$config['tags']]);
-        }
+        $tags = $config['tags'] ?? [];
 
         switch ($type) {
-            case 'doctrine_dbal':
-                $ref = $config['connection'] ? sprintf('doctrine.dbal.%s_connection', $config['connection']) : 'database_connection';
-                $definition->replaceArgument(0, new Reference($ref));
-
-                return $definition;
-            case 'graphite':
-                $definition->replaceArgument(0, $config['host']);
-                $definition->replaceArgument(1, $config['port'] ?? 2003);
-                $definition->replaceArgument(2, $config['protocol'] ?? 'tcp');
-
-                return $definition;
             case 'influxdb_v1':
                 if (!class_exists(\InfluxDB\Client::class)) {
                     throw new \LogicException('The "influxdb/influxdb-php" package is required to use the "influxdb" collector.');
                 }
+
                 if ($config['service']) {
-                    $definition->replaceArgument(0, new Reference($config['service']));
+                    $database = new Reference($config['service']);
                 } else {
                     $database = new ChildDefinition('beberlei_metrics.collector_proto.influxdb_v1.database');
-                    $database->replaceArgument(0, sprintf('influxdb://%s:%s@%s:%s/%s',
+                    $database->replaceArgument('$dsn', sprintf('influxdb://%s:%s@%s:%s/%s',
                         $config['username'],
                         $config['password'],
                         $config['host'],
                         $config['port'] ?? 8086,
                         $config['database'],
                     ));
-                    $definition->replaceArgument(0, $database);
                 }
 
-                return $definition;
-            case 'logger':
-            case 'null':
-            case 'memory':
+                $definition->replaceArgument('$database', $database);
+                $definition->replaceArgument('$tags', $tags);
+
                 return $definition;
             case 'prometheus':
                 if (!class_exists(CollectorRegistry::class)) {
                     throw new \LogicException('The "promphp/prometheus_client_php" package is required to use the "prometheus" collector.');
                 }
+
                 if ($config['service']) {
-                    $definition->replaceArgument(0, new Reference($config['service']));
+                    $registryId = $config['service'];
                 } else {
-                    $database = new ChildDefinition('beberlei_metrics.collector_proto.prometheus.registry');
-                    $container->setDefinition($id = 'beberlei_metrics.collector.' . $name . '.prometheus.registry', $database);
+                    $container->setDefinition(
+                        $registryId = 'beberlei_metrics.collector.' . $name . '.prometheus.registry',
+                        new ChildDefinition('beberlei_metrics.collector_proto.prometheus.registry'),
+                    );
 
-                    $definition->replaceArgument(0, new Reference($id));
                     if (!$container->hasAlias(CollectorRegistry::class)) {
-                        $container->setAlias(CollectorRegistry::class, $id);
+                        $container->setAlias(CollectorRegistry::class, $registryId);
                     }
                 }
 
-                $definition->replaceArgument(1, $config['namespace']);
+                $definition->replaceArgument('$registry', new Reference($registryId));
+                $definition->replaceArgument('$namespace', $config['namespace']);
+                $definition->replaceArgument('$tags', $tags);
+
+                return $definition;
+            case 'graphite':
+                $definition->replaceArgument('$host', $config['host']);
+                $definition->replaceArgument('$port', $config['port'] ?? 2003);
+                $definition->replaceArgument('$protocol', $config['protocol'] ?? 'tcp');
 
                 return $definition;
             case 'statsd':
             case 'dogstatsd':
-                $definition->replaceArgument(0, $config['host']);
-                $definition->replaceArgument(1, $config['port'] ?? 8125);
-                $definition->replaceArgument(2, $config['prefix']);
+                $definition->replaceArgument('$host', $config['host']);
+                $definition->replaceArgument('$port', $config['port'] ?? 8125);
+                $definition->replaceArgument('$prefix', $config['prefix']);
 
                 return $definition;
             case 'telegraf':
-                $definition->replaceArgument(0, $config['host']);
-                $definition->replaceArgument(1, $config['port'] ?? 8125);
-                $definition->replaceArgument(2, $config['prefix']);
+                $definition->replaceArgument('$host', $config['host']);
+                $definition->replaceArgument('$port', $config['port'] ?? 8125);
+                $definition->replaceArgument('$prefix', $config['prefix']);
+                $definition->replaceArgument('$tags', $tags);
 
+                return $definition;
+            case 'doctrine_dbal':
+                $ref = $config['connection'] ? sprintf('doctrine.dbal.%s_connection', $config['connection']) : 'database_connection';
+                $definition->replaceArgument('$conn', new Reference($ref));
+
+                return $definition;
+            case 'logger':
+            case 'memory':
+            case 'null':
                 return $definition;
             default:
                 throw new \InvalidArgumentException(sprintf('The type "%s" is not supported.', $type));

diff --git a/src/MetricsBundle/Resources/config/metrics.xml b/src/MetricsBundle/Resources/config/metrics.xml
@@ -7,19 +7,19 @@
     <services>
         <!-- Prototype / Collector -->
         <service id="beberlei_metrics.collector_proto.doctrine_dbal" class="Beberlei\Metrics\Collector\DoctrineDBAL" abstract="true">
-            <argument /> <!-- Doctrine DBAL connection, set by the extension -->
+            <argument key="$conn" type="abstract">should be defined the extension</argument>
         </service>
         <service id="beberlei_metrics.collector_proto.graphite" class="Beberlei\Metrics\Collector\Graphite" abstract="true">
-            <argument /> <!-- host, set by the extension -->
-            <argument /> <!-- port, set by the extension -->
-            <argument /> <!-- protocol, set by the extension -->
+            <argument key="$host" type="abstract">should be defined the extension</argument>
+            <argument key="$port" type="abstract">should be defined the extension</argument>
+            <argument key="$protocol" type="abstract">should be defined the extension</argument>
         </service>
         <service id="beberlei_metrics.collector_proto.influxdb_v1.database" class="InfluxDB\Database" abstract="true">
             <factory class="InfluxDB\Client" method="fromDSN" />
-            <argument /> <!-- InfluxDB DSN, set by the extension -->
+            <argument key="$dsn" type="abstract">should be defined the extension</argument>
         </service>
         <service id="beberlei_metrics.collector_proto.influxdb_v1" class="Beberlei\Metrics\Collector\InfluxDbV1" abstract="true">
-            <argument /> <!-- InfluxDB dataase, set by the extension -->
+            <argument key="$database" type="abstract">should be defined the extension</argument>
         </service>
         <service id="beberlei_metrics.collector_proto.logger" class="Beberlei\Metrics\Collector\Logger" abstract="true">
             <argument type="service" id="logger" />
@@ -33,23 +33,25 @@
             </argument>
         </service>
         <service id="beberlei_metrics.collector_proto.prometheus" class="Beberlei\Metrics\Collector\Prometheus" abstract="true">
-            <argument /> <!-- Prometheus collector registry, set by the extension -->
-            <argument /> <!-- Prometheus namespace, set by the extension -->
+            <argument key="$registry" type="abstract">should be defined the extension</argument>
+            <argument key="$namespace" type="abstract">should be defined the extension</argument>
+            <argument key="$tags" type="abstract">should be defined the extension</argument>
         </service>
         <service id="beberlei_metrics.collector_proto.statsd" class="Beberlei\Metrics\Collector\StatsD" abstract="true">
-            <argument /> <!-- host, set by the extension -->
-            <argument /> <!-- port, set by the extension -->
-            <argument /> <!-- prefix, set by the extension -->
+            <argument key="$host" type="abstract">should be defined the extension</argument>
+            <argument key="$port" type="abstract">should be defined the extension</argument>
+            <argument key="$prefix" type="abstract">should be defined the extension</argument>
         </service>
         <service id="beberlei_metrics.collector_proto.dogstatsd" class="Beberlei\Metrics\Collector\DogStatsD" abstract="true">
-            <argument /> <!-- host, set by the extension -->
-            <argument /> <!-- port, set by the extension -->
-            <argument /> <!-- prefix, set by the extension -->
+            <argument key="$host" type="abstract">should be defined the extension</argument>
+            <argument key="$port" type="abstract">should be defined the extension</argument>
+            <argument key="$prefix" type="abstract">should be defined the extension</argument>
         </service>
         <service id="beberlei_metrics.collector_proto.telegraf" class="Beberlei\Metrics\Collector\Telegraf" abstract="true">
-            <argument /> <!-- host, set by the extension -->
-            <argument /> <!-- port, set by the extension -->
-            <argument /> <!-- prefix, set by the extension -->
+            <argument key="$host" type="abstract">should be defined the extension</argument>
+            <argument key="$port" type="abstract">should be defined the extension</argument>
+            <argument key="$prefix" type="abstract">should be defined the extension</argument>
+            <argument key="$tags" type="abstract">should be defined the extension</argument>
         </service>
         <service id="beberlei_metrics.collector_proto.memory" class="Beberlei\Metrics\Collector\InMemory" abstract="true">
         </service>