From 5ec7528c11beac83ce47a2c057b1013739428a82 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 7 Oct 2024 14:48:11 -0400 Subject: [PATCH 1/6] chore(ci_vis): move configurations to envier --- ddtrace/internal/ci_visibility/recorder.py | 24 +++----- ddtrace/internal/ci_visibility/utils.py | 3 +- ddtrace/internal/ci_visibility/writer.py | 5 +- ddtrace/internal/telemetry/writer.py | 4 +- ddtrace/settings/civis.py | 61 +++++++++++++++++++ ddtrace/settings/config.py | 12 ---- .../test_ci_visibility_api_client.py | 2 +- 7 files changed, 80 insertions(+), 31 deletions(-) create mode 100644 ddtrace/settings/civis.py diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index ac2b5ee8f4f..838fa30a5ff 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -78,6 +78,8 @@ from ddtrace.internal.utils.formats import asbool from ddtrace.internal.utils.http import verify_url from ddtrace.internal.writer.writer import Response +from ddtrace.settings.civis import ci_config +from ddtrace.settings.civis import test_config if TYPE_CHECKING: # pragma: no cover @@ -217,7 +219,7 @@ def __init__(self, tracer=None, config=None, service=None): self._git_data: GitData = get_git_data_from_tags(self._tags) - if ddconfig._ci_visibility_agentless_enabled: + if ci_config._agentless_enabled: if not self._api_key: raise EnvironmentError( "DD_CIVISIBILITY_AGENTLESS_ENABLED is set, but DD_API_KEY is not set, so ddtrace " @@ -231,7 +233,7 @@ def __init__(self, tracer=None, config=None, service=None): self._configurations, self._api_key, self._dd_site, - ddconfig._ci_visibility_agentless_url if ddconfig._ci_visibility_agentless_url else None, + ci_config._agentless_url if ci_config._agentless_url else None, self._service, ddconfig.env, ) @@ -304,7 +306,7 @@ def _check_enabled_features(self): # DEV: Remove this ``if`` once ITR is in GA _error_return_value = TestVisibilityAPISettings() - if not ddconfig._ci_visibility_intelligent_testrunner_enabled: + if not ci_config._itr_enabled: return _error_return_value if not self._api_client: @@ -402,10 +404,7 @@ def test_skipping_enabled(cls): def is_efd_enabled(cls): if cls._instance is None: return False - return ( - cls._instance._api_settings.early_flake_detection.enabled - and ddconfig._test_visibility_early_flake_detection_enabled - ) + return cls._instance._api_settings.early_flake_detection.enabled and ci_config.early_flake_detection @classmethod def should_collect_coverage(cls): @@ -476,7 +475,7 @@ def _should_skip_path(self, path, name, test_skipping_mode=None): def enable(cls, tracer=None, config=None, service=None): # type: (Optional[Tracer], Optional[Any], Optional[str]) -> None log.debug("Enabling %s", cls.__name__) - if ddconfig._ci_visibility_agentless_enabled: + if ci_config._agentless_enabled: if not os.getenv("_CI_DD_API_KEY", os.getenv("DD_API_KEY")): log.critical( "%s disabled: environment variable DD_CIVISIBILITY_AGENTLESS_ENABLED is true but" @@ -551,10 +550,7 @@ def _start_service(self): self._unique_test_ids = unique_test_ids log.info("Unique tests fetched for Early Flake Detection: %s", len(self._unique_test_ids)) else: - if ( - self._api_settings.early_flake_detection.enabled - and not ddconfig._test_visibility_early_flake_detection_enabled - ): + if self._api_settings.early_flake_detection.enabled and not ci_config.early_flake_detection: log.warning( "Early Flake Detection is enabled by API but disabled by " "DD_TEST_VISIBILITY_EARLY_FLAKE_DETECTION_ENABLED environment variable" @@ -783,8 +779,8 @@ def set_test_session_name(cls, test_command: str) -> None: 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 + if test_config.session_name: + test_session_name = test_config.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 diff --git a/ddtrace/internal/ci_visibility/utils.py b/ddtrace/internal/ci_visibility/utils.py index e2af9a62b14..6bf11510642 100644 --- a/ddtrace/internal/ci_visibility/utils.py +++ b/ddtrace/internal/ci_visibility/utils.py @@ -10,6 +10,7 @@ from ddtrace.ext import test from ddtrace.internal.ci_visibility.constants import CIVISIBILITY_LOG_FILTER_RE from ddtrace.internal.logger import get_logger +from ddtrace.settings.civis import ci_config log = get_logger(__name__) @@ -105,7 +106,7 @@ def take_over_logger_stream_handler(remove_ddtrace_stream_handlers=True): log.debug("CIVisibility not taking over ddtrace logger handler because debug mode is enabled") return - level = ddconfig.ci_visibility_log_level + level = ci_config.log_level if level.upper() == "NONE": log.debug("CIVisibility not taking over ddtrace logger because level is set to: %s", level) diff --git a/ddtrace/internal/ci_visibility/writer.py b/ddtrace/internal/ci_visibility/writer.py index 45b801fca93..6fa241214e4 100644 --- a/ddtrace/internal/ci_visibility/writer.py +++ b/ddtrace/internal/ci_visibility/writer.py @@ -12,6 +12,7 @@ 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.settings.civis import ci_config from ddtrace.vendor.dogstatsd import DogStatsd # noqa:F401 from .. import agent @@ -119,8 +120,8 @@ def __init__( if use_evp: intake_url = intake_url if intake_url else agent.get_trace_url() intake_cov_url = intake_url - elif config._ci_visibility_agentless_url: - intake_url = intake_url if intake_url else config._ci_visibility_agentless_url + elif ci_config._agentless_url: + intake_url = intake_url if intake_url else ci_config._agentless_url intake_cov_url = intake_url if not intake_url: intake_url = "%s.%s" % (AGENTLESS_BASE_URL, os.getenv("DD_SITE", AGENTLESS_DEFAULT_SITE)) diff --git a/ddtrace/internal/telemetry/writer.py b/ddtrace/internal/telemetry/writer.py index 3612f704c83..878c62f6c56 100644 --- a/ddtrace/internal/telemetry/writer.py +++ b/ddtrace/internal/telemetry/writer.py @@ -12,6 +12,8 @@ from typing import Tuple # noqa:F401 from typing import Union # noqa:F401 +from ddtrace.settings.civis import ci_config + from ...internal import atexit from ...internal import forksafe from ...internal.compat import parse @@ -243,7 +245,7 @@ def __init__(self, is_periodic=True, agentless=None): self._enabled = config._telemetry_enabled if agentless is None: - agentless = config._ci_visibility_agentless_enabled or config._dd_api_key is not None + agentless = ci_config._agentless_enabled or config._dd_api_key is not None if agentless and not config._dd_api_key: log.debug("Disabling telemetry: no Datadog API key found in agentless mode") diff --git a/ddtrace/settings/civis.py b/ddtrace/settings/civis.py new file mode 100644 index 00000000000..24ac51cfde6 --- /dev/null +++ b/ddtrace/settings/civis.py @@ -0,0 +1,61 @@ +from envier import En + + +class CIVisConfig(En): + __prefix__ = "dd.civisibility" + + _itr_enabled = En.v( + bool, + "itr.enabled", + default=False, + help_type="Boolean", + help="Enable ....", + ) + + _agentless_enabled = En.v( + bool, + "agentless.enabled", + default=False, + help_type="Boolean", + help="Enable ....", + ) + + _agentless_url = En.v( + str, + "agentless.url", + default="", + help_type="String", + help="Enable ....", + ) + + log_level = En.v( + str, + "log_level", + default="info", + help_type="String", + help="Enable ....", + ) + + early_flake_detection = En.v( + bool, + "early.flake.detection", + default=True, + help_type="Boolean", + help="Enable ....", + ) + + +class CITestConfig(En): + __prefix__ = "dd.test" + + test_session_name = En.v( + str, + "session.name", + default=None, + help_type="String", + help="....", + ) + + +ci_config = CIVisConfig() +test_config = CITestConfig() diff --git a/ddtrace/settings/config.py b/ddtrace/settings/config.py index a569070c61a..cf3e3fc9ab2 100644 --- a/ddtrace/settings/config.py +++ b/ddtrace/settings/config.py @@ -580,18 +580,6 @@ def __init__(self): 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)) - 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=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) - ) - self._otel_enabled = asbool(os.getenv("DD_TRACE_OTEL_ENABLED", False)) if self._otel_enabled: # Replaces the default otel api runtime context with DDRuntimeContext diff --git a/tests/ci_visibility/api_client/test_ci_visibility_api_client.py b/tests/ci_visibility/api_client/test_ci_visibility_api_client.py index 99bdb0067fe..415f049a2e5 100644 --- a/tests/ci_visibility/api_client/test_ci_visibility_api_client.py +++ b/tests/ci_visibility/api_client/test_ci_visibility_api_client.py @@ -48,7 +48,7 @@ def _patch_env_for_testing(): "ddtrace.internal.ci_visibility.recorder.CIVisibility._check_enabled_features", return_value=TestVisibilityAPISettings(), ), mock.patch( - "ddtrace.config._ci_visibility_agentless_enabled", True + "ddtrace.settings.civis.ci_config._agentless_enabled", True ): # Rebuild the config (yes, this is horrible) new_ddconfig = Config() From a14cae013919cc7a6479270edbf6abb57c78971f Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 7 Oct 2024 14:56:25 -0400 Subject: [PATCH 2/6] update codeowners file --- .github/CODEOWNERS | 1 + ddtrace/internal/ci_visibility/recorder.py | 2 +- ddtrace/settings/civis.py | 8 +++++--- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 44ad3487cfd..cdb3908b655 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -169,3 +169,4 @@ tests/opentelemetry/ @DataDog/apm-sdk-api-python tests/tracer/ @DataDog/apm-sdk-api-python # Override because order matters tests/tracer/test_ci.py @DataDog/ci-app-libraries +ddtrace/settings/civis.py @DataDog/ci-app-libraries diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index 838fa30a5ff..efc0a4a45aa 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -780,7 +780,7 @@ def set_test_session_name(cls, test_command: str) -> None: return if test_config.session_name: - test_session_name = test_config.test_session_name + test_session_name = test_config.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 diff --git a/ddtrace/settings/civis.py b/ddtrace/settings/civis.py index 24ac51cfde6..e874e088723 100644 --- a/ddtrace/settings/civis.py +++ b/ddtrace/settings/civis.py @@ -1,3 +1,5 @@ +import typing as t + from envier import En @@ -30,7 +32,7 @@ class CIVisConfig(En): log_level = En.v( str, - "log_level", + "log.level", default="info", help_type="String", help="Enable ....", @@ -48,8 +50,8 @@ class CIVisConfig(En): class CITestConfig(En): __prefix__ = "dd.test" - test_session_name = En.v( - str, + session_name = En.v( + t.Optional[str], "session.name", default=None, help_type="String", From e7cb25fbcec1ad9992a042c47719fbb70e1cefd2 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 8 Oct 2024 14:48:44 -0400 Subject: [PATCH 3/6] update tests to mock envier --- ddtrace/internal/ci_visibility/recorder.py | 19 ++++++++--------- ddtrace/internal/ci_visibility/utils.py | 4 ++-- ddtrace/internal/ci_visibility/writer.py | 6 +++--- ddtrace/internal/telemetry/writer.py | 4 ++-- tests/ci_visibility/test_ci_visibility.py | 5 +---- tests/ci_visibility/util.py | 11 ++++++++-- tests/telemetry/test_writer.py | 24 ++++++++++++---------- tests/utils.py | 19 +++++++++++------ 8 files changed, 52 insertions(+), 40 deletions(-) diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index efc0a4a45aa..9b837168a65 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -78,8 +78,7 @@ from ddtrace.internal.utils.formats import asbool from ddtrace.internal.utils.http import verify_url from ddtrace.internal.writer.writer import Response -from ddtrace.settings.civis import ci_config -from ddtrace.settings.civis import test_config +from ddtrace.settings import civis if TYPE_CHECKING: # pragma: no cover @@ -219,7 +218,7 @@ def __init__(self, tracer=None, config=None, service=None): self._git_data: GitData = get_git_data_from_tags(self._tags) - if ci_config._agentless_enabled: + if civis.ci_config._agentless_enabled: if not self._api_key: raise EnvironmentError( "DD_CIVISIBILITY_AGENTLESS_ENABLED is set, but DD_API_KEY is not set, so ddtrace " @@ -233,7 +232,7 @@ def __init__(self, tracer=None, config=None, service=None): self._configurations, self._api_key, self._dd_site, - ci_config._agentless_url if ci_config._agentless_url else None, + civis.ci_config._agentless_url if civis.ci_config._agentless_url else None, self._service, ddconfig.env, ) @@ -306,7 +305,7 @@ def _check_enabled_features(self): # DEV: Remove this ``if`` once ITR is in GA _error_return_value = TestVisibilityAPISettings() - if not ci_config._itr_enabled: + if not civis.ci_config._itr_enabled: return _error_return_value if not self._api_client: @@ -404,7 +403,7 @@ def test_skipping_enabled(cls): def is_efd_enabled(cls): if cls._instance is None: return False - return cls._instance._api_settings.early_flake_detection.enabled and ci_config.early_flake_detection + return cls._instance._api_settings.early_flake_detection.enabled and civis.ci_config.early_flake_detection @classmethod def should_collect_coverage(cls): @@ -475,7 +474,7 @@ def _should_skip_path(self, path, name, test_skipping_mode=None): def enable(cls, tracer=None, config=None, service=None): # type: (Optional[Tracer], Optional[Any], Optional[str]) -> None log.debug("Enabling %s", cls.__name__) - if ci_config._agentless_enabled: + if civis.ci_config._agentless_enabled: if not os.getenv("_CI_DD_API_KEY", os.getenv("DD_API_KEY")): log.critical( "%s disabled: environment variable DD_CIVISIBILITY_AGENTLESS_ENABLED is true but" @@ -550,7 +549,7 @@ def _start_service(self): self._unique_test_ids = unique_test_ids log.info("Unique tests fetched for Early Flake Detection: %s", len(self._unique_test_ids)) else: - if self._api_settings.early_flake_detection.enabled and not ci_config.early_flake_detection: + if self._api_settings.early_flake_detection.enabled and not civis.ci_config.early_flake_detection: log.warning( "Early Flake Detection is enabled by API but disabled by " "DD_TEST_VISIBILITY_EARLY_FLAKE_DETECTION_ENABLED environment variable" @@ -779,8 +778,8 @@ def set_test_session_name(cls, test_command: str) -> None: log.debug("Not setting test session name because no CIVisibilityEventClient is active") return - if test_config.session_name: - test_session_name = test_config.session_name + if civis.test_config.session_name: + test_session_name = civis.test_config.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 diff --git a/ddtrace/internal/ci_visibility/utils.py b/ddtrace/internal/ci_visibility/utils.py index 6bf11510642..dfc3a5accf3 100644 --- a/ddtrace/internal/ci_visibility/utils.py +++ b/ddtrace/internal/ci_visibility/utils.py @@ -10,7 +10,7 @@ from ddtrace.ext import test from ddtrace.internal.ci_visibility.constants import CIVISIBILITY_LOG_FILTER_RE from ddtrace.internal.logger import get_logger -from ddtrace.settings.civis import ci_config +from ddtrace.settings import civis log = get_logger(__name__) @@ -106,7 +106,7 @@ def take_over_logger_stream_handler(remove_ddtrace_stream_handlers=True): log.debug("CIVisibility not taking over ddtrace logger handler because debug mode is enabled") return - level = ci_config.log_level + level = civis.ci_config.log_level if level.upper() == "NONE": log.debug("CIVisibility not taking over ddtrace logger because level is set to: %s", level) diff --git a/ddtrace/internal/ci_visibility/writer.py b/ddtrace/internal/ci_visibility/writer.py index 6fa241214e4..d4daa69221b 100644 --- a/ddtrace/internal/ci_visibility/writer.py +++ b/ddtrace/internal/ci_visibility/writer.py @@ -12,7 +12,7 @@ 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.settings.civis import ci_config +from ddtrace.settings import civis from ddtrace.vendor.dogstatsd import DogStatsd # noqa:F401 from .. import agent @@ -120,8 +120,8 @@ def __init__( if use_evp: intake_url = intake_url if intake_url else agent.get_trace_url() intake_cov_url = intake_url - elif ci_config._agentless_url: - intake_url = intake_url if intake_url else ci_config._agentless_url + elif civis.ci_config._agentless_url: + intake_url = intake_url if intake_url else civis.ci_config._agentless_url intake_cov_url = intake_url if not intake_url: intake_url = "%s.%s" % (AGENTLESS_BASE_URL, os.getenv("DD_SITE", AGENTLESS_DEFAULT_SITE)) diff --git a/ddtrace/internal/telemetry/writer.py b/ddtrace/internal/telemetry/writer.py index 878c62f6c56..f073de10b2c 100644 --- a/ddtrace/internal/telemetry/writer.py +++ b/ddtrace/internal/telemetry/writer.py @@ -12,7 +12,7 @@ from typing import Tuple # noqa:F401 from typing import Union # noqa:F401 -from ddtrace.settings.civis import ci_config +from ddtrace.settings import civis from ...internal import atexit from ...internal import forksafe @@ -245,7 +245,7 @@ def __init__(self, is_periodic=True, agentless=None): self._enabled = config._telemetry_enabled if agentless is None: - agentless = ci_config._agentless_enabled or config._dd_api_key is not None + agentless = civis.ci_config._agentless_enabled or config._dd_api_key is not None if agentless and not config._dd_api_key: log.debug("Disabling telemetry: no Datadog API key found in agentless mode") diff --git a/tests/ci_visibility/test_ci_visibility.py b/tests/ci_visibility/test_ci_visibility.py index 4e7561a0bfb..49849f3d25c 100644 --- a/tests/ci_visibility/test_ci_visibility.py +++ b/tests/ci_visibility/test_ci_visibility.py @@ -41,7 +41,6 @@ from tests.utils import DummyCIVisibilityWriter from tests.utils import DummyTracer from tests.utils import TracerTestCase -from tests.utils import override_global_config TEST_SHA_1 = "b3672ea5cbc584124728c48a443825d2940e0ddd" @@ -567,9 +566,7 @@ def tearDown(self): def test_civisibilitywriter_agentless_url(self): with _ci_override_env(dict(DD_API_KEY="foobar.baz")): - with override_global_config({"_ci_visibility_agentless_url": "https://foo.bar"}), mock.patch( - "ddtrace.internal.ci_visibility.writer.config._ci_visibility_agentless_url", "https://foo.bar" - ): + with mock.patch("ddtrace.settings.civis.ci_config._agentless_url", "https://foo.bar"): dummy_writer = DummyCIVisibilityWriter() assert dummy_writer.intake_url == "https://foo.bar" diff --git a/tests/ci_visibility/util.py b/tests/ci_visibility/util.py index 748b416baee..dca12dcf508 100644 --- a/tests/ci_visibility/util.py +++ b/tests/ci_visibility/util.py @@ -13,6 +13,8 @@ from ddtrace.internal.ci_visibility.git_client import CIVisibilityGitClient from ddtrace.internal.ci_visibility.recorder import CIVisibility from ddtrace.internal.test_visibility._internal_item_ids import InternalTestId +from ddtrace.settings.civis import CITestConfig +from ddtrace.settings.civis import CIVisConfig from tests.utils import DummyCIVisibilityWriter from tests.utils import override_env @@ -206,5 +208,10 @@ def _ci_override_env( new_vars: t.Optional[t.Dict[str, str]] = None, mock_ci_env=False, replace_os_env=True, full_clear=False ): env_vars = _get_default_ci_env_vars(new_vars, mock_ci_env, full_clear) - with override_env(env_vars, replace_os_env=replace_os_env), mock.patch("ddtrace.tracer", ddtrace.Tracer()): - yield + with override_env(env_vars, replace_os_env=replace_os_env): + ci_config = CIVisConfig() + test_config = CITestConfig() + with mock.patch("ddtrace.settings.civis.ci_config", ci_config), mock.patch( + "ddtrace.settings.civis.test_config", test_config + ), mock.patch("ddtrace.tracer", ddtrace.Tracer()): + yield diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 058d01f1c46..070b6dd4ddc 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -604,8 +604,8 @@ def _get_request_body(payload, payload_type, seq_id=1): def test_telemetry_writer_agent_setup(): - with override_global_config( - {"_dd_site": "datad0g.com", "_dd_api_key": "foobarkey", "_ci_visibility_agentless_enabled": False} + with override_global_config({"_dd_site": "datad0g.com", "_dd_api_key": "foobarkey"}), mock.patch( + "ddtrace.settings.civis.ci_config._agentless_enabled", False ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter(agentless=False) assert new_telemetry_writer._enabled @@ -625,8 +625,8 @@ def test_telemetry_writer_agent_setup(): ], ) def test_telemetry_writer_agent_setup_agentless_arg_overrides_env(env_agentless, arg_agentless, expected_endpoint): - with override_global_config( - {"_dd_site": "datad0g.com", "_dd_api_key": "foobarkey", "_ci_visibility_agentless_enabled": env_agentless} + with override_global_config({"_dd_site": "datad0g.com", "_dd_api_key": "foobarkey"}), mock.patch( + "ddtrace.settings.civis.ci_config._agentless_enabled", env_agentless ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter(agentless=arg_agentless) # Note: other tests are checking whether values bet set properly, so we're only looking at agentlessness here @@ -634,8 +634,8 @@ def test_telemetry_writer_agent_setup_agentless_arg_overrides_env(env_agentless, def test_telemetry_writer_agentless_setup(): - with override_global_config( - {"_dd_site": "datad0g.com", "_dd_api_key": "foobarkey", "_ci_visibility_agentless_enabled": True} + with override_global_config({"_dd_site": "datad0g.com", "_dd_api_key": "foobarkey"}), mock.patch( + "ddtrace.settings.civis.ci_config._agentless_enabled", True ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter() assert new_telemetry_writer._enabled @@ -645,8 +645,8 @@ def test_telemetry_writer_agentless_setup(): def test_telemetry_writer_agentless_setup_eu(): - with override_global_config( - {"_dd_site": "datadoghq.eu", "_dd_api_key": "foobarkey", "_ci_visibility_agentless_enabled": True} + with override_global_config({"_dd_site": "datadoghq.eu", "_dd_api_key": "foobarkey"}), mock.patch( + "ddtrace.settings.civis.ci_config._agentless_enabled", True ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter() assert new_telemetry_writer._enabled @@ -656,8 +656,8 @@ def test_telemetry_writer_agentless_setup_eu(): def test_telemetry_writer_agentless_disabled_without_api_key(): - with override_global_config( - {"_dd_site": "datad0g.com", "_dd_api_key": None, "_ci_visibility_agentless_enabled": True} + with override_global_config({"_dd_site": "datad0g.com", "_dd_api_key": None}), mock.patch( + "ddtrace.settings.civis.ci_config._agentless_enabled", True ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter() assert not new_telemetry_writer._enabled @@ -676,7 +676,9 @@ def test_telemetry_writer_is_using_agentless_by_default_if_api_key_is_available( def test_telemetry_writer_is_using_agent_by_default_if_api_key_is_not_available(): - with override_global_config({"_dd_api_key": None, "_ci_visibility_agentless_enabled": False}): + with override_global_config({"_dd_api_key": None}), mock.patch( + "ddtrace.settings.civis.ci_config._agentless_enabled", False + ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter() assert new_telemetry_writer._enabled assert new_telemetry_writer._client._endpoint == "telemetry/proxy/api/v2/apmtelemetry" diff --git a/tests/utils.py b/tests/utils.py index 62389e0fbba..41139cdc1f6 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -131,7 +131,6 @@ def override_global_config(values): "_obfuscation_query_string_pattern", "global_query_string_obfuscation_disabled", "_ci_visibility_agentless_url", - "_ci_visibility_agentless_enabled", "_subexec_sensitive_user_wildcards", "_remote_config_enabled", "_remote_config_poll_interval", @@ -196,16 +195,24 @@ def override_config(integration, values): >>> with self.override_config('flask', dict(service_name='test-service')): # Your test """ - options = getattr(ddtrace.config, integration) + integration_config = getattr(ddtrace.config, integration) + with override_attributes(integration_config, values): + yield - original = dict((key, options.get(key)) for key in values.keys()) - options.update(values) +@contextlib.contextmanager +def override_attributes(obj, attributes): + # store current attributes + original_attrs = dict((key, getattr(obj, key)) for key in attributes.keys()) try: + # override attributes + for key, value in attributes.items(): + setattr(obj, key, value) yield finally: - options.update(original) - ddtrace.config._reset() + # restore original attributes + for key, value in original_attrs.items(): + setattr(obj, key, value) @contextlib.contextmanager From adbc1391ce9ac3680329626259d279576235a2af Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 8 Oct 2024 14:51:39 -0400 Subject: [PATCH 4/6] remove config that no longer exists --- tests/ci_visibility/test_ci_visibility.py | 2 -- tests/utils.py | 19 +++++-------------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/tests/ci_visibility/test_ci_visibility.py b/tests/ci_visibility/test_ci_visibility.py index 49849f3d25c..a345c27da77 100644 --- a/tests/ci_visibility/test_ci_visibility.py +++ b/tests/ci_visibility/test_ci_visibility.py @@ -571,7 +571,6 @@ def test_civisibilitywriter_agentless_url(self): assert dummy_writer.intake_url == "https://foo.bar" def test_civisibilitywriter_coverage_agentless_url(self): - ddtrace.internal.ci_visibility.writer.config._ci_visibility_agentless_url = "" with _ci_override_env( dict( DD_API_KEY="foobar.baz", @@ -590,7 +589,6 @@ def test_civisibilitywriter_coverage_agentless_url(self): _get_connection.assert_called_once_with("https://citestcov-intake.datadoghq.com", 2.0) def test_civisibilitywriter_coverage_agentless_with_intake_url_param(self): - ddtrace.internal.ci_visibility.writer.config._ci_visibility_agentless_url = "" with _ci_override_env( dict( DD_API_KEY="foobar.baz", diff --git a/tests/utils.py b/tests/utils.py index 41139cdc1f6..4c2a307eae3 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -130,7 +130,6 @@ def override_global_config(values): "_trace_compute_stats", "_obfuscation_query_string_pattern", "global_query_string_obfuscation_disabled", - "_ci_visibility_agentless_url", "_subexec_sensitive_user_wildcards", "_remote_config_enabled", "_remote_config_poll_interval", @@ -195,24 +194,16 @@ def override_config(integration, values): >>> with self.override_config('flask', dict(service_name='test-service')): # Your test """ - integration_config = getattr(ddtrace.config, integration) - with override_attributes(integration_config, values): - yield + options = getattr(ddtrace.config, integration) + original = dict((key, options.get(key)) for key in values.keys()) -@contextlib.contextmanager -def override_attributes(obj, attributes): - # store current attributes - original_attrs = dict((key, getattr(obj, key)) for key in attributes.keys()) + options.update(values) try: - # override attributes - for key, value in attributes.items(): - setattr(obj, key, value) yield finally: - # restore original attributes - for key, value in original_attrs.items(): - setattr(obj, key, value) + options.update(original) + ddtrace.config._reset() @contextlib.contextmanager From 539c91a7cce5ca5db54760bc7aad4a802e6f5c19 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 8 Oct 2024 16:29:21 -0400 Subject: [PATCH 5/6] fix envars --- ddtrace/settings/civis.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ddtrace/settings/civis.py b/ddtrace/settings/civis.py index e874e088723..59be8a50f0f 100644 --- a/ddtrace/settings/civis.py +++ b/ddtrace/settings/civis.py @@ -8,7 +8,7 @@ class CIVisConfig(En): _itr_enabled = En.v( bool, - "itr.enabled", + "itr_enabled", default=False, help_type="Boolean", help="Enable ....", @@ -16,7 +16,7 @@ class CIVisConfig(En): _agentless_enabled = En.v( bool, - "agentless.enabled", + "agentless_enabled", default=False, help_type="Boolean", help="Enable ....", @@ -24,7 +24,7 @@ class CIVisConfig(En): _agentless_url = En.v( str, - "agentless.url", + "agentless_url", default="", help_type="String", help="Enable ....", @@ -32,7 +32,7 @@ class CIVisConfig(En): log_level = En.v( str, - "log.level", + "log_level", default="info", help_type="String", help="Enable ....", @@ -40,7 +40,7 @@ class CIVisConfig(En): early_flake_detection = En.v( bool, - "early.flake.detection", + "early_flake_detection", default=True, help_type="Boolean", help="Enable ....", From 2c5c4833b6b805bac6bcdd27876352978672de7b Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Wed, 9 Oct 2024 01:41:21 -0400 Subject: [PATCH 6/6] make all ci configs public --- ddtrace/internal/ci_visibility/recorder.py | 8 ++++---- ddtrace/internal/ci_visibility/writer.py | 4 ++-- ddtrace/internal/telemetry/writer.py | 2 +- ddtrace/settings/civis.py | 6 +++--- .../api_client/test_ci_visibility_api_client.py | 2 +- tests/ci_visibility/test_ci_visibility.py | 2 +- tests/telemetry/test_writer.py | 12 ++++++------ 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index 9b837168a65..9a744e31a2f 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -218,7 +218,7 @@ def __init__(self, tracer=None, config=None, service=None): self._git_data: GitData = get_git_data_from_tags(self._tags) - if civis.ci_config._agentless_enabled: + if civis.ci_config.agentless_enabled: if not self._api_key: raise EnvironmentError( "DD_CIVISIBILITY_AGENTLESS_ENABLED is set, but DD_API_KEY is not set, so ddtrace " @@ -232,7 +232,7 @@ def __init__(self, tracer=None, config=None, service=None): self._configurations, self._api_key, self._dd_site, - civis.ci_config._agentless_url if civis.ci_config._agentless_url else None, + civis.ci_config.agentless_url if civis.ci_config.agentless_url else None, self._service, ddconfig.env, ) @@ -305,7 +305,7 @@ def _check_enabled_features(self): # DEV: Remove this ``if`` once ITR is in GA _error_return_value = TestVisibilityAPISettings() - if not civis.ci_config._itr_enabled: + if not civis.ci_config.itr_enabled: return _error_return_value if not self._api_client: @@ -474,7 +474,7 @@ def _should_skip_path(self, path, name, test_skipping_mode=None): def enable(cls, tracer=None, config=None, service=None): # type: (Optional[Tracer], Optional[Any], Optional[str]) -> None log.debug("Enabling %s", cls.__name__) - if civis.ci_config._agentless_enabled: + if civis.ci_config.agentless_enabled: if not os.getenv("_CI_DD_API_KEY", os.getenv("DD_API_KEY")): log.critical( "%s disabled: environment variable DD_CIVISIBILITY_AGENTLESS_ENABLED is true but" diff --git a/ddtrace/internal/ci_visibility/writer.py b/ddtrace/internal/ci_visibility/writer.py index d4daa69221b..4438ba113ec 100644 --- a/ddtrace/internal/ci_visibility/writer.py +++ b/ddtrace/internal/ci_visibility/writer.py @@ -120,8 +120,8 @@ def __init__( if use_evp: intake_url = intake_url if intake_url else agent.get_trace_url() intake_cov_url = intake_url - elif civis.ci_config._agentless_url: - intake_url = intake_url if intake_url else civis.ci_config._agentless_url + elif civis.ci_config.agentless_url: + intake_url = intake_url if intake_url else civis.ci_config.agentless_url intake_cov_url = intake_url if not intake_url: intake_url = "%s.%s" % (AGENTLESS_BASE_URL, os.getenv("DD_SITE", AGENTLESS_DEFAULT_SITE)) diff --git a/ddtrace/internal/telemetry/writer.py b/ddtrace/internal/telemetry/writer.py index f073de10b2c..9712c866a2b 100644 --- a/ddtrace/internal/telemetry/writer.py +++ b/ddtrace/internal/telemetry/writer.py @@ -245,7 +245,7 @@ def __init__(self, is_periodic=True, agentless=None): self._enabled = config._telemetry_enabled if agentless is None: - agentless = civis.ci_config._agentless_enabled or config._dd_api_key is not None + agentless = civis.ci_config.agentless_enabled or config._dd_api_key is not None if agentless and not config._dd_api_key: log.debug("Disabling telemetry: no Datadog API key found in agentless mode") diff --git a/ddtrace/settings/civis.py b/ddtrace/settings/civis.py index 59be8a50f0f..ced15ff3a91 100644 --- a/ddtrace/settings/civis.py +++ b/ddtrace/settings/civis.py @@ -6,7 +6,7 @@ class CIVisConfig(En): __prefix__ = "dd.civisibility" - _itr_enabled = En.v( + itr_enabled = En.v( bool, "itr_enabled", default=False, @@ -14,7 +14,7 @@ class CIVisConfig(En): help="Enable ....", ) - _agentless_enabled = En.v( + agentless_enabled = En.v( bool, "agentless_enabled", default=False, @@ -22,7 +22,7 @@ class CIVisConfig(En): help="Enable ....", ) - _agentless_url = En.v( + agentless_url = En.v( str, "agentless_url", default="", diff --git a/tests/ci_visibility/api_client/test_ci_visibility_api_client.py b/tests/ci_visibility/api_client/test_ci_visibility_api_client.py index 415f049a2e5..16bf8828490 100644 --- a/tests/ci_visibility/api_client/test_ci_visibility_api_client.py +++ b/tests/ci_visibility/api_client/test_ci_visibility_api_client.py @@ -48,7 +48,7 @@ def _patch_env_for_testing(): "ddtrace.internal.ci_visibility.recorder.CIVisibility._check_enabled_features", return_value=TestVisibilityAPISettings(), ), mock.patch( - "ddtrace.settings.civis.ci_config._agentless_enabled", True + "ddtrace.settings.civis.ci_config.agentless_enabled", True ): # Rebuild the config (yes, this is horrible) new_ddconfig = Config() diff --git a/tests/ci_visibility/test_ci_visibility.py b/tests/ci_visibility/test_ci_visibility.py index a345c27da77..9805604296d 100644 --- a/tests/ci_visibility/test_ci_visibility.py +++ b/tests/ci_visibility/test_ci_visibility.py @@ -566,7 +566,7 @@ def tearDown(self): def test_civisibilitywriter_agentless_url(self): with _ci_override_env(dict(DD_API_KEY="foobar.baz")): - with mock.patch("ddtrace.settings.civis.ci_config._agentless_url", "https://foo.bar"): + with mock.patch("ddtrace.settings.civis.ci_config.agentless_url", "https://foo.bar"): dummy_writer = DummyCIVisibilityWriter() assert dummy_writer.intake_url == "https://foo.bar" diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 070b6dd4ddc..40e96b1cd61 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -605,7 +605,7 @@ def _get_request_body(payload, payload_type, seq_id=1): def test_telemetry_writer_agent_setup(): with override_global_config({"_dd_site": "datad0g.com", "_dd_api_key": "foobarkey"}), mock.patch( - "ddtrace.settings.civis.ci_config._agentless_enabled", False + "ddtrace.settings.civis.ci_config.agentless_enabled", False ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter(agentless=False) assert new_telemetry_writer._enabled @@ -626,7 +626,7 @@ def test_telemetry_writer_agent_setup(): ) def test_telemetry_writer_agent_setup_agentless_arg_overrides_env(env_agentless, arg_agentless, expected_endpoint): with override_global_config({"_dd_site": "datad0g.com", "_dd_api_key": "foobarkey"}), mock.patch( - "ddtrace.settings.civis.ci_config._agentless_enabled", env_agentless + "ddtrace.settings.civis.ci_config.agentless_enabled", env_agentless ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter(agentless=arg_agentless) # Note: other tests are checking whether values bet set properly, so we're only looking at agentlessness here @@ -635,7 +635,7 @@ def test_telemetry_writer_agent_setup_agentless_arg_overrides_env(env_agentless, def test_telemetry_writer_agentless_setup(): with override_global_config({"_dd_site": "datad0g.com", "_dd_api_key": "foobarkey"}), mock.patch( - "ddtrace.settings.civis.ci_config._agentless_enabled", True + "ddtrace.settings.civis.ci_config.agentless_enabled", True ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter() assert new_telemetry_writer._enabled @@ -646,7 +646,7 @@ def test_telemetry_writer_agentless_setup(): def test_telemetry_writer_agentless_setup_eu(): with override_global_config({"_dd_site": "datadoghq.eu", "_dd_api_key": "foobarkey"}), mock.patch( - "ddtrace.settings.civis.ci_config._agentless_enabled", True + "ddtrace.settings.civis.ci_config.agentless_enabled", True ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter() assert new_telemetry_writer._enabled @@ -657,7 +657,7 @@ def test_telemetry_writer_agentless_setup_eu(): def test_telemetry_writer_agentless_disabled_without_api_key(): with override_global_config({"_dd_site": "datad0g.com", "_dd_api_key": None}), mock.patch( - "ddtrace.settings.civis.ci_config._agentless_enabled", True + "ddtrace.settings.civis.ci_config.agentless_enabled", True ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter() assert not new_telemetry_writer._enabled @@ -677,7 +677,7 @@ def test_telemetry_writer_is_using_agentless_by_default_if_api_key_is_available( def test_telemetry_writer_is_using_agent_by_default_if_api_key_is_not_available(): with override_global_config({"_dd_api_key": None}), mock.patch( - "ddtrace.settings.civis.ci_config._agentless_enabled", False + "ddtrace.settings.civis.ci_config.agentless_enabled", False ): new_telemetry_writer = ddtrace.internal.telemetry.TelemetryWriter() assert new_telemetry_writer._enabled