-
Notifications
You must be signed in to change notification settings - Fork 395
Add a fallback to inferred route for AppSec Api security sampler #5006
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
Conversation
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage 🔗 Commit SHA: f400f71 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
BenchmarksBenchmark execution time: 2025-11-03 13:14:47 Comparing candidate commit f400f71 in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 43 metrics, 2 unstable metrics. scenario:profiling - Allocations (baseline)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should change the route inference interface to encapsulate some additional logic which now exposed to all calls to this module
Typing analysisNote: Ignored files are excluded from the next sections. Untyped methodsThis PR introduces 1 partially typed method. Partially typed methods (+1-0)❌ Introduced:If you believe a method or an attribute is rightfully untyped or partially typed, you can add |
2ab8036 to
2cc3588
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌟 Exceptionally well done! 🔥
What does this PR do?
This PR changes
AppSec::APISecuity::RouteExtractorto infer the route if it cannot be extracted, instead of falling back to request path. This route is then used for AppSec API security sampler.Motivation:
We want to have more accurate sampling for requests to barebones Rack applications, or Rack-based frameworks that we don't instrument directly.
Change log entry
None. This is an internal change.
Additional Notes:
APPSEC-59868
How to test the change?
CI.