From f7ffe6d344f0732ed290621bb208041368372a49 Mon Sep 17 00:00:00 2001 From: Federico Mon Date: Sun, 16 Jul 2023 10:56:19 +0200 Subject: [PATCH] chore(ci-visibility): capture timeout errors (#6352) CI Visibility: Capture timeout errors in requests to the backend ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] 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/internal/ci_visibility/recorder.py | 17 +++++++++-- ddtrace/internal/compat.py | 7 +++++ tests/ci_visibility/test_ci_visibility.py | 35 ++++++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index 576cbd8d5e0..d3845a8a173 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -19,6 +19,7 @@ from ddtrace.internal.agent import get_trace_url from ddtrace.internal.ci_visibility.filters import TraceCiVisibilityFilter from ddtrace.internal.compat import JSONDecodeError +from ddtrace.internal.compat import TimeoutError from ddtrace.internal.compat import parse from ddtrace.internal.logger import get_logger from ddtrace.internal.service import Service @@ -41,6 +42,7 @@ log = get_logger(__name__) TEST_SKIPPING_LEVEL = "suite" +DEFAULT_TIMEOUT = 15 def _extract_repository_name_from_url(repository_url): @@ -61,7 +63,7 @@ def _get_git_repo(): def _do_request(method, url, payload, headers): # type: (str, str, str, Dict) -> Response try: - conn = get_connection(url) + conn = get_connection(url, timeout=DEFAULT_TIMEOUT) log.debug("Sending request: %s %s %s %s", method, url, payload, headers) conn.request("POST", url, payload, headers) resp = compat.get_connection_response(conn) @@ -172,7 +174,11 @@ def _check_enabled_features(self): }, } } - response = _do_request("POST", url, json.dumps(payload), _headers) + try: + response = _do_request("POST", url, json.dumps(payload), _headers) + except TimeoutError: + log.warning("Request timeout while fetching enabled features") + return False, False try: parsed = json.loads(response.body) except JSONDecodeError: @@ -260,7 +266,12 @@ def _fetch_tests_to_skip(self): log.warning("Cannot make requests to skippable endpoint if mode is not agentless or evp proxy") return - response = _do_request("POST", url, json.dumps(payload), _headers) + try: + response = _do_request("POST", url, json.dumps(payload), _headers) + except TimeoutError: + log.warning("Request timeout while fetching skippable tests") + self._test_suites_to_skip = [] + return if response.status >= 400: log.warning("Test skips request responded with status %d", response.status) diff --git a/ddtrace/internal/compat.py b/ddtrace/internal/compat.py index 4c2e15fc4af..d84b0c1d21b 100644 --- a/ddtrace/internal/compat.py +++ b/ddtrace/internal/compat.py @@ -35,6 +35,7 @@ "Queue", "stringify", "StringIO", + "TimeoutError", "urlencode", "parse", "reraise", @@ -45,6 +46,12 @@ PY2 = sys.version_info[0] == 2 PY3 = sys.version_info[0] == 3 +try: + from builtin import TimeoutError +except ImportError: + # Purposely shadowing a missing python builtin + from socket import timeout as TimeoutError # noqa: A001 + if not PY2: long = int diff --git a/tests/ci_visibility/test_ci_visibility.py b/tests/ci_visibility/test_ci_visibility.py index 746f373fd3a..9a015bac449 100644 --- a/tests/ci_visibility/test_ci_visibility.py +++ b/tests/ci_visibility/test_ci_visibility.py @@ -15,6 +15,7 @@ from ddtrace.internal.ci_visibility.git_client import CIVisibilityGitClient from ddtrace.internal.ci_visibility.git_client import CIVisibilityGitClientSerializerV1 from ddtrace.internal.ci_visibility.recorder import _extract_repository_name_from_url +from ddtrace.internal.compat import TimeoutError from ddtrace.internal.utils.http import Response from ddtrace.span import Span from tests.utils import DummyCIVisibilityWriter @@ -99,6 +100,40 @@ def test_ci_visibility_service_enable_with_app_key_and_itr_disabled(_do_request) CIVisibility.disable() +@mock.patch("ddtrace.internal.ci_visibility.recorder._do_request", side_effect=TimeoutError) +def test_ci_visibility_service_settings_timeout(_do_request): + with override_env( + dict( + DD_API_KEY="foobar.baz", + DD_APP_KEY="foobar", + DD_CIVISIBILITY_AGENTLESS_ENABLED="1", + DD_CIVISIBILITY_ITR_ENABLED="1", + ) + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + CIVisibility.enable(service="test-service") + assert CIVisibility._instance._code_coverage_enabled_by_api is False + assert CIVisibility._instance._test_skipping_enabled_by_api is False + CIVisibility.disable() + + +@mock.patch("ddtrace.internal.ci_visibility.recorder.CIVisibility._check_enabled_features", return_value=(True, True)) +@mock.patch("ddtrace.internal.ci_visibility.recorder._do_request", side_effect=TimeoutError) +def test_ci_visibility_service_skippable_timeout(_do_request, _check_enabled_features): + with override_env( + dict( + DD_API_KEY="foobar.baz", + DD_APP_KEY="foobar", + DD_CIVISIBILITY_AGENTLESS_ENABLED="1", + DD_CIVISIBILITY_ITR_ENABLED="1", + ) + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + 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_app_key_and_itr_enabled(_do_request): with override_env(