From 8563d2e411e05186cf65fcb08cc48ed3d94820db Mon Sep 17 00:00:00 2001 From: Federico Mon Date: Wed, 17 May 2023 12:36:53 +0200 Subject: [PATCH 1/2] fix(ci-visibility): remove extra requests performed if env var not set (#5872) CI App: Remove extra requests performed if ``DD_CIVISIBILITY_ITR_ENABLED`` env var not set or False. ## 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/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] OPTIONAL: PR description includes explicit acknowledgement of the performance implications of the change as reported in the benchmarks PR comment. ## Reviewer Checklist - [ ] Title is accurate. - [ ] No unnecessary changes are introduced. - [ ] Description motivates each change. - [ ] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Testing strategy adequately addresses listed risk(s). - [ ] Change is maintainable (easy to change, telemetry, documentation). - [ ] Release note makes sense to a user of the library. - [ ] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. --------- Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com> (cherry picked from commit 9ff8aae9a2bd3f198e277db9c87776408135fc19) --- ddtrace/internal/ci_visibility/recorder.py | 5 +++++ .../fix-ci-app-api-call-12f848d25f93d02d.yaml | 4 ++++ tests/ci_visibility/test_ci_visibility.py | 18 +++++++++++++++++- 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/fix-ci-app-api-call-12f848d25f93d02d.yaml diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index 5cec1e10ae1..51d705f91ea 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -102,6 +102,11 @@ def _check_enabled_features(self): # type: () -> Tuple[bool, bool] if not self._app_key: return False, False + + # DEV: Remove this ``if`` once ITR is in GA + if not ddconfig._ci_visibility_intelligent_testrunner_enabled: + return False, False + url = "https://api.%s/api/v2/libraries/tests/services/setting" % self._dd_site _headers = {"dd-api-key": self._api_key, "dd-application-key": self._app_key} payload = { diff --git a/releasenotes/notes/fix-ci-app-api-call-12f848d25f93d02d.yaml b/releasenotes/notes/fix-ci-app-api-call-12f848d25f93d02d.yaml new file mode 100644 index 00000000000..0f8c77244ef --- /dev/null +++ b/releasenotes/notes/fix-ci-app-api-call-12f848d25f93d02d.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + CI Visibility: This fix resolves an issue where the tracer was doing extra requests if the ``DD_CIVISIBILITY_ITR_ENABLED`` env var was not set. diff --git a/tests/ci_visibility/test_ci_visibility.py b/tests/ci_visibility/test_ci_visibility.py index 9930d926cf4..39584747a52 100644 --- a/tests/ci_visibility/test_ci_visibility.py +++ b/tests/ci_visibility/test_ci_visibility.py @@ -55,8 +55,24 @@ def test_ci_visibility_service_enable(): @mock.patch("ddtrace.internal.ci_visibility.recorder._do_request") -def test_ci_visibility_service_enable_with_app_key(_do_request): +def test_ci_visibility_service_enable_with_app_key_and_itr_disabled(_do_request): with override_env(dict(DD_API_KEY="foobar.baz", DD_APP_KEY="foobar")): + _do_request.return_value = Response( + status=200, + body='{"data":{"id":"1234","type":"ci_app_tracers_test_service_settings","attributes":' + '{"code_coverage":true,"tests_skipping":true}}}', + ) + 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._do_request") +def test_ci_visibility_service_enable_with_app_key_and_itr_enabled(_do_request): + + with override_env(dict(DD_API_KEY="foobar.baz", DD_APP_KEY="foobar", DD_CIVISIBILITY_ITR_ENABLED="1")): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() _do_request.return_value = Response( status=200, body='{"data":{"id":"1234","type":"ci_app_tracers_test_service_settings","attributes":' From 8953e69f14df423b571fd1dcd26ad642bc7d6f4e Mon Sep 17 00:00:00 2001 From: Federico Mon Date: Wed, 17 May 2023 19:40:35 +0200 Subject: [PATCH 2/2] fix(ci-visibility): missing required config entry --- ddtrace/settings/config.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ddtrace/settings/config.py b/ddtrace/settings/config.py index c308daf087e..4e7732d3d40 100644 --- a/ddtrace/settings/config.py +++ b/ddtrace/settings/config.py @@ -289,6 +289,9 @@ def __init__(self): self._ci_visibility_agentless_enabled = asbool(os.getenv("DD_CIVISIBILITY_AGENTLESS_ENABLED", default=False)) self._ci_visibility_agentless_url = os.getenv("DD_CIVISIBILITY_AGENTLESS_URL", default="") + self._ci_visibility_intelligent_testrunner_enabled = asbool( + os.getenv("DD_CIVISIBILITY_ITR_ENABLED", default=False) + ) def __getattr__(self, name): if name not in self._config: