-
Notifications
You must be signed in to change notification settings - Fork 463
feat(apisec): implement new API Security sampler #3315
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
feat(apisec): implement new API Security sampler #3315
Conversation
Implementation of the new API Security sampler defined by the [latest RFC](https://docs.google.com/document/d/1PYoHms9PPXR8V_5_T5-KXAhoFDKQYA8mTnmS12xkGOE/edit?tab=t.0). This change makes the API Security sampler make decisions specific to a given endpoint (method + route + response status code) instead of using a simplistic sampling rate. This allows for improved coverage and accuracy of schema extraction as part of API Security. This change uses the sampler implementation from github.com/DataDog/appsec-internal-go#39.
Datadog ReportBranch report: ✅ 0 Failed, 4451 Passed, 66 Skipped, 3m 42.86s Total Time |
BenchmarksBenchmark execution time: 2025-04-03 10:47:28 Comparing candidate commit dcc6eb5 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 55 metrics, 1 unstable metrics. |
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.
Do you feel like working on apisec telemetry metrics in another PR ?
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.
Rubber stamp for the profiling bits (just go.mod/go.sum changes)
750d466
to
55c3db6
Compare
68b606f
to
2dde675
Compare
@@ -108,6 +108,7 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { | |||
spanopts = append(spanopts, httptraceinternal.HeaderTagsFromRequest(req, r.config.headerTags)) | |||
resource := r.config.resourceNamer(r, req) | |||
httptrace.TraceAndServe(r.Router, w, req, &httptrace.ServeConfig{ | |||
Framework: "github.com/gorilla/mux", |
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.
@RomainMuller Do all HTTP contribs need to register this Framework
field? It's to add this to contribution guidelines.
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.
AppSec basically uses this to tag telemetry when we are unable to obtain some information... So this is optional, but I reckon helps if present (that might be useful for APM telemetry, too, in the future?)
Implementation of the new API Security sampler defined by the [latest RFC](https://docs.google.com/document/d/1PYoHms9PPXR8V_5_T5-KXAhoFDKQYA8mTnmS12xkGOE/edit?tab=t.0). This change makes the API Security sampler make decisions specific to a given endpoint (method + route + response status code) instead of using a simplistic sampling rate. This allows for improved coverage and accuracy of schema extraction as part of API Security. This change uses the sampler implementation from DataDog/appsec-internal-go#39.
Implementation of the new API Security sampler defined by the latest RFC.
This change makes the API Security sampler make decisions specific to a given endpoint (method + route + response status code) instead of using a simplistic sampling rate. This allows for improved coverage and accuracy of schema extraction as part of API Security.
This change uses the sampler implementation from DataDog/appsec-internal-go#39.
Reviewer's Checklist
v2-dev
branch and reviewed by @DataDog/apm-go.Unsure? Have a question? Request a review!