Skip to content

Commit

Permalink
[mq] working branch - merge cc649bc on top of main at a664aab
Browse files Browse the repository at this point in the history
{"baseBranch":"main","baseCommit":"a664aab0a9d53e8dbb874d68933613de4cbecb1d","createdAt":"2024-11-21T13:37:42.446817Z","gitlabPipelineId":"49608212","headSha":"cc649bcf7f8fcdb0af4dc1ea756cb518dc80d0ca","id":"3a79c945-094b-4dca-bb41-dee25b810447","priority":"200","pullRequestNumber":"11478","queuedAt":"2024-11-22T11:03:16.085110Z","status":"STATUS_QUEUED","temporalExecutionRunId":"1d25c0cb-6e3e-4d11-9203-719c130128e5","temporalExecutionWorkflowId":"ea5769d3-80b5-4a91-9d26-1f34fb7406a0_38"}
  • Loading branch information
dd-mergequeue[bot] authored Nov 22, 2024
2 parents 3dc6130 + cc649bc commit af4d566
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 1 deletion.
24 changes: 23 additions & 1 deletion ddtrace/internal/ci_visibility/recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,14 @@ def __init__(self, tracer=None, config=None, service=None):
self._git_data: GitData = get_git_data_from_tags(self._tags)

dd_env = os.getenv("_CI_DD_ENV", ddconfig.env)
dd_env_msg = ""

if ddconfig._ci_visibility_agentless_enabled:
# In agentless mode, normalize an unset env to none (this is already done by the backend in most cases, so
# it does not override default behavior)
if dd_env is None:
dd_env = "none"
dd_env_msg = " (not set in environment)"
if not self._api_key:
raise EnvironmentError(
"DD_CIVISIBILITY_AGENTLESS_ENABLED is set, but DD_API_KEY is not set, so ddtrace "
Expand All @@ -242,6 +248,11 @@ def __init__(self, tracer=None, config=None, service=None):
dd_env,
)
elif self._agent_evp_proxy_is_available():
# In EVP-proxy cases, if an env is not provided, we need to get the agent's default env in order to make
# the correct decision:
if dd_env is None:
dd_env = self._agent_get_default_env()
dd_env_msg = " (default environment provided by agent)"
self._requests_mode = REQUESTS_MODE.EVP_PROXY_EVENTS
requests_mode_str = "EVP Proxy"
self._api_client = EVPProxyTestVisibilityAPIClient(
Expand Down Expand Up @@ -269,7 +280,7 @@ def __init__(self, tracer=None, config=None, service=None):

self._configure_writer(coverage_enabled=self._collect_coverage_enabled, url=self.tracer._agent_url)

log.info("Service: %s (env: %s)", self._service, dd_env)
log.info("Service: %s (env: %s%s)", self._service, dd_env, dd_env_msg)
log.info("Requests mode: %s", requests_mode_str)
log.info("Git metadata upload enabled: %s", self._should_upload_git_metadata)
log.info("API-provided settings: coverage collection: %s", self._api_settings.coverage_enabled)
Expand Down Expand Up @@ -394,6 +405,17 @@ def _agent_evp_proxy_is_available(self):
return True
return False

def _agent_get_default_env(self):
# type: () -> Optional[str]
try:
info = agent.info(self.tracer._agent_url)
except Exception:
return "none"

if info:
return info.get("config", {}).get("default_env", "none")
return "none"

@classmethod
def is_itr_enabled(cls):
# cls.enabled guarantees _instance is not None
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
fixes:
- |
CI Visibility: fixes an issue where the CIVisbility service would incorrectly default the tracer env to ``None``
in EVP proxy mode if ``DD_ENV`` was not specified but the agent had a default environment set to a value other
than ``none`` (eg: using ``DD_APM_ENV`` in the agent's environment).
106 changes: 106 additions & 0 deletions tests/ci_visibility/api_client/test_ci_visibility_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,8 @@ def test_civisibility_api_client_agentless_env_config_success(self, env_vars, ex
configurations["custom"] = _expected_config.pop("custom_configurations")
if "dd_service" not in _expected_config:
_expected_config["dd_service"] = "dd-test-py"
if "dd_env" not in _expected_config:
_expected_config["dd_env"] = "none"

git_data = GitData("[email protected]:TestDog/dd-test-py.git", "notmainbranch", "mytestcommitsha1234")
with _ci_override_env(_env_vars, full_clear=True), _patch_env_for_testing():
Expand Down Expand Up @@ -425,6 +427,7 @@ def test_civisibility_api_client_evp_proxy_config_success(self, env_vars, expect
configurations=configurations,
git_data=git_data,
agent_url="http://patchedagenturl:6218",
dd_env="none",
**_expected_config,
)
CIVisibility.enable()
Expand All @@ -435,3 +438,106 @@ def test_civisibility_api_client_evp_proxy_config_success(self, env_vars, expect
assert CIVisibility._instance._api_client.__dict__ == expected_client.__dict__
finally:
CIVisibility.disable()

def test_civisibility_api_client_evp_respects_agent_default_config(self):
"""Tests that, if no DD_ENV is provided in EVP mode, the agent's default env is used"""
agent_info_response = json.loads(
"""
{
"version": "7.49.1",
"git_commit": "1790cab",
"endpoints": [
"/v0.3/traces",
"/v0.3/services",
"/v0.4/traces",
"/v0.4/services",
"/v0.5/traces",
"/v0.7/traces",
"/profiling/v1/input",
"/telemetry/proxy/",
"/v0.6/stats",
"/v0.1/pipeline_stats",
"/evp_proxy/v1/",
"/evp_proxy/v2/",
"/evp_proxy/v3/",
"/debugger/v1/input",
"/debugger/v1/diagnostics",
"/symdb/v1/input",
"/dogstatsd/v1/proxy",
"/dogstatsd/v2/proxy",
"/v0.7/config",
"/config/set"
],
"client_drop_p0s": true,
"span_meta_structs": true,
"long_running_spans": true,
"config": {
"default_env": "not_the_default_default_env",
"target_tps": 10,
"max_eps": 200,
"receiver_port": 8126,
"receiver_socket": "",
"connection_limit": 0,
"receiver_timeout": 0,
"max_request_bytes": 26214400,
"statsd_port": 8125,
"max_memory": 0,
"max_cpu": 0,
"analyzed_spans_by_service": {},
"obfuscation": {
"elastic_search": true,
"mongo": true,
"sql_exec_plan": false,
"sql_exec_plan_normalize": false,
"http": {
"remove_query_string": false,
"remove_path_digits": false
},
"remove_stack_traces": false,
"redis": {
"Enabled": true,
"RemoveAllArgs": false
},
"memcached": {
"Enabled": true,
"KeepCommand": false
}
}
}
}
"""
)

configurations = {
"os.architecture": "testarch64",
"os.platform": "Not Actually Linux",
"os.version": "1.2.3-test",
"runtime.name": "CPythonTest",
"runtime.version": "1.2.3",
}

git_data = GitData("[email protected]:TestDog/dd-test-py.git", "notmainbranch", "mytestcommitsha1234")
with _ci_override_env(full_clear=True), _patch_env_for_testing(), mock.patch(
"ddtrace.internal.ci_visibility.recorder.CIVisibility._agent_evp_proxy_is_available", return_value=True
), mock.patch("ddtrace.internal.agent.info", return_value=agent_info_response), mock.patch(
"ddtrace.internal.agent.get_trace_url", return_value="http://shouldntbeused:6218"
), mock.patch(
"ddtrace.internal.ci_visibility.recorder.ddtrace.tracer._agent_url", "http://patchedagenturl:6218"
):
try:
expected_client = EVPProxyTestVisibilityAPIClient(
itr_skipping_level=ITR_SKIPPING_LEVEL.TEST,
configurations=configurations,
git_data=git_data,
agent_url="http://patchedagenturl:6218",
dd_env="not_the_default_default_env",
dd_service="dd-test-py",
)
CIVisibility.enable()
assert CIVisibility.enabled is True
assert CIVisibility._instance is not None
assert CIVisibility._instance._api_client is not None

assert CIVisibility._instance._api_client.__dict__ == expected_client.__dict__
finally:
CIVisibility.disable()

0 comments on commit af4d566

Please sign in to comment.