Skip to content

Commit

Permalink
chore(logging)!: set default value of DD_CALL_BASIC_CONFIG to False (#…
Browse files Browse the repository at this point in the history
…3168)

ddtrace should not call `logging.basicConfig` since it is not a logging library.  

This PR updates the default value of ``DD_CALL_BASIC_CONFIG`` to False and logs a deprecation warning if DD_CALL_BASIC_CONFIG configuration is set to True.

## Checklist
- [ ] Added to the correct milestone.
- [ ] Tests provided or description of manual testing performed is included in the code or PR.
- [ ] Library documentation is updated.
- [ ] [Corp site](https://github.com/DataDog/documentation/) documentation is updated (link to the PR).
  • Loading branch information
mabdinur authored and majorgreys committed Feb 18, 2022
1 parent 5507db4 commit b68efdb
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 23 deletions.
8 changes: 7 additions & 1 deletion ddtrace/bootstrap/sitecustomize.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from ddtrace.internal.utils.formats import parse_tags_str
from ddtrace.tracer import DD_LOG_FORMAT # noqa
from ddtrace.tracer import debug_mode
from ddtrace.vendor.debtcollector import deprecate


if config.logs_injection:
Expand All @@ -40,8 +41,13 @@
# upon initializing it the first time.
# See https://github.com/python/cpython/blob/112e4afd582515fcdcc0cde5012a4866e5cfda12/Lib/logging/__init__.py#L1550
# Debug mode from the tracer will do a basicConfig so only need to do this otherwise
call_basic_config = asbool(os.environ.get("DD_CALL_BASIC_CONFIG", "true"))
call_basic_config = asbool(os.environ.get("DD_CALL_BASIC_CONFIG", "false"))
if not debug_mode and call_basic_config:
deprecate(
"ddtrace.tracer.logging.basicConfig",
message="`logging.basicConfig()` should be called in a user's application."
" ``DD_CALL_BASIC_CONFIG`` will be removed in a future version.",
)
if config.logs_injection:
logging.basicConfig(format=DD_LOG_FORMAT)
else:
Expand Down
7 changes: 6 additions & 1 deletion ddtrace/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,18 @@
log = get_logger(__name__)

debug_mode = asbool(os.getenv("DD_TRACE_DEBUG", default=False))
call_basic_config = asbool(os.environ.get("DD_CALL_BASIC_CONFIG", "true"))
call_basic_config = asbool(os.environ.get("DD_CALL_BASIC_CONFIG", "false"))

DD_LOG_FORMAT = "%(asctime)s %(levelname)s [%(name)s] [%(filename)s:%(lineno)d] {}- %(message)s".format(
"[dd.service=%(dd.service)s dd.env=%(dd.env)s dd.version=%(dd.version)s"
" dd.trace_id=%(dd.trace_id)s dd.span_id=%(dd.span_id)s] "
)
if debug_mode and not hasHandlers(log) and call_basic_config:
debtcollector.deprecate(
"ddtrace.tracer.logging.basicConfig",
message="`logging.basicConfig()` should be called in a user's application."
" ``DD_CALL_BASIC_CONFIG`` will be removed in a future version.",
)
if config.logs_injection:
# We need to ensure logging is patched in case the tracer logs during initialization
patch(logging=True)
Expand Down
2 changes: 1 addition & 1 deletion docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ below:
.. _dd-call-basic-config:
* - ``DD_CALL_BASIC_CONFIG``
- Boolean
- True
- False
- Controls whether ``logging.basicConfig`` is called in ``ddtrace-run`` or when debug mode is enabled.

.. _dd-trace-agent-url:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
upgrade:
- |
Default value of ``DD_CALL_BASIC_CONFIG`` was updated from ``True`` to `False`. Call `logging.basicConfig()` to configure logging in your application.
deprecations:
- |
DD_CALL_BASIC_CONFIG is deprecated and will be removed in a future version.
29 changes: 15 additions & 14 deletions tests/commands/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def test_debug_enabling(self):
assert b"Test success" in out
assert b"DATADOG TRACER CONFIGURATION" not in out

with self.override_env(dict(DD_TRACE_DEBUG="true")):
with self.override_env(dict(DD_TRACE_DEBUG="true", DD_CALL_BASIC_CONFIG="true")):
out = subprocess.check_output(
["ddtrace-run", "python", "tests/commands/ddtrace_run_debug.py"],
stderr=subprocess.STDOUT,
Expand Down Expand Up @@ -267,7 +267,7 @@ def test_global_trace_tags(self):

def test_logs_injection(self):
"""Ensure logs injection works"""
with self.override_env(dict(DD_LOGS_INJECTION="true")):
with self.override_env(dict(DD_LOGS_INJECTION="true", DD_CALL_BASIC_CONFIG="true")):
out = subprocess.check_output(["ddtrace-run", "python", "tests/commands/ddtrace_run_logs_injection.py"])
assert out.startswith(b"Test success")

Expand All @@ -280,6 +280,19 @@ def test_gevent_patch_all(self):
out = subprocess.check_output(["ddtrace-run", "python", "tests/commands/ddtrace_run_gevent.py"])
assert out.startswith(b"Test success")

def test_debug_mode(self):
with self.override_env(dict(DD_CALL_BASIC_CONFIG="true")):
p = subprocess.Popen(
["ddtrace-run", "--debug", "python", "-c", "''"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)

p.wait()
assert p.returncode == 0
assert p.stdout.read() == six.b("")
assert six.b("ddtrace.sampler") in p.stderr.read()


def test_env_profiling_enabled(monkeypatch):
"""DD_PROFILING_ENABLED allows enabling the global profiler."""
Expand Down Expand Up @@ -378,18 +391,6 @@ def test_return_code():
assert p.returncode == 4


def test_debug_mode():
p = subprocess.Popen(
["ddtrace-run", "--debug", "python", "-c", "''"],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
p.wait()
assert p.returncode == 0
assert p.stdout.read() == six.b("")
assert six.b("ddtrace.sampler") in p.stderr.read()


def test_info_no_configs():
p = subprocess.Popen(
["ddtrace-run", "--info"],
Expand Down
12 changes: 9 additions & 3 deletions tests/integration/test_debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,9 @@ def test_startup_logs_sampling_rules():
def test_error_output_ddtracerun_debug_mode():
p = subprocess.Popen(
["ddtrace-run", "python", "tests/integration/hello.py"],
env=dict(DD_TRACE_AGENT_URL="http://localhost:8126", DD_TRACE_DEBUG="true", **os.environ),
env=dict(
DD_TRACE_AGENT_URL="http://localhost:8126", DD_TRACE_DEBUG="true", DD_CALL_BASIC_CONFIG="true", **os.environ
),
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
Expand All @@ -392,7 +394,9 @@ def test_error_output_ddtracerun_debug_mode():
# No connection to agent, debug mode enabled
p = subprocess.Popen(
["ddtrace-run", "python", "tests/integration/hello.py"],
env=dict(DD_TRACE_AGENT_URL="http://localhost:4321", DD_TRACE_DEBUG="true", **os.environ),
env=dict(
DD_TRACE_AGENT_URL="http://localhost:4321", DD_TRACE_DEBUG="true", DD_CALL_BASIC_CONFIG="true", **os.environ
),
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
Expand Down Expand Up @@ -434,7 +438,9 @@ def test_error_output_ddtracerun():
def test_debug_span_log():
p = subprocess.Popen(
["python", "-c", 'import os; print(os.environ);import ddtrace; ddtrace.tracer.trace("span").finish()'],
env=dict(DD_TRACE_AGENT_URL="http://localhost:8126", DD_TRACE_DEBUG="true", **os.environ),
env=dict(
DD_TRACE_AGENT_URL="http://localhost:8126", DD_TRACE_DEBUG="true", DD_CALL_BASIC_CONFIG="true", **os.environ
),
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
Expand Down
8 changes: 5 additions & 3 deletions tests/integration/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def test_debug_mode():

p = subprocess.Popen(
[sys.executable, "-c", "import ddtrace"],
env=dict(DD_TRACE_DEBUG="true"),
env=dict(DD_TRACE_DEBUG="true", DD_CALL_BASIC_CONFIG="true"),
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
)
Expand Down Expand Up @@ -600,6 +600,7 @@ def test_regression_logging_in_context(tmpdir, logs_injection, debug_mode, patch
env=dict(
DD_TRACE_LOGS_INJECTION=str(logs_injection).lower(),
DD_TRACE_DEBUG=str(debug_mode).lower(),
DD_CALL_BASIC_CONFIG="true",
),
)
try:
Expand All @@ -622,7 +623,7 @@ def test_call_basic_config(ddtrace_run_python_code_in_subprocess, call_basic_con
When false
We do not call logging.basicConfig()
When not set
We call logging.basicConfig()
We do not call logging.basicConfig()
"""
env = os.environ.copy()

Expand All @@ -632,7 +633,7 @@ def test_call_basic_config(ddtrace_run_python_code_in_subprocess, call_basic_con
env["DD_CALL_BASIC_CONFIG"] = str(call_basic_config).lower()
has_root_handlers = call_basic_config
else:
has_root_handlers = True
has_root_handlers = False

out, err, status, pid = ddtrace_run_python_code_in_subprocess(
"""
Expand Down Expand Up @@ -756,6 +757,7 @@ def test_ddtrace_run_startup_logging_injection(ddtrace_run_python_code_in_subpro
env = os.environ.copy()
env["DD_TRACE_DEBUG"] = "true"
env["DD_LOGS_INJECTION"] = "true"
env["DD_CALL_BASIC_CONFIG"] = "true"

# DEV: We don't actually have to execute any code to validate this
out, err, status, pid = ddtrace_run_python_code_in_subprocess("", env=env)
Expand Down

0 comments on commit b68efdb

Please sign in to comment.