Conversation
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: c1f1e7a | Docs | View more details | Give us feedback! |
c7dd814 to
05846cd
Compare
d96d39b to
fda30f7
Compare
fda30f7 to
c1f1e7a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1f1e7a7ec
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| if (conf.enable_status() == | ||
| FinalizedConfigSettings::enable_status::DISABLED) { | ||
| Library::set_active(false); |
There was a problem hiding this comment.
looks like the default of active_ should be false instead
| datadog::telemetry::Product{datadog::telemetry::Product::Name::appsec, | ||
| security::Library::active(), | ||
| datadog_semver_nginx_mod, | ||
| {}, | ||
| {}, | ||
| {}}); |
There was a problem hiding this comment.
Is this what you really want? By this time, this will have been run:
set_active(status == ENABLED)
if status is UNSPECIFIED, it will report false. But aftwerwards RC may enable it. So this will report false for everyone using RC to activate/deactive appsec via RC.
Summary
app-startedtelemetry for WAF-enabled buildssecurity::Libraryand the nginx module versionDependency
Depends on DataDog/dd-trace-cpp#366, which fixes serialization of registered telemetry products.
Validation
unit_tests: passedenabled=false,version=1.22.0enabled=true,version=1.22.0