system-tests icon indicating copy to clipboard operation
system-tests copied to clipboard

Excluding PII identifiers should work for DI tracers

Open tylfin opened this issue 6 months ago • 3 comments

Motivation

This pull request enhances the PII redaction tests by introducing support for excluded identifiers. The changes ensure that specific identifiers, such as cookie and sessionid, are excluded from redaction and are validated accordingly. The most important updates include modifications to the test logic, the addition of new test scenarios, and updates to the configuration for excluded identifiers.

Changes

  • Support for excluded identifiers in tests:
    • Introduced the EXCLUDED_IDENTIFIERS list in tests/debugger/test_debugger_pii.py to specify identifiers that should not be redacted, such as cookie and sessionid.
    • Updated the _assert and _validate_pii_keyword_redaction methods to handle excluded identifiers, ensuring they are present in snapshots and not improperly redacted. [1] [2] [3]
  • New test scenarios for excluded identifiers:
    • Added a new test case, test_pii_redaction_excluded_identifiers, to validate the behavior of excluded identifiers. This includes handling missing features and known bugs across different libraries.
  • Environment variable for excluded identifiers:
    • Added DD_DYNAMIC_INSTRUMENTATION_REDACTION_EXCLUDED_IDENTIFIERS to the DEBUGGER_PII_REDACTION scenario specifying cookie, _2fa, and sessionid.

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes (if something not related to your task is failing, you can ignore it)
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner. We're working on refining the codeowners file quickly.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

:rocket: Once your PR is reviewed, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • [ ] If PR title starts with [<language>], double-check that only <language> is impacted by the change
  • [ ] No system-tests internal is modified. Otherwise, I have the approval from R&P team
  • [ ] CI is green, or failing jobs are not related to this change (and you are 100% sure about this statement)
  • [ ] A docker base image is modified?
    • [ ] the relevant build-XXX-image label is present
  • [ ] A scenario is added (or removed)?

tylfin avatar Apr 25 '25 14:04 tylfin