Merge branch 'release/1.6.0'

artprima · Dec 22, 2020 · 4fd3a01 · 4fd3a01
2 parents ff99c66 + 4555635
commit 4fd3a01
Show file tree

Hide file tree

Showing 27 changed files with 269 additions and 139 deletions.
diff --git a/.github/workflows/code-style.yml b/.github/workflows/code-style.yml
@@ -0,0 +1,43 @@
+name: Coding Style
+
+on:
+  push:
+    branches: [ master, develop ]
+  pull_request:
+    branches: [ master, develop ]
+
+jobs:
+  run:
+    runs-on: ubuntu-latest
+    name: PHP 7.4
+    steps:
+    - name: Checkout
+      uses: actions/checkout@v2
+
+    - name: Setup PHP
+      uses: shivammathur/setup-php@v2
+      with:
+        php-version: "7.4"
+        extensions: mbstring, intl
+        ini-values: post_max_size=256M
+        coverage: xdebug        
+        tools: php-cs-fixer, phpunit
+    - name: Get composer cache directory
+      id: composer-cache
+      run: echo "::set-output name=dir::$(composer config cache-files-dir)"
+    - name: Cache composer dependencies
+      uses: actions/cache@v2
+      with:
+        path: ${{ steps.composer-cache.outputs.dir }}
+        # Use composer.json for key, if composer.lock is not committed.
+        # key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}
+        key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
+        restore-keys: ${{ runner.os }}-composer-
+    - name: Install Composer dependencies
+      run: |
+        composer install --no-progress --prefer-dist --optimize-autoloader
+    - name: Check code style issues
+      run: |
+        vendor/bin/phpcs --config-set installed_paths vendor/escapestudios/symfony2-coding-standard
+        composer run-script php-cs-fixer-check
+        composer run-script phpcs-check
diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml
@@ -18,7 +18,7 @@ jobs:
     strategy:
       matrix:
         operating-system: [ubuntu-latest]
-        php-versions: ['7.2', '7.3', '7.4']
+        php-versions: ['7.2', '7.3', '7.4', '8.0']
     name: PHP ${{ matrix.php-versions }} Test on ${{ matrix.operating-system }}
     steps:
     - name: Checkout
@@ -46,7 +46,7 @@ jobs:
     - name: Install Composer dependencies
       run: |
         composer install --no-progress --prefer-dist --optimize-autoloader
-    - name: Run Tests
+    - name: Run PHPUnit Tests
       run: vendor/bin/phpunit --coverage-text --coverage-clover coverage.xml
 
     - name: Upload coverage to Codecov

diff --git a/.github/workflows/symfony.yml b/.github/workflows/symfony.yml
@@ -18,8 +18,8 @@ jobs:
     strategy:
       matrix:
         operating-system: [ubuntu-latest]
-        symfony-versions: ['4.4', '5.1']
-        php-versions: ['7.2', '7.3', '7.4']
+        symfony-versions: ['4.4', '5.1', '5.2']
+        php-versions: ['7.2', '7.3', '7.4', '8.0']
     name: PHP ${{ matrix.php-versions }} / Symfony ${{ matrix.symfony-versions }} Test on ${{ matrix.operating-system }}
     steps:
     - name: Checkout

diff --git a/.gitignore b/.gitignore
@@ -4,3 +4,6 @@ composer.lock
 
 .php_cs.cache
 coverage
+
+/.phpcs-cache
+/phpcs.xml
diff --git a/.php_cs.dist b/.php_cs.dist
@@ -0,0 +1,17 @@
+<?php
+
+$finder = PhpCsFixer\Finder::create()
+    ->in(__DIR__)
+    ->exclude('var')
+    ->exclude('config')
+;
+
+return PhpCsFixer\Config::create()
+    ->setRiskyAllowed(true)
+    ->setRules([
+        '@Symfony' => true,
+        'array_syntax' => ['syntax' => 'short'],
+        'declare_strict_types' => true,
+    ])
+    ->setFinder($finder)
+;
diff --git a/DependencyInjection/ArtprimaPrometheusMetricsExtension.php b/DependencyInjection/ArtprimaPrometheusMetricsExtension.php
@@ -4,13 +4,13 @@
 
 namespace Artprima\PrometheusMetricsBundle\DependencyInjection;
 
-use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\Config\FileLocator;
-use Symfony\Component\HttpKernel\DependencyInjection\Extension;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Loader;
+use Symfony\Component\HttpKernel\DependencyInjection\Extension;
 
 /**
- * This is the class that loads and manages your bundle configuration.
+ * This is the class that loads and manages the bundle configuration.
  *
  * @see http://symfony.com/doc/current/cookbook/bundles/extension.html
  */

diff --git a/DependencyInjection/Compiler/IgnoredRoutesPass.php b/DependencyInjection/Compiler/IgnoredRoutesPass.php
@@ -8,9 +8,12 @@
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 
+/**
+ * IgnoredRoutesPass is a compilation pass that sets ignored routes argument for the metrics.
+ */
 class IgnoredRoutesPass implements CompilerPassInterface
 {
-    public function process(ContainerBuilder $container)
+    public function process(ContainerBuilder $container): void
     {
         if (!$container->hasDefinition(RequestCounterListener::class)) {
             return;

diff --git a/DependencyInjection/Compiler/RegisterMetricsGeneratorPass.php b/DependencyInjection/Compiler/RegisterMetricsGeneratorPass.php
@@ -9,6 +9,10 @@
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
 
+/**
+ * RegisterMetricsGeneratorPass is a compilation pass that registers all metrics classes taged as
+ * prometheus_metrics_bundle.metrics_generator.
+ */
 class RegisterMetricsGeneratorPass implements CompilerPassInterface
 {
     public function process(ContainerBuilder $container)

diff --git a/DependencyInjection/Compiler/ResolveAdapterDefinitionPass.php b/DependencyInjection/Compiler/ResolveAdapterDefinitionPass.php
@@ -10,6 +10,9 @@
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 
+/**
+ * ResolveAdapterDefinitionPass is a compilation pass that sets the metrics backend storage adapter.
+ */
 class ResolveAdapterDefinitionPass implements CompilerPassInterface
 {
     public function process(ContainerBuilder $container)

diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -17,7 +17,7 @@ class Configuration implements ConfigurationInterface
     /**
      * {@inheritdoc}
      */
-    public function getConfigTreeBuilder()
+    public function getConfigTreeBuilder(): TreeBuilder
     {
         $treeBuilder = new TreeBuilder('artprima_prometheus_metrics');
         $rootNode = $treeBuilder->getRootNode();
@@ -50,7 +50,9 @@ public function getConfigTreeBuilder()
                                 ->always()
                                 // here we force casting `float` to `string` to avoid TypeError when working with Redis
                                 // see for more details: https://github.com/phpredis/phpredis/issues/1538
-                                ->then(function ($v) { return (string) $v; })
+                                ->then(function ($v) {
+                                    return (string) $v;
+                                })
                             ->end()
                         ->end()
                         ->booleanNode('persistent_connections')->end()

diff --git a/EventListener/RequestCounterListener.php b/EventListener/RequestCounterListener.php
@@ -11,7 +11,7 @@
 use Symfony\Component\HttpKernel\Event\TerminateEvent;
 
 /**
- * Class RequestCounterListener.
+ * Class RequestCounterListener is an event listener that calls the registered metric handlers.
  */
 class RequestCounterListener implements LoggerAwareInterface
 {

diff --git a/Metrics/AppMetrics.php b/Metrics/AppMetrics.php
@@ -11,7 +11,12 @@
 use Symfony\Component\Stopwatch\Stopwatch;
 
 /**
- * Class AppMetrics.
+ * Class AppMetrics is an implementation of basic metrics collector that is turned on by default.
+ *
+ * Collected metrics:
+ * - requests (per method and route)
+ * - responses (per method, route and response type)
+ * - request duration histogram (per method and route)
  */
 class AppMetrics implements MetricsGeneratorInterface
 {
@@ -32,17 +37,17 @@ class AppMetrics implements MetricsGeneratorInterface
      */
     private $collectionRegistry;
 
-    public function init(string $namespace, CollectorRegistry $collectionRegistry)
+    public function init(string $namespace, CollectorRegistry $collectionRegistry): void
     {
-        $this->namespace          = $namespace;
+        $this->namespace = $namespace;
         $this->collectionRegistry = $collectionRegistry;
     }
 
-    public function collectRequest(RequestEvent $event)
+    public function collectRequest(RequestEvent $event): void
     {
-        $request       = $event->getRequest();
+        $request = $event->getRequest();
         $requestMethod = $request->getMethod();
-        $requestRoute  = $request->attributes->get('_route');
+        $requestRoute = $request->attributes->get('_route');
 
         // do not track "OPTIONS" requests
         if ('OPTIONS' === $requestMethod) {
@@ -53,100 +58,86 @@ public function collectRequest(RequestEvent $event)
         $this->incRequestsTotal($requestMethod, $requestRoute);
     }
 
-    private function setInstance(string $value): void
-    {
-        $name = 'instance_name';
-        try {
-            // the trick with try/catch let's us setting the instance name only once
-            $this->collectionRegistry->getGauge($this->namespace, $name);
-        } catch (MetricNotFoundException $e) {
-            /** @noinspection PhpUnhandledExceptionInspection */
-            $gauge = $this->collectionRegistry->registerGauge(
-                $this->namespace,
-                $name,
-                'app instance name',
-                ['instance']
-            );
-            $gauge->set(1, [$value]);
-        }
-    }
-
-    private function incRequestsTotal(?string $method = null, ?string $route = null): void
-    {
-        $counter = $this->collectionRegistry->getOrRegisterCounter(
-            $this->namespace,
-            'http_requests_total',
-            'total request count',
-            ['action']
-        );
-
-        $counter->inc(['all']);
-
-        if (null !== $method && null !== $route) {
-            $counter->inc([sprintf('%s-%s', $method, $route)]);
-        }
-    }
-
-    public function collectResponse(TerminateEvent $event)
+    public function collectResponse(TerminateEvent $event): void
     {
         $response = $event->getResponse();
-        $request  = $event->getRequest();
+        $request = $event->getRequest();
 
         $requestMethod = $request->getMethod();
-        $requestRoute  = $request->attributes->get('_route');
+        $requestRoute = $request->attributes->get('_route');
 
         if ($response->getStatusCode() >= 200 && $response->getStatusCode() < 300) {
-            $this->inc2xxResponsesTotal($requestMethod, $requestRoute);
+            $this->incResponsesTotal('2xx', $requestMethod, $requestRoute);
+        } elseif ($response->getStatusCode() >= 300 && $response->getStatusCode() < 400) {
+            $this->incResponsesTotal('3xx', $requestMethod, $requestRoute);
         } elseif ($response->getStatusCode() >= 400 && $response->getStatusCode() < 500) {
-            $this->inc4xxResponsesTotal($requestMethod, $requestRoute);
+            $this->incResponsesTotal('4xx', $requestMethod, $requestRoute);
         } elseif ($response->getStatusCode() >= 500) {
-            $this->inc5xxResponsesTotal($requestMethod, $requestRoute);
+            $this->incResponsesTotal('5xx', $requestMethod, $requestRoute);
         }
 
-        if($this->stopwatch && $this->stopwatch->isStarted('execution_time')) {
+        if ($this->stopwatch && $this->stopwatch->isStarted('execution_time')) {
             $evt = $this->stopwatch->stop('execution_time');
             if (null !== $evt) {
                 $this->setRequestDuration($evt->getDuration() / 1000, $requestMethod, $requestRoute);
             }
         }
     }
 
-    private function inc2xxResponsesTotal(?string $method = null, ?string $route = null): void
+    public function collectStart(RequestEvent $event): void
     {
-        $counter = $this->collectionRegistry->getOrRegisterCounter(
-            $this->namespace,
-            'http_2xx_responses_total',
-            'total 2xx response count',
-            ['action']
-        );
-        $counter->inc(['all']);
+        // do not track "OPTIONS" requests
+        if ($event->getRequest()->isMethod('OPTIONS')) {
+            return;
+        }
 
-        if (null !== $method && null !== $route) {
-            $counter->inc([sprintf('%s-%s', $method, $route)]);
+        if (class_exists(self::STOPWATCH_CLASS)) {
+            $className = self::STOPWATCH_CLASS;
+            $this->stopwatch = new $className();
+            $this->stopwatch->start('execution_time');
         }
     }
 
-    private function inc4xxResponsesTotal(?string $method = null, ?string $route = null): void
+    private function setInstance(string $value): void
+    {
+        $name = 'instance_name';
+        try {
+            // the trick with try/catch let's us setting the instance name only once
+            $this->collectionRegistry->getGauge($this->namespace, $name);
+        } catch (MetricNotFoundException $e) {
+            /** @noinspection PhpUnhandledExceptionInspection */
+            $gauge = $this->collectionRegistry->registerGauge(
+                $this->namespace,
+                $name,
+                'app instance name',
+                ['instance']
+            );
+            $gauge->set(1, [$value]);
+        }
+    }
+
+    private function incRequestsTotal(?string $method = null, ?string $route = null): void
     {
         $counter = $this->collectionRegistry->getOrRegisterCounter(
             $this->namespace,
-            'http_4xx_responses_total',
-            'total 4xx response count',
+            'http_requests_total',
+            'total request count',
             ['action']
         );
+
         $counter->inc(['all']);
 
         if (null !== $method && null !== $route) {
             $counter->inc([sprintf('%s-%s', $method, $route)]);
         }
     }
 
-    private function inc5xxResponsesTotal(?string $method = null, ?string $route = null): void
+    private function incResponsesTotal(string $type, ?string $method = null, ?string $route = null): void
     {
         $counter = $this->collectionRegistry->getOrRegisterCounter(
             $this->namespace,
-            'http_5xx_responses_total',
-            'total 5xx response count',
+            sprintf('http_%s_responses_total', $type),
+            sprintf('total %s response count', $type),
             ['action']
         );
         $counter->inc(['all']);
@@ -170,18 +161,4 @@ private function setRequestDuration(float $duration, ?string $method = null, ?st
             $histogram->observe($duration, [sprintf('%s-%s', $method, $route)]);
         }
     }
-
-    public function collectStart(RequestEvent $event)
-    {
-        // do not track "OPTIONS" requests
-        if ($event->getRequest()->isMethod('OPTIONS')) {
-            return;
-        }
-
-        if (class_exists(self::STOPWATCH_CLASS)) {
-            $className       = self::STOPWATCH_CLASS;
-            $this->stopwatch = new $className();
-            $this->stopwatch->start('execution_time');
-        }
-    }
 }