ci(appsec): fix ddwaf circular import by mabdinur · Pull Request #12013 · DataDog/dd-trace-py

mabdinur · 2025-01-21T18:56:17Z

Fix the following CI failures: https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-py/-/jobs/774636553

Reproduction:

from ddtrace.appsec._metrics import _set_waf_init_metric

ddtrace/appsec/_metrics defines _set_waf_init_metric
ddtrace/appsec/_metrics imports ddtrace.appsec._proccessors
ddtrace.appsec._proccessors imports _set_waf_init_metric
Boom: Circular import

The fix here is to avoid importing ddwaf from ddtrace.appsec._processor in ddtrace.appsec._metrics. Instead we can import ddwaf directly from ddtrace.appsec._ddwaf.

This issue does not occur if the ddtrace.appsec._processor module is imported before ddtrace.appsec._metrics. This is why we do not see this error in most of our appsec tests. However the order of imports is not guaranteed by the slots check. This explains the flakiness in this job.

Checklist

PR author has checked that all the criteria below are met
The PR description includes an overview of the change
The PR description articulates the motivation for the change
The change includes tests OR the PR description describes a testing strategy
The PR description notes risks associated with the change, if any
Newly-added code is easy to change
The change follows the library release note guidelines
The change includes or references documentation updates if necessary
Backport labels are set (if applicable)

Reviewer Checklist

Reviewer has checked that all the criteria below are met
Title is accurate
All changes are related to the pull request's stated goal
Avoids breaking API changes
Testing strategy adequately addresses listed risks
Newly-added code is easy to change
Release note makes sense to a user of the library
If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment
Backport labels are set in a manner that is consistent with the release branch maintenance policy

github-actions · 2025-01-21T18:56:51Z

CODEOWNERS have been resolved as:

ddtrace/appsec/_metrics.py                                              @DataDog/asm-python

datadog-dd-trace-py-rkomorn · 2025-01-21T19:09:17Z

Datadog Report

Branch report: munir/fix-ddwaf-circular-import
Commit report: 8b1c81e
Test service: dd-trace-py

✅ 0 Failed, 130 Passed, 1468 Skipped, 4m 28.11s Total duration (35m 41.38s time saved)

pr-commenter · 2025-01-21T19:35:31Z

Benchmarks

Benchmark execution time: 2025-01-21 19:35:29

Comparing candidate commit 8b1c81e in PR branch munir/fix-ddwaf-circular-import with baseline commit f198341 in branch main.

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

