Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: do not return default service if config.service was user set and equals inferred service [backport 3.1] #12502

Open
wants to merge 2 commits into
base: 3.1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ddtrace/settings/_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,10 @@ def _get_service(self, default=None):
# use the inferred base service value, and instead use the default if included. If we
# didn't do this, we would have a massive breaking change from adding inferred_base_service,
# which would be replacing any integration defaults since service is no longer None.
if self.service and self.service == self._inferred_base_service:
# We also check if the service was user provided since an edge case may be that
# DD_SERVICE == inferred base service, which would force the default to be returned
# even though we want config.service in this case.
if self.service and self.service == self._inferred_base_service and not self._is_user_provided_service:
return default if default is not None else self.service

# TODO: This method can be replaced with `config.service`.
Expand Down
58 changes: 58 additions & 0 deletions tests/internal/service_name/test_inferred_base_service.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import os
import pathlib
import shlex
import subprocess
import tempfile
import textwrap
from unittest.mock import patch

import pytest
Expand Down Expand Up @@ -32,6 +35,7 @@ def mock_file_system():
(base_path / "modules" / "no" / "python_files").mkdir(parents=True)
(base_path / "apps" / "app1").mkdir(parents=True)
(base_path / "apps" / "app2" / "cmd").mkdir(parents=True)
(base_path / "app" / "app2" / "cmd").mkdir(parents=True)

# Create Python and other files
(base_path / "__pycache__" / "app.cpython-311.pyc").touch()
Expand Down Expand Up @@ -182,3 +186,57 @@ def test_module_exists(module_name, should_exist):
exists = _module_exists(module_name)

assert exists == should_exist, f"Module {module_name} existence check failed."


@pytest.mark.parametrize(
"cmd,default,expected",
[
("ddtrace-run python app/web.py", None, "app"),
("ddtrace-run python app/web.py", "test-app", "test-app"),
("DD_SERVICE=app ddtrace-run python app/web.py", None, "app"),
("DD_SERVICE=app ddtrace-run python app/web.py", "test-appp", "app"),
],
)
def test_get_service(cmd, default, expected, testdir):
cmd_parts = shlex.split(cmd)

env = os.environ.copy()
# Extract environment variables from the command (e.g., DD_SERVICE=app)
env_vars = {}
command = []
for part in cmd_parts:
if "=" in part and not part.startswith(("'", '"')):
key, value = part.split("=", 1)
env_vars[key] = value
else:
command.append(part)
env.update(env_vars)

app_dir = testdir.mkdir("app")

web_py_content = textwrap.dedent(
f"""
from ddtrace import config

# Retrieve the service name using the _get_service method
service = config._get_service(default={repr(default)})

# Assert that the detected service name matches the expected value
assert service == {repr(expected)}, f"Expected service '{repr(expected)}', but got '{{service}}'"
"""
)

# Create the web.py file within the app directory
app_dir.join("web.py").write(web_py_content)
app_dir.join("__init__.py").write("# Init for web package")

# Execute the command using subprocess
result = subprocess.run(command, cwd=testdir.tmpdir, capture_output=True, text=True, env=env)

assert result.returncode == 0, (
f"Command failed with return code {result.returncode}\n"
f"STDOUT:\n{result.stdout}\n"
f"STDERR:\n{result.stderr}"
)

assert "AssertionError" not in result.stderr, "AssertionError found in stderr"
Loading