From e15898b7ed40dd736fa240c6a04865a655748760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADtor=20De=20Ara=C3=BAjo?= Date: Wed, 4 Sep 2024 15:10:16 +0100 Subject: [PATCH] fix(ci_visibility): handle errors while fetching skippable test data [backport 2.10] Backport 82650538b from #10460 to 2.10. Handle any exceptions while fetching skippable test data, defaulting to skipping no tests if an exception happens. ## 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: Romain Komorn <136473744+romainkomorndatadog@users.noreply.github.com> (cherry picked from commit 82650538b376a545e23362c1dd7f124ac79d6c88) --- ddtrace/internal/ci_visibility/recorder.py | 6 ++++++ ...ion-in-tests-to-skip-675db3b857f8e232.yaml | 6 ++++++ tests/ci_visibility/test_ci_visibility.py | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+) create mode 100644 releasenotes/notes/ci-visibility-handle-exception-in-tests-to-skip-675db3b857f8e232.yaml diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index 91d4aa7d89c..595e0688996 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -548,6 +548,12 @@ def _fetch_tests_to_skip(self, skipping_mode: str): self._test_suites_to_skip = [] self._tests_to_skip = defaultdict(list) + except Exception: + log.warning("Error retrieving skippable test data, no tests will be skipped", exc_info=True) + error_type = ERROR_TYPES.UNKNOWN + self._test_suites_to_skip = [] + self._tests_to_skip = defaultdict(list) + finally: record_itr_skippable_request( sw.elapsed() * 1000, diff --git a/releasenotes/notes/ci-visibility-handle-exception-in-tests-to-skip-675db3b857f8e232.yaml b/releasenotes/notes/ci-visibility-handle-exception-in-tests-to-skip-675db3b857f8e232.yaml new file mode 100644 index 00000000000..fe63970e61a --- /dev/null +++ b/releasenotes/notes/ci-visibility-handle-exception-in-tests-to-skip-675db3b857f8e232.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + CI Visibility: This fix resolves an issue where exceptions other than timeouts and connection errors raised + while fetching the list of skippable tests for ITR were not being handled correctly and caused the tracer to + crash. diff --git a/tests/ci_visibility/test_ci_visibility.py b/tests/ci_visibility/test_ci_visibility.py index 221a8d7be46..527ca3c59fa 100644 --- a/tests/ci_visibility/test_ci_visibility.py +++ b/tests/ci_visibility/test_ci_visibility.py @@ -207,6 +207,25 @@ def test_ci_visibility_service_skippable_timeout(_do_request, _check_enabled_fea CIVisibility.disable() +@mock.patch( + "ddtrace.internal.ci_visibility.recorder.CIVisibility._check_enabled_features", + return_value=_CIVisibilitySettings(True, True, False, True), +) +@mock.patch("ddtrace.internal.ci_visibility.recorder._do_request", side_effect=ValueError) +def test_ci_visibility_service_skippable_other_error(_do_request, _check_enabled_features): + with override_env( + dict( + DD_API_KEY="foobar.baz", + DD_APP_KEY="foobar", + DD_CIVISIBILITY_AGENTLESS_ENABLED="1", + ) + ), _dummy_noop_git_client(): + ddtrace.internal.ci_visibility.recorder.ddconfig = _get_default_civisibility_ddconfig() + CIVisibility.enable(service="test-service") + assert CIVisibility._instance._test_suites_to_skip == [] + CIVisibility.disable() + + @mock.patch("ddtrace.internal.ci_visibility.recorder._do_request") def test_ci_visibility_service_enable_with_itr_enabled(_do_request): with override_env(