From 3fd90caf37b831d6c6046fd0cd525d9531ae5093 Mon Sep 17 00:00:00 2001 From: Romain Komorn <136473744+romainkomorndatadog@users.noreply.github.com> Date: Fri, 4 Oct 2024 15:11:35 +0100 Subject: [PATCH 01/20] chore(ci_visibility): fix module path collection (#10944) Fixes the way module path was being computed. Now uses the parent of the `item`'s `path` attribute if available, and if not, uses `item.module.__path__`'s parent. Otherwise it defaults to the current directory. ## 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) --- ddtrace/contrib/pytest/_utils.py | 8 ++++++- tests/contrib/pytest/test_pytest.py | 33 ++++++++++++++++++++++++----- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/ddtrace/contrib/pytest/_utils.py b/ddtrace/contrib/pytest/_utils.py index 01a849c356..a4a23d5eed 100644 --- a/ddtrace/contrib/pytest/_utils.py +++ b/ddtrace/contrib/pytest/_utils.py @@ -99,7 +99,13 @@ def _get_test_parameters_json(item) -> t.Optional[str]: def _get_module_path_from_item(item: pytest.Item) -> Path: - return Path(item.nodeid.rpartition("/")[0]).absolute() + try: + item_path = getattr(item, "path", None) + if item_path is not None: + return item.path.absolute().parent + return Path(item.module.__file__).absolute().parent + except Exception: # noqa: E722 + return Path.cwd() def _get_session_command(session: pytest.Session): diff --git a/tests/contrib/pytest/test_pytest.py b/tests/contrib/pytest/test_pytest.py index 7be2135430..1ba0d66e83 100644 --- a/tests/contrib/pytest/test_pytest.py +++ b/tests/contrib/pytest/test_pytest.py @@ -1463,8 +1463,14 @@ def test_pytest_module_path(self): os.chdir(str(package_outer_dir)) with open("test_outer_abc.py", "w+") as fd: fd.write( - """def test_ok(): - assert True""" + textwrap.dedent( + """ + import pytest + + @pytest.mark.parametrize("paramslash", ["c/d", "/d/c", "f/"]) + def test_ok_1(paramslash): + assert True""" + ) ) os.mkdir("test_inner_package") os.chdir("test_inner_package") @@ -1472,14 +1478,22 @@ def test_pytest_module_path(self): pass with open("test_inner_abc.py", "w+") as fd: fd.write( - """def test_ok(): - assert True""" + textwrap.dedent( + """ + import pytest + + @pytest.mark.parametrize("slashparam", ["a/b", "/b/a", "a/"]) + def test_ok_2(slashparam): + assert True + + """ + ) ) self.testdir.chdir() self.inline_run("--ddtrace") spans = self.pop_spans() - assert len(spans) == 7 + assert len(spans) == 11 test_module_spans = sorted( [span for span in spans if span.get_tag("type") == "test_module_end"], key=lambda s: s.get_tag("test.module"), @@ -1494,6 +1508,15 @@ def test_pytest_module_path(self): assert test_suite_spans[0].get_tag("test.suite") == "test_inner_abc.py" assert test_suite_spans[1].get_tag("test.suite") == "test_outer_abc.py" + test_spans = _get_spans_from_list(spans, "test") + for test_span in test_spans: + if test_span.get_tag("test.name").startswith("test_ok_1"): + assert test_span.get_tag("test.module_path") == "test_outer_package" + elif test_span.get_tag("test.name").startswith("test_ok_2"): + assert test_span.get_tag("test.module_path") == "test_outer_package/test_inner_package" + else: + raise ValueError("Unexpected span name") + def test_pytest_module_path_empty(self): """ Test that running pytest without module will create an empty module span with empty path. From 37ae36573d79a8a400559b3643b926df30745452 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 4 Oct 2024 14:19:40 +0000 Subject: [PATCH 02/20] chore: update redis latest version to 5.1.0 (#10933) ## 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](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 - [ ] 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: quinna-h <175135214+quinna-h@users.noreply.github.com> --- .riot/requirements/115aba5.txt | 6 +++--- .riot/requirements/1683324.txt | 6 +++--- .riot/requirements/177912e.txt | 12 ++++++------ .riot/requirements/181128c.txt | 16 ++++++++-------- .riot/requirements/18b32f4.txt | 8 ++++---- .riot/requirements/1916976.txt | 16 ++++++++-------- .riot/requirements/195ecad.txt | 12 ++++++------ .riot/requirements/1d32f58.txt | 16 ++++++++-------- .riot/requirements/315c2cb.txt | 16 ++++++++-------- .riot/requirements/74e07bf.txt | 12 ++++++------ .riot/requirements/9232661.txt | 10 +++++----- .riot/requirements/9e36105.txt | 6 +++--- .riot/requirements/baf46ab.txt | 16 ++++++++-------- .riot/requirements/c961281.txt | 8 ++++---- .riot/requirements/f4b1bd3.txt | 16 ++++++++-------- 15 files changed, 88 insertions(+), 88 deletions(-) diff --git a/.riot/requirements/115aba5.txt b/.riot/requirements/115aba5.txt index f165e70bbf..0d4bef52ed 100644 --- a/.riot/requirements/115aba5.txt +++ b/.riot/requirements/115aba5.txt @@ -2,12 +2,12 @@ # This file is autogenerated by pip-compile with Python 3.7 # by the following command: # -# pip-compile --config=pyproject.toml --no-annotate --resolver=backtracking .riot/requirements/115aba5.in +# pip-compile --allow-unsafe --config=pyproject.toml --no-annotate --resolver=backtracking .riot/requirements/115aba5.in # async-timeout==4.0.3 -attrs==23.2.0 +attrs==24.2.0 coverage[toml]==7.2.7 -exceptiongroup==1.2.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 importlib-metadata==6.7.0 iniconfig==2.0.0 diff --git a/.riot/requirements/1683324.txt b/.riot/requirements/1683324.txt index 70f7284a8a..ffd915a20d 100644 --- a/.riot/requirements/1683324.txt +++ b/.riot/requirements/1683324.txt @@ -2,12 +2,12 @@ # This file is autogenerated by pip-compile with Python 3.7 # by the following command: # -# pip-compile --config=pyproject.toml --no-annotate --resolver=backtracking .riot/requirements/1683324.in +# pip-compile --allow-unsafe --config=pyproject.toml --no-annotate --resolver=backtracking .riot/requirements/1683324.in # async-timeout==4.0.3 -attrs==23.2.0 +attrs==24.2.0 coverage[toml]==7.2.7 -exceptiongroup==1.2.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 importlib-metadata==6.7.0 iniconfig==2.0.0 diff --git a/.riot/requirements/177912e.txt b/.riot/requirements/177912e.txt index 0fe298c21d..41ae45d3e5 100644 --- a/.riot/requirements/177912e.txt +++ b/.riot/requirements/177912e.txt @@ -2,23 +2,23 @@ # This file is autogenerated by pip-compile with Python 3.10 # by the following command: # -# pip-compile --no-annotate .riot/requirements/177912e.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/177912e.in # async-timeout==4.0.3 -attrs==23.2.0 -coverage[toml]==7.5.4 -exceptiongroup==1.2.1 +attrs==24.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 pytest-randomly==3.15.0 redis==5.0.1 sortedcontainers==2.4.0 -tomli==2.0.1 +tomli==2.0.2 diff --git a/.riot/requirements/181128c.txt b/.riot/requirements/181128c.txt index bbbfce393f..5f601f7256 100644 --- a/.riot/requirements/181128c.txt +++ b/.riot/requirements/181128c.txt @@ -2,25 +2,25 @@ # This file is autogenerated by pip-compile with Python 3.9 # by the following command: # -# pip-compile --no-annotate .riot/requirements/181128c.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/181128c.in # async-timeout==4.0.3 -attrs==23.2.0 -coverage[toml]==7.5.4 -exceptiongroup==1.2.1 +attrs==24.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 -importlib-metadata==8.0.0 +importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 pytest-randomly==3.15.0 redis==5.0.1 sortedcontainers==2.4.0 -tomli==2.0.1 -zipp==3.19.2 +tomli==2.0.2 +zipp==3.20.2 diff --git a/.riot/requirements/18b32f4.txt b/.riot/requirements/18b32f4.txt index ce02e20acd..ee6c10ee40 100644 --- a/.riot/requirements/18b32f4.txt +++ b/.riot/requirements/18b32f4.txt @@ -2,17 +2,17 @@ # This file is autogenerated by pip-compile with Python 3.11 # by the following command: # -# pip-compile --no-annotate .riot/requirements/18b32f4.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/18b32f4.in # -attrs==23.2.0 -coverage[toml]==7.5.4 +attrs==24.2.0 +coverage[toml]==7.6.1 hypothesis==6.45.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 diff --git a/.riot/requirements/1916976.txt b/.riot/requirements/1916976.txt index d883e6de3c..11f5e647fb 100644 --- a/.riot/requirements/1916976.txt +++ b/.riot/requirements/1916976.txt @@ -2,25 +2,25 @@ # This file is autogenerated by pip-compile with Python 3.9 # by the following command: # -# pip-compile --no-annotate .riot/requirements/1916976.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/1916976.in # async-timeout==4.0.3 -attrs==23.2.0 -coverage[toml]==7.5.4 -exceptiongroup==1.2.1 +attrs==24.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 -importlib-metadata==8.0.0 +importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 pytest-randomly==3.15.0 redis==4.6.0 sortedcontainers==2.4.0 -tomli==2.0.1 -zipp==3.19.2 +tomli==2.0.2 +zipp==3.20.2 diff --git a/.riot/requirements/195ecad.txt b/.riot/requirements/195ecad.txt index 71ab9cd490..984f240c0e 100644 --- a/.riot/requirements/195ecad.txt +++ b/.riot/requirements/195ecad.txt @@ -2,23 +2,23 @@ # This file is autogenerated by pip-compile with Python 3.10 # by the following command: # -# pip-compile --no-annotate .riot/requirements/195ecad.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/195ecad.in # async-timeout==4.0.3 -attrs==23.2.0 -coverage[toml]==7.5.4 -exceptiongroup==1.2.1 +attrs==24.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 pytest-randomly==3.15.0 redis==4.6.0 sortedcontainers==2.4.0 -tomli==2.0.1 +tomli==2.0.2 diff --git a/.riot/requirements/1d32f58.txt b/.riot/requirements/1d32f58.txt index 2e623af931..6574a8ae4b 100644 --- a/.riot/requirements/1d32f58.txt +++ b/.riot/requirements/1d32f58.txt @@ -2,25 +2,25 @@ # This file is autogenerated by pip-compile with Python 3.9 # by the following command: # -# pip-compile --no-annotate .riot/requirements/1d32f58.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/1d32f58.in # async-timeout==4.0.3 -attrs==23.2.0 -coverage[toml]==7.5.4 -exceptiongroup==1.2.1 +attrs==24.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 -importlib-metadata==8.0.0 +importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 pytest-randomly==3.15.0 redis==4.6.0 sortedcontainers==2.4.0 -tomli==2.0.1 -zipp==3.19.2 +tomli==2.0.2 +zipp==3.20.2 diff --git a/.riot/requirements/315c2cb.txt b/.riot/requirements/315c2cb.txt index f76fe91235..3c1b0f30c8 100644 --- a/.riot/requirements/315c2cb.txt +++ b/.riot/requirements/315c2cb.txt @@ -2,25 +2,25 @@ # This file is autogenerated by pip-compile with Python 3.8 # by the following command: # -# pip-compile --no-annotate .riot/requirements/315c2cb.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/315c2cb.in # async-timeout==4.0.3 -attrs==23.2.0 -coverage[toml]==7.5.4 -exceptiongroup==1.2.1 +attrs==24.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 -importlib-metadata==8.0.0 +importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 pytest-randomly==3.15.0 redis==4.6.0 sortedcontainers==2.4.0 -tomli==2.0.1 -zipp==3.19.2 +tomli==2.0.2 +zipp==3.20.2 diff --git a/.riot/requirements/74e07bf.txt b/.riot/requirements/74e07bf.txt index 670f96c773..f80d2c91be 100644 --- a/.riot/requirements/74e07bf.txt +++ b/.riot/requirements/74e07bf.txt @@ -2,23 +2,23 @@ # This file is autogenerated by pip-compile with Python 3.10 # by the following command: # -# pip-compile --no-annotate .riot/requirements/74e07bf.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/74e07bf.in # async-timeout==4.0.3 -attrs==23.2.0 -coverage[toml]==7.5.4 -exceptiongroup==1.2.1 +attrs==24.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 pytest-randomly==3.15.0 redis==4.6.0 sortedcontainers==2.4.0 -tomli==2.0.1 +tomli==2.0.2 diff --git a/.riot/requirements/9232661.txt b/.riot/requirements/9232661.txt index 184749d781..65ab43360b 100644 --- a/.riot/requirements/9232661.txt +++ b/.riot/requirements/9232661.txt @@ -2,20 +2,20 @@ # This file is autogenerated by pip-compile with Python 3.12 # by the following command: # -# pip-compile --no-annotate .riot/requirements/9232661.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/9232661.in # -attrs==23.2.0 -coverage[toml]==7.5.4 +attrs==24.2.0 +coverage[toml]==7.6.1 hypothesis==6.45.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 pytest-randomly==3.15.0 -redis==5.0.7 +redis==5.1.0 sortedcontainers==2.4.0 diff --git a/.riot/requirements/9e36105.txt b/.riot/requirements/9e36105.txt index 0596dbe2b0..9849b90804 100644 --- a/.riot/requirements/9e36105.txt +++ b/.riot/requirements/9e36105.txt @@ -2,12 +2,12 @@ # This file is autogenerated by pip-compile with Python 3.7 # by the following command: # -# pip-compile --config=pyproject.toml --no-annotate --resolver=backtracking .riot/requirements/9e36105.in +# pip-compile --allow-unsafe --config=pyproject.toml --no-annotate --resolver=backtracking .riot/requirements/9e36105.in # async-timeout==4.0.3 -attrs==23.2.0 +attrs==24.2.0 coverage[toml]==7.2.7 -exceptiongroup==1.2.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 importlib-metadata==6.7.0 iniconfig==2.0.0 diff --git a/.riot/requirements/baf46ab.txt b/.riot/requirements/baf46ab.txt index 7aa767d1ec..7e6602cef3 100644 --- a/.riot/requirements/baf46ab.txt +++ b/.riot/requirements/baf46ab.txt @@ -2,25 +2,25 @@ # This file is autogenerated by pip-compile with Python 3.8 # by the following command: # -# pip-compile --no-annotate .riot/requirements/baf46ab.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/baf46ab.in # async-timeout==4.0.3 -attrs==23.2.0 -coverage[toml]==7.5.4 -exceptiongroup==1.2.1 +attrs==24.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 -importlib-metadata==8.0.0 +importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 pytest-randomly==3.15.0 redis==4.6.0 sortedcontainers==2.4.0 -tomli==2.0.1 -zipp==3.19.2 +tomli==2.0.2 +zipp==3.20.2 diff --git a/.riot/requirements/c961281.txt b/.riot/requirements/c961281.txt index 65346f6985..cb412a89b4 100644 --- a/.riot/requirements/c961281.txt +++ b/.riot/requirements/c961281.txt @@ -2,17 +2,17 @@ # This file is autogenerated by pip-compile with Python 3.11 # by the following command: # -# pip-compile --no-annotate .riot/requirements/c961281.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/c961281.in # -attrs==23.2.0 -coverage[toml]==7.5.4 +attrs==24.2.0 +coverage[toml]==7.6.1 hypothesis==6.45.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 diff --git a/.riot/requirements/f4b1bd3.txt b/.riot/requirements/f4b1bd3.txt index 06f7e7334b..eadbf79ff0 100644 --- a/.riot/requirements/f4b1bd3.txt +++ b/.riot/requirements/f4b1bd3.txt @@ -2,25 +2,25 @@ # This file is autogenerated by pip-compile with Python 3.8 # by the following command: # -# pip-compile --no-annotate .riot/requirements/f4b1bd3.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/f4b1bd3.in # async-timeout==4.0.3 -attrs==23.2.0 -coverage[toml]==7.5.4 -exceptiongroup==1.2.1 +attrs==24.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 -importlib-metadata==8.0.0 +importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 packaging==24.1 pluggy==1.5.0 -pytest==8.2.2 +pytest==8.3.3 pytest-asyncio==0.23.7 pytest-cov==5.0.0 pytest-mock==3.14.0 pytest-randomly==3.15.0 redis==5.0.1 sortedcontainers==2.4.0 -tomli==2.0.1 -zipp==3.19.2 +tomli==2.0.2 +zipp==3.20.2 From 66563bf9037e7bc9dde3baa6a4356ed19629661e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 4 Oct 2024 15:37:04 +0000 Subject: [PATCH 03/20] chore: update starlette latest version to 0.39.2 (#10946) ## 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: quinna-h <175135214+quinna-h@users.noreply.github.com> --- .riot/requirements/112dc22.txt | 6 +++--- .riot/requirements/1250d61.txt | 2 +- .riot/requirements/156e3cc.txt | 4 ++-- .riot/requirements/17a0f7f.txt | 4 ++-- .riot/requirements/18392ae.txt | 8 ++++---- .riot/requirements/1b846e9.txt | 6 +++--- .riot/requirements/1c489e9.txt | 2 +- .riot/requirements/2c855a9.txt | 6 +++--- .riot/requirements/41529f2.txt | 6 +++--- .riot/requirements/4b1629e.txt | 6 +++--- .riot/requirements/7b8e50e.txt | 6 +++--- .riot/requirements/7f7863d.txt | 6 +++--- .riot/requirements/7ff8c97.txt | 6 +++--- .riot/requirements/91a3315.txt | 2 +- .riot/requirements/b06b6cb.txt | 6 +++--- .riot/requirements/c52f9f6.txt | 8 ++++---- .riot/requirements/d6ceb22.txt | 6 +++--- .riot/requirements/d88b3ac.txt | 6 +++--- .riot/requirements/ec338d4.txt | 8 ++++---- 19 files changed, 52 insertions(+), 52 deletions(-) diff --git a/.riot/requirements/112dc22.txt b/.riot/requirements/112dc22.txt index 44620089fc..5a7a17b01c 100644 --- a/.riot/requirements/112dc22.txt +++ b/.riot/requirements/112dc22.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 @@ -35,7 +35,7 @@ sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 starlette==0.14.2 -tomli==2.0.1 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 zipp==3.20.2 diff --git a/.riot/requirements/1250d61.txt b/.riot/requirements/1250d61.txt index b20ba77d79..90fc52cde9 100644 --- a/.riot/requirements/1250d61.txt +++ b/.riot/requirements/1250d61.txt @@ -18,7 +18,7 @@ h11==0.14.0 httpcore==0.17.3 httpx==0.24.1 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==6.7.0 iniconfig==2.0.0 mock==5.1.0 diff --git a/.riot/requirements/156e3cc.txt b/.riot/requirements/156e3cc.txt index 5c9eb621c6..1525da6c41 100644 --- a/.riot/requirements/156e3cc.txt +++ b/.riot/requirements/156e3cc.txt @@ -14,10 +14,10 @@ coverage[toml]==7.6.1 databases==0.8.0 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 diff --git a/.riot/requirements/17a0f7f.txt b/.riot/requirements/17a0f7f.txt index 933ab038ba..d6725f817c 100644 --- a/.riot/requirements/17a0f7f.txt +++ b/.riot/requirements/17a0f7f.txt @@ -14,10 +14,10 @@ coverage[toml]==7.6.1 databases==0.8.0 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 diff --git a/.riot/requirements/18392ae.txt b/.riot/requirements/18392ae.txt index dc666ee33b..933de189d6 100644 --- a/.riot/requirements/18392ae.txt +++ b/.riot/requirements/18392ae.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 @@ -33,7 +33,7 @@ requests==2.32.3 sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 -starlette==0.38.5 -tomli==2.0.1 +starlette==0.39.2 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 diff --git a/.riot/requirements/1b846e9.txt b/.riot/requirements/1b846e9.txt index e13a11851f..4d174ec147 100644 --- a/.riot/requirements/1b846e9.txt +++ b/.riot/requirements/1b846e9.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 @@ -35,7 +35,7 @@ sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 starlette==0.33.0 -tomli==2.0.1 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 zipp==3.20.2 diff --git a/.riot/requirements/1c489e9.txt b/.riot/requirements/1c489e9.txt index 791aa3de79..92254158db 100644 --- a/.riot/requirements/1c489e9.txt +++ b/.riot/requirements/1c489e9.txt @@ -18,7 +18,7 @@ h11==0.14.0 httpcore==0.17.3 httpx==0.24.1 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==6.7.0 iniconfig==2.0.0 mock==5.1.0 diff --git a/.riot/requirements/2c855a9.txt b/.riot/requirements/2c855a9.txt index 131d09f5fd..0145630e74 100644 --- a/.riot/requirements/2c855a9.txt +++ b/.riot/requirements/2c855a9.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 @@ -35,7 +35,7 @@ sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 starlette==0.14.2 -tomli==2.0.1 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 zipp==3.20.2 diff --git a/.riot/requirements/41529f2.txt b/.riot/requirements/41529f2.txt index 128b7c4607..83a605e288 100644 --- a/.riot/requirements/41529f2.txt +++ b/.riot/requirements/41529f2.txt @@ -14,10 +14,10 @@ coverage[toml]==7.6.1 databases==0.8.0 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 @@ -32,6 +32,6 @@ requests==2.32.3 sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 -starlette==0.38.5 +starlette==0.39.2 typing-extensions==4.12.2 urllib3==2.2.3 diff --git a/.riot/requirements/4b1629e.txt b/.riot/requirements/4b1629e.txt index d106e237cb..76be9c2a6d 100644 --- a/.riot/requirements/4b1629e.txt +++ b/.riot/requirements/4b1629e.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 @@ -34,6 +34,6 @@ sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 starlette==0.20.4 -tomli==2.0.1 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 diff --git a/.riot/requirements/7b8e50e.txt b/.riot/requirements/7b8e50e.txt index ca2e84d225..cd7fc9062b 100644 --- a/.riot/requirements/7b8e50e.txt +++ b/.riot/requirements/7b8e50e.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 @@ -35,7 +35,7 @@ sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 starlette==0.20.4 -tomli==2.0.1 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 zipp==3.20.2 diff --git a/.riot/requirements/7f7863d.txt b/.riot/requirements/7f7863d.txt index 745b95125d..361a348330 100644 --- a/.riot/requirements/7f7863d.txt +++ b/.riot/requirements/7f7863d.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 @@ -35,7 +35,7 @@ sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 starlette==0.20.4 -tomli==2.0.1 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 zipp==3.20.2 diff --git a/.riot/requirements/7ff8c97.txt b/.riot/requirements/7ff8c97.txt index a28ae997cf..ab3925fdd6 100644 --- a/.riot/requirements/7ff8c97.txt +++ b/.riot/requirements/7ff8c97.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 @@ -34,6 +34,6 @@ sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 starlette==0.33.0 -tomli==2.0.1 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 diff --git a/.riot/requirements/91a3315.txt b/.riot/requirements/91a3315.txt index 093907a93d..59c2416955 100644 --- a/.riot/requirements/91a3315.txt +++ b/.riot/requirements/91a3315.txt @@ -18,7 +18,7 @@ h11==0.14.0 httpcore==0.17.3 httpx==0.24.1 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==6.7.0 iniconfig==2.0.0 mock==5.1.0 diff --git a/.riot/requirements/b06b6cb.txt b/.riot/requirements/b06b6cb.txt index 101c222e6a..f74a362923 100644 --- a/.riot/requirements/b06b6cb.txt +++ b/.riot/requirements/b06b6cb.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 @@ -34,6 +34,6 @@ sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 starlette==0.15.0 -tomli==2.0.1 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 diff --git a/.riot/requirements/c52f9f6.txt b/.riot/requirements/c52f9f6.txt index 6acace5157..c65ef57f78 100644 --- a/.riot/requirements/c52f9f6.txt +++ b/.riot/requirements/c52f9f6.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 @@ -34,8 +34,8 @@ requests==2.32.3 sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 -starlette==0.38.5 -tomli==2.0.1 +starlette==0.39.2 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 zipp==3.20.2 diff --git a/.riot/requirements/d6ceb22.txt b/.riot/requirements/d6ceb22.txt index 872e1a3370..0749513724 100644 --- a/.riot/requirements/d6ceb22.txt +++ b/.riot/requirements/d6ceb22.txt @@ -14,10 +14,10 @@ coverage[toml]==7.6.1 databases==0.8.0 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 @@ -32,6 +32,6 @@ requests==2.32.3 sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 -starlette==0.38.5 +starlette==0.39.2 typing-extensions==4.12.2 urllib3==2.2.3 diff --git a/.riot/requirements/d88b3ac.txt b/.riot/requirements/d88b3ac.txt index f3c951b12a..b1cbde620f 100644 --- a/.riot/requirements/d88b3ac.txt +++ b/.riot/requirements/d88b3ac.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 @@ -35,7 +35,7 @@ sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 starlette==0.33.0 -tomli==2.0.1 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 zipp==3.20.2 diff --git a/.riot/requirements/ec338d4.txt b/.riot/requirements/ec338d4.txt index 87ab6f1bf1..e5a0a713b2 100644 --- a/.riot/requirements/ec338d4.txt +++ b/.riot/requirements/ec338d4.txt @@ -15,10 +15,10 @@ databases==0.8.0 exceptiongroup==1.2.2 greenlet==3.0.3 h11==0.14.0 -httpcore==1.0.5 +httpcore==1.0.6 httpx==0.27.2 hypothesis==6.45.0 -idna==3.9 +idna==3.10 importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 @@ -34,8 +34,8 @@ requests==2.32.3 sniffio==1.3.1 sortedcontainers==2.4.0 sqlalchemy==1.4.54 -starlette==0.38.5 -tomli==2.0.1 +starlette==0.39.2 +tomli==2.0.2 typing-extensions==4.12.2 urllib3==2.2.3 zipp==3.20.2 From e8cb0905a299fc843094928b71c315e0289e94d0 Mon Sep 17 00:00:00 2001 From: wantsui Date: Fri, 4 Oct 2024 14:14:40 -0400 Subject: [PATCH 04/20] chore: update changelog for version 2.12.3 (#10945) - [x] update changelog for version 2.12.3 --- CHANGELOG.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09d3445d24..8146b8f4dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,39 @@ Changelogs for versions not listed here can be found at https://github.com/DataD --- +## 2.12.3 + +### Bug Fixes + +- Code Security + - This fix resolves an issue where exploit prevention was not properly blocking requests with custom redirection actions. + - Ensure the `Initializer` object is always reset and freed before the Python runtime. + +- LLM Observability + - Fixes an issue where the OpenAI and LangChain integrations would still submit integration metrics even in agentless mode. Integration metrics are now disabled if using agentless mode via `LLMObs.enable(agentless_enabled=True)` or setting `DD_LLMOBS_AGENTLESS_ENABLED=1`. + - Resolves an issue in the `LLMObs.annotate()` method where non-JSON serializable arguments were discarded entirely. Now, the `LLMObs.annotate()` method safely handles non-JSON-serializable arguments by defaulting to a placeholder text. + - Resolves an issue where attempting to tag non-JSON serializable request/response parameters resulted in a `TypeError` in the OpenAI, LangChain, Bedrock, and Anthropic integrations. + - Resolves an issue where attempting to tag non-JSON serializable request arguments caused a `TypeError`. The Anthropic integration now safely tags non-JSON serializable arguments with a default placeholder text. + - Resolves an issue where attempting to tag non-JSON serializable tool config arguments resulted in a `TypeError`. The LangChain integration now safely tags non-JSON serializable arguments with a default placeholder text. + +- Profiling + - All files with platform-dependent code have had their filenames updated to reflect the platform they are for. This fixes issues where the wrong file would be used on a given platform. + - Improves the error message when the native exporter fails to load and stops profiling from starting if ddtrace is also being injected. + - Enables endpoint profiling for stack v2, `DD_PROFILING_STACK_V2_ENABLED` is set. + - Fixes endpoint profiling when using libdatadog exporter, either with `DD_PROFILING_EXPORT_LIBDD_ENABLED` or `DD_PROFILING_TIMELINE_ENABLED`. + - Enables code provenance when using libdatadog exporter, `DD_PROFILING_EXPORT_LIBDD_ENABLED`, `DD_PROFILING_STACK_V2_ENABLED`, or `DD_PROFILING_TIMELINE_ENABLED`. + - Fixes an issue where flamegraph was upside down for stack v2, `DD_PROFILING_STACK_V2_ENABLED`. + +- Tracing + - Fixes an issue where `celery.apply` spans didn't close if the `after_task_publish` or `task_postrun` signals didn't get sent when using `apply_async`, which can happen if there is an internal exception during the handling of the task. This update also marks the span as an error if an exception occurs. + - Fixes an issue where `celery.apply` spans using task_protocol 1 didn't close by improving the check for the task id in the body. + - Fixes circular imports raised when psycopg automatic instrumentation is enabled. + - Removes a reference cycle that caused unnecessary garbage collection for top-level spans. + - Fixed an issue where a `TypeError` exception would be raised if the first message's `topic()` returned `None` during consumption. + - Kinesis: Resolves an issue where unparsable data in a Kinesis record would cause a NoneType error. + +--- + ## 2.13.1 From e0dd63b3f65ac73a4ae623de56c2e4ea93a837d9 Mon Sep 17 00:00:00 2001 From: erikayasuda <153395705+erikayasuda@users.noreply.github.com> Date: Fri, 4 Oct 2024 14:36:52 -0400 Subject: [PATCH 05/20] ci: clean up github mq configs and add dd mq configs (#10924) GitHub jobs are not running when PRs are added to the DD MQ. We need to add a branch trigger for `'mq-working-branch**'` for them to run on the merge queue. ## 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 - [ ] 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) --- .github/workflows/build_deploy.yml | 1 + .github/workflows/build_python_3.yml | 3 --- .github/workflows/codeql-analysis.yml | 4 +--- .github/workflows/django-overhead-profile.yml | 5 ++--- .github/workflows/encoders-profile.yml | 5 ++--- .github/workflows/flask-overhead-profile.yml | 5 ++--- .github/workflows/pypa_musllinux_1_2_i686.yml | 4 +--- .github/workflows/requirements-locks.yml | 5 ++--- .github/workflows/rust-ci.yml | 3 --- .github/workflows/system-tests.yml | 4 +--- .github/workflows/test_frameworks.yml | 4 +--- .github/workflows/testrunner.yml | 4 +--- .github/workflows/unit_tests.yml | 4 +--- 13 files changed, 15 insertions(+), 36 deletions(-) diff --git a/.github/workflows/build_deploy.yml b/.github/workflows/build_deploy.yml index 6078279f84..78333b890c 100644 --- a/.github/workflows/build_deploy.yml +++ b/.github/workflows/build_deploy.yml @@ -10,6 +10,7 @@ on: # before merging/releasing - build_deploy* - 'upgrade-latest-*' + - 'mq-working-branch**' pull_request: release: types: diff --git a/.github/workflows/build_python_3.yml b/.github/workflows/build_python_3.yml index d1f606790a..86f952dff4 100644 --- a/.github/workflows/build_python_3.yml +++ b/.github/workflows/build_python_3.yml @@ -1,9 +1,6 @@ name: Build Python 3 on: - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] workflow_call: inputs: cibw_build: diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 264977d270..e3a3edf4ec 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -4,13 +4,11 @@ on: push: branches: - main + - 'mq-working-branch**' pull_request: # The branches below must be a subset of the branches above branches: - main - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] jobs: analyze: diff --git a/.github/workflows/django-overhead-profile.yml b/.github/workflows/django-overhead-profile.yml index 65cf3d707f..f581749fe8 100644 --- a/.github/workflows/django-overhead-profile.yml +++ b/.github/workflows/django-overhead-profile.yml @@ -3,14 +3,13 @@ on: push: branches: - main + - 'mq-working-branch**' pull_request: paths: - 'ddtrace/**' - 'scripts/profiles/django-simple/**' - '.github/workflows/django-overhead-profile.yml' - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] + jobs: django-overhead-profile: runs-on: ubuntu-latest diff --git a/.github/workflows/encoders-profile.yml b/.github/workflows/encoders-profile.yml index 61bad5df93..887d96ead3 100644 --- a/.github/workflows/encoders-profile.yml +++ b/.github/workflows/encoders-profile.yml @@ -3,14 +3,13 @@ on: push: branches: - main + - 'mq-working-branch**' pull_request: paths: - 'ddtrace/internal/_encoding.pyx' - 'scripts/profiles/encoders/**' - '.github/workflows/encoders-profile.yml' - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] + jobs: encoders-profile: runs-on: ubuntu-latest diff --git a/.github/workflows/flask-overhead-profile.yml b/.github/workflows/flask-overhead-profile.yml index d6062c1d8f..4adefed18e 100644 --- a/.github/workflows/flask-overhead-profile.yml +++ b/.github/workflows/flask-overhead-profile.yml @@ -3,14 +3,13 @@ on: push: branches: - main + - 'mq-working-branch**' pull_request: paths: - 'ddtrace/**' - 'scripts/profiles/flask-simple/**' - '.github/workflows/flask-overhead-profile.yml' - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] + jobs: flask-overhead-profile: runs-on: ubuntu-latest diff --git a/.github/workflows/pypa_musllinux_1_2_i686.yml b/.github/workflows/pypa_musllinux_1_2_i686.yml index 460b932618..f8f21f88a5 100644 --- a/.github/workflows/pypa_musllinux_1_2_i686.yml +++ b/.github/workflows/pypa_musllinux_1_2_i686.yml @@ -5,11 +5,9 @@ on: push: branches: - 'main' + - 'mq-working-branch**' paths: - 'docker/**' - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] jobs: build-and-publish: diff --git a/.github/workflows/requirements-locks.yml b/.github/workflows/requirements-locks.yml index ac6b1dce8f..507ec17bbf 100644 --- a/.github/workflows/requirements-locks.yml +++ b/.github/workflows/requirements-locks.yml @@ -3,11 +3,10 @@ on: push: branches: - main + - 'mq-working-branch**' pull_request: types: [opened, reopened, synchronize] - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] + jobs: validate: name: Check requirements lockfiles diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 7400aef733..668aa507f8 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -4,9 +4,6 @@ on: pull_request: paths: - src/** - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] jobs: check: diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index e40b909532..5b3ee98cfe 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -4,13 +4,11 @@ on: push: branches: - main + - 'mq-working-branch**' pull_request: workflow_dispatch: {} schedule: - cron: '00 04 * * 2-6' - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] jobs: system-tests-build-agent: diff --git a/.github/workflows/test_frameworks.yml b/.github/workflows/test_frameworks.yml index 34dafa6fcd..e56256b0d5 100644 --- a/.github/workflows/test_frameworks.yml +++ b/.github/workflows/test_frameworks.yml @@ -4,10 +4,8 @@ on: push: branches: - main + - 'mq-working-branch**' pull_request: - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] concurrency: group: ${{ github.workflow }}-${{ github.ref }} diff --git a/.github/workflows/testrunner.yml b/.github/workflows/testrunner.yml index a7708ffc7c..f8341d1bc0 100644 --- a/.github/workflows/testrunner.yml +++ b/.github/workflows/testrunner.yml @@ -5,11 +5,9 @@ on: push: branches: - 'main' + - 'mq-working-branch**' paths: - 'docker/**' - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] jobs: build-and-publish: diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index cf9a8799a6..b3d1981a5b 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -4,11 +4,9 @@ on: push: branches: - main + - 'mq-working-branch**' pull_request: workflow_dispatch: {} - merge_group: - # Trigger jobs when PR is added to merge queue - types: [checks_requested] jobs: unit-tests: From 1138db7b0ed9ebc6c3d4d5416efc619b7f3df4ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADtor=20De=20Ara=C3=BAjo?= Date: Sun, 6 Oct 2024 16:58:35 +0100 Subject: [PATCH 06/20] feat(ci_visibility): set test session name from environment and test command (#10881) Add support for `test_session.name` tag, set from `DD_TEST_SESSION_NAME` or from job id + test command. ## 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) --- ddtrace/contrib/pytest/_plugin_v1.py | 6 +- ddtrace/contrib/unittest/patch.py | 3 + ddtrace/ext/test.py | 3 + ddtrace/internal/ci_visibility/encoder.py | 13 ++-- ddtrace/internal/ci_visibility/recorder.py | 28 +++++++ ddtrace/internal/ci_visibility/writer.py | 13 +++- ddtrace/settings/config.py | 1 + docs/configuration.rst | 10 +++ ...dd_test_session_name-d4ab1537577a1cb8.yaml | 6 ++ .../api/test_ext_test_visibility_api.py | 17 +++++ tests/ci_visibility/test_ci_visibility.py | 74 ++++++++++++++++++- tests/ci_visibility/test_encoder.py | 18 +---- tests/contrib/pytest/test_pytest.py | 19 +++++ tests/contrib/unittest/test_unittest.py | 16 ++++ 14 files changed, 202 insertions(+), 25 deletions(-) create mode 100644 releasenotes/notes/ci_visibility-feature-dd_test_session_name-d4ab1537577a1cb8.yaml diff --git a/ddtrace/contrib/pytest/_plugin_v1.py b/ddtrace/contrib/pytest/_plugin_v1.py index 6cd7b11397..a3a3b504b0 100644 --- a/ddtrace/contrib/pytest/_plugin_v1.py +++ b/ddtrace/contrib/pytest/_plugin_v1.py @@ -472,13 +472,17 @@ def pytest_sessionstart(session): service=_CIVisibility._instance._service, span_type=SpanTypes.TEST, ) + test_command = _get_pytest_command(session.config) test_session_span.set_tag_str(COMPONENT, "pytest") test_session_span.set_tag_str(SPAN_KIND, KIND) test_session_span.set_tag_str(test.FRAMEWORK, FRAMEWORK) test_session_span.set_tag_str(test.FRAMEWORK_VERSION, pytest.__version__) test_session_span.set_tag_str(_EVENT_TYPE, _SESSION_TYPE) - test_session_span.set_tag_str(test.COMMAND, _get_pytest_command(session.config)) + test_session_span.set_tag_str(test.COMMAND, test_command) test_session_span.set_tag_str(_SESSION_ID, str(test_session_span.span_id)) + + _CIVisibility.set_test_session_name(test_command=test_command) + if _CIVisibility.test_skipping_enabled(): test_session_span.set_tag_str(test.ITR_TEST_SKIPPING_ENABLED, "true") test_session_span.set_tag( diff --git a/ddtrace/contrib/unittest/patch.py b/ddtrace/contrib/unittest/patch.py index 9c40bab4f4..2c8bdd299a 100644 --- a/ddtrace/contrib/unittest/patch.py +++ b/ddtrace/contrib/unittest/patch.py @@ -650,6 +650,9 @@ def _start_test_session_span(instance) -> ddtrace.Span: test.ITR_TEST_CODE_COVERAGE_ENABLED, "true" if _CIVisibility._instance._collect_coverage_enabled else "false", ) + + _CIVisibility.set_test_session_name(test_command=test_command) + if _CIVisibility.test_skipping_enabled(): _set_test_skipping_tags_to_span(test_session_span) else: diff --git a/ddtrace/ext/test.py b/ddtrace/ext/test.py index 0829f6ec06..5e3d3b04d8 100644 --- a/ddtrace/ext/test.py +++ b/ddtrace/ext/test.py @@ -84,6 +84,9 @@ ITR_UNSKIPPABLE = "test.itr.unskippable" ITR_FORCED_RUN = "test.itr.forced_run" +# Test Session Name +TEST_SESSION_NAME = "test_session.name" + class Status(Enum): PASS = "pass" diff --git a/ddtrace/internal/ci_visibility/encoder.py b/ddtrace/internal/ci_visibility/encoder.py index 096c18ec4c..aa54a72199 100644 --- a/ddtrace/internal/ci_visibility/encoder.py +++ b/ddtrace/internal/ci_visibility/encoder.py @@ -37,25 +37,26 @@ class CIVisibilityEncoderV01(BufferedEncoder): content_type = "application/msgpack" - ALLOWED_METADATA_KEYS = ("language", "library_version", "runtime-id", "env") PAYLOAD_FORMAT_VERSION = 1 TEST_SUITE_EVENT_VERSION = 1 TEST_EVENT_VERSION = 2 ENDPOINT_TYPE = ENDPOINT.TEST_CYCLE def __init__(self, *args): + # DEV: args are not used here, but are used by BufferedEncoder's __cinit__() method, + # which is called implicitly by Cython. super(CIVisibilityEncoderV01, self).__init__() self._lock = threading.RLock() self._metadata = {} self._init_buffer() - self._metadata = {} def __len__(self): with self._lock: return len(self.buffer) - def set_metadata(self, metadata): - self._metadata.update(metadata) + def set_metadata(self, event_type, metadata): + # type: (str, Dict[str, str]) -> None + self._metadata.setdefault(event_type, {}).update(metadata) def _init_buffer(self): with self._lock: @@ -82,10 +83,10 @@ def _build_payload(self, traces): if not normalized_spans: return None record_endpoint_payload_events_count(endpoint=ENDPOINT.TEST_CYCLE, count=len(normalized_spans)) - self._metadata = {k: v for k, v in self._metadata.items() if k in self.ALLOWED_METADATA_KEYS} + # TODO: Split the events in several payloads as needed to avoid hitting the intake's maximum payload size. return CIVisibilityEncoderV01._pack_payload( - {"version": self.PAYLOAD_FORMAT_VERSION, "metadata": {"*": self._metadata}, "events": normalized_spans} + {"version": self.PAYLOAD_FORMAT_VERSION, "metadata": self._metadata, "events": normalized_spans} ) @staticmethod diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index 4d774c691a..ac2b5ee8f4 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -65,6 +65,7 @@ from ddtrace.internal.ci_visibility.git_data import GitData from ddtrace.internal.ci_visibility.git_data import get_git_data_from_tags from ddtrace.internal.ci_visibility.telemetry.constants import TEST_FRAMEWORKS +from ddtrace.internal.ci_visibility.writer import CIVisibilityEventClient from ddtrace.internal.ci_visibility.writer import CIVisibilityWriter from ddtrace.internal.codeowners import Codeowners from ddtrace.internal.compat import parse @@ -765,6 +766,32 @@ def is_unknown_ci(cls) -> bool: return instance._tags.get(ci.PROVIDER_NAME) is None + def _get_ci_visibility_event_client(self) -> Optional[CIVisibilityEventClient]: + writer = self.tracer._writer + if isinstance(writer, CIVisibilityWriter): + for client in writer._clients: + if isinstance(client, CIVisibilityEventClient): + return client + + return None + + @classmethod + def set_test_session_name(cls, test_command: str) -> None: + instance = cls.get_instance() + client = instance._get_ci_visibility_event_client() + if not client: + log.debug("Not setting test session name because no CIVisibilityEventClient is active") + return + + if ddconfig.test_session_name: + test_session_name = ddconfig.test_session_name + else: + job_name = instance._tags.get(ci.JOB_NAME) + test_session_name = f"{job_name}-{test_command}" if job_name else test_command + + log.debug("Setting test session name: %s", test_session_name) + client.set_test_session_name(test_session_name) + @classmethod def is_unique_test(cls, test_id: Union[TestId, InternalTestId]) -> bool: instance = cls.get_instance() @@ -834,6 +861,7 @@ def _on_discover_session( ) CIVisibility.add_session(session) + CIVisibility.set_test_session_name(test_command=discover_args.test_command) @_requires_civisibility_enabled diff --git a/ddtrace/internal/ci_visibility/writer.py b/ddtrace/internal/ci_visibility/writer.py index a54e45bb6b..45b801fca9 100644 --- a/ddtrace/internal/ci_visibility/writer.py +++ b/ddtrace/internal/ci_visibility/writer.py @@ -6,6 +6,11 @@ import ddtrace from ddtrace import config +from ddtrace.ext import SpanTypes +from ddtrace.ext.test import TEST_SESSION_NAME +from ddtrace.internal.ci_visibility.constants import MODULE_TYPE +from ddtrace.internal.ci_visibility.constants import SESSION_TYPE +from ddtrace.internal.ci_visibility.constants import SUITE_TYPE from ddtrace.internal.utils.time import StopWatch from ddtrace.vendor.dogstatsd import DogStatsd # noqa:F401 @@ -43,15 +48,21 @@ class CIVisibilityEventClient(WriterClientBase): def __init__(self): encoder = CIVisibilityEncoderV01(0, 0) encoder.set_metadata( + "*", { "language": "python", "env": config.env, "runtime-id": get_runtime_id(), "library_version": ddtrace.__version__, - } + }, ) super(CIVisibilityEventClient, self).__init__(encoder) + def set_test_session_name(self, test_session_name: str) -> None: + if isinstance(self.encoder, CIVisibilityEncoderV01): + for event_type in [SESSION_TYPE, MODULE_TYPE, SUITE_TYPE, SpanTypes.TEST]: + self.encoder.set_metadata(event_type, {TEST_SESSION_NAME: test_session_name}) + class CIVisibilityCoverageClient(WriterClientBase): def __init__(self, intake_url, headers=None, itr_suite_skipping_mode=False): diff --git a/ddtrace/settings/config.py b/ddtrace/settings/config.py index bc08e1c8a9..a569070c61 100644 --- a/ddtrace/settings/config.py +++ b/ddtrace/settings/config.py @@ -587,6 +587,7 @@ def __init__(self): os.getenv("DD_CIVISIBILITY_ITR_ENABLED", default=True) ) self.ci_visibility_log_level = os.getenv("DD_CIVISIBILITY_LOG_LEVEL", default="info") + self.test_session_name = os.getenv("DD_TEST_SESSION_NAME") self._test_visibility_early_flake_detection_enabled = asbool( os.getenv("DD_CIVISIBILITY_EARLY_FLAKE_DETECTION_ENABLED", default=True) ) diff --git a/docs/configuration.rst b/docs/configuration.rst index 091554b1f6..45673747b7 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -537,6 +537,16 @@ The following environment variables for the tracer are supported: version_added: v1.9.0: + DD_TEST_SESSION_NAME: + type: String + default: (autodetected) + description: | + Configures the ``CIVisibility`` service to use the given string as the value of the ``test_session.name`` tag in + test events. If not specified, this string will be constructed from the CI job id (if available) and the test + command used to start the test session. + version_added: + v2.16.0: + DD_CIVISIBILITY_AGENTLESS_ENABLED: type: Boolean default: False diff --git a/releasenotes/notes/ci_visibility-feature-dd_test_session_name-d4ab1537577a1cb8.yaml b/releasenotes/notes/ci_visibility-feature-dd_test_session_name-d4ab1537577a1cb8.yaml new file mode 100644 index 0000000000..e5311a51c3 --- /dev/null +++ b/releasenotes/notes/ci_visibility-feature-dd_test_session_name-d4ab1537577a1cb8.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + CI Visibility: This adds the `test_session.name` tag to test events. The test session name can be set via the + DD_TEST_SESSION_NAME environment variable. If DD_TEST_SESSION_NAME is not specified, the test session name is set + from the CI job id and the test command. diff --git a/tests/ci_visibility/api/test_ext_test_visibility_api.py b/tests/ci_visibility/api/test_ext_test_visibility_api.py index 0907fa2f24..f45395dcde 100644 --- a/tests/ci_visibility/api/test_ext_test_visibility_api.py +++ b/tests/ci_visibility/api/test_ext_test_visibility_api.py @@ -1,8 +1,10 @@ from os import getcwd as os_getcwd from pathlib import Path +from unittest import mock import pytest +from ddtrace.ext.test_visibility import api from ddtrace.ext.test_visibility.api import TestSourceFileInfo @@ -53,3 +55,18 @@ def test_source_file_info_enforces_start_line_less_than_end_line(self): with pytest.raises(ValueError): # start_line cannot be None if end_line is provided _ = TestSourceFileInfo(Path("/absolute/path/my_file_name"), end_line=1) + + +class TestCIDiscoverTestSessionName: + def test_discover_set_test_session_name(self): + """Check that the test command is used to set the test session name.""" + api.enable_test_visibility() + + with mock.patch( + "ddtrace.internal.ci_visibility.recorder.CIVisibility.set_test_session_name" + ) as set_test_session_name_mock: + api.TestSession.discover("some_test_command", "dd_manual_test_fw", "1.0.0") + + api.disable_test_visibility() + + set_test_session_name_mock.assert_called_once_with(test_command="some_test_command") diff --git a/tests/ci_visibility/test_ci_visibility.py b/tests/ci_visibility/test_ci_visibility.py index 7ead5af1be..4e7561a0bf 100644 --- a/tests/ci_visibility/test_ci_visibility.py +++ b/tests/ci_visibility/test_ci_visibility.py @@ -10,6 +10,7 @@ from unittest.mock import Mock import mock +import msgpack import pytest import ddtrace @@ -36,6 +37,7 @@ from tests.ci_visibility.util import _ci_override_env from tests.ci_visibility.util import _get_default_civisibility_ddconfig from tests.ci_visibility.util import _patch_dummy_writer +from tests.ci_visibility.util import set_up_mock_civisibility from tests.utils import DummyCIVisibilityWriter from tests.utils import DummyTracer from tests.utils import TracerTestCase @@ -560,6 +562,7 @@ def tearDown(self): if CIVisibility.enabled: CIVisibility.disable() except Exception: + # no-dd-sa:python-best-practices/no-silent-exception pass def test_civisibilitywriter_agentless_url(self): @@ -1091,10 +1094,10 @@ def test_civisibility_enable_respects_passed_in_tracer(): class TestIsITRSkippable: - """Tests whether the CIVisibilty.is_item_itr_skippable work properly (the _suite_ and _test_ level methods are + """Tests whether the CIVisibility.is_item_itr_skippable work properly (the _suite_ and _test_ level methods are assumed to be working since they are called by is_item_itr_skippable in a wrapper-like way). - These tests mock CIVisibility._instance._test_suites_to_skip and _tests_to_skip , implying that + These tests mock CIVisibility._instance._test_suites_to_skip and _tests_to_skip, implying that CIVisibility._fetch_tests_to_skip() ran successfully. In test-level skipping mode, only the following tests should be skippable: @@ -1310,3 +1313,70 @@ def test_is_item_itr_skippable_suite_level(self): # Check all tests are not skippable for test_id in self._get_all_test_ids(): assert CIVisibility.is_item_itr_skippable(test_id) is False + + +class TestCIVisibilitySetTestSessionName(TracerTestCase): + def tearDown(self): + try: + if CIVisibility.enabled: + CIVisibility.disable() + except Exception: + # no-dd-sa:python-best-practices/no-silent-exception + pass + + def assert_test_session_name(self, name): + """Check that the payload metadata contains the test session name attributes.""" + payload = msgpack.loads( + CIVisibility._instance.tracer._writer._clients[0].encoder._build_payload([[Span("foo")]]) + ) + assert payload["metadata"]["test_session_end"] == {"test_session.name": name} + assert payload["metadata"]["test_suite_end"] == {"test_session.name": name} + assert payload["metadata"]["test_module_end"] == {"test_session.name": name} + assert payload["metadata"]["test"] == {"test_session.name": name} + + def test_set_test_session_name_from_command(self): + """When neither DD_TEST_SESSION_NAME nor a job id is provided, the test session name should be the test + command. + """ + with _ci_override_env(dict()), set_up_mock_civisibility(), _patch_dummy_writer(): + CIVisibility.enable() + CIVisibility.set_test_session_name(test_command="some_command") + self.assert_test_session_name("some_command") + + def test_set_test_session_name_from_dd_test_session_name_env_var(self): + """When DD_TEST_SESSION_NAME is provided, the test session name should be its value.""" + with _ci_override_env( + dict( + DD_TEST_SESSION_NAME="the_name", + ) + ), set_up_mock_civisibility(), _patch_dummy_writer(): + CIVisibility.enable() + CIVisibility.set_test_session_name(test_command="some_command") + self.assert_test_session_name("the_name") + + def test_set_test_session_name_from_job_name_and_command(self): + """When DD_TEST_SESSION_NAME is not provided, but a job id is, the test session name should be constructed from + the job id and test command. + """ + with _ci_override_env( + dict( + GITLAB_CI="1", + CI_JOB_NAME="the_job", + ) + ), set_up_mock_civisibility(), _patch_dummy_writer(): + CIVisibility.enable() + CIVisibility.set_test_session_name(test_command="some_command") + self.assert_test_session_name("the_job-some_command") + + def test_set_test_session_name_from_dd_test_session_name_env_var_priority(self): + """When both DD_TEST_SESSION_NAME and job id are provided, DD_TEST_SESSION_NAME wins.""" + with _ci_override_env( + dict( + GITLAB_CI="1", + CI_JOB_NAME="the_job", + DD_TEST_SESSION_NAME="the_name", + ) + ), set_up_mock_civisibility(), _patch_dummy_writer(): + CIVisibility.enable() + CIVisibility.set_test_session_name(test_command="some_command") + self.assert_test_session_name("the_name") diff --git a/tests/ci_visibility/test_encoder.py b/tests/ci_visibility/test_encoder.py index 46d6dd7f8c..402668a624 100644 --- a/tests/ci_visibility/test_encoder.py +++ b/tests/ci_visibility/test_encoder.py @@ -43,11 +43,7 @@ def test_encode_traces_civisibility_v0(): test_trace[1].set_tag_str("type", "test") encoder = CIVisibilityEncoderV01(0, 0) - encoder.set_metadata( - { - "language": "python", - } - ) + encoder.set_metadata("*", {"language": "python"}) for trace in traces: encoder.put(trace) payload, num_traces = encoder.encode() @@ -91,11 +87,7 @@ def test_encode_traces_civisibility_v0(): def test_encode_traces_civisibility_v0_no_traces(): encoder = CIVisibilityEncoderV01(0, 0) - encoder.set_metadata( - { - "language": "python", - } - ) + encoder.set_metadata("*", {"language": "python"}) payload, _ = encoder.encode() assert payload is None @@ -104,11 +96,7 @@ def test_encode_traces_civisibility_v0_empty_traces(): traces = [[], []] encoder = CIVisibilityEncoderV01(0, 0) - encoder.set_metadata( - { - "language": "python", - } - ) + encoder.set_metadata("*", {"language": "python"}) for trace in traces: encoder.put(trace) payload, size = encoder.encode() diff --git a/tests/contrib/pytest/test_pytest.py b/tests/contrib/pytest/test_pytest.py index 1ba0d66e83..d6db2838a4 100644 --- a/tests/contrib/pytest/test_pytest.py +++ b/tests/contrib/pytest/test_pytest.py @@ -220,6 +220,25 @@ def test_ok(): test_span = spans[0] assert test_span.get_tag("test.command") == "pytest -p no:randomly --ddtrace {}".format(file_name) + def test_pytest_command_test_session_name(self): + """Test that the pytest run command is used to set the test session name.""" + py_file = self.testdir.makepyfile( + """ + def test_ok(): + assert True + """ + ) + file_name = os.path.basename(py_file.strpath) + + with mock.patch( + "ddtrace.internal.ci_visibility.recorder.CIVisibility.set_test_session_name" + ) as set_test_session_name_mock: + self.inline_run("--ddtrace", file_name) + + set_test_session_name_mock.assert_called_once_with( + test_command="pytest -p no:randomly --ddtrace {}".format(file_name) + ) + def test_ini_no_ddtrace(self): """Test ini config, overridden by --no-ddtrace cli parameter.""" self.testdir.makefile(".ini", pytest="[pytest]\nddtrace=1\n") diff --git a/tests/contrib/unittest/test_unittest.py b/tests/contrib/unittest/test_unittest.py index 0a1ca63cd5..b2af61b47d 100644 --- a/tests/contrib/unittest/test_unittest.py +++ b/tests/contrib/unittest/test_unittest.py @@ -54,6 +54,22 @@ def _dummy_check_enabled_features(self): ): yield + def test_unittest_set_test_session_name(self): + """Check that the unittest command is used to set the test session name.""" + _set_tracer(self.tracer) + + class UnittestExampleTestCase(unittest.TestCase): + def test_will_pass_first(self): + self.assertTrue(2 == 2) + + with mock.patch( + "ddtrace.internal.ci_visibility.recorder.CIVisibility.set_test_session_name" + ) as set_test_session_name_mock: + suite = unittest.TestLoader().loadTestsFromTestCase(UnittestExampleTestCase) + unittest.TextTestRunner(verbosity=0).run(suite) + + set_test_session_name_mock.assert_called_once_with(test_command="python -m unittest") + def test_unittest_pass_single(self): """Test with a `unittest` test which should pass.""" _set_tracer(self.tracer) From 371959147f8fd24ef367e02174a72574e57808ea Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Sun, 6 Oct 2024 14:59:53 -0400 Subject: [PATCH 07/20] chore(profiling): mypy ignores `dd_wrapper/_deps` directory (#10951) `ddtrace/internal/datadog/profiling/build/dd_wrapper/_deps` could contain Python files from googletest. ## 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 - [ ] 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) --- mypy.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy.ini b/mypy.ini index 9e27bb1313..d33cc17d01 100644 --- a/mypy.ini +++ b/mypy.ini @@ -2,7 +2,7 @@ files = ddtrace, ddtrace/profiling/_build.pyx, docs -exclude = ddtrace/appsec/_iast/_taint_tracking/cmake-build-debug/|ddtrace/appsec/_iast/_taint_tracking/_vendor/ +exclude = ddtrace/appsec/_iast/_taint_tracking/cmake-build-debug/|ddtrace/appsec/_iast/_taint_tracking/_vendor/|ddtrace/internal/datadog/profiling/build/dd_wrapper/_deps # mypy thinks .pyx files are scripts and errors out if it finds multiple scripts scripts_are_modules = true show_error_codes = true From dc1dc3a3b3a54a222c7bcd0ff076000247103fe4 Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Mon, 7 Oct 2024 04:47:33 -0400 Subject: [PATCH 08/20] chore: set build parallel level for hatch ddtrace_unit_tests (#10952) ## 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 - [ ] 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) --- hatch.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/hatch.toml b/hatch.toml index 0c619d273f..c582f8f6a9 100644 --- a/hatch.toml +++ b/hatch.toml @@ -369,6 +369,7 @@ dependencies = [ [envs.ddtrace_unit_tests.env-vars] DD_IAST_ENABLED = "false" DD_REMOTE_CONFIGURATION_ENABLED = "false" +CMAKE_BUILD_PARALLEL_LEVEL="6" [envs.ddtrace_unit_tests.scripts] test = [ From c4cfa38673986f41ddb82aa7d027375040ae5c3a Mon Sep 17 00:00:00 2001 From: Juanjo Alvarez Martinez Date: Mon, 7 Oct 2024 11:41:17 +0200 Subject: [PATCH 09/20] chore(iast): fix all known memleaks in the native module + safety fixes (#10947) Signed-off-by: Juanjo Alvarez ## Description - Move the `TaintedObjectPtr` from `TaintedObject*` to `share_ptr`. This fixes the infamous `flask-appsec-iast-p100` leak on the `add_aspect`. - Fix a leak when iterating a `Py_List` on `AspectJoin`. - Handle a missing case in `AspectOperatorAdd`. - Fix leak in `args_are_text_and_same_type`. - Add some nullptr-checks. Thanks @avara1986 for the joint work on this. ## 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) --------- Signed-off-by: Juanjo Alvarez Co-authored-by: Alberto Vara --- .gitignore | 13 ++- .../_taint_tracking/Aspects/AspectJoin.cpp | 13 +-- .../Aspects/AspectOperatorAdd.cpp | 36 ++++++--- .../Initializer/Initializer.cpp | 27 +------ .../_taint_tracking/Initializer/Initializer.h | 2 - .../TaintTracking/TaintRange.cpp | 8 +- .../TaintTracking/TaintRange.h | 4 +- .../TaintTracking/TaintedObject.cpp | 23 ------ .../TaintTracking/TaintedObject.h | 9 +-- .../_taint_tracking/Utils/StringUtils.cpp | 6 ++ .../_iast/_taint_tracking/Utils/StringUtils.h | 14 +++- .../_taint_tracking/tests/test_add_aspect.cpp | 79 +++++++++++++++++++ .../_taint_tracking/tests/test_common.hpp | 18 +++++ .../_taint_tracking/tests/test_helpers.cpp | 1 - .../tests/test_index_aspect.cpp | 1 - .../_taint_tracking/tests/test_other.cpp | 1 - scripts/iast/mod_leak_functions.py | 4 +- 17 files changed, 168 insertions(+), 91 deletions(-) create mode 100644 ddtrace/appsec/_iast/_taint_tracking/tests/test_add_aspect.cpp diff --git a/.gitignore b/.gitignore index ce0f64a943..f6b1906f30 100644 --- a/.gitignore +++ b/.gitignore @@ -141,10 +141,21 @@ ddtrace/_version.py artifacts/ # IAST cpp +ddtrace/appsec/_iast/_taint_tracking/cmake_install.cmake +ddtrace/appsec/_iast/_taint_tracking/CMakeCache.txt +ddtrace/appsec/_iast/_taint_tracking/Makefile ddtrace/appsec/_iast/_taint_tracking/cmake-build-debug/* ddtrace/appsec/_iast/_taint_tracking/_deps/* ddtrace/appsec/_iast/_taint_tracking/CMakeFiles/* - +ddtrace/appsec/_iast/_taint_tracking/tests/CMakeFiles/* +ddtrace/appsec/_iast/_taint_tracking/tests/cmake_install.cmake +ddtrace/appsec/_iast/_taint_tracking/tests/CMakeCache.txt +ddtrace/appsec/_iast/_taint_tracking/tests/Makefile +ddtrace/appsec/_iast/_taint_tracking/tests/CTestTestfile.cmake +ddtrace/appsec/_iast/_taint_tracking/tests/native_tests* +ddtrace/appsec/_iast/_taint_tracking/_vendor/pybind11/Makefile +ddtrace/appsec/_iast/_taint_tracking/_vendor/pybind11/CMakeFiles/* +ddtrace/appsec/_iast/_taint_tracking/_vendor/pybind11/cmake_install.cmake # CircleCI generated config .circleci/config.gen.yml diff --git a/ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectJoin.cpp b/ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectJoin.cpp index 60d4aa3dea..476246e2a8 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectJoin.cpp +++ b/ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectJoin.cpp @@ -55,7 +55,7 @@ aspect_join_str(PyObject* sep, PyObject* new_result{ new_pyobject_id(result) }; set_tainted_object(new_result, result_to, tx_taint_map); - Py_DecRef(result); + Py_DECREF(result); return new_result; } @@ -134,7 +134,7 @@ aspect_join(PyObject* sep, PyObject* result, PyObject* iterable_elements, const PyObject* new_result{ new_pyobject_id(result) }; set_tainted_object(new_result, result_to, tx_taint_map); - Py_DecRef(result); + Py_DECREF(result); return new_result; } @@ -158,9 +158,10 @@ api_join_aspect(PyObject* self, PyObject* const* args, const Py_ssize_t nargs) PyObject* list_aux = PyList_New(0); while ((item = PyIter_Next(iterator))) { PyList_Append(list_aux, item); + Py_DECREF(item); } arg0 = list_aux; - Py_DecRef(iterator); + Py_DECREF(iterator); decref_arg0 = true; } } @@ -181,7 +182,7 @@ api_join_aspect(PyObject* self, PyObject* const* args, const Py_ssize_t nargs) if (has_pyerr()) { if (decref_arg0) { - Py_DecRef(arg0); + Py_DECREF(arg0); } return nullptr; } @@ -190,13 +191,13 @@ api_join_aspect(PyObject* self, PyObject* const* args, const Py_ssize_t nargs) if (not ctx_map or ctx_map->empty() or get_pyobject_size(result) == 0) { // Empty result cannot have taint ranges if (decref_arg0) { - Py_DecRef(arg0); + Py_DECREF(arg0); } return result; } auto res = aspect_join(sep, result, arg0, ctx_map); if (decref_arg0) { - Py_DecRef(arg0); + Py_DECREF(arg0); } return res; }); diff --git a/ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectOperatorAdd.cpp b/ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectOperatorAdd.cpp index 0d613375ac..a92a1436f9 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectOperatorAdd.cpp +++ b/ddtrace/appsec/_iast/_taint_tracking/Aspects/AspectOperatorAdd.cpp @@ -21,17 +21,16 @@ add_aspect(PyObject* result_o, const size_t len_candidate_text{ get_pyobject_size(candidate_text) }; const size_t len_text_to_add{ get_pyobject_size(text_to_add) }; - if (len_text_to_add == 0 and len_candidate_text > 0) { - return candidate_text; - } - if (len_text_to_add > 0 and len_candidate_text == 0 and text_to_add == result_o) { - return text_to_add; + // Appended from or with nothing, ranges didn't change + if ((len_text_to_add == 0 and len_candidate_text > 0) || + (len_text_to_add > 0 and len_candidate_text == 0 and text_to_add == result_o)) { + return result_o; } const auto& to_candidate_text = get_tainted_object(candidate_text, tx_taint_map); if (to_candidate_text and to_candidate_text->get_ranges().size() >= TaintedObject::TAINT_RANGE_LIMIT) { const auto& res_new_id = new_pyobject_id(result_o); - Py_DecRef(result_o); + Py_DECREF(result_o); // If left side is already at the maximum taint ranges, we just reuse its // ranges, we don't need to look at left side. set_tainted_object(res_new_id, to_candidate_text, tx_taint_map); @@ -44,12 +43,20 @@ add_aspect(PyObject* result_o, } if (!to_text_to_add) { const auto& res_new_id = new_pyobject_id(result_o); - Py_DecRef(result_o); + Py_DECREF(result_o); set_tainted_object(res_new_id, to_candidate_text, tx_taint_map); return res_new_id; } - auto tainted = initializer->allocate_tainted_object_copy(to_candidate_text); + if (!to_candidate_text) { + const auto tainted = initializer->allocate_tainted_object(); + tainted->add_ranges_shifted(to_text_to_add, static_cast(len_candidate_text)); + set_tainted_object(result_o, tainted, tx_taint_map); + } + + // At this point we have both to_candidate_text and to_text_to_add to we add the + // ranges from both to result_o + const auto tainted = initializer->allocate_tainted_object_copy(to_candidate_text); tainted->add_ranges_shifted(to_text_to_add, static_cast(len_candidate_text)); set_tainted_object(result_o, tainted, tx_taint_map); @@ -84,6 +91,9 @@ api_add_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs) // PyNumber_Add actually works for any type! result_o = PyNumber_Add(candidate_text, text_to_add); + if (result_o == nullptr) { + return nullptr; + } TRY_CATCH_ASPECT("add_aspect", return result_o, , { const auto tx_map = Initializer::get_tainting_map(); @@ -116,8 +126,6 @@ api_add_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs) PyObject* api_add_inplace_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs) { - PyObject* result_o = nullptr; - if (nargs != 2) { py::set_error(PyExc_ValueError, MSG_ERROR_N_PARAMS); return nullptr; @@ -125,7 +133,10 @@ api_add_inplace_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs) PyObject* candidate_text = args[0]; PyObject* text_to_add = args[1]; - result_o = PyNumber_InPlaceAdd(candidate_text, text_to_add); + PyObject* result_o = PyNumber_InPlaceAdd(candidate_text, text_to_add); + if (result_o == nullptr) { + return nullptr; + } TRY_CATCH_ASPECT("add_inplace_aspect", return result_o, , { const auto tx_map = Initializer::get_tainting_map(); @@ -152,7 +163,6 @@ api_add_inplace_aspect(PyObject* self, PyObject* const* args, Py_ssize_t nargs) is_notinterned_notfasttainted_unicode(text_to_add)) { return result_o; } - candidate_text = add_aspect(result_o, candidate_text, text_to_add, tx_map); - return candidate_text; + return add_aspect(result_o, candidate_text, text_to_add, tx_map); }); } \ No newline at end of file diff --git a/ddtrace/appsec/_iast/_taint_tracking/Initializer/Initializer.cpp b/ddtrace/appsec/_iast/_taint_tracking/Initializer/Initializer.cpp index 73b934366e..657d8a92e1 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/Initializer/Initializer.cpp +++ b/ddtrace/appsec/_iast/_taint_tracking/Initializer/Initializer.cpp @@ -14,7 +14,7 @@ Initializer::Initializer() { // Fill the taintedobjects stack for (int i = 0; i < TAINTEDOBJECTS_STACK_SIZE; i++) { - available_taintedobjects_stack.push(new TaintedObject()); + available_taintedobjects_stack.push(make_shared()); } // Fill the ranges stack @@ -41,9 +41,6 @@ Initializer::clear_tainting_map(const TaintRangeMapTypePtr& tx_map) return; } std::lock_guard lock(active_map_addreses_mutex); - for (const auto& [fst, snd] : *tx_map) { - snd.second->decref(); - } tx_map->clear(); active_map_addreses.erase(tx_map.get()); } @@ -114,12 +111,12 @@ TaintedObjectPtr Initializer::allocate_tainted_object() { if (!available_taintedobjects_stack.empty()) { - const auto& toptr = available_taintedobjects_stack.top(); + const auto toptr = available_taintedobjects_stack.top(); available_taintedobjects_stack.pop(); return toptr; } // Stack is empty, create new object - return new TaintedObject(); + return make_shared(); } TaintedObjectPtr @@ -147,24 +144,6 @@ Initializer::allocate_tainted_object_copy(const TaintedObjectPtr& from) return allocate_ranges_into_taint_object_copy(from->ranges_); } -void -Initializer::release_tainted_object(TaintedObjectPtr tobj) -{ - if (!tobj) { - return; - } - - tobj->reset(); - if (available_taintedobjects_stack.size() < TAINTEDOBJECTS_STACK_SIZE) { - available_taintedobjects_stack.push(tobj); - return; - } - - // Stack full, just delete the object (but to a reset before so ranges are - // reused or freed) - delete tobj; -} - TaintRangePtr Initializer::allocate_taint_range(const RANGE_START start, const RANGE_LENGTH length, const Source& origin) { diff --git a/ddtrace/appsec/_iast/_taint_tracking/Initializer/Initializer.h b/ddtrace/appsec/_iast/_taint_tracking/Initializer/Initializer.h index d453ce5527..6d7fbf3649 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/Initializer/Initializer.h +++ b/ddtrace/appsec/_iast/_taint_tracking/Initializer/Initializer.h @@ -140,8 +140,6 @@ class Initializer */ TaintedObjectPtr allocate_tainted_object_copy(const TaintedObjectPtr& from); - void release_tainted_object(TaintedObjectPtr tobj); - // FIXME: these should be static functions of TaintRange // IMPORTANT: if the returned object is not assigned to the map, you have // responsibility of calling release_taint_range on it or you'll have a leak. diff --git a/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintRange.cpp b/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintRange.cpp index a15274cd5e..ed50046cec 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintRange.cpp +++ b/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintRange.cpp @@ -181,9 +181,8 @@ set_ranges(PyObject* str, const TaintRangeRefs& ranges, const TaintRangeMapTypeP auto new_tainted_object = initializer->allocate_ranges_into_taint_object(ranges); set_fast_tainted_if_notinterned_unicode(str); - new_tainted_object->incref(); if (it != tx_map->end()) { - it->second.second->decref(); + it->second.second.reset(); it->second = std::make_pair(get_internal_hash(str), new_tainted_object); return true; } @@ -312,7 +311,6 @@ get_tainted_object(PyObject* str, const TaintRangeMapTypePtr& tx_map) } if (get_internal_hash(str) != it->second.first) { - it->second.second->decref(); tx_map->erase(it); return nullptr; } @@ -368,15 +366,13 @@ set_tainted_object(PyObject* str, TaintedObjectPtr tainted_object, const TaintRa // If the tainted object is different, we need to decref the previous one // and incref the new one. But if it's the same object, we can avoid both // operations, since they would be redundant. - it->second.second->decref(); - tainted_object->incref(); + it->second.second.reset(); it->second = std::make_pair(get_internal_hash(str), tainted_object); } // Update the hash, because for bytearrays it could have changed after the extend operation it->second.first = get_internal_hash(str); return; } - tainted_object->incref(); tx_map->insert({ obj_id, std::make_pair(get_internal_hash(str), tainted_object) }); } diff --git a/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintRange.h b/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintRange.h index 6d6e1a9a27..efb817a335 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintRange.h +++ b/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintRange.h @@ -17,7 +17,7 @@ namespace py = pybind11; class TaintedObject; // Alias -using TaintedObjectPtr = TaintedObject*; +using TaintedObjectPtr = shared_ptr; #ifdef NDEBUG // Decide wether to use abseil @@ -134,7 +134,7 @@ api_is_unicode_and_not_fast_tainted(const py::handle& str) return is_notinterned_notfasttainted_unicode(str.ptr()); } -TaintedObject* +TaintedObjectPtr get_tainted_object(PyObject* str, const TaintRangeMapTypePtr& tx_taint_map); Py_hash_t diff --git a/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintedObject.cpp b/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintedObject.cpp index 3a5aa2aa27..3b00a5d28e 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintedObject.cpp +++ b/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintedObject.cpp @@ -140,34 +140,11 @@ void TaintedObject::reset() { move_ranges_to_stack(); - rc_ = 0; if (initializer) { ranges_.reserve(RANGES_INITIAL_RESERVE); } } -void -TaintedObject::incref() -{ - rc_++; -} - -void -TaintedObject::decref() -{ - if (--rc_ <= 0) { - release(); - } -} - -void -TaintedObject::release() -{ - // If rc_ is negative, there is a bug. - // assert(rc_ == 0); - initializer->release_tainted_object(this); -} - void pyexport_taintedobject(const py::module& m) { diff --git a/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintedObject.h b/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintedObject.h index a4198424a7..88198fabce 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintedObject.h +++ b/ddtrace/appsec/_iast/_taint_tracking/TaintTracking/TaintedObject.h @@ -8,7 +8,6 @@ class TaintedObject private: TaintRangeRefs ranges_; - size_t rc_{}; public: constexpr static int TAINT_RANGE_LIMIT = 100; @@ -36,7 +35,7 @@ class TaintedObject [[nodiscard]] TaintRangeRefs get_ranges_copy() const { return ranges_; } - void add_ranges_shifted(TaintedObject* tainted_object, + void add_ranges_shifted(TaintedObjectPtr tainted_object, RANGE_START offset, RANGE_LENGTH max_length = -1, RANGE_START orig_offset = -1); @@ -53,12 +52,6 @@ class TaintedObject void move_ranges_to_stack(); void reset(); - - void incref(); - - void decref(); - - void release(); }; void diff --git a/ddtrace/appsec/_iast/_taint_tracking/Utils/StringUtils.cpp b/ddtrace/appsec/_iast/_taint_tracking/Utils/StringUtils.cpp index 0fd274925b..2df5ffa5e2 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/Utils/StringUtils.cpp +++ b/ddtrace/appsec/_iast/_taint_tracking/Utils/StringUtils.cpp @@ -112,6 +112,9 @@ new_pyobject_id(PyObject* tainted_object) const auto res = PyObject_CallFunctionObjArgs(bytes_join_ptr.ptr(), val, NULL); Py_XDECREF(val); Py_XDECREF(empty_bytes); + if (res == nullptr) { + return tainted_object; + } return res; } @@ -138,6 +141,9 @@ new_pyobject_id(PyObject* tainted_object) Py_XDECREF(val); Py_XDECREF(empty_bytes); Py_XDECREF(empty_bytearray); + if (res == nullptr) { + return tainted_object; + } return res; } return tainted_object; diff --git a/ddtrace/appsec/_iast/_taint_tracking/Utils/StringUtils.h b/ddtrace/appsec/_iast/_taint_tracking/Utils/StringUtils.h index d0dbf1ddf8..d2da5dcd82 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/Utils/StringUtils.h +++ b/ddtrace/appsec/_iast/_taint_tracking/Utils/StringUtils.h @@ -81,12 +81,22 @@ template bool args_are_text_and_same_type(PyObject* first, PyObject* second, Args... args) { + if (first == nullptr || second == nullptr) { + return false; + } + + const auto type_first = PyObject_Type(first); + const auto type_second = PyObject_Type(second); + // Check if both first and second are valid text types and of the same type - if (first == nullptr || second == nullptr || !is_text(first) || !is_text(second) || - PyObject_Type(first) != PyObject_Type(second)) { + if (!is_text(first) || !is_text(second) || type_first != type_second) { + Py_XDECREF(type_first); + Py_XDECREF(type_second); return false; } + Py_XDECREF(type_first); + Py_XDECREF(type_second); // Recursively check the rest of the arguments return args_are_text_and_same_type(second, args...); } diff --git a/ddtrace/appsec/_iast/_taint_tracking/tests/test_add_aspect.cpp b/ddtrace/appsec/_iast/_taint_tracking/tests/test_add_aspect.cpp new file mode 100644 index 0000000000..85ac7dc3d9 --- /dev/null +++ b/ddtrace/appsec/_iast/_taint_tracking/tests/test_add_aspect.cpp @@ -0,0 +1,79 @@ +#include "Aspects/Helpers.h" + +#include +#include +#include + +using AspectAddCheck = PyEnvWithContext; + +TEST_F(AspectAddCheck, check_api_add_aspect_strings_candidate_text_empty) +{ + PyObject* candidate_text = this->StringToPyObjectStr(""); + PyObject* text_to_add = this->StringToPyObjectStr("def"); + PyObject* args_array[2]; + args_array[0] = candidate_text; + args_array[1] = text_to_add; + auto result = api_add_aspect(nullptr, args_array, 2); + EXPECT_FALSE(has_pyerr()); + + std::string result_string = this->PyObjectStrToString(result); + + EXPECT_EQ(result_string, "def"); + Py_DecRef(candidate_text); + Py_DecRef(text_to_add); + Py_DecRef(result); +} + +TEST_F(AspectAddCheck, check_api_add_aspect_strings_text_to_add_empty) +{ + PyObject* candidate_text = this->StringToPyObjectStr("abc"); + PyObject* text_to_add = this->StringToPyObjectStr(""); + PyObject* args_array[2]; + args_array[0] = candidate_text; + args_array[1] = text_to_add; + auto result = api_add_aspect(nullptr, args_array, 2); + EXPECT_FALSE(has_pyerr()); + + std::string result_string = this->PyObjectStrToString(result); + + EXPECT_EQ(result_string, "abc"); + Py_DecRef(candidate_text); + Py_DecRef(text_to_add); + Py_DecRef(result); +} + +TEST_F(AspectAddCheck, check_api_add_aspect_strings) +{ + PyObject* candidate_text = this->StringToPyObjectStr("abc"); + PyObject* text_to_add = this->StringToPyObjectStr("def"); + PyObject* args_array[2]; + args_array[0] = candidate_text; + args_array[1] = text_to_add; + auto result = api_add_aspect(nullptr, args_array, 2); + EXPECT_FALSE(has_pyerr()); + + std::string result_string = this->PyObjectStrToString(result); + + EXPECT_EQ(result_string, "abcdef"); + Py_DecRef(candidate_text); + Py_DecRef(text_to_add); + Py_DecRef(result); +} + +TEST_F(AspectAddCheck, check_api_add_aspect_bytes) +{ + PyObject* candidate_text = this->StringToPyObjectBytes("abc"); + PyObject* text_to_add = this->StringToPyObjectBytes("def"); + PyObject* args_array[2]; + args_array[0] = candidate_text; + args_array[1] = text_to_add; + auto result = api_add_aspect(nullptr, args_array, 2); + EXPECT_FALSE(has_pyerr()); + + std::string result_string = this->PyObjectBytesToString(result); + + EXPECT_EQ(result_string, "abcdef"); + Py_DecRef(candidate_text); + Py_DecRef(text_to_add); + Py_DecRef(result); +} diff --git a/ddtrace/appsec/_iast/_taint_tracking/tests/test_common.hpp b/ddtrace/appsec/_iast/_taint_tracking/tests/test_common.hpp index 81af3566b5..b7338e3d68 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/tests/test_common.hpp +++ b/ddtrace/appsec/_iast/_taint_tracking/tests/test_common.hpp @@ -29,4 +29,22 @@ class PyEnvWithContext : public ::testing::Test initializer.reset(); py::finalize_interpreter(); } + + public: + PyObject* StringToPyObjectStr(const string& ob) { return PyUnicode_FromString(ob.c_str()); } + string PyObjectStrToString(PyObject* ob) + { + PyObject* utf8_str = PyUnicode_AsEncodedString(ob, "utf-8", "strict"); + const char* res_data = PyBytes_AsString(utf8_str); + std::string res_string(res_data); + Py_DecRef(utf8_str); + return res_string; + } + PyObject* StringToPyObjectBytes(const string& ob) { return PyBytes_FromString(ob.c_str()); } + string PyObjectBytesToString(PyObject* ob) + { + const char* res_data = PyBytes_AsString(ob); + std::string res_string(res_data); + return res_string; + } }; diff --git a/ddtrace/appsec/_iast/_taint_tracking/tests/test_helpers.cpp b/ddtrace/appsec/_iast/_taint_tracking/tests/test_helpers.cpp index 1078e23a1e..825a03ba78 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/tests/test_helpers.cpp +++ b/ddtrace/appsec/_iast/_taint_tracking/tests/test_helpers.cpp @@ -1,5 +1,4 @@ #include -#include #include #include diff --git a/ddtrace/appsec/_iast/_taint_tracking/tests/test_index_aspect.cpp b/ddtrace/appsec/_iast/_taint_tracking/tests/test_index_aspect.cpp index 444325d8fd..2961cc80ea 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/tests/test_index_aspect.cpp +++ b/ddtrace/appsec/_iast/_taint_tracking/tests/test_index_aspect.cpp @@ -1,7 +1,6 @@ #include "Aspects/Helpers.h" #include -#include #include using AspectIndexCheck = PyEnvWithContext; diff --git a/ddtrace/appsec/_iast/_taint_tracking/tests/test_other.cpp b/ddtrace/appsec/_iast/_taint_tracking/tests/test_other.cpp index 940e52222b..0a3ccf063b 100644 --- a/ddtrace/appsec/_iast/_taint_tracking/tests/test_other.cpp +++ b/ddtrace/appsec/_iast/_taint_tracking/tests/test_other.cpp @@ -1,6 +1,5 @@ #include #include -#include // jjj #include using PyByteArray_Check = PyEnvCheck; diff --git a/scripts/iast/mod_leak_functions.py b/scripts/iast/mod_leak_functions.py index e43db154db..e55480a1d3 100644 --- a/scripts/iast/mod_leak_functions.py +++ b/scripts/iast/mod_leak_functions.py @@ -24,7 +24,9 @@ def test_doit(): string3 = string1 + string2 # 2 propagation ranges: hiroot1234 string4 = "-".join([string3, string3, string3]) # 6 propagation ranges: hiroot1234-hiroot1234-hiroot1234 - string5 = string4[0:20] # 1 propagation range: hiroot1234-hiroot123 + string4_2 = string1 + string4_2 += " " + " ".join(string_ for string_ in [string4, string4, string4]) + string5 = string4_2[0:20] # 1 propagation range: hiroot1234-hiroot123 string6 = string5.title() # 1 propagation range: Hiroot1234-Hiroot123 string7 = string6.upper() # 1 propagation range: HIROOT1234-HIROOT123 string8 = "%s_notainted" % string7 # 1 propagation range: HIROOT1234-HIROOT123_notainted From 6ae8477f0317c26aca50cbdca3e8288af9cebd08 Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Mon, 7 Oct 2024 06:56:04 -0400 Subject: [PATCH 10/20] chore(ci): parallelize unittest github actions (#10954) 1. Use matrix in actions: No need to specify archs as GitHub actions runners only support specific archs for given OSes and we've only been running ubuntu-latest (x86_64), windows-latest (x86_64) and macos-latest (arm64). 2. Pass python version to hatch run. ## 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 - [ ] 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) --- .github/workflows/unit_tests.yml | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index b3d1981a5b..4abce66d34 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -14,26 +14,15 @@ jobs: strategy: fail-fast: false matrix: - include: - - os: ubuntu-latest - archs: x86_64 i686 - #- os: arm-4core-linux - # archs: aarch64 - - os: windows-latest - archs: AMD64 x86 - - os: macos-latest - archs: arm64 + os: [ubuntu-latest, windows-latest, macos-latest] + # Keep this in sync with hatch.toml + python-version: ["3.7", "3.10", "3.12"] steps: - uses: actions/checkout@v4 # Include all history and tags with: fetch-depth: 0 - - uses: actions/setup-python@v5 - name: Install Python - with: - python-version: '3.12' - - uses: actions-rust-lang/setup-rust-toolchain@v1 - name: Install latest stable toolchain and rustfmt run: rustup update stable && rustup default stable && rustup component add rustfmt clippy @@ -44,4 +33,4 @@ jobs: version: "1.12.0" - name: Run tests - run: hatch run ddtrace_unit_tests:test + run: hatch run +py=${{ matrix.python-version }} ddtrace_unit_tests:test From f446733e079039f9b79197d0c0aff2b0a1461ea3 Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Mon, 7 Oct 2024 08:09:35 -0400 Subject: [PATCH 11/20] chore(profiling): use FindPython3 with Python3_ROOT_DIR hint from setup.py (#10950) [FindPython3](https://cmake.org/cmake/help/latest/module/FindPython3.html) finds installed Python executable, headers, and library using `Python3_ROOT_DIR` [hint](https://cmake.org/cmake/help/latest/module/FindPython3.html#hints). ## 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) --- .../datadog/profiling/build_standalone.sh | 3 --- .../profiling/crashtracker/CMakeLists.txt | 20 +++++++----------- .../datadog/profiling/ddup/CMakeLists.txt | 21 ++++++++----------- .../datadog/profiling/stack_v2/CMakeLists.txt | 1 + setup.py | 18 +--------------- 5 files changed, 19 insertions(+), 44 deletions(-) diff --git a/ddtrace/internal/datadog/profiling/build_standalone.sh b/ddtrace/internal/datadog/profiling/build_standalone.sh index 238399e7d0..62ca35ba19 100755 --- a/ddtrace/internal/datadog/profiling/build_standalone.sh +++ b/ddtrace/internal/datadog/profiling/build_standalone.sh @@ -73,9 +73,6 @@ cmake_args=( -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_VERBOSE_MAKEFILE=ON -DLIB_INSTALL_DIR=$(realpath $MY_DIR)/lib - -DPython3_INCLUDE_DIRS=$(python3 -c "from distutils.sysconfig import get_python_inc; print(get_python_inc())") - -DPython3_EXECUTABLE=$(which python3) - -DPython3_LIBRARY=$(python3 -c "import distutils.sysconfig as sysconfig; print(sysconfig.get_config_var('LIBDIR'))") ) # Initial build targets; no matter what, dd_wrapper is the base dependency, so it's always built diff --git a/ddtrace/internal/datadog/profiling/crashtracker/CMakeLists.txt b/ddtrace/internal/datadog/profiling/crashtracker/CMakeLists.txt index e9fdd8f391..678ef83291 100644 --- a/ddtrace/internal/datadog/profiling/crashtracker/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/crashtracker/CMakeLists.txt @@ -17,31 +17,27 @@ include(FindLibdatadog) add_subdirectory(../dd_wrapper ${CMAKE_CURRENT_BINARY_DIR}/../dd_wrapper_build) +find_package(Python3 COMPONENTS Interpreter Development) # Make sure we have necessary Python variables if(NOT Python3_INCLUDE_DIRS) message(FATAL_ERROR "Python3_INCLUDE_DIRS not found") endif() -# This sets some parameters for the target build, which can only be defined by setup.py -set(ENV{PY_MAJOR_VERSION} ${PY_MAJOR_VERSION}) -set(ENV{PY_MINOR_VERSION} ${PY_MINOR_VERSION}) -set(ENV{PY_MICRO_VERSION} ${PY_MICRO_VERSION}) - -# if PYTHON_EXECUTABLE is unset or empty, but Python3_EXECUTABLE is set, use that -if(NOT PYTHON_EXECUTABLE AND Python3_EXECUTABLE) - set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE}) -endif() - # If we still don't have a Python executable, we can't continue -if(NOT PYTHON_EXECUTABLE) +if(NOT Python3_EXECUTABLE) message(FATAL_ERROR "Python executable not found") endif() +# This sets some parameters for the target build +set(ENV{PY_MAJOR_VERSION} ${Python3_VERSION_MAJOR}) +set(ENV{PY_MINOR_VERSION} ${Python3_VERSION_MINOR}) +set(ENV{PY_MICRO_VERSION} ${Python3_VERSION_PATCH}) + # Cythonize the .pyx file set(CRASHTRACKER_CPP_SRC ${CMAKE_CURRENT_BINARY_DIR}/_crashtracker.cpp) add_custom_command( OUTPUT ${CRASHTRACKER_CPP_SRC} - COMMAND ${PYTHON_EXECUTABLE} -m cython ${CMAKE_CURRENT_LIST_DIR}/_crashtracker.pyx -o ${CRASHTRACKER_CPP_SRC} + COMMAND ${Python3_EXECUTABLE} -m cython ${CMAKE_CURRENT_LIST_DIR}/_crashtracker.pyx -o ${CRASHTRACKER_CPP_SRC} DEPENDS ${CMAKE_CURRENT_LIST_DIR}/_crashtracker.pyx) # Specify the target C-extension that we want to build diff --git a/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt b/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt index fcb7e25d20..e0e71bad11 100644 --- a/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/ddup/CMakeLists.txt @@ -22,31 +22,28 @@ include(FindLibdatadog) # things may yet be factored differently. add_subdirectory(../dd_wrapper ${CMAKE_CURRENT_BINARY_DIR}/../dd_wrapper_build) +find_package(Python3 COMPONENTS Interpreter Development) + # Make sure we have necessary Python variables if(NOT Python3_INCLUDE_DIRS) message(FATAL_ERROR "Python3_INCLUDE_DIRS not found") endif() -# This sets some parameters for the target build, which can only be defined by setup.py -set(ENV{PY_MAJOR_VERSION} ${PY_MAJOR_VERSION}) -set(ENV{PY_MINOR_VERSION} ${PY_MINOR_VERSION}) -set(ENV{PY_MICRO_VERSION} ${PY_MICRO_VERSION}) - -# if PYTHON_EXECUTABLE is unset or empty, but Python3_EXECUTABLE is set, use that -if(NOT PYTHON_EXECUTABLE AND Python3_EXECUTABLE) - set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE}) -endif() - # If we still don't have a Python executable, we can't continue -if(NOT PYTHON_EXECUTABLE) +if(NOT Python3_EXECUTABLE) message(FATAL_ERROR "Python executable not found") endif() +# This sets some parameters for the target build +set(ENV{PY_MAJOR_VERSION} ${Python3_VERSION_MAJOR}) +set(ENV{PY_MINOR_VERSION} ${Python3_VERSION_MINOR}) +set(ENV{PY_MICRO_VERSION} ${Python3_VERSION_PATCH}) + # Cythonize the .pyx file set(DDUP_CPP_SRC ${CMAKE_CURRENT_BINARY_DIR}/_ddup.cpp) add_custom_command( OUTPUT ${DDUP_CPP_SRC} - COMMAND ${PYTHON_EXECUTABLE} -m cython ${CMAKE_CURRENT_LIST_DIR}/_ddup.pyx -o ${DDUP_CPP_SRC} + COMMAND ${Python3_EXECUTABLE} -m cython ${CMAKE_CURRENT_LIST_DIR}/_ddup.pyx -o ${DDUP_CPP_SRC} DEPENDS ${CMAKE_CURRENT_LIST_DIR}/_ddup.pyx) # Specify the target C-extension that we want to build diff --git a/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt b/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt index 8bcc96276d..bc0caae58d 100644 --- a/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt +++ b/ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt @@ -21,6 +21,7 @@ include(FindCppcheck) # design is unknown. Hack it for now. add_subdirectory(../dd_wrapper ${CMAKE_CURRENT_BINARY_DIR}/../dd_wrapper_build) +find_package(Python3 COMPONENTS Interpreter Development) # Make sure we have necessary Python variables if(NOT Python3_INCLUDE_DIRS) message(FATAL_ERROR "Python3_INCLUDE_DIRS not found") diff --git a/setup.py b/setup.py index 196a2c452c..f45e9b086c 100644 --- a/setup.py +++ b/setup.py @@ -337,18 +337,12 @@ def build_extension_cmake(self, ext): cmake_build_dir = Path(self.build_lib.replace("lib.", "cmake."), ext.name).resolve() cmake_build_dir.mkdir(parents=True, exist_ok=True) - # Get development paths - python_include = sysconfig.get_paths()["include"] - python_lib = sysconfig.get_config_var("LIBDIR") - # Which commands are passed to _every_ cmake invocation cmake_args = ext.cmake_args or [] cmake_args += [ "-S{}".format(ext.source_dir), # cmake>=3.13 "-B{}".format(cmake_build_dir), # cmake>=3.13 - "-DPython3_INCLUDE_DIRS={}".format(python_include), - "-DPython3_LIBRARIES={}".format(python_lib), - "-DPYTHON_EXECUTABLE={}".format(sys.executable), + "-DPython3_ROOT_DIR={}".format(sysconfig.get_config_var("prefix")), "-DCMAKE_BUILD_TYPE={}".format(ext.build_type), "-DLIB_INSTALL_DIR={}".format(output_dir), "-DEXTENSION_NAME={}".format(extension_basename), @@ -536,11 +530,6 @@ def get_exts_for(name): CMakeExtension( "ddtrace.internal.datadog.profiling.ddup._ddup", source_dir=DDUP_DIR, - cmake_args=[ - "-DPY_MAJOR_VERSION={}".format(sys.version_info.major), - "-DPY_MINOR_VERSION={}".format(sys.version_info.minor), - "-DPY_MICRO_VERSION={}".format(sys.version_info.micro), - ], optional=False, ) ) @@ -549,11 +538,6 @@ def get_exts_for(name): CMakeExtension( "ddtrace.internal.datadog.profiling.crashtracker._crashtracker", source_dir=CRASHTRACKER_DIR, - cmake_args=[ - "-DPY_MAJOR_VERSION={}".format(sys.version_info.major), - "-DPY_MINOR_VERSION={}".format(sys.version_info.minor), - "-DPY_MICRO_VERSION={}".format(sys.version_info.micro), - ], optional=False, ) ) From 6536952412af541a83264d083729993db45a7e24 Mon Sep 17 00:00:00 2001 From: Quinna Halim Date: Mon, 7 Oct 2024 09:22:22 -0400 Subject: [PATCH 12/20] chore(pymongo): service name override in pymongo (#10917) This PR makes the following changes: - Setting `config.pymongo['service']` did not override service name, due to previously having a `Pin` overriding. This changes the `Pin` on the MongoClient` to `None` - We should use `ext_service` instead of `int_service` for pymongo ## 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) --- ddtrace/contrib/internal/pymongo/client.py | 8 +++-- ddtrace/contrib/internal/pymongo/patch.py | 9 +++--- tests/contrib/pymongo/test.py | 35 ++++++++++++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/ddtrace/contrib/internal/pymongo/client.py b/ddtrace/contrib/internal/pymongo/client.py index f8441f3091..cb7466ac7a 100644 --- a/ddtrace/contrib/internal/pymongo/client.py +++ b/ddtrace/contrib/internal/pymongo/client.py @@ -10,10 +10,12 @@ # project import ddtrace +from ddtrace import Pin from ddtrace import config from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY from ddtrace.constants import SPAN_KIND from ddtrace.constants import SPAN_MEASURED_KEY +from ddtrace.contrib import trace_utils from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes from ddtrace.ext import db @@ -67,7 +69,7 @@ def __getddpin__(client): client.__getddpin__ = functools.partial(__getddpin__, client) # Set a pin on the traced mongo client - ddtrace.Pin(service=_DEFAULT_SERVICE).onto(client) + Pin(service=None).onto(client) # The function is exposed in the public API, but it is not used in the codebase. @@ -128,7 +130,7 @@ def _datadog_trace_operation(operation, wrapped): span = pin.tracer.trace( schematize_database_operation("pymongo.cmd", database_provider="mongodb"), span_type=SpanTypes.MONGODB, - service=pin.service, + service=trace_utils.ext_service(pin, config.pymongo), ) span.set_tag_str(COMPONENT, config.pymongo.integration_name) @@ -251,7 +253,7 @@ def _trace_cmd(cmd, socket_instance, address): s = pin.tracer.trace( schematize_database_operation("pymongo.cmd", database_provider="mongodb"), span_type=SpanTypes.MONGODB, - service=pin.service, + service=trace_utils.ext_service(pin, config.pymongo), ) s.set_tag_str(COMPONENT, config.pymongo.integration_name) diff --git a/ddtrace/contrib/internal/pymongo/patch.py b/ddtrace/contrib/internal/pymongo/patch.py index 5032ce3b48..0c0927ffea 100644 --- a/ddtrace/contrib/internal/pymongo/patch.py +++ b/ddtrace/contrib/internal/pymongo/patch.py @@ -16,6 +16,8 @@ from ddtrace.internal.wrapping import unwrap as _u from ddtrace.internal.wrapping import wrap as _w +from ....internal.schema import schematize_service_name + # keep TracedMongoClient import to maintain backwards compatibility from .client import TracedMongoClient # noqa: F401 from .client import _trace_mongo_client_init @@ -46,10 +48,7 @@ _CHECKOUT_FN_NAME = "get_socket" if pymongo.version_tuple < (4, 5) else "checkout" -config._add( - "pymongo", - dict(_default_service="pymongo"), -) +config._add("pymongo", dict(_default_service=schematize_service_name("pymongo"))) def get_version(): @@ -119,7 +118,7 @@ def traced_get_socket(func, args, kwargs): with pin.tracer.trace( "pymongo.%s" % _CHECKOUT_FN_NAME, - service=trace_utils.int_service(pin, config.pymongo), + service=trace_utils.ext_service(pin, config.pymongo), span_type=SpanTypes.MONGODB, ) as span: span.set_tag_str(COMPONENT, config.pymongo.integration_name) diff --git a/tests/contrib/pymongo/test.py b/tests/contrib/pymongo/test.py index 69633b13e7..f439036fcc 100644 --- a/tests/contrib/pymongo/test.py +++ b/tests/contrib/pymongo/test.py @@ -544,6 +544,27 @@ def test_user_specified_service_v0(self): assert len(spans) == 2 assert spans[1].service != "mysvc" + @TracerTestCase.run_in_subprocess(env_overrides=dict()) + def test_user_specified_service_default_override(self): + """ + Override service name using config + """ + from ddtrace import config + from ddtrace import patch + + cfg = config.pymongo + cfg["service"] = "new-mongo" + patch(pymongo=True) + assert cfg.service == "new-mongo", f"service name is {cfg.service}" + tracer = DummyTracer() + client = pymongo.MongoClient(port=MONGO_CONFIG["port"]) + Pin.get_from(client).clone(tracer=tracer).onto(client) + + client["testdb"].drop_collection("whatever") + spans = tracer.pop() + assert spans + assert spans[0].service == "new-mongo", f"service name is {spans[0].service}" + @TracerTestCase.run_in_subprocess(env_overrides=dict(DD_SERVICE="mysvc", DD_TRACE_SPAN_ATTRIBUTE_SCHEMA="v1")) def test_user_specified_service_v1(self): """ @@ -563,6 +584,20 @@ def test_user_specified_service_v1(self): assert len(spans) == 2 assert spans[1].service == "mysvc" + @TracerTestCase.run_in_subprocess(env_overrides=dict()) + def test_unspecified_service_v0(self): + """ + v0: When a user does not specify a service for the app + The pymongo integration should use pymongo. + """ + tracer = DummyTracer() + client = pymongo.MongoClient(port=MONGO_CONFIG["port"]) + Pin.get_from(client).clone(tracer=tracer).onto(client) + client["testdb"].drop_collection("whatever") + spans = tracer.pop() + assert len(spans) == 2 + assert spans[1].service == "pymongo" + @TracerTestCase.run_in_subprocess( env_overrides=dict(DD_PYMONGO_SERVICE="mypymongo", DD_TRACE_SPAN_ATTRIBUTE_SCHEMA="v0") ) From d8c8fa9a71dba6da02b1772456c9c4460dd6ca42 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 7 Oct 2024 13:23:32 +0000 Subject: [PATCH 13/20] chore: update bottle latest version to 0.13.1 (#10956) ## 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: quinna-h <175135214+quinna-h@users.noreply.github.com> Co-authored-by: Quinna Halim --- .riot/requirements/1280196.txt | 36 +++++++++++++++++----------------- .riot/requirements/15dee3b.txt | 34 ++++++++++++++++---------------- .riot/requirements/18b1b66.txt | 16 +++++++-------- .riot/requirements/573fdbf.txt | 36 +++++++++++++++++----------------- .riot/requirements/760d56e.txt | 18 ++++++++--------- .riot/requirements/8c110bf.txt | 34 ++++++++++++++++---------------- 6 files changed, 87 insertions(+), 87 deletions(-) diff --git a/.riot/requirements/1280196.txt b/.riot/requirements/1280196.txt index ea2303daf1..55d4fd22d3 100644 --- a/.riot/requirements/1280196.txt +++ b/.riot/requirements/1280196.txt @@ -2,28 +2,28 @@ # This file is autogenerated by pip-compile with Python 3.8 # by the following command: # -# pip-compile --no-annotate .riot/requirements/1280196.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/1280196.in # -attrs==23.1.0 -beautifulsoup4==4.12.2 -bottle==0.12.25 -coverage[toml]==7.3.4 -exceptiongroup==1.2.0 +attrs==24.2.0 +beautifulsoup4==4.12.3 +bottle==0.13.1 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 -importlib-metadata==7.0.0 +importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 -packaging==23.2 -pluggy==1.3.0 -pytest==7.4.3 -pytest-cov==4.1.0 -pytest-mock==3.12.0 +packaging==24.1 +pluggy==1.5.0 +pytest==8.3.3 +pytest-cov==5.0.0 +pytest-mock==3.14.0 pytest-randomly==3.15.0 sortedcontainers==2.4.0 -soupsieve==2.5 -tomli==2.0.1 -waitress==2.1.2 -webob==1.8.7 -webtest==3.0.0 -zipp==3.17.0 +soupsieve==2.6 +tomli==2.0.2 +waitress==3.0.0 +webob==1.8.8 +webtest==3.0.1 +zipp==3.20.2 diff --git a/.riot/requirements/15dee3b.txt b/.riot/requirements/15dee3b.txt index 748fe265b4..2b8691e851 100644 --- a/.riot/requirements/15dee3b.txt +++ b/.riot/requirements/15dee3b.txt @@ -2,28 +2,28 @@ # This file is autogenerated by pip-compile with Python 3.9 # by the following command: # -# pip-compile --no-annotate .riot/requirements/15dee3b.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/15dee3b.in # -attrs==23.1.0 -beautifulsoup4==4.12.2 +attrs==24.2.0 +beautifulsoup4==4.12.3 bottle==0.12.25 -coverage[toml]==7.3.4 -exceptiongroup==1.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 -importlib-metadata==7.0.0 +importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 -packaging==23.2 -pluggy==1.3.0 -pytest==7.4.3 -pytest-cov==4.1.0 -pytest-mock==3.12.0 +packaging==24.1 +pluggy==1.5.0 +pytest==8.3.3 +pytest-cov==5.0.0 +pytest-mock==3.14.0 pytest-randomly==3.15.0 sortedcontainers==2.4.0 -soupsieve==2.5 -tomli==2.0.1 -waitress==2.1.2 -webob==1.8.7 -webtest==3.0.0 -zipp==3.17.0 +soupsieve==2.6 +tomli==2.0.2 +waitress==3.0.0 +webob==1.8.8 +webtest==3.0.1 +zipp==3.20.2 diff --git a/.riot/requirements/18b1b66.txt b/.riot/requirements/18b1b66.txt index 026db1fa37..222f1e2b51 100644 --- a/.riot/requirements/18b1b66.txt +++ b/.riot/requirements/18b1b66.txt @@ -2,21 +2,21 @@ # This file is autogenerated by pip-compile with Python 3.7 # by the following command: # -# pip-compile --config=pyproject.toml --no-annotate --resolver=backtracking .riot/requirements/18b1b66.in +# pip-compile --allow-unsafe --config=pyproject.toml --no-annotate --resolver=backtracking .riot/requirements/18b1b66.in # -attrs==23.1.0 -beautifulsoup4==4.12.2 +attrs==24.2.0 +beautifulsoup4==4.12.3 bottle==0.12.25 coverage[toml]==7.2.7 -exceptiongroup==1.2.0 +exceptiongroup==1.2.2 hypothesis==6.45.0 importlib-metadata==6.7.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 -packaging==23.2 +packaging==24.0 pluggy==1.2.0 -pytest==7.4.3 +pytest==7.4.4 pytest-cov==4.1.0 pytest-mock==3.11.1 pytest-randomly==3.12.0 @@ -25,6 +25,6 @@ soupsieve==2.4.1 tomli==2.0.1 typing-extensions==4.7.1 waitress==2.1.2 -webob==1.8.7 -webtest==3.0.0 +webob==1.8.8 +webtest==3.0.1 zipp==3.15.0 diff --git a/.riot/requirements/573fdbf.txt b/.riot/requirements/573fdbf.txt index effce0978a..d6aff21e2a 100644 --- a/.riot/requirements/573fdbf.txt +++ b/.riot/requirements/573fdbf.txt @@ -2,28 +2,28 @@ # This file is autogenerated by pip-compile with Python 3.9 # by the following command: # -# pip-compile --no-annotate .riot/requirements/573fdbf.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/573fdbf.in # -attrs==23.1.0 -beautifulsoup4==4.12.2 -bottle==0.12.25 -coverage[toml]==7.3.4 -exceptiongroup==1.2.0 +attrs==24.2.0 +beautifulsoup4==4.12.3 +bottle==0.13.1 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 -importlib-metadata==7.0.0 +importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 -packaging==23.2 -pluggy==1.3.0 -pytest==7.4.3 -pytest-cov==4.1.0 -pytest-mock==3.12.0 +packaging==24.1 +pluggy==1.5.0 +pytest==8.3.3 +pytest-cov==5.0.0 +pytest-mock==3.14.0 pytest-randomly==3.15.0 sortedcontainers==2.4.0 -soupsieve==2.5 -tomli==2.0.1 -waitress==2.1.2 -webob==1.8.7 -webtest==3.0.0 -zipp==3.17.0 +soupsieve==2.6 +tomli==2.0.2 +waitress==3.0.0 +webob==1.8.8 +webtest==3.0.1 +zipp==3.20.2 diff --git a/.riot/requirements/760d56e.txt b/.riot/requirements/760d56e.txt index 53727148b5..d3e80a7a66 100644 --- a/.riot/requirements/760d56e.txt +++ b/.riot/requirements/760d56e.txt @@ -2,21 +2,21 @@ # This file is autogenerated by pip-compile with Python 3.7 # by the following command: # -# pip-compile --config=pyproject.toml --no-annotate --resolver=backtracking .riot/requirements/760d56e.in +# pip-compile --allow-unsafe --config=pyproject.toml --no-annotate --resolver=backtracking .riot/requirements/760d56e.in # -attrs==23.1.0 -beautifulsoup4==4.12.2 -bottle==0.12.25 +attrs==24.2.0 +beautifulsoup4==4.12.3 +bottle==0.13.1 coverage[toml]==7.2.7 -exceptiongroup==1.2.0 +exceptiongroup==1.2.2 hypothesis==6.45.0 importlib-metadata==6.7.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 -packaging==23.2 +packaging==24.0 pluggy==1.2.0 -pytest==7.4.3 +pytest==7.4.4 pytest-cov==4.1.0 pytest-mock==3.11.1 pytest-randomly==3.12.0 @@ -25,6 +25,6 @@ soupsieve==2.4.1 tomli==2.0.1 typing-extensions==4.7.1 waitress==2.1.2 -webob==1.8.7 -webtest==3.0.0 +webob==1.8.8 +webtest==3.0.1 zipp==3.15.0 diff --git a/.riot/requirements/8c110bf.txt b/.riot/requirements/8c110bf.txt index fed864f8b5..0a57b2a151 100644 --- a/.riot/requirements/8c110bf.txt +++ b/.riot/requirements/8c110bf.txt @@ -2,28 +2,28 @@ # This file is autogenerated by pip-compile with Python 3.8 # by the following command: # -# pip-compile --no-annotate .riot/requirements/8c110bf.in +# pip-compile --allow-unsafe --no-annotate .riot/requirements/8c110bf.in # -attrs==23.1.0 -beautifulsoup4==4.12.2 +attrs==24.2.0 +beautifulsoup4==4.12.3 bottle==0.12.25 -coverage[toml]==7.3.4 -exceptiongroup==1.2.0 +coverage[toml]==7.6.1 +exceptiongroup==1.2.2 hypothesis==6.45.0 -importlib-metadata==7.0.0 +importlib-metadata==8.5.0 iniconfig==2.0.0 mock==5.1.0 opentracing==2.4.0 -packaging==23.2 -pluggy==1.3.0 -pytest==7.4.3 -pytest-cov==4.1.0 -pytest-mock==3.12.0 +packaging==24.1 +pluggy==1.5.0 +pytest==8.3.3 +pytest-cov==5.0.0 +pytest-mock==3.14.0 pytest-randomly==3.15.0 sortedcontainers==2.4.0 -soupsieve==2.5 -tomli==2.0.1 -waitress==2.1.2 -webob==1.8.7 -webtest==3.0.0 -zipp==3.17.0 +soupsieve==2.6 +tomli==2.0.2 +waitress==3.0.0 +webob==1.8.8 +webtest==3.0.1 +zipp==3.20.2 From b380f21795366b2d8f28619893a8c0127ca6b709 Mon Sep 17 00:00:00 2001 From: Romain Komorn <136473744+romainkomorndatadog@users.noreply.github.com> Date: Mon, 7 Oct 2024 15:02:15 +0100 Subject: [PATCH 14/20] chore(ci_visibility): separate pytest outcome processing into its own function (#10960) This separates the outcome processing for `pytest` into a separate function to allow for easier refactoring of code for Early Flake Detection retries. It also makes the status an optional argument for various `finish()` calls to allow for finishing without overriding the test status. ## 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: Alberto Vara --- ddtrace/contrib/pytest/_plugin_v2.py | 54 +++++++++++++-------- ddtrace/internal/ci_visibility/api/_test.py | 5 +- ddtrace/internal/test_visibility/api.py | 4 +- static-analysis.datadog.yml | 7 ++- 4 files changed, 44 insertions(+), 26 deletions(-) diff --git a/ddtrace/contrib/pytest/_plugin_v2.py b/ddtrace/contrib/pytest/_plugin_v2.py index 903f7b9f31..7981899260 100644 --- a/ddtrace/contrib/pytest/_plugin_v2.py +++ b/ddtrace/contrib/pytest/_plugin_v2.py @@ -29,6 +29,7 @@ from ddtrace.ext import test from ddtrace.ext.test_visibility import ITR_SKIPPING_LEVEL from ddtrace.ext.test_visibility.api import TestExcInfo +from ddtrace.ext.test_visibility.api import TestStatus from ddtrace.ext.test_visibility.api import disable_test_visibility from ddtrace.ext.test_visibility.api import enable_test_visibility from ddtrace.ext.test_visibility.api import is_test_visibility_enabled @@ -54,6 +55,12 @@ _NODEID_REGEX = re.compile("^((?P.*)/(?P[^/]*?))::(?P.*?)$") +class _TestOutcome(t.NamedTuple): + status: t.Optional[TestStatus] = None + skip_reason: t.Optional[str] = None + exc_info: t.Optional[TestExcInfo] = None + + def _handle_itr_should_skip(item, test_id) -> bool: """Checks whether a test should be skipped @@ -71,11 +78,11 @@ def _handle_itr_should_skip(item, test_id) -> bool: # Marking the test as forced run also applies to its hierarchy InternalTest.mark_itr_forced_run(test_id) return False - else: - InternalTest.mark_itr_skipped(test_id) - # Marking the test as skipped by ITR so that it appears in pytest's output - item.add_marker(pytest.mark.skip(reason=SKIPPED_BY_ITR_REASON)) # TODO don't rely on internal for reason - return True + + InternalTest.mark_itr_skipped(test_id) + # Marking the test as skipped by ITR so that it appears in pytest's output + item.add_marker(pytest.mark.skip(reason=SKIPPED_BY_ITR_REASON)) # TODO don't rely on internal for reason + return True return False @@ -311,7 +318,7 @@ def pytest_runtest_protocol(item, nextitem) -> None: return -def _pytest_runtest_makereport(item, call, outcome): +def _process_outcome(item, call, outcome) -> _TestOutcome: result = outcome.get_result() test_id = _get_test_id_from_item(item) @@ -322,7 +329,7 @@ def _pytest_runtest_makereport(item, call, outcome): # - it was skipped by ITR # - it was marked with skipif if InternalTest.is_finished(test_id): - return + return _TestOutcome() # In cases where a test was marked as XFAIL, the reason is only available during when call.when == "call", so we # add it as a tag immediately: @@ -339,7 +346,7 @@ def _pytest_runtest_makereport(item, call, outcome): # DEV NOTE: some skip scenarios (eg: skipif) have an exception during setup if call.when != "teardown" and not (has_exception or result.failed): - return + return _TestOutcome() xfail = hasattr(result, "wasxfail") or "xfail" in result.keywords xfail_reason_tag = InternalTest.get_tag(test_id, XFAIL_REASON) if xfail else None @@ -349,8 +356,8 @@ def _pytest_runtest_makereport(item, call, outcome): # that's why no XFAIL_REASON or test.RESULT tags will be added. if result.skipped: if InternalTest.was_skipped_by_itr(test_id): - # Items that were skipped by ITR already have their status set - return + # Items that were skipped by ITR already have their status and reason set + return _TestOutcome() if xfail and not has_skip_keyword: # XFail tests that fail are recorded skipped by pytest, should be passed instead @@ -358,11 +365,9 @@ def _pytest_runtest_makereport(item, call, outcome): InternalTest.set_tag(test_id, test.RESULT, test.Status.XFAIL.value) if xfail_reason_tag is None: InternalTest.set_tag(test_id, XFAIL_REASON, getattr(result, "wasxfail", "XFail")) - InternalTest.mark_pass(test_id) - return + return _TestOutcome(TestStatus.PASS) - InternalTest.mark_skip(test_id, _extract_reason(call)) - return + return _TestOutcome(TestStatus.SKIP, _extract_reason(call)) if result.passed: if xfail and not has_skip_keyword and not item.config.option.runxfail: @@ -371,24 +376,33 @@ def _pytest_runtest_makereport(item, call, outcome): InternalTest.set_tag(test_id, XFAIL_REASON, "XFail") InternalTest.set_tag(test_id, test.RESULT, test.Status.XPASS.value) - InternalTest.mark_pass(test_id) - return + return _TestOutcome(TestStatus.PASS) if xfail and not has_skip_keyword and not item.config.option.runxfail: # XPass (strict=True) are recorded failed by pytest, longrepr contains reason if xfail_reason_tag is None: InternalTest.set_tag(test_id, XFAIL_REASON, getattr(result, "longrepr", "XFail")) InternalTest.set_tag(test_id, test.RESULT, test.Status.XPASS.value) - InternalTest.mark_fail(test_id) - return + return _TestOutcome(TestStatus.FAIL) exc_info = TestExcInfo(call.excinfo.type, call.excinfo.value, call.excinfo.tb) if call.excinfo else None - InternalTest.mark_fail(test_id, exc_info) + return _TestOutcome(status=TestStatus.FAIL, exc_info=exc_info) + + +def _pytest_runtest_makereport(item: pytest.Item, call: pytest.CallInfo, outcome: pytest.TestReport) -> None: + test_id = _get_test_id_from_item(item) + + test_outcome = _process_outcome(item, call, outcome) + + if test_outcome.status is None and call.when != "teardown": + return + + InternalTest.finish(test_id, test_outcome.status, test_outcome.skip_reason, test_outcome.exc_info) @pytest.hookimpl(hookwrapper=True) -def pytest_runtest_makereport(item, call) -> None: +def pytest_runtest_makereport(item: pytest.Item, call: pytest.CallInfo) -> None: """Store outcome for tracing.""" outcome: pytest.TestReport outcome = yield diff --git a/ddtrace/internal/ci_visibility/api/_test.py b/ddtrace/internal/ci_visibility/api/_test.py index b48de33364..7f9316c5ca 100644 --- a/ddtrace/internal/ci_visibility/api/_test.py +++ b/ddtrace/internal/ci_visibility/api/_test.py @@ -126,13 +126,14 @@ def _telemetry_record_event_finished(self): def finish_test( self, - status: TestStatus, + status: Optional[TestStatus] = None, reason: Optional[str] = None, exc_info: Optional[TestExcInfo] = None, override_finish_time: Optional[float] = None, ) -> None: log.debug("Test Visibility: finishing %s, with status: %s, reason: %s", self, status, reason) - self.set_status(status) + if status is not None: + self.set_status(status) if reason is not None: self.set_tag(test.SKIP_REASON, reason) if exc_info is not None: diff --git a/ddtrace/internal/test_visibility/api.py b/ddtrace/internal/test_visibility/api.py index cba827646f..618dcf2499 100644 --- a/ddtrace/internal/test_visibility/api.py +++ b/ddtrace/internal/test_visibility/api.py @@ -115,7 +115,7 @@ class FinishArgs(NamedTuple): """InternalTest allows finishing with an overridden finish time (for EFD and other retry purposes)""" test_id: InternalTestId - status: TestStatus + status: t.Optional[TestStatus] = None skip_reason: t.Optional[str] = None exc_info: t.Optional[TestExcInfo] = None override_finish_time: t.Optional[float] = None @@ -124,7 +124,7 @@ class FinishArgs(NamedTuple): @_catch_and_log_exceptions def finish( item_id: InternalTestId, - status: ext_api.TestStatus, + status: t.Optional[ext_api.TestStatus] = None, reason: t.Optional[str] = None, exc_info: t.Optional[ext_api.TestExcInfo] = None, override_finish_time: t.Optional[float] = None, diff --git a/static-analysis.datadog.yml b/static-analysis.datadog.yml index 05e121907f..156f3713b4 100644 --- a/static-analysis.datadog.yml +++ b/static-analysis.datadog.yml @@ -1,9 +1,12 @@ rulesets: - python-best-practices: rules: - nested-blocks: + comment-fixme-todo-ownership: ignore: - "**" max-function-lines: - ignore: + ignore: - "tests/ci_visibility/api/fake_runner_*.py" + nested-blocks: + ignore: + - "**" From 80a3ee85f7620e32d60cd26925a398dd81790f15 Mon Sep 17 00:00:00 2001 From: Sam Brenner <106700075+sabrenner@users.noreply.github.com> Date: Mon, 7 Oct 2024 11:20:11 -0400 Subject: [PATCH 15/20] fix(botocore): bedrock cross-region inference model name does not throw (#10949) ## What does this PR do? Fixes #10772 Model IDs with cross-region inference would throw because we assumed `modelID` would only have the `provider` and `model_name`, when it could have the region as well. This would result in: ``` File /python3.11/site-packages/ddtrace/contrib/internal/botocore/services/bedrock.py:321, in patched_bedrock_api_call(original_func, instance, args, kwargs, function_vars) 319 params = function_vars.get("params") 320 pin = function_vars.get("pin") --> 321 model_provider, model_name = params.get("modelId").split(".") 322 integration = function_vars.get("integration") 323 submit_to_llmobs = integration.llmobs_enabled and "embed" not in model_name ValueError: too many values to unpack (expected 2) ``` We are not tagging that cross-region inference in this PR, just resolving the error. ### Testing To test this change, a singular `anthropic_message` test case and corresponding cassette was modified. This had an unfortunate side effect of affecting an LLMObs test as well. I can add a different test instead, but I believe that would require an additional 60+ line cassette which isn't totally needed, so I opted for this instead. ## 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) --- ddtrace/contrib/internal/botocore/services/bedrock.py | 6 +++++- ...k-cross-region-inference-model-fix-179b7f1ddd6e8e02.yaml | 4 ++++ .../bedrock_cassettes/anthropic_message_invoke.yaml | 2 +- tests/contrib/botocore/test_bedrock.py | 1 + tests/contrib/botocore/test_bedrock_llmobs.py | 4 ++++ 5 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/botocore-bedrock-cross-region-inference-model-fix-179b7f1ddd6e8e02.yaml diff --git a/ddtrace/contrib/internal/botocore/services/bedrock.py b/ddtrace/contrib/internal/botocore/services/bedrock.py index fca33630f5..df547e8f09 100644 --- a/ddtrace/contrib/internal/botocore/services/bedrock.py +++ b/ddtrace/contrib/internal/botocore/services/bedrock.py @@ -318,7 +318,11 @@ def handle_bedrock_response( def patched_bedrock_api_call(original_func, instance, args, kwargs, function_vars): params = function_vars.get("params") pin = function_vars.get("pin") - model_provider, model_name = params.get("modelId").split(".") + model_meta = params.get("modelId").split(".") + if len(model_meta) == 2: + model_provider, model_name = model_meta + else: + _, model_provider, model_name = model_meta # cross-region inference integration = function_vars.get("integration") submit_to_llmobs = integration.llmobs_enabled and "embed" not in model_name with core.context_with_data( diff --git a/releasenotes/notes/botocore-bedrock-cross-region-inference-model-fix-179b7f1ddd6e8e02.yaml b/releasenotes/notes/botocore-bedrock-cross-region-inference-model-fix-179b7f1ddd6e8e02.yaml new file mode 100644 index 0000000000..be6f6d0dc2 --- /dev/null +++ b/releasenotes/notes/botocore-bedrock-cross-region-inference-model-fix-179b7f1ddd6e8e02.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + botocore: fixes bedrock model and model provider interpretation from ``modelId`` when using cross-region inference. diff --git a/tests/contrib/botocore/bedrock_cassettes/anthropic_message_invoke.yaml b/tests/contrib/botocore/bedrock_cassettes/anthropic_message_invoke.yaml index a4abf1ac29..dfc0235d6c 100644 --- a/tests/contrib/botocore/bedrock_cassettes/anthropic_message_invoke.yaml +++ b/tests/contrib/botocore/bedrock_cassettes/anthropic_message_invoke.yaml @@ -21,7 +21,7 @@ interactions: - !!binary | YXR0ZW1wdD0x method: POST - uri: https://bedrock-runtime.us-east-1.amazonaws.com/model/anthropic.claude-3-sonnet-20240229-v1%3A0/invoke + uri: https://bedrock-runtime.us-east-1.amazonaws.com/model/us.anthropic.claude-3-sonnet-20240229-v1%3A0/invoke response: body: string: '{"id":"msg_01E6sPP1ksqicYCaBrkvzna8","type":"message","role":"assistant","content":[{"type":"text","text":"Hobbits diff --git a/tests/contrib/botocore/test_bedrock.py b/tests/contrib/botocore/test_bedrock.py index 90c7669cba..3c91b147a9 100644 --- a/tests/contrib/botocore/test_bedrock.py +++ b/tests/contrib/botocore/test_bedrock.py @@ -173,6 +173,7 @@ def test_anthropic_invoke(bedrock_client, request_vcr): @pytest.mark.snapshot def test_anthropic_message_invoke(bedrock_client, request_vcr): body, model = json.dumps(_REQUEST_BODIES["anthropic_message"]), _MODELS["anthropic_message"] + model = "us." + model with request_vcr.use_cassette("anthropic_message_invoke.yaml"): response = bedrock_client.invoke_model(body=body, modelId=model) json.loads(response.get("body").read()) diff --git a/tests/contrib/botocore/test_bedrock_llmobs.py b/tests/contrib/botocore/test_bedrock_llmobs.py index ae2fb7894c..b7e80651f3 100644 --- a/tests/contrib/botocore/test_bedrock_llmobs.py +++ b/tests/contrib/botocore/test_bedrock_llmobs.py @@ -128,6 +128,10 @@ def _test_llmobs_invoke(cls, provider, bedrock_client, mock_llmobs_span_writer, } with get_request_vcr().use_cassette(cassette_name): body, model = json.dumps(body), _MODELS[provider] + if provider == "anthropic_message": + # we do this to re-use a cassette which tests + # cross-region inference + model = "us." + model response = bedrock_client.invoke_model(body=body, modelId=model) json.loads(response.get("body").read()) span = mock_tracer.pop_traces()[0][0] From 0bf1fe939368f1818e484e22861ba54a74ea5e3c Mon Sep 17 00:00:00 2001 From: "Gabriele N. Tornetta" Date: Mon, 7 Oct 2024 18:44:34 +0100 Subject: [PATCH 16/20] ci: enable code origin for spans in django profile (#10955) We enable Code Origin for Spans in the Django profile job to collect profiling data with the featured turned on. ## 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 - [ ] 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) --- .github/workflows/django-overhead-profile.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/django-overhead-profile.yml b/.github/workflows/django-overhead-profile.yml index f581749fe8..01a1626e0d 100644 --- a/.github/workflows/django-overhead-profile.yml +++ b/.github/workflows/django-overhead-profile.yml @@ -15,6 +15,7 @@ jobs: runs-on: ubuntu-latest env: PREFIX: ${{ github.workspace }}/prefix + DD_CODE_ORIGIN_FOR_SPANS_ENABLED: "1" defaults: run: working-directory: ddtrace From 2af13d93b5071339f22024be423cb86ec89641de Mon Sep 17 00:00:00 2001 From: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> Date: Mon, 7 Oct 2024 21:33:48 +0200 Subject: [PATCH 17/20] chore(ci): fix alpine test (#10964) Fix for alpine test failing on gitlab after some changes on system tests. ## 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) --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 643a2ec990..a352f7f72e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -38,7 +38,7 @@ package-oci: onboarding_tests_installer: parallel: matrix: - - ONBOARDING_FILTER_WEBLOG: [test-app-python,test-app-python-container,test-app-python-alpine-libgcc] + - ONBOARDING_FILTER_WEBLOG: [test-app-python,test-app-python-container,test-app-python-alpine] onboarding_tests_k8s_injection: From 5f25f36e0650169109d0c117d2d680033682e218 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 7 Oct 2024 16:24:44 -0400 Subject: [PATCH 18/20] fix(tracing): ensure http.url obfuscation is consistent with other libraries (#10914) When ``DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP`` is set to an empty string ``global_query_string_obfuscation_disabled`` should be set to False. ## 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 - [ ] 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) --- ddtrace/contrib/trace_utils.py | 18 ++++--- ddtrace/internal/utils/http.py | 10 +--- ddtrace/settings/config.py | 18 +++---- ...fuscation-consistent-8a2361bf3298e1be.yaml | 4 ++ tests/tracer/test_env_vars.py | 4 +- tests/tracer/test_http.py | 10 ---- tests/tracer/test_trace_utils.py | 51 +++++++++++++++++++ 7 files changed, 77 insertions(+), 38 deletions(-) create mode 100644 releasenotes/notes/make-obfuscation-consistent-8a2361bf3298e1be.yaml diff --git a/ddtrace/contrib/trace_utils.py b/ddtrace/contrib/trace_utils.py index 3cc0cbdac4..9fba9c0a7b 100644 --- a/ddtrace/contrib/trace_utils.py +++ b/ddtrace/contrib/trace_utils.py @@ -404,14 +404,18 @@ def ext_service(pin, int_config, default=None): def _set_url_tag(integration_config, span, url, query): # type: (IntegrationConfig, Span, str, str) -> None - - if integration_config.http_tag_query_string: # Tagging query string in http.url - if config.global_query_string_obfuscation_disabled: # No redacting of query strings - span.set_tag_str(http.URL, url) - else: # Redact query strings - span.set_tag_str(http.URL, redact_url(url, config._obfuscation_query_string_pattern, query)) - else: # Not tagging query string in http.url + if not integration_config.http_tag_query_string: + span.set_tag_str(http.URL, strip_query_string(url)) + elif config.global_query_string_obfuscation_disabled: + # TODO(munir): This case exists for backwards compatibility. To remove query strings from URLs, + # users should set ``DD_TRACE_HTTP_CLIENT_TAG_QUERY_STRING=False``. This case should be + # removed when config.global_query_string_obfuscation_disabled is removed (v3.0). + span.set_tag_str(http.URL, url) + elif getattr(config._obfuscation_query_string_pattern, "pattern", None) == b"": + # obfuscation is disabled when DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP="" span.set_tag_str(http.URL, strip_query_string(url)) + else: + span.set_tag_str(http.URL, redact_url(url, config._obfuscation_query_string_pattern, query)) def set_http_meta( diff --git a/ddtrace/internal/utils/http.py b/ddtrace/internal/utils/http.py index 48867cf968..cba605a152 100644 --- a/ddtrace/internal/utils/http.py +++ b/ddtrace/internal/utils/http.py @@ -74,21 +74,13 @@ def strip_query_string(url): def redact_query_string(query_string, query_string_obfuscation_pattern): - # type: (str, Optional[re.Pattern]) -> Union[bytes, str] - if query_string_obfuscation_pattern is None: - return query_string - + # type: (str, re.Pattern) -> Union[bytes, str] bytes_query = query_string if isinstance(query_string, bytes) else query_string.encode("utf-8") return query_string_obfuscation_pattern.sub(b"", bytes_query) def redact_url(url, query_string_obfuscation_pattern, query_string=None): # type: (str, re.Pattern, Optional[str]) -> Union[str,bytes] - - # Avoid further processing if obfuscation is disabled - if query_string_obfuscation_pattern is None: - return url - parts = compat.parse.urlparse(url) redacted_query = None diff --git a/ddtrace/settings/config.py b/ddtrace/settings/config.py index a569070c61..6de9774f25 100644 --- a/ddtrace/settings/config.py +++ b/ddtrace/settings/config.py @@ -567,18 +567,16 @@ def __init__(self): dd_trace_obfuscation_query_string_regexp = os.getenv( "DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP", DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP_DEFAULT ) - self.global_query_string_obfuscation_disabled = True # If empty obfuscation pattern + self.global_query_string_obfuscation_disabled = dd_trace_obfuscation_query_string_regexp == "" self._obfuscation_query_string_pattern = None self.http_tag_query_string = True # Default behaviour of query string tagging in http.url - if dd_trace_obfuscation_query_string_regexp != "": - self.global_query_string_obfuscation_disabled = False # Not empty obfuscation pattern - try: - self._obfuscation_query_string_pattern = re.compile( - dd_trace_obfuscation_query_string_regexp.encode("ascii") - ) - except Exception: - log.warning("Invalid obfuscation pattern, disabling query string tracing", exc_info=True) - self.http_tag_query_string = False # Disable query string tagging if malformed obfuscation pattern + try: + self._obfuscation_query_string_pattern = re.compile( + dd_trace_obfuscation_query_string_regexp.encode("ascii") + ) + except Exception: + log.warning("Invalid obfuscation pattern, disabling query string tracing", exc_info=True) + self.http_tag_query_string = False # Disable query string tagging if malformed obfuscation pattern # Test Visibility config items self._ci_visibility_agentless_enabled = asbool(os.getenv("DD_CIVISIBILITY_AGENTLESS_ENABLED", default=False)) diff --git a/releasenotes/notes/make-obfuscation-consistent-8a2361bf3298e1be.yaml b/releasenotes/notes/make-obfuscation-consistent-8a2361bf3298e1be.yaml new file mode 100644 index 0000000000..fd4e51ec99 --- /dev/null +++ b/releasenotes/notes/make-obfuscation-consistent-8a2361bf3298e1be.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + integrations: Ensures that ``http.url`` span tag contains the full query string when ``DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP `` is set to an empty string. diff --git a/tests/tracer/test_env_vars.py b/tests/tracer/test_env_vars.py index 0191ece9e5..272dd75119 100644 --- a/tests/tracer/test_env_vars.py +++ b/tests/tracer/test_env_vars.py @@ -8,7 +8,7 @@ "env_var_name,env_var_value,expected_obfuscation_config,expected_global_query_string_obfuscation_disabled," "expected_http_tag_query_string", [ - ("DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP", "", None, True, True), + ("DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP", "", 're.compile(b"")', True, True), ( "DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP", "(?i)(?:p(?:ass)?w(?:or))", @@ -64,7 +64,7 @@ def test_obfuscation_querystring_pattern_env_var( ], env=env, ) - assert b"AssertionError" not in out + assert b"AssertionError" not in out, out @pytest.mark.parametrize( diff --git a/tests/tracer/test_http.py b/tests/tracer/test_http.py index 03b003b1de..5a8a02ebf5 100644 --- a/tests/tracer/test_http.py +++ b/tests/tracer/test_http.py @@ -45,16 +45,6 @@ def test_strip_query_string(url): ) -@pytest.mark.parametrize("url", _url_fixtures()) -def test_redact_url_obfuscation_disabled_without_param(url): - assert redact_url(url, None, None) == url - - -@pytest.mark.parametrize("url", _url_fixtures()) -def test_redact_url_obfuscation_disabled_with_param(url): - assert redact_url(url, None, "query_string") == url - - @pytest.mark.parametrize("url", _url_fixtures()) def test_redact_url_not_redacts_without_param(url): res = redact_url(url, re.compile(b"\\@"), None) diff --git a/tests/tracer/test_trace_utils.py b/tests/tracer/test_trace_utils.py index 07195e2900..d9c5508df9 100644 --- a/tests/tracer/test_trace_utils.py +++ b/tests/tracer/test_trace_utils.py @@ -1021,6 +1021,57 @@ def test_sanitized_url_in_http_meta(span, int_config): assert span.get_tag(http.URL) == FULL_URL +@pytest.mark.subprocess(env={"DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP": ""}) +def test_url_in_http_with_empty_obfuscation_regex(): + from ddtrace import config + from ddtrace import tracer + from ddtrace.contrib.trace_utils import set_http_meta + from ddtrace.ext import http + + assert config._obfuscation_query_string_pattern.pattern == b"", config._obfuscation_query_string_pattern + + SENSITIVE_URL = "http://weblog:7777/?application_key=123" + config._add("myint", dict()) + with tracer.trace("s") as span: + set_http_meta( + span, + config.myint, + method="GET", + url=SENSITIVE_URL, + status_code=200, + ) + assert span.get_tag(http.URL) == SENSITIVE_URL + + +# TODO(munir): Remove this test when global_query_string_obfuscation_disabled is removed +@pytest.mark.subprocess(env={"DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP": ""}) +def test_url_in_http_with_obfuscation_enabled_and_empty_regex(): + # Test that query strings are not added to urls when the obfuscation regex is an empty string + # and obfuscation is enabled (not disabled xD) + from ddtrace import config + from ddtrace import tracer + from ddtrace.contrib.trace_utils import set_http_meta + from ddtrace.ext import http + + # assert obfuscation is disabled when the regex is an empty string + assert config.global_query_string_obfuscation_disabled is True + assert config._obfuscation_query_string_pattern is not None + + # Enable obfucation with an empty regex + config.global_query_string_obfuscation_disabled = False + + config._add("myint", dict()) + with tracer.trace("s") as span: + set_http_meta( + span, + config.myint, + method="GET", + url="http://weblog:7777/?application_key=123", + status_code=200, + ) + assert span.get_tag(http.URL) == "http://weblog:7777/", span._meta + + def test_url_in_http_meta(span, int_config): SENSITIVE_QS_URL = "http://example.com/search?token=03cb9f67dbbc4cb8b963629951e10934&q=query#frag?ment" REDACTED_URL = "http://example.com/search?&q=query#frag?ment" From 885df93249cd18b4968cfabbab4aca939cbbd6a5 Mon Sep 17 00:00:00 2001 From: Alberto Vara Date: Tue, 8 Oct 2024 09:32:59 +0200 Subject: [PATCH 19/20] chore(iast): remove IAST deny list elements (#10961) A memory leak was introduced in #10540 when "py" was removed from the deny list. This caused a leak in FastAPI with the `pypika` package. #10846 patched the issue, and #10947 resolved it. Now, we're re-enabling those packages. This PR is tested in #10902 ## 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) --- ddtrace/appsec/_iast/_ast/ast_patching.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ddtrace/appsec/_iast/_ast/ast_patching.py b/ddtrace/appsec/_iast/_ast/ast_patching.py index 9aa7298538..572fca02ce 100644 --- a/ddtrace/appsec/_iast/_ast/ast_patching.py +++ b/ddtrace/appsec/_iast/_ast/ast_patching.py @@ -300,11 +300,6 @@ "uvicorn.", "anyio.", "httpcore.", - "pypika.", - "pydantic.", - "pydantic_core.", - "pydantic_settings.", - "tomli.", ) From f757fbfcc49f53e18811152fd51457b2f60c1ab4 Mon Sep 17 00:00:00 2001 From: "Gabriele N. Tornetta" Date: Tue, 8 Oct 2024 09:33:37 +0100 Subject: [PATCH 20/20] chore(di): guard against mutable module container (#10841) When disovering functions on module import, we look at the module as a container of function-like objects. If the module is partially loaded, its `__dict__` might mutate. We ensure to iterate over a copy of the module's `__dict__` when iterating over it. We expect the original module to mutate dynamically at runtime, so by the time we make a copy of its underlying lookup dict, we are able to detected at the very least everything that corresponds to the actual source code. ## 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 - [ ] 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) --- ddtrace/debugging/_function/discovery.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ddtrace/debugging/_function/discovery.py b/ddtrace/debugging/_function/discovery.py index 5e92ad05a1..9cabb4b3a0 100644 --- a/ddtrace/debugging/_function/discovery.py +++ b/ddtrace/debugging/_function/discovery.py @@ -66,7 +66,9 @@ def __init__( origin: Optional[Union[Tuple["ContainerIterator", ContainerKey], Tuple[FullyNamedFunction, str]]] = None, ) -> None: if isinstance(container, (type, ModuleType)): - self._iter = iter(container.__dict__.items()) + # DEV: A module object could be partially initialised, therefore + # __dict__ can mutate. + self._iter = iter(container.__dict__.copy().items()) self.__name__ = container.__name__ elif isinstance(container, tuple):