Skip to content

Commit ed0d4db

Browse files
ZylphrexZylphrex
authored
feat(profiling): Extract qualified name for each frame (getsentry#1669)
Currently, we use `code.co_name` for the frame name. This does not include the name of the class if it was a method. This tries to extract the qualified name for each frame where possible. - methods: *typically* have `self` as a positional argument and we can inspect it to extract the class name - class methods: *typically* have `cls` as a positional argument and we can inspect it to extract the class name - static methods: no obvious way of extract the class name
1 parent 3bc8bb8 commit ed0d4db

2 files changed

Lines changed: 146 additions & 29 deletions

File tree

‎sentry_sdk/profiler.py‎

Lines changed: 53 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,33 +16,31 @@
1616
import platform
1717
import random
1818
import signal
19+
import sys
1920
import threading
2021
import time
21-
import sys
2222
import uuid
23-
24-
from collections import deque
23+
from collections import deque, namedtuple
2524
from contextlib import contextmanager
2625

2726
import sentry_sdk
2827
from sentry_sdk._compat import PY33
29-
3028
from sentry_sdk._types import MYPY
3129
from sentry_sdk.utils import nanosecond_time
3230

3331
if MYPY:
32+
from types import FrameType
3433
from typing import Any
3534
from typing import Deque
3635
from typing import Dict
3736
from typing import Generator
3837
from typing import List
3938
from typing import Optional
4039
from typing import Sequence
41-
from typing import Tuple
4240
import sentry_sdk.tracing
4341

44-
Frame = Any
45-
FrameData = Tuple[str, str, int]
42+
43+
FrameData = namedtuple("FrameData", ["name", "file", "line"])
4644

4745

4846
_sample_buffer = None # type: Optional[_SampleBuffer]
@@ -115,7 +113,7 @@ def _sample_stack(*args, **kwargs):
115113
(
116114
nanosecond_time(),
117115
[
118-
(tid, _extract_stack(frame))
116+
(tid, extract_stack(frame))
119117
for tid, frame in sys._current_frames().items()
120118
],
121119
)
@@ -126,8 +124,8 @@ def _sample_stack(*args, **kwargs):
126124
MAX_STACK_DEPTH = 128
127125

128126

129-
def _extract_stack(frame):
130-
# type: (Frame) -> Sequence[FrameData]
127+
def extract_stack(frame, max_stack_depth=MAX_STACK_DEPTH):
128+
# type: (Optional[FrameType], int) -> Sequence[FrameData]
131129
"""
132130
Extracts the stack starting the specified frame. The extracted stack
133131
assumes the specified frame is the top of the stack, and works back
@@ -137,22 +135,52 @@ def _extract_stack(frame):
137135
only the first `MAX_STACK_DEPTH` frames will be returned.
138136
"""
139137

140-
stack = deque(maxlen=MAX_STACK_DEPTH) # type: Deque[FrameData]
138+
stack = deque(maxlen=max_stack_depth) # type: Deque[FrameType]
141139

142140
while frame is not None:
143-
stack.append(
144-
(
145-
# co_name only contains the frame name.
146-
# If the frame was a class method,
147-
# the class name will NOT be included.
148-
frame.f_code.co_name,
149-
frame.f_code.co_filename,
150-
frame.f_code.co_firstlineno,
151-
)
152-
)
141+
stack.append(frame)
153142
frame = frame.f_back
154143

155-
return stack
144+
return [
145+
FrameData(
146+
name=get_frame_name(frame),
147+
file=frame.f_code.co_filename,
148+
line=frame.f_lineno,
149+
)
150+
for frame in stack
151+
]
152+
153+
154+
def get_frame_name(frame):
155+
# type: (FrameType) -> str
156+
157+
# in 3.11+, there is a frame.f_code.co_qualname that
158+
# we should consider using instead where possible
159+
160+
# co_name only contains the frame name. If the frame was a method,
161+
# the class name will NOT be included.
162+
name = frame.f_code.co_name
163+
164+
# if it was a method, we can get the class name by inspecting
165+
# the f_locals for the `self` argument
166+
try:
167+
if "self" in frame.f_locals:
168+
return "{}.{}".format(frame.f_locals["self"].__class__.__name__, name)
169+
except AttributeError:
170+
pass
171+
172+
# if it was a class method, (decorated with `@classmethod`)
173+
# we can get the class name by inspecting the f_locals for the `cls` argument
174+
try:
175+
if "cls" in frame.f_locals:
176+
return "{}.{}".format(frame.f_locals["cls"].__name__, name)
177+
except AttributeError:
178+
pass
179+
180+
# nothing we can do if it is a staticmethod (decorated with @staticmethod)
181+
182+
# we've done all we can, time to give up and return what we have
183+
return name
156184

157185

158186
class Profile(object):
@@ -287,9 +315,9 @@ def slice_profile(self, start_ns, stop_ns):
287315
frames[frame] = len(frames)
288316
frames_list.append(
289317
{
290-
"name": frame[0],
291-
"file": frame[1],
292-
"line": frame[2],
318+
"name": frame.name,
319+
"file": frame.file,
320+
"line": frame.line,
293321
}
294322
)
295323
current_stack.append(frames[frame])

‎tests/test_profiler.py‎

Lines changed: 93 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1+
import inspect
12
import platform
23
import sys
34
import threading
45

56
import pytest
67

7-
from sentry_sdk.profiler import setup_profiler
8+
from sentry_sdk.profiler import extract_stack, get_frame_name, setup_profiler
89

910

1011
minimum_python_33 = pytest.mark.skipif(
@@ -20,7 +21,6 @@
2021
def test_profiler_invalid_mode(teardown_profiling):
2122
with pytest.raises(ValueError):
2223
setup_profiler({"_experiments": {"profiler_mode": "magic"}})
23-
# make sure to clean up at the end of the test
2424

2525

2626
@unix_only
@@ -52,10 +52,99 @@ def join(self, timeout=None):
5252
thread.start()
5353
thread.join()
5454

55-
# make sure to clean up at the end of the test
56-
5755

56+
@unix_only
5857
@pytest.mark.parametrize("mode", ["sleep", "event", "sigprof", "sigalrm"])
5958
def test_profiler_valid_mode(mode, teardown_profiling):
6059
# should not raise any exceptions
6160
setup_profiler({"_experiments": {"profiler_mode": mode}})
61+
62+
63+
def get_frame(depth=1):
64+
"""
65+
This function is not exactly true to its name. Depending on
66+
how it is called, the true depth of the stack can be deeper
67+
than the argument implies.
68+
"""
69+
if depth <= 0:
70+
raise ValueError("only positive integers allowed")
71+
if depth > 1:
72+
return get_frame(depth=depth - 1)
73+
return inspect.currentframe()
74+
75+
76+
class GetFrame:
77+
def instance_method(self):
78+
return inspect.currentframe()
79+
80+
@classmethod
81+
def class_method(cls):
82+
return inspect.currentframe()
83+
84+
@staticmethod
85+
def static_method():
86+
return inspect.currentframe()
87+
88+
89+
@pytest.mark.parametrize(
90+
("frame", "frame_name"),
91+
[
92+
pytest.param(
93+
get_frame(),
94+
"get_frame",
95+
id="function",
96+
),
97+
pytest.param(
98+
(lambda: inspect.currentframe())(),
99+
"<lambda>",
100+
id="lambda",
101+
),
102+
pytest.param(
103+
GetFrame().instance_method(),
104+
"GetFrame.instance_method",
105+
id="instance_method",
106+
),
107+
pytest.param(
108+
GetFrame().class_method(),
109+
"GetFrame.class_method",
110+
id="class_method",
111+
),
112+
pytest.param(
113+
GetFrame().static_method(),
114+
"GetFrame.static_method",
115+
id="static_method",
116+
marks=pytest.mark.skip(reason="unsupported"),
117+
),
118+
],
119+
)
120+
def test_get_frame_name(frame, frame_name):
121+
assert get_frame_name(frame) == frame_name
122+
123+
124+
@pytest.mark.parametrize(
125+
("depth", "max_stack_depth", "actual_depth"),
126+
[
127+
pytest.param(1, 128, 1, id="less than"),
128+
pytest.param(256, 128, 128, id="greater than"),
129+
pytest.param(128, 128, 128, id="equals"),
130+
],
131+
)
132+
def test_extract_stack_with_max_depth(depth, max_stack_depth, actual_depth):
133+
# introduce a lambda that we'll be looking for in the stack
134+
frame = (lambda: get_frame(depth=depth))()
135+
136+
# plus 1 because we introduced a lambda intentionally that we'll
137+
# look for in the final stack to make sure its in the right position
138+
base_stack_depth = len(inspect.stack()) + 1
139+
140+
# increase the max_depth by the `base_stack_depth` to account
141+
# for the extra frames pytest will add
142+
stack = extract_stack(frame, max_stack_depth + base_stack_depth)
143+
assert len(stack) == base_stack_depth + actual_depth
144+
145+
for i in range(actual_depth):
146+
assert stack[i].name == "get_frame", i
147+
148+
# index 0 contains the inner most frame on the stack, so the lamdba
149+
# should be at index `actual_depth`
150+
assert stack[actual_depth].name == "<lambda>", actual_depth

0 commit comments

Comments
 (0)