-
Notifications
You must be signed in to change notification settings - Fork 377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[APPSEC-56187] Rework WAF span metrics collection #4291
[APPSEC-56187] Rework WAF span metrics collection #4291
Conversation
* Replace result object in instrumentation tests
Datadog ReportBranch report: ✅ 0 Failed, 22063 Passed, 1477 Skipped, 5m 10.33s Total Time |
8ab0c68
to
d7769f1
Compare
BenchmarksBenchmark execution time: 2025-01-15 18:21:08 Comparing candidate commit b6e2f20 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 31 metrics, 2 unstable metrics. |
a24600c
to
7a8e713
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4291 +/- ##
=======================================
Coverage 97.71% 97.71%
=======================================
Files 1356 1358 +2
Lines 82489 82498 +9
Branches 4219 4221 +2
=======================================
+ Hits 80604 80615 +11
+ Misses 1885 1883 -2 ☔ View full report in Codecov by Sentry. |
7a8e713
to
b6e2f20
Compare
What does this PR do?
Encapsulates metrics collection into
AppSec::Context
which exposes public API to call security checks. And restructure some other classes responsibilities.Motivation:
The end goal is to reduce abstraction leak from instrumentation and accumulate it in a single place for the future refactoring.
Change log entry
No.
Additional Notes:
In a nutshell
AppSec::Processor::Context
moved intoAppSec::SecurityEngine::Runner
and lost some responsibilities it should not has.AppSec::SecurityEngine::Result::{Ok, Match, Error}
with united interface and stats.Context
too to remove the knowledge leak from security engine side.AppSec
module is now responsible for knowing is API-Security enabled or not.How to test the change?
If the CI and System-Tests are passing - we are good to go.