Skip to content

feat(internal/appsec): activation metric and config key#3959

Merged
RomainMuller merged 4 commits into
mainfrom
eliottness/APPSEC-58859/rc-activation-plus-telemetry-env-var
Sep 15, 2025
Merged

feat(internal/appsec): activation metric and config key#3959
RomainMuller merged 4 commits into
mainfrom
eliottness/APPSEC-58859/rc-activation-plus-telemetry-env-var

Conversation

@eliottness

@eliottness eliottness commented Sep 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR adds the DD_APPSEC_ENABLED configuration that was supposed to already be there but was removed by stableconfig APM work. It also adds the appsec.enabled telemetyr metric to also report the value of appsec being enabled or not. This PR ALSO fixes a bug that stableconfig introduced by returning an error even when the env var is not set which made us branch out of appsec.Start too early. And since telemetry logs are dropped backend side we could not have find out earlier.

System-tests run: https://github.com/DataDog/dd-trace-go/actions/runs/17610780213

Motivation

Better data on appsec enablement and how it was enabled.

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • New code is free of linting errors. You can check this by running ./scripts/lint.sh locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@pr-commenter

pr-commenter Bot commented Sep 10, 2025 •
edited
Loading

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2025-09-10 14:49:31

Comparing candidate commit f8ed3c7 in PR branch eliottness/APPSEC-58859/rc-activation-plus-telemetry-env-var with baseline commit e7e5c7f in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 24 metrics, 0 unstable metrics.

eliottness
eliottness commented Sep 10, 2025
View reviewed changes
Comment thread internal/appsec/appsec.go
Comment on lines +58 to 60
// Even when DD_APPSEC_ENABLED is an empty string or unset, an error can be returned here
log.Debug("appsec: could not determine the AppSec enablement mode from DD_APPSEC_ENABLED: %s", err.Error())
}

@eliottness eliottness Sep 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where an error of the style: "value "" is not a boolean" was reported, which made it so that we never registered RC products for remote activation

@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Sep 10, 2025 •
edited by datadog-official Bot
Loading

Copy link
Copy Markdown

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: f8ed3c7 | Docs | Was this helpful? Give us feedback!

@eliottness
fix tests
f8a8237 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
fix tests
8261a50 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness
eliottness marked this pull request as ready for review September 10, 2025 14:34
@eliottness
eliottness requested a review from a team as a code owner September 10, 2025 14:34
@eliottness
fix tests
f8ed3c7 
Signed-off-by: Eliott Bouhana <[email protected]>
@eliottness

eliottness commented Sep 10, 2025

Copy link
Copy Markdown
Contributor Author

/merge

@dd-devflow-routing-codex

dd-devflow-routing-codex Bot commented Sep 10, 2025 •
edited
Loading

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2025-09-10 14:36:10 UTC ℹ️ Start processing command /merge

2025-09-10 14:36:27 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2025-09-10 18:37:11 UTC ⚠️ MergeQueue: This merge request was unqueued

devflow unqueued this merge request: It did not become mergeable within the expected time

@RomainMuller
RomainMuller merged commit 1edd172 into main Sep 15, 2025
264 of 265 checks passed
@RomainMuller
RomainMuller deleted the eliottness/APPSEC-58859/rc-activation-plus-telemetry-env-var branch September 15, 2025 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RomainMuller RomainMuller RomainMuller approved these changes

Assignees

No one assigned

Labels

mergequeue-status: removed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@eliottness @RomainMuller