-
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-56188] Replace Scope with Context #4277
Conversation
Datadog ReportBranch report: ✅ 0 Failed, 22187 Passed, 1477 Skipped, 5m 23.48s Total Time |
BenchmarksBenchmark execution time: 2025-01-13 11:20:12 Comparing candidate commit 92b6443 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 31 metrics, 2 unstable metrics. |
4c768f4
to
7d8c852
Compare
7d8c852
to
92b6443
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4277 +/- ##
=======================================
Coverage 97.72% 97.72%
=======================================
Files 1354 1354
Lines 82421 82415 -6
Branches 4220 4220
=======================================
- Hits 80543 80539 -4
+ Misses 1878 1876 -2 ☔ View full report in Codecov by Sentry. |
What does this PR do?
Replace irresponsible class
AppSec::Scope
withAppSec::Context
and move some functionality into it.Motivation:
In order to implement RASP/WAF telemetry and metrics we need to adjust the domain of the AppSec. The main point -
AppSec::Context
represents request context and hides complexity of the security engine calls (and gathering statistics) from the instrumentation.You will see some obvious issues with existing code because of that change, but that's the plan. We move slowly with a guaranteed compatibility.
Change log entry
No.
Additional Notes:
This is the first PR which establishes the base to develop metrics collection. In this PR introduces as minimum as possible changes to stay compatible, but move forward.
How to test the change?
CI (specifically systems tests) should be enough.