Skip to content

chore(appsec): remove internal _dd.appsec.usr.id and _dd.appsec.usr.login tags - #19852

Draft
florentinl wants to merge 2 commits into
mainfrom
florentin.labelle/remove-appsec-usr-tags
Draft

chore(appsec): remove internal _dd.appsec.usr.id and _dd.appsec.usr.login tags#19852
florentinl wants to merge 2 commits into
mainfrom
florentin.labelle/remove-appsec-usr-tags

Conversation

@florentinl

Copy link
Copy Markdown
Contributor

Description

Removes the internal _dd.appsec.usr.id and _dd.appsec.usr.login span tags, along with the USER_LOGIN_USERID / USER_LOGIN_USERNAME constants and the now-dead conditionals / locals that only existed to populate them.

Context

These tags were introduced by the Automated user lifecycle tracking RFC for accuracy analysis of automated user ID collection — i.e. comparing the SDK-provided usr.id against the auto-instrumented value. An audit of every write/read site shows:

  • Outside anonymization mode, _dd.appsec.usr.id is always a verbatim duplicate of usr.id (both derived from the same user_id variable after the same _maybe_hash step). It carries no additional information for pure-auto customers.
  • The user login value is already carried by the public event tags appsec.events.users.login.{success,failure}.usr.login and appsec.events.users.signup.usr.login.
  • The user id value is already carried by usr.id (set via set_user) and, for login-failure / signup / SDK-success events, by the public event tags appsec.events.users.{login,signup}.*.usr.id.
  • The only remaining non-redundant signal (SDK+auto in anonymization mode, where usr.id is raw and _dd.appsec.usr.id is hashed) can be derived from _dd.appsec.user.collection_mode together with usr.id.

The tracer never reads these tags; they are write-only. This is a behavior change for the AppSec backend's accuracy-analysis pipeline, which is why this opens as a draft — backend sign-off is required before merging.

What is removed

  • ddtrace/appsec/_constants.pyUSER_LOGIN_USERID / USER_LOGIN_USERNAME constant definitions.
  • ddtrace/appsec/_trace_utils.py — 5 write sites + 4 collapsing if … != SDK conditionals.
  • ddtrace/appsec/track_user_sdk.py — 3 write sites.
  • ddtrace/appsec/_contrib/django/__init__.py — 4 write sites + the dead hash_login local in the ANON auth branch.
  • Tests — 6 assertions on the removed tags.

What is preserved (unchanged)

  • usr.id (set via set_user).
  • The public event tags: appsec.events.users.login.{success,failure}.usr.{login,id}, appsec.events.users.signup.usr.{login,id}.
  • _dd.appsec.user.collection_mode, _dd.appsec.events.*.auto.mode, _dd.appsec.events.*.sdk, manual.keep, and all libddwaf addresses.

Testing

  • python3 -m py_compile on all modified files.
  • scripts/lint ruff-fix + scripts/lint format_check pass on the changed files.
  • Existing AppSec login/signup/auth-tracking tests updated to drop assertions on the removed tags; the assertions on usr.id and the public event tags remain and continue to validate the user-identity data path.

TODO before merging: run the relevant AppSec test suites via the run-tests skill (django appsec, contrib appsec login/signup events) to confirm no regressions.

Risks

  • Backend accuracy analysis: the AppSec backend may currently consume _dd.appsec.usr.id / _dd.appsec.usr.login specifically (rather than usr.id or the public event tags) to populate user-identity fields or run accuracy comparisons. If so, removing them is a breaking change for that pipeline. Requires sign-off from the ASM Respond/Monitor backend team.
  • RFC compliance: these tags are mandated by the accepted Automated user lifecycle tracking RFC. Removing them technically deviates from the spec; an RFC update (or explicit waiver) should accompany the merge.
  • No public API is affected. No customer-facing tag is removed (usr.id, usr.login, and the appsec.events.* tags are untouched).

Additional Notes

  • Suggested PR label: changelog/no-changelog (removal of internal _dd.* tags; no customer-facing API change).
  • Net diff: 6 files changed, 1 insertion, 38 deletions.
  • Follow-up: if the backend confirms the tags are unused, an RFC update should retire them from the spec.

…ogin tags

These internal tags were mandated by the Automated user lifecycle
tracking RFC for accuracy analysis of automated user ID collection
(SDK vs auto-instrumentation). Outside of anonymization mode they are
always a verbatim duplicate of usr.id (or the public
appsec.events.users.{login,signup}.*.usr.id event tags), and the
remaining SDK+auto accuracy signal can be derived from
_dd.appsec.user.collection_mode together with usr.id.

