Skip to content

Commit ade1b4c

Browse files
feat(metrics): Do not count a request as processed if bouncer did not bounce at all
1 parent 778fc96 commit ade1b4c

File tree

3 files changed

+32
-25
lines changed

3 files changed

+32
-25
lines changed

CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,18 @@ As far as possible, we try to adhere to [Symfony guidelines](https://symfony.com
1111

1212
---
1313

14+
15+
## [4.1.0](https://github.com/crowdsecurity/php-cs-bouncer/releases/tag/v4.1.0) - 2025-01-10
16+
[_Compare with previous release_](https://github.com/crowdsecurity/php-cs-bouncer/compare/v4.0.0...v4.1.0)
17+
18+
19+
### Changed
20+
21+
- Do not save origins count when the bouncer does not bounce the IP, due to business logic. This avoids sending a
22+
"processed" usage metrics to the LAPI when the IP is not bounced.
23+
24+
---
25+
1426
## [4.0.0](https://github.com/crowdsecurity/php-cs-bouncer/releases/tag/v4.0.0) - 2025-01-09
1527
[_Compare with previous release_](https://github.com/crowdsecurity/php-cs-bouncer/compare/v3.2.0...v4.0.0)
1628

src/AbstractBouncer.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -665,11 +665,6 @@ private function handleBounceExclusion(string $message): bool
665665
'type' => 'SHOULD_NOT_BOUNCE',
666666
'message' => $message,
667667
]);
668-
// Increment clean origin count
669-
$this->getRemediationEngine()->updateMetricsOriginsCount(
670-
AbstractCache::CLEAN,
671-
Constants::REMEDIATION_BYPASS
672-
);
673668

674669
return false;
675670
}

tests/Integration/AbstractBouncerTest.php

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -712,9 +712,9 @@ public function testRun()
712712
);
713713
$originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
714714
$this->assertEquals(
715-
['clean' => ['bypass' => 1]],
715+
null,
716716
$originCountItem,
717-
'The origin count for clean should be 1'
717+
'The origin count for clean should be null'
718718
);
719719

720720
// Test 3: bouncing URI
@@ -739,9 +739,9 @@ public function testRun()
739739
$this->assertEquals(true, $bouncer->run(), 'Should bounce uri');
740740
$originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
741741
$this->assertEquals(
742-
['clean' => ['bypass' => 2]],
742+
['clean' => ['bypass' => 1]],
743743
$originCountItem,
744-
'The origin count for clean should be 2'
744+
'The origin count for clean should be 1'
745745
);
746746

747747
// Test 4: not bouncing URI if disabled
@@ -768,9 +768,9 @@ public function testRun()
768768
$this->assertEquals(false, $bouncer->run(), 'Should not bounce if disabled');
769769
$originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
770770
$this->assertEquals(
771-
['clean' => ['bypass' => 3]],
771+
['clean' => ['bypass' => 1]],
772772
$originCountItem,
773-
'The origin count for clean should be 3'
773+
'The origin count for clean should still be 1 as we did not bounce at all due to config'
774774
);
775775

776776
PHPUnitUtil::assertRegExp(
@@ -830,9 +830,9 @@ public function testRun()
830830
);
831831
$originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
832832
$this->assertEquals(
833-
['clean' => ['bypass' => 3]],
833+
['clean' => ['bypass' => 1]],
834834
$originCountItem,
835-
'The origin count for clean should be still 3'
835+
'The origin count for clean should be still 1'
836836
);
837837

838838
// Test 6: NOT throw error if config says so
@@ -871,9 +871,9 @@ public function testRun()
871871
}
872872
$originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
873873
$this->assertEquals(
874-
['clean' => ['bypass' => 3]],
874+
['clean' => ['bypass' => 1]],
875875
$originCountItem,
876-
'The origin count for clean should be still 3'
876+
'The origin count for clean should be still 1'
877877
);
878878

879879
$this->assertEquals('', $error, 'Should not throw error');
@@ -919,9 +919,9 @@ public function testRun()
919919
);
920920
$originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
921921
$this->assertEquals(
922-
['clean' => ['bypass' => 4]],
922+
['clean' => ['bypass' => 2]],
923923
$originCountItem,
924-
'The origin count for clean should be 4'
924+
'The origin count for clean should be 2'
925925
);
926926

927927
// Test 8 : forced X-Forwarded-for usage
@@ -959,9 +959,9 @@ public function testRun()
959959
);
960960
$originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
961961
$this->assertEquals(
962-
['clean' => ['bypass' => 5]],
962+
['clean' => ['bypass' => 3]],
963963
$originCountItem,
964-
'The origin count for clean should be 5'
964+
'The origin count for clean should be 3'
965965
);
966966

967967
// Test 9 non-authorized
@@ -995,9 +995,9 @@ public function testRun()
995995
);
996996
$originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
997997
$this->assertEquals(
998-
['clean' => ['bypass' => 6]],
998+
['clean' => ['bypass' => 4]],
999999
$originCountItem,
1000-
'The origin count for clean should be 6'
1000+
'The origin count for clean should be 4'
10011001
);
10021002

10031003
// Test 10 authorized
@@ -1030,20 +1030,20 @@ public function testRun()
10301030
);
10311031
$originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
10321032
$this->assertEquals(
1033-
['clean' => ['bypass' => 7]],
1033+
['clean' => ['bypass' => 5]],
10341034
$originCountItem,
1035-
'The origin count for clean should be 7'
1035+
'The origin count for clean should be 5'
10361036
);
10371037
// Test 11: push metrics
10381038
$result = $bouncer->pushUsageMetrics(self::BOUNCER_NAME, self::BOUNCER_VERSION, self::BOUNCER_TYPE);
10391039
$this->assertEquals(
10401040
[
10411041
'name' => 'processed',
1042-
'value' => 7,
1042+
'value' => 5,
10431043
'unit' => 'request',
10441044
],
10451045
$result['remediation_components'][0]['metrics'][0]['items'][0],
1046-
'Should have pushed metrics'
1046+
'Should have pushed 5 processed metrics'
10471047
);
10481048
$originCountItem = $cache->getItem(AbstractCache::ORIGINS_COUNT)->get();
10491049
$this->assertEquals(

0 commit comments

Comments
 (0)