feat(metrics): Do not count a request as processed if bouncer did not…

… bounce at all
crowdsecurity · Jan 10, 2025 · ade1b4c · ade1b4c
1 parent 778fc96
commit ade1b4c
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,18 @@ As far as possible, we try to adhere to [Symfony guidelines](https://symfony.com
 
 ---
 
+
+## [4.1.0](https://github.com/crowdsecurity/php-cs-bouncer/releases/tag/v4.1.0) - 2025-01-10
+[_Compare with previous release_](https://github.com/crowdsecurity/php-cs-bouncer/compare/v4.0.0...v4.1.0)
+
+
+### Changed
+
+- Do not save origins count when the bouncer does not bounce the IP, due to business logic. This avoids sending a 
+  "processed" usage metrics to the LAPI when the IP is not bounced.
+
+---
+
 ## [4.0.0](https://github.com/crowdsecurity/php-cs-bouncer/releases/tag/v4.0.0) - 2025-01-09
 [_Compare with previous release_](https://github.com/crowdsecurity/php-cs-bouncer/compare/v3.2.0...v4.0.0)
 

diff --git a/src/AbstractBouncer.php b/src/AbstractBouncer.php
@@ -665,11 +665,6 @@ private function handleBounceExclusion(string $message): bool
             'type' => 'SHOULD_NOT_BOUNCE',
             'message' => $message,
         ]);
-        // Increment clean origin count
-        $this->getRemediationEngine()->updateMetricsOriginsCount(
-            AbstractCache::CLEAN,
-            Constants::REMEDIATION_BYPASS
-        );
 
         return false;
     }

diff --git a/tests/Integration/AbstractBouncerTest.php b/tests/Integration/AbstractBouncerTest.php
@@ -712,9 +712,9 @@ public function testRun()
         );
         $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
         $this->assertEquals(
-            ['clean' => ['bypass' => 1]],
+            null,
             $originCountItem,
-            'The origin count for clean should be 1'
+            'The origin count for clean should be null'
         );
 
         // Test 3: bouncing URI
@@ -739,9 +739,9 @@ public function testRun()
         $this->assertEquals(true, $bouncer->run(), 'Should bounce uri');
         $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
         $this->assertEquals(
-            ['clean' => ['bypass' => 2]],
+            ['clean' => ['bypass' => 1]],
             $originCountItem,
-            'The origin count for clean should be 2'
+            'The origin count for clean should be 1'
         );
 
         // Test 4:  not bouncing URI if disabled
@@ -768,9 +768,9 @@ public function testRun()
         $this->assertEquals(false, $bouncer->run(), 'Should not bounce if disabled');
         $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
         $this->assertEquals(
-            ['clean' => ['bypass' => 3]],
+            ['clean' => ['bypass' => 1]],
             $originCountItem,
-            'The origin count for clean should be 3'
+            'The origin count for clean should still be 1 as we did not bounce at all due to config'
         );
 
         PHPUnitUtil::assertRegExp(
@@ -830,9 +830,9 @@ public function testRun()
         );
         $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
         $this->assertEquals(
-            ['clean' => ['bypass' => 3]],
+            ['clean' => ['bypass' => 1]],
             $originCountItem,
-            'The origin count for clean should be still 3'
+            'The origin count for clean should be still 1'
         );
 
         // Test 6: NOT throw error if config says so
@@ -871,9 +871,9 @@ public function testRun()
         }
         $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
         $this->assertEquals(
-            ['clean' => ['bypass' => 3]],
+            ['clean' => ['bypass' => 1]],
             $originCountItem,
-            'The origin count for clean should be still 3'
+            'The origin count for clean should be still 1'
         );
 
         $this->assertEquals('', $error, 'Should not throw error');
@@ -919,9 +919,9 @@ public function testRun()
         );
         $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
         $this->assertEquals(
-            ['clean' => ['bypass' => 4]],
+            ['clean' => ['bypass' => 2]],
             $originCountItem,
-            'The origin count for clean should be 4'
+            'The origin count for clean should be 2'
         );
 
         // Test 8 : forced X-Forwarded-for usage
@@ -959,9 +959,9 @@ public function testRun()
         );
         $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
         $this->assertEquals(
-            ['clean' => ['bypass' => 5]],
+            ['clean' => ['bypass' => 3]],
             $originCountItem,
-            'The origin count for clean should be 5'
+            'The origin count for clean should be 3'
         );
 
         // Test 9 non-authorized
@@ -995,9 +995,9 @@ public function testRun()
         );
         $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
         $this->assertEquals(
-            ['clean' => ['bypass' => 6]],
+            ['clean' => ['bypass' => 4]],
             $originCountItem,
-            'The origin count for clean should be 6'
+            'The origin count for clean should be 4'
         );
 
         // Test 10 authorized
@@ -1030,20 +1030,20 @@ public function testRun()
         );
         $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
         $this->assertEquals(
-            ['clean' => ['bypass' => 7]],
+            ['clean' => ['bypass' => 5]],
             $originCountItem,
-            'The origin count for clean should be 7'
+            'The origin count for clean should be 5'
         );
         // Test 11: push metrics
         $result = $bouncer->pushUsageMetrics(self::BOUNCER_NAME, self::BOUNCER_VERSION, self::BOUNCER_TYPE);
         $this->assertEquals(
             [
                 'name' => 'processed',
-                'value' => 7,
+                'value' => 5,
                 'unit' => 'request',
             ],
             $result['remediation_components'][0]['metrics'][0]['items'][0],
-            'Should have pushed metrics'
+            'Should have pushed 5 processed metrics'
         );
         $originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
         $this->assertEquals(