Remove the USER_LOGIN_USERID / USER_LOGIN_USERNAME constants, all
write sites, and the now-dead conditionals and local variables that
only existed to populate them. The public event tags and usr.id are
unaffected.
@florentinl florentinl added the changelog/no-changelog A changelog entry is not required for this PR. label Aug 25, 2026
@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codeowners resolved as

Resolved from the full PR diff against main using the target branch CODEOWNERS file.
CODEOWNERS team requests not listed below are not required by the current file set.

ddtrace/appsec/_constants.py                                            @DataDog/asm-python
ddtrace/appsec/_contrib/django/__init__.py                              @DataDog/asm-python
ddtrace/appsec/_trace_utils.py                                          @DataDog/asm-python
ddtrace/appsec/track_user_sdk.py                                        @DataDog/asm-python
docs/rfc-addendum-remove-appsec-usr-tags.md                             @DataDog/python-guild
tests/appsec/contrib_appsec/utils.py                                    @DataDog/asm-python
tests/appsec/integrations/django_tests/test_appsec_django.py            @DataDog/asm-python

@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Aug 25, 2026

Copy link
Copy Markdown

Dependency direction analysis

⚠️ Existing dependency direction violations

There are 250 dependency direction violations that already exist on the base branch and have not been changed by this PR.

Show existing violations (showing 5 of 250 highest severity)
ddtrace.internal.tracemethods -×-> ddtrace.trace  (internal-core -> product:tracing, score=135)
ddtrace.internal.ci_visibility.filters -×-> ddtrace.trace  (product:ci_visibility -> product:tracing, score=133)
ddtrace.llmobs._integrations.langgraph -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=133)
ddtrace.appsec._contrib.flask -×-> ddtrace.trace  (product:appsec -> product:tracing, score=133)
ddtrace.llmobs._integrations.pydantic_ai -×-> ddtrace.trace  (product:llmobs -> product:tracing, score=133)

To see all violations, download the layers-base.json and layers-pr.json artifacts from this CI job and run:

uv run --script scripts/import-analysis/layers.py compare layers-base.json layers-pr.json

@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Pipelines  Tests

⚠️ Warnings

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 25 Pipeline jobs failed

System Tests | tracer-release / End-to-end #1 / uds-flask 1 — ❌ 12 tests failed · 🔧 Needs a code fix, caused by this PR

View more details · View in GitHub Actions

12 failed tests. KeyError: '_dd.appsec.usr.login' in tests/appsec/test_automated_login_events.py

Showing tests most relevant to this failure.

❌ tests.appsec.test_automated_login_events.Test_V3_Login_Events.test_login_sdk_failure_basic[uds-flask] from system_tests_suite
KeyError: '_dd.appsec.usr.login'

self = <tests.appsec.test_automated_login_events.Test_V3_Login_Events object at 0x7f0c5c794770>

    def test_login_sdk_failure_basic(self):
        for request in self.r_sdk_failure:
            assert request.status_code == 401
            for _, _, span in interfaces.library.get_spans(request=request):
                assert_priority(span)
                meta = span.get("meta", {})
...
❌ tests.appsec.test_automated_login_events.Test_V3_Login_Events.test_login_sdk_failure_local[uds-flask] from system_tests_suite
KeyError: '_dd.appsec.usr.login'

self = <tests.appsec.test_automated_login_events.Test_V3_Login_Events object at 0x7f0c5c794890>

    def test_login_sdk_failure_local(self):
        for request in self.r_sdk_failure:
            assert request.status_code == 401
            for _, _, span in interfaces.library.get_spans(request=request):
                assert_priority(span)
                meta = span.get("meta", {})
...
❌ tests.appsec.test_automated_login_events.Test_V3_Login_Events.test_login_sdk_success_basic[uds-flask] from system_tests_suite
KeyError: '_dd.appsec.usr.login'

self = <tests.appsec.test_automated_login_events.Test_V3_Login_Events object at 0x7f0c5c794b30>

    def test_login_sdk_success_basic(self):
        for request in self.r_sdk_success:
            assert request.status_code == 200
            for _, _, span in interfaces.library.get_spans(request=request):
                assert_priority(span)
                meta = span.get("meta", {})
...
↳ and 9 more — View all
System Tests | tracer-release / End-to-end #10 / uwsgi-poc 10 — ❌ 2 tests failed · 🔧 Needs a code fix, caused by this PR

View more details · View in GitHub Actions

2 failed tests due to KeyError: '_dd.appsec.usr.login' during login event assertions in tests/appsec/test_asm_standalone.py.

Showing tests most relevant to this failure.

❌ tests.appsec.test_asm_standalone.Test_UserEventsStandalone_Automated.test_user_login_failure_event_generates_asm_event[uwsgi-poc] from system_tests_suite
KeyError: '_dd.appsec.usr.login'

self = <tests.appsec.test_asm_standalone.Test_UserEventsStandalone_Automated object at 0x7fdbd04dea80>

    def test_user_login_failure_event_generates_asm_event(self):
        trace_id = 1212121212121212122
        meta = self._get_standalone_span_meta(trace_id)
        assert meta is not None
>       assert meta["_dd.appsec.usr.login"] == INVALID_USER
E       KeyError: '_dd.appsec.usr.login'
...
❌ tests.appsec.test_asm_standalone.Test_UserEventsStandalone_Automated.test_user_login_success_event_generates_asm_event[uwsgi-poc] from system_tests_suite
KeyError: '_dd.appsec.usr.login'

self = <tests.appsec.test_asm_standalone.Test_UserEventsStandalone_Automated object at 0x7fdbd04df290>

    def test_user_login_success_event_generates_asm_event(self):
        trace_id = 1212121212121212111
        meta = self._get_standalone_span_meta(trace_id)
        assert meta is not None
>       assert meta["_dd.appsec.usr.login"] == USER
E       KeyError: '_dd.appsec.usr.login'
...
System Tests | tracer-release / End-to-end #2 / uwsgi-poc 2 — ❌ 12 tests failed · 🔧 Needs a code fix, caused by this PR

View more details · View in GitHub Actions

KeyError: '_dd.appsec.usr.login' in multiple login tests, causing 12 total test failures.

Showing tests most relevant to this failure.

❌ tests.appsec.test_automated_login_events.Test_V3_Login_Events.test_login_sdk_failure_basic[uwsgi-poc] from system_tests_suite
KeyError: '_dd.appsec.usr.login'

self = <tests.appsec.test_automated_login_events.Test_V3_Login_Events object at 0x7f99e2ec6ff0>

    def test_login_sdk_failure_basic(self):
        for request in self.r_sdk_failure:
            assert request.status_code == 401
            for _, _, span in interfaces.library.get_spans(request=request):
                assert_priority(span)
                meta = span.get("meta", {})
...
❌ tests.appsec.test_automated_login_events.Test_V3_Login_Events.test_login_sdk_failure_local[uwsgi-poc] from system_tests_suite
KeyError: '_dd.appsec.usr.login'

self = <tests.appsec.test_automated_login_events.Test_V3_Login_Events object at 0x7f99e2ec7a10>

    def test_login_sdk_failure_local(self):
        for request in self.r_sdk_failure:
            assert request.status_code == 401
            for _, _, span in interfaces.library.get_spans(request=request):
                assert_priority(span)
                meta = span.get("meta", {})
...
❌ tests.appsec.test_automated_login_events.Test_V3_Login_Events.test_login_sdk_success_basic[uwsgi-poc] from system_tests_suite
KeyError: '_dd.appsec.usr.login'

self = <tests.appsec.test_automated_login_events.Test_V3_Login_Events object at 0x7f99e2ec6480>

    def test_login_sdk_success_basic(self):
        for request in self.r_sdk_success:
            assert request.status_code == 200
            for _, _, span in interfaces.library.get_spans(request=request):
                assert_priority(span)
                meta = span.get("meta", {})
...
↳ and 9 more — View all

View all 25 failed jobs.

📋 Copy fix prompt
CI on my pull request is failing. Help me find and fix the root cause of each failing job below — they were flagged as caused by changes in this PR, so focus on the diff. For each job, explain the failure and propose a fix.

Before you start, set up the Datadog software-delivery tooling so you can
query the CI data yourself:

1. Check whether you already have the Datadog software-delivery MCP tools
   (e.g. a `search_datadog_ci_pipeline_events` tool) and the `unblock-pr` skill.
2. If either is missing, STOP and ask me for permission before installing
   anything. Do not install or run anything until I have said yes.
3. Only with my explicit approval, set up the Datadog software-delivery MCP
   server and skills by following:
     https://docs.datadoghq.com/getting_started/software_delivery_mcp_tools/
   then restart so the skill is picked up.
4. If I decline, skip all of the above and work from the context below alone.

