Repository navigation
chore: exclude stacktrace C extension's new location - #875
emmettbutler wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
More details
The removal glob targets the relocated compiled extension while preserving the security modules needed at startup. The retained legacy path also covers older ddtrace layouts; no reportable defect was identified.
🤖 Bits Code Review · Commit e2c2dff
|
@emmettbutler I am currently doing a release for this layer, please don't merge this until at least tomorrow afternoon (assuming this ddtrace change is going out in 4.17 or later, if it's already out pls let me know asap and I'll pause the release) 🙏 |
rithikanarayan
left a comment
There was a problem hiding this comment.
blocking til release is done, dm if the comment about ddtrace version this responds to is relevant
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 1 Pipeline job failed
ℹ️ Info🔄 Datadog auto-retried 1 job - 0 passed on retry Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: e2c2dff | Docs | View more details | Give us feedback! |
…urity (#21162) ## Description Removes the 10 dependency direction violations from IAST (`product:iast`) into AAP (`product:aap`). Every one of them pointed at code both products use, so that code now lives in a new `ddtrace/internal/security/` package, which both may import. Being part of `ddtrace.internal`, the package imports no product code. Extracted from AAP modules: - `STACK_TRACE` from `ddtrace/appsec/_constants.py` → `ddtrace/internal/security/_constants.py` - `get_entry_span` and the `_ASM_CONTEXT` core-context key from `ddtrace/appsec/_asm_request_context.py` → `ddtrace/internal/security/_context.py`. `report_stack` needs the entry span; it now looks it up through the shared key instead of importing `_asm_request_context`. - `_asm_manual_keep` from `ddtrace/appsec/_trace_utils.py` → `ddtrace/internal/security/_trace_utils.py` The original AAP modules re-export the extracted names, so existing AAP call sites are unchanged. IAST now imports everything from `ddtrace.internal.security`. ## Risks - The C extension's import path changed from `ddtrace.appsec._shared._stacktrace` to `ddtrace.internal.security._stacktrace`. Packaging that removes the extension by its old path (as the AWS Lambda layer does to keep it out of serverless deployments) needs to remove it at the new path; otherwise the unused extension is shipped. Nothing imports it at serverless startup, so this affects package size only. There's a PR updating the exclusion rule in the Lambda layer here: DataDog/datadog-lambda-python#875 Co-authored-by: emmett.butler <emmett.butler@datadoghq.com>
dd-trace-py is moving its stacktrace C extension, which only IAST uses, from
ddtrace/appsec/_shared/toddtrace/internal/security/. The layer already stripsddtrace/appsec/_sharedfor this reason. The new directory also contains modules AAP imports at startup, so only the compiled extension is removed, not the whole directory.The existing
ddtrace/appsec/_sharedentry is kept so the layer still strips the extension from ddtrace versions released before the move.rm -rfignores whichever path doesn't exist, so this works with both old and new ddtrace versions, and the merge order relative to the dd-trace-py change doesn't matter.Testing Guidelines
The extension isn't imported at serverless startup; dd-trace-py's
tests/internal/test_serverless.pyasserts this at its new module path.Additional Notes
Types of Changes
Check all that apply
🤖 Generated with Claude Code