Fix the following CI failures: https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-py/-/jobs/774636553 Reproduction: ``` from ddtrace.appsec._metrics import _set_waf_init_metric ``` - `ddtrace/appsec/_metrics` defines [_set_waf_init_metric](https://github.com/DataDog/dd-trace-py/blob/130b69b367e22311fa5fea7e3cc0910396e968c4/ddtrace/appsec/_metrics.py#L48-L49) - `ddtrace/appsec/_metrics` imports [ddtrace.appsec._proccessors](https://github.com/DataDog/dd-trace-py/blob/130b69b367e22311fa5fea7e3cc0910396e968c4/ddtrace/appsec/_metrics.py#L4-L5) - `ddtrace.appsec._proccessors` imports [_set_waf_init_metric](https://github.com/DataDog/dd-trace-py/blob/130b69b367e22311fa5fea7e3cc0910396e968c4/ddtrace/appsec/_processor.py#L443-L444) - Boom: Circular import The fix here is to avoid importing `ddwaf` from `ddtrace.appsec._processor` in `ddtrace.appsec._metrics`. Instead we can import ddwaf directly from `ddtrace.appsec._ddwaf`. This issue does not occur if the `ddtrace.appsec._processor` module is imported before `ddtrace.appsec._metrics`. This is why we do not see this error in most of our appsec tests. However the order of imports is not guaranteed by the slots check. This explains the flakiness in this job. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) (cherry picked from commit 67e1c46)

Backport 67e1c46 from #12013 to 2.20. Fix the following CI failures: https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-py/-/jobs/774636553 Reproduction: ``` from ddtrace.appsec._metrics import _set_waf_init_metric ``` - `ddtrace/appsec/_metrics` defines [_set_waf_init_metric](https://github.com/DataDog/dd-trace-py/blob/130b69b367e22311fa5fea7e3cc0910396e968c4/ddtrace/appsec/_metrics.py#L48-L49) - `ddtrace/appsec/_metrics` imports [ddtrace.appsec._proccessors](https://github.com/DataDog/dd-trace-py/blob/130b69b367e22311fa5fea7e3cc0910396e968c4/ddtrace/appsec/_metrics.py#L4-L5) - `ddtrace.appsec._proccessors` imports [_set_waf_init_metric](https://github.com/DataDog/dd-trace-py/blob/130b69b367e22311fa5fea7e3cc0910396e968c4/ddtrace/appsec/_processor.py#L443-L444) - Boom: Circular import The fix here is to avoid importing `ddwaf` from `ddtrace.appsec._processor` in `ddtrace.appsec._metrics`. Instead we can import ddwaf directly from `ddtrace.appsec._ddwaf`. This issue does not occur if the `ddtrace.appsec._processor` module is imported before `ddtrace.appsec._metrics`. This is why we do not see this error in most of our appsec tests. However the order of imports is not guaranteed by the slots check. This explains the flakiness in this job. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>

Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - #11987 - #12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - #11987 - #12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) (cherry picked from commit 4f0bcb5)

Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - #11987 - #12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - #11987 - #12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. (cherry picked from commit 4f0bcb5) ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com>

Fix the following CI failures: https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-py/-/jobs/774636553 Reproduction: ``` from ddtrace.appsec._metrics import _set_waf_init_metric ``` - `ddtrace/appsec/_metrics` defines [_set_waf_init_metric](https://github.com/DataDog/dd-trace-py/blob/130b69b367e22311fa5fea7e3cc0910396e968c4/ddtrace/appsec/_metrics.py#L48-L49) - `ddtrace/appsec/_metrics` imports [ddtrace.appsec._proccessors](https://github.com/DataDog/dd-trace-py/blob/130b69b367e22311fa5fea7e3cc0910396e968c4/ddtrace/appsec/_metrics.py#L4-L5) - `ddtrace.appsec._proccessors` imports [_set_waf_init_metric](https://github.com/DataDog/dd-trace-py/blob/130b69b367e22311fa5fea7e3cc0910396e968c4/ddtrace/appsec/_processor.py#L443-L444) - Boom: Circular import The fix here is to avoid importing `ddwaf` from `ddtrace.appsec._processor` in `ddtrace.appsec._metrics`. Instead we can import ddwaf directly from `ddtrace.appsec._ddwaf`. This issue does not occur if the `ddtrace.appsec._processor` module is imported before `ddtrace.appsec._metrics`. This is why we do not see this error in most of our appsec tests. However the order of imports is not guaranteed by the slots check. This explains the flakiness in this job. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - #11987 - #12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - #11987 - #12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) (cherry picked from commit 4f0bcb5)

Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - #11987 - #12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) (cherry picked from commit 4f0bcb5)

github-actions · 2025-02-13T15:46:00Z

The backport to 2.19 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.19 2.19
# Navigate to the new working tree
cd .worktrees/backport-2.19
# Create a new branch
git switch --create backport-12013-to-2.19
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 67e1c46e0044be304843eb1403bc1f9b7c370000
# Push it to GitHub
git push --set-upstream origin backport-12013-to-2.19
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.19

Then, create a pull request where the base branch is 2.19 and the compare/head branch is backport-12013-to-2.19.

Backport 4f0bcb5 from #12102 to 2.20. Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - #11987 - #12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com>

…t 2.20] (#12270) Co-authored-by: Federico Mon <federico.mon@datadoghq.com> fix(asm): make sure iast is not loaded by exploit prevention if disabled [backport 2.20] (#12271) Co-authored-by: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> chore(asm): don't load appsec modules (iast)... [backport 2.20] (#12298) The goal is to make sure no appsec module is loaded if appsec is disabled. This PR is the first one of 2, handling IAST. It removes all non guarded IAST import from outside appsec. - ensure we don't load any iast module if iast is disabled - replace `_is_iast_enabled()` by a field `_iast_enabled` in the asm config APPSEC-56626 - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) (cherry picked from commit ea2a9a5) --------- Co-authored-by: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> Co-authored-by: Alberto Vara <alberto.vara@datadoghq.com> chore(asm): clean libddwaf loading [backport 2.20] (#12320) Backport 4f0bcb5 from #12102 to 2.20. Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - #11987 - #12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> fix(asm): decouple appsec [backport 2.20] (#12319) Backport #12212 to 2.20 - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> chore(iast): relase note of #12298 [backport 2.20] (#12317) Release note of the PR #12298 - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

Backport 4f0bcb5 from #12102 to 2.20. Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - #11987 - #12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com>

fix(appsec): fix ddwaf circular import

8b1c81e

mabdinur closed this Jan 21, 2025

mabdinur reopened this Jan 24, 2025

mabdinur marked this pull request as ready for review January 24, 2025 18:50

mabdinur requested a review from a team as a code owner January 24, 2025 18:50

Merge branch 'main' into munir/fix-ddwaf-circular-import

e28bbf4

mabdinur enabled auto-merge (squash) January 24, 2025 19:36

taegyunkim approved these changes Jan 24, 2025

View reviewed changes

erikayasuda added the changelog/no-changelog A changelog entry is not required for this PR. label Jan 24, 2025

erikayasuda closed this Jan 24, 2025

auto-merge was automatically disabled January 24, 2025 20:09
Pull request was closed

erikayasuda reopened this Jan 24, 2025

erikayasuda enabled auto-merge (squash) January 24, 2025 20:10

gnufede approved these changes Jan 25, 2025

View reviewed changes

erikayasuda merged commit 67e1c46 into main Jan 25, 2025

erikayasuda deleted the munir/fix-ddwaf-circular-import branch January 25, 2025 16:13

christophe-papazian mentioned this pull request Jan 27, 2025

chore(asm): clean libddwaf loading #12102

Merged

2 tasks

Yun-Kim added the backport 2.20 label Jan 27, 2025

github-actions Bot mentioned this pull request Jan 27, 2025

ci(appsec): fix ddwaf circular import [backport 2.20] #12103

Merged

2 tasks

erikayasuda mentioned this pull request Jan 29, 2025

chore(asm): clean libddwaf loading [forwardport 3.x] #12145

Merged

2 tasks

github-actions Bot mentioned this pull request Feb 13, 2025

chore(asm): clean libddwaf loading [backport 2.20] #12320

Merged

2 tasks

avara1986 mentioned this pull request Feb 13, 2025

chore(asm): clean libddwaf loading [Backport 2.19] #12322

Closed

4 tasks

avara1986 added the backport 2.19 label Feb 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci(appsec): fix ddwaf circular import#12013

ci(appsec): fix ddwaf circular import#12013
erikayasuda merged 2 commits intomainfrom
munir/fix-ddwaf-circular-import

mabdinur commented Jan 21, 2025 •

edited by erikayasuda

Loading

Uh oh!

github-actions Bot commented Jan 21, 2025

Uh oh!

datadog-dd-trace-py-rkomorn Bot commented Jan 21, 2025

Uh oh!

pr-commenter Bot commented Jan 21, 2025

Uh oh!

github-actions Bot commented Feb 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

mabdinur commented Jan 21, 2025 • edited by erikayasuda Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Reviewer Checklist

Uh oh!

github-actions Bot commented Jan 21, 2025

Uh oh!

datadog-dd-trace-py-rkomorn Bot commented Jan 21, 2025

Datadog Report

Uh oh!

pr-commenter Bot commented Jan 21, 2025

Benchmarks

Uh oh!

github-actions Bot commented Feb 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mabdinur commented Jan 21, 2025 •

edited by erikayasuda

Loading