Then run /unblock-pr — it will pull the CI data itself. The job context below is what we already know.

If /unblock-pr is not available — because I declined the setup above, or it did not install — work from the context below instead.

Datadog has already classified this failure as caused by changes in this PR.
Take that as given and work the fix:

1. Locate the change. Diff this branch against its base and find the change
   that produces this error. Explain the mechanism, don't just name a file:
     git fetch origin && git diff $(git merge-base origin/main HEAD)...HEAD
2. Reproduce it locally. Run the failing job's command or test before
   proposing anything.
3. Propose the smallest fix that addresses the root cause — not a workaround,
   not a broadened assertion, not a disabled or skipped test.
4. Re-run the same command to confirm, and say exactly what you ran.
5. If the failure turns out to be intermittent rather than deterministic, say
   so plainly instead of "fixing" it — that is a flaky test, and patching it
   hides the problem.

If the right move is to re-run the job rather than change code, use the job
link in the context below. For GitHub Actions: `gh run rerun <run-id> --failed`,
where the run ID is the number after `/runs/` in that URL (not the trailing
number, which is the job ID).

Branch: florentin.labelle/remove-appsec-usr-tags

System Tests | tracer-release / End-to-end #1 / uds-flask 1
Commit: c92b73d4b3f07d1447a1a4190d9f2bf6c27d6441
Error (code / test):
12 failed tests. KeyError: '_dd.appsec.usr.login' in tests/appsec/test_automated_login_events.py
CI job: https://github.com/DataDog/dd-trace-py/actions/runs/32859204347/job/97841743059

System Tests | tracer-release / End-to-end #10 / uwsgi-poc 10
Commit: c92b73d4b3f07d1447a1a4190d9f2bf6c27d6441
Error (code / test):
2 failed tests due to KeyError: '_dd.appsec.usr.login' during login event assertions in tests/appsec/test_asm_standalone.py.
CI job: https://github.com/DataDog/dd-trace-py/actions/runs/32859204347/job/97841746288

System Tests | tracer-release / End-to-end #2 / uwsgi-poc 2
Commit: c92b73d4b3f07d1447a1a4190d9f2bf6c27d6441
Error (code / test):
KeyError: '_dd.appsec.usr.login' in multiple login tests, causing 12 total test failures.
CI job: https://github.com/DataDog/dd-trace-py/actions/runs/32859204347/job/97841746809

Plus 17 more failing jobs not shown here.

ℹ️ Info

No other issues found (see more)

❄️ No new flaky tests detected

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 1119f21 | Docs | View more details | Give us feedback!

@cit-pr-commenter-54b7da

cit-pr-commenter-54b7da Bot commented Aug 25, 2026

Copy link
Copy Markdown

Circular import analysis

⚠️ Existing circular imports

There are 3 circular imports that already exist on the base branch and have not been changed by this PR.

ddtrace.errortracking._handled_exceptions.bytecode_injector -> ddtrace.errortracking._handled_exceptions.callbacks -> ddtrace.errortracking._handled_exceptions.collector -> ddtrace.errortracking._handled_exceptions.bytecode_reporting -> ddtrace.errortracking._handled_exceptions.bytecode_injector
ddtrace.llmobs -> ddtrace.llmobs._evaluators -> ddtrace.llmobs._evaluators.format -> ddtrace.llmobs._experiment -> ddtrace.llmobs
ddtrace.appsec._asm_request_context -> ddtrace.appsec._iast._iast_request_context_base -> ddtrace.appsec._iast._iast_env -> ddtrace.appsec._iast.reporter -> ddtrace.appsec._exploit_prevention.stack_traces -> ddtrace.appsec._asm_request_context

@pr-commenter

pr-commenter Bot commented Aug 25, 2026

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-08-25 14:40:57

Comparing candidate commit 1119f21 in PR branch florentin.labelle/remove-appsec-usr-tags with baseline commit b1d9e9b in branch main.

📊 Benchmarking dashboard

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

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:iastaspectsospath-ospathbasename_aspect

  • 🟥 execution_time [+138.048µs; +145.102µs] or [+33.690%; +35.411%]

scenario:iastaspectssplit-rsplit_aspect

  • 🟥 execution_time [+23.101µs; +26.793µs] or [+16.787%; +19.470%]

Proposes retiring the two internal write-only tags introduced by the
Automated user lifecycle tracking RFC. Documents the motivation
(redundancy with usr.id and the public appsec.events.* event tags in
all collection modes) and the backend derivation path from
collection_mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/no-changelog A changelog entry is not required for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant