Skip to content

Commit 244ffac

Browse files
chore(profiling): remove '_self_' prefixes from attributes following unwrapt in Lock Profiler code
1 parent a22de70 commit 244ffac

File tree

2 files changed

+34
-47
lines changed

2 files changed

+34
-47
lines changed

ddtrace/profiling/collector/_lock.py

Lines changed: 34 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,13 @@ class _ProfiledLock:
4040

4141
__slots__ = (
4242
"__wrapped__",
43-
"_self_tracer",
44-
"_self_max_nframes",
45-
"_self_capture_sampler",
46-
"_self_endpoint_collection_enabled",
47-
"_self_init_loc",
48-
"_self_acquired_at",
49-
"_self_name",
43+
"tracer",
44+
"max_nframes",
45+
"capture_sampler",
46+
"endpoint_collection_enabled",
47+
"init_location",
48+
"acquired_time",
49+
"name",
5050
)
5151

5252
def __init__(
@@ -58,16 +58,16 @@ def __init__(
5858
endpoint_collection_enabled: bool,
5959
) -> None:
6060
self.__wrapped__: Any = wrapped
61-
self._self_tracer: Optional[Tracer] = tracer
62-
self._self_max_nframes: int = max_nframes
63-
self._self_capture_sampler: collector.CaptureSampler = capture_sampler
64-
self._self_endpoint_collection_enabled: bool = endpoint_collection_enabled
61+
self.tracer: Optional[Tracer] = tracer
62+
self.max_nframes: int = max_nframes
63+
self.capture_sampler: collector.CaptureSampler = capture_sampler
64+
self.endpoint_collection_enabled: bool = endpoint_collection_enabled
6565
# Frame depth: 0=__init__, 1=_profiled_allocate_lock, 2=_LockAllocatorWrapper.__call__, 3=caller
6666
frame: FrameType = sys._getframe(3)
6767
code: CodeType = frame.f_code
68-
self._self_init_loc: str = "%s:%d" % (os.path.basename(code.co_filename), frame.f_lineno)
69-
self._self_acquired_at: int = 0
70-
self._self_name: Optional[str] = None
68+
self.init_location: str = f"{os.path.basename(code.co_filename)}:{frame.f_lineno}"
69+
self.acquired_time: int = 0
70+
self.name: Optional[str] = None
7171

7272
def __aenter__(self, *args: Any, **kwargs: Any) -> Any:
7373
return self._acquire(self.__wrapped__.__aenter__, *args, **kwargs)
@@ -76,7 +76,7 @@ def __aexit__(self, *args: Any, **kwargs: Any) -> Any:
7676
return self._release(self.__wrapped__.__aexit__, *args, **kwargs)
7777

7878
def _acquire(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) -> Any:
79-
if not self._self_capture_sampler.capture():
79+
if not self.capture_sampler.capture():
8080
return inner_func(*args, **kwargs)
8181

8282
start: int = time.monotonic_ns()
@@ -85,7 +85,7 @@ def _acquire(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) ->
8585
finally:
8686
try:
8787
end: int = time.monotonic_ns()
88-
self._self_acquired_at = end
88+
self.acquired_time = end
8989

9090
thread_id: int
9191
thread_name: str
@@ -96,10 +96,8 @@ def _acquire(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) ->
9696
task_frame: Optional[FrameType]
9797
task_id, task_name, task_frame = _task.get_task(thread_id)
9898

99-
self._maybe_update_self_name()
100-
lock_name: str = (
101-
"%s:%s" % (self._self_init_loc, self._self_name) if self._self_name else self._self_init_loc
102-
)
99+
self._update_name()
100+
lock_name: str = "%s:%s" % (self.init_location, self.name) if self.name else self.init_location
103101

104102
frame: FrameType
105103
if task_frame is None:
@@ -110,7 +108,7 @@ def _acquire(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) ->
110108
frame = task_frame
111109

112110
frames: List[DDFrame]
113-
frames, _ = _traceback.pyframe_to_frames(frame, self._self_max_nframes)
111+
frames, _ = _traceback.pyframe_to_frames(frame, self.max_nframes)
114112

115113
thread_native_id: int = _threading.get_thread_native_id(thread_id)
116114

@@ -122,8 +120,8 @@ def _acquire(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) ->
122120
handle.push_task_id(task_id)
123121
handle.push_task_name(task_name)
124122

125-
if self._self_tracer is not None:
126-
handle.push_span(self._self_tracer.current_span())
123+
if self.tracer is not None:
124+
handle.push_span(self.tracer.current_span())
127125
for ddframe in frames:
128126
handle.push_frame(ddframe.function_name, ddframe.file_name, 0, ddframe.lineno)
129127
handle.flush_sample()
@@ -134,15 +132,15 @@ def acquire(self, *args: Any, **kwargs: Any) -> Any:
134132
return self._acquire(self.__wrapped__.acquire, *args, **kwargs)
135133

136134
def _release(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) -> None:
137-
start: Optional[int] = getattr(self, "_self_acquired_at", None)
135+
start: Optional[int] = getattr(self, "acquired_time", None)
138136
try:
139137
# Though it should generally be avoided to call release() from
140138
# multiple threads, it is possible to do so. In that scenario, the
141139
# following statement code will raise an AttributeError. This should
142140
# not be propagated to the caller and to the users. The inner_func
143141
# will raise an RuntimeError as the threads are trying to release()
144142
# and unlocked lock, and the expected behavior is to propagate that.
145-
del self._self_acquired_at
143+
del self.acquired_time
146144
except AttributeError:
147145
# We just ignore the error, if the attribute is not found.
148146
pass
@@ -161,9 +159,7 @@ def _release(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) ->
161159
task_frame: Optional[FrameType]
162160
task_id, task_name, task_frame = _task.get_task(thread_id)
163161

164-
lock_name: str = (
165-
"%s:%s" % (self._self_init_loc, self._self_name) if self._self_name else self._self_init_loc
166-
)
162+
lock_name: str = "%s:%s" % (self.init_location, self.name) if self.name else self.init_location
167163

168164
frame: FrameType
169165
if task_frame is None:
@@ -173,7 +169,7 @@ def _release(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) ->
173169
frame = task_frame
174170

175171
frames: List[DDFrame]
176-
frames, _ = _traceback.pyframe_to_frames(frame, self._self_max_nframes)
172+
frames, _ = _traceback.pyframe_to_frames(frame, self.max_nframes)
177173

178174
thread_native_id: int = _threading.get_thread_native_id(thread_id)
179175

@@ -185,8 +181,8 @@ def _release(self, inner_func: Callable[..., Any], *args: Any, **kwargs: Any) ->
185181
handle.push_task_id(task_id)
186182
handle.push_task_name(task_name)
187183

188-
if self._self_tracer is not None:
189-
handle.push_span(self._self_tracer.current_span())
184+
if self.tracer is not None:
185+
handle.push_span(self.tracer.current_span())
190186
for ddframe in frames:
191187
handle.push_frame(ddframe.function_name, ddframe.file_name, 0, ddframe.lineno)
192188
handle.flush_sample()
@@ -200,7 +196,7 @@ def __enter__(self, *args: Any, **kwargs: Any) -> Any:
200196
def __exit__(self, *args: Any, **kwargs: Any) -> None:
201197
self._release(self.__wrapped__.__exit__, *args, **kwargs)
202198

203-
def _find_self_name(self, var_dict: Dict[str, Any]) -> Optional[str]:
199+
def _find_name(self, var_dict: Dict[str, Any]) -> Optional[str]:
204200
for name, value in var_dict.items():
205201
if name.startswith("__") or isinstance(value, ModuleType):
206202
continue
@@ -210,7 +206,6 @@ def _find_self_name(self, var_dict: Dict[str, Any]) -> Optional[str]:
210206
for attribute in dir(value):
211207
try:
212208
if not attribute.startswith("__") and getattr(value, attribute) is self:
213-
self._self_name = attribute
214209
return attribute
215210
except AttributeError:
216211
# Accessing unset attributes in __slots__ raises AttributeError.
@@ -219,8 +214,8 @@ def _find_self_name(self, var_dict: Dict[str, Any]) -> Optional[str]:
219214

220215
# Get lock acquire/release call location and variable name the lock is assigned to
221216
# This function propagates ValueError if the frame depth is <= 3.
222-
def _maybe_update_self_name(self) -> None:
223-
if self._self_name is not None:
217+
def _update_name(self) -> None:
218+
if self.name is not None:
224219
return
225220
# We expect the call stack to be like this:
226221
# 0: this
@@ -245,17 +240,17 @@ def _maybe_update_self_name(self) -> None:
245240
frame = sys._getframe(3)
246241

247242
# First, look at the local variables of the caller frame, and then the global variables
248-
self._self_name = self._find_self_name(frame.f_locals) or self._find_self_name(frame.f_globals)
243+
self.name = self._find_name(frame.f_locals) or self._find_name(frame.f_globals)
249244

250-
if not self._self_name:
251-
self._self_name = ""
245+
if not self.name:
246+
self.name = ""
252247

253248
def locked(self) -> bool:
254249
"""Return True if lock is currently held."""
255250
return self.__wrapped__.locked()
256251

257252
def __repr__(self) -> str:
258-
return f"<_ProfiledLock({self.__wrapped__!r}) at {self._self_init_loc}>"
253+
return f"<_ProfiledLock({self.__wrapped__!r}) at {self.init_location}>"
259254

260255
def __bool__(self) -> bool:
261256
"""For use in with statements."""

tests/profiling_v2/collector/test_threading.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,7 @@ def test_patch():
9797
lock = threading.Lock
9898
collector = ThreadingLockCollector()
9999
collector.start()
100-
<<<<<<< HEAD
101100
assert lock == collector._original_lock
102-
=======
103-
assert lock == collector._original
104-
>>>>>>> 2577e6d3a6 (Remove 'wrapt' dependency from the Lock Profiler)
105101
# After patching, threading.Lock is replaced with our wrapper
106102
# The old reference (lock) points to the original builtin Lock class
107103
assert lock != threading.Lock # They're different after patching
@@ -110,11 +106,7 @@ def test_patch():
110106
collector.stop()
111107
# After stopping, everything is restored
112108
assert lock == threading.Lock
113-
<<<<<<< HEAD
114109
assert collector._original_lock == threading.Lock
115-
=======
116-
assert collector._original == threading.Lock
117-
>>>>>>> 2577e6d3a6 (Remove 'wrapt' dependency from the Lock Profiler)
118110

119111

120112
@pytest.mark.skipif(not sys.platform.startswith("linux"), reason="only works on linux")

0 commit comments

Comments
 (0)