From 914bbbbe4ae35a67c4e897d5f2d715600736e0aa Mon Sep 17 00:00:00 2001 From: "Gabriele N. Tornetta" Date: Wed, 13 Sep 2023 17:44:33 +0100 Subject: [PATCH] refactor(debugging): make safety module internal (#6810) The safety submodule of `ddtrace.debugging` has been made public (according to the naming conventions) by accident. The API of this module is not documented and we do not expect any usage outside the library itself. ## 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/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. --------- Co-authored-by: Yun Kim Co-authored-by: Yun Kim <35776586+Yun-Kim@users.noreply.github.com> --- ddtrace/debugging/_expressions.py | 2 +- ddtrace/debugging/{safety.py => _safety.py} | 0 ddtrace/debugging/_signal/model.py | 4 ++-- ddtrace/debugging/_signal/snapshot.py | 10 +++++----- ddtrace/debugging/_signal/utils.py | 2 +- tests/debugging/test_safety.py | 14 +++++++------- 6 files changed, 16 insertions(+), 16 deletions(-) rename ddtrace/debugging/{safety.py => _safety.py} (100%) diff --git a/ddtrace/debugging/_expressions.py b/ddtrace/debugging/_expressions.py index 5406ca267d0..5c2ef5efac9 100644 --- a/ddtrace/debugging/_expressions.py +++ b/ddtrace/debugging/_expressions.py @@ -40,7 +40,7 @@ from bytecode import Compare from bytecode import Instr -from ddtrace.debugging.safety import safe_getitem +from ddtrace.debugging._safety import safe_getitem from ddtrace.internal.compat import PYTHON_VERSION_INFO as PY diff --git a/ddtrace/debugging/safety.py b/ddtrace/debugging/_safety.py similarity index 100% rename from ddtrace/debugging/safety.py rename to ddtrace/debugging/_safety.py diff --git a/ddtrace/debugging/_signal/model.py b/ddtrace/debugging/_signal/model.py index ddb45d1787a..38830083cf1 100644 --- a/ddtrace/debugging/_signal/model.py +++ b/ddtrace/debugging/_signal/model.py @@ -15,7 +15,7 @@ import six from ddtrace.context import Context -from ddtrace.debugging import safety +from ddtrace.debugging import _safety from ddtrace.debugging._expressions import DDExpressionEvaluationError from ddtrace.debugging._probe.model import FunctionLocationMixin from ddtrace.debugging._probe.model import LineLocationMixin @@ -83,7 +83,7 @@ def _eval_condition(self, _locals=None): return False def _enrich_args(self, retval, exc_info, duration): - _locals = list(self.args or safety.get_args(self.frame)) + _locals = list(self.args or _safety.get_args(self.frame)) _locals.append(("@duration", duration / 1e6)) # milliseconds if exc_info[1] is None: _locals.append(("@return", retval)) diff --git a/ddtrace/debugging/_signal/snapshot.py b/ddtrace/debugging/_signal/snapshot.py index 38614c25ac0..f6292e78374 100644 --- a/ddtrace/debugging/_signal/snapshot.py +++ b/ddtrace/debugging/_signal/snapshot.py @@ -8,7 +8,7 @@ import attr -from ddtrace.debugging import safety +from ddtrace.debugging import _safety from ddtrace.debugging._expressions import DDExpressionEvaluationError from ddtrace.debugging._probe.model import CaptureLimits from ddtrace.debugging._probe.model import DEFAULT_CAPTURE_LIMITS @@ -144,7 +144,7 @@ def enter(self): probe = self.probe frame = self.frame - _args = list(self.args or safety.get_args(frame)) + _args = list(self.args or _safety.get_args(frame)) if probe.evaluate_at == ProbeEvaluateTimingForMethod.EXIT: return @@ -190,7 +190,7 @@ def exit(self, retval, exc_info, duration): if probe.take_snapshot: self.return_capture = _capture_context( - self.args or safety.get_args(self.frame), _locals, exc_info, limits=probe.limits + self.args or _safety.get_args(self.frame), _locals, exc_info, limits=probe.limits ) self.duration = duration self.state = SignalState.DONE @@ -213,8 +213,8 @@ def line(self): return self.line_capture = _capture_context( - self.args or safety.get_args(frame), - safety.get_locals(frame), + self.args or _safety.get_args(frame), + _safety.get_locals(frame), sys.exc_info(), limits=probe.limits, ) diff --git a/ddtrace/debugging/_signal/utils.py b/ddtrace/debugging/_signal/utils.py index be166fa7c0b..3fb79238cc8 100644 --- a/ddtrace/debugging/_signal/utils.py +++ b/ddtrace/debugging/_signal/utils.py @@ -13,7 +13,7 @@ from ddtrace.debugging._probe.model import MAXLEN from ddtrace.debugging._probe.model import MAXLEVEL from ddtrace.debugging._probe.model import MAXSIZE -from ddtrace.debugging.safety import get_fields +from ddtrace.debugging._safety import get_fields from ddtrace.internal.compat import BUILTIN_CONTAINER_TYPES from ddtrace.internal.compat import BUILTIN_SIMPLE_TYPES from ddtrace.internal.compat import CALLABLE_TYPES diff --git a/tests/debugging/test_safety.py b/tests/debugging/test_safety.py index 90fa8fc82a4..e0f1b858719 100644 --- a/tests/debugging/test_safety.py +++ b/tests/debugging/test_safety.py @@ -4,15 +4,15 @@ import pytest -from ddtrace.debugging import safety +from ddtrace.debugging import _safety def test_get_args(): def assert_args(args): - assert set(dict(safety.get_args(inspect.currentframe().f_back)).keys()) == args + assert set(dict(_safety.get_args(inspect.currentframe().f_back)).keys()) == args def assert_locals(_locals): - assert set(dict(safety.get_locals(inspect.currentframe().f_back)).keys()) == _locals + assert set(dict(_safety.get_locals(inspect.currentframe().f_back)).keys()) == _locals def arg_and_kwargs(a, **kwargs): assert_args({"a", "kwargs"}) @@ -55,7 +55,7 @@ def property_with_side_effect(self): def test_get_fields_side_effects(): - assert safety.get_fields(SideEffects()) == {} + assert _safety.get_fields(SideEffects()) == {} # ---- Slots ---- @@ -75,8 +75,8 @@ def __init__(self): super(B, self).__init__() self.b = "b" - assert safety.get_fields(A()) == {"a": "a"} - assert safety.get_fields(B()) == {"a": "a", "b": "b"} + assert _safety.get_fields(A()) == {"a": "a"} + assert _safety.get_fields(B()) == {"a": "a", "b": "b"} def test_safe_dict(): @@ -87,4 +87,4 @@ def __dict__(self): raise NotImplementedError() with pytest.raises(AttributeError): - safety._safe_dict(Foo()) + _safety._safe_dict(Foo())