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
feat(profiling): Extract qualified name for each frame #1669
Changes from 3 commits
830e5d0
7c259de
2f141c7
2289868
160b484
4308636
a6a1f1b
4f2820b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,33 +16,31 @@ | |
import platform | ||
import random | ||
import signal | ||
import sys | ||
import threading | ||
import time | ||
import sys | ||
import uuid | ||
|
||
from collections import deque | ||
from collections import deque, namedtuple | ||
from contextlib import contextmanager | ||
|
||
import sentry_sdk | ||
from sentry_sdk._compat import PY33 | ||
|
||
from sentry_sdk._types import MYPY | ||
from sentry_sdk.utils import nanosecond_time | ||
|
||
if MYPY: | ||
from types import FrameType | ||
from typing import Any | ||
from typing import Deque | ||
from typing import Dict | ||
from typing import Generator | ||
from typing import List | ||
from typing import Optional | ||
from typing import Sequence | ||
from typing import Tuple | ||
import sentry_sdk.tracing | ||
|
||
Frame = Any | ||
FrameData = Tuple[str, str, int] | ||
|
||
FrameData = namedtuple("FrameData", ["name", "file", "line"]) | ||
|
||
|
||
_sample_buffer = None # type: Optional[_SampleBuffer] | ||
|
@@ -127,7 +125,7 @@ def _sample_stack(*args, **kwargs): | |
|
||
|
||
def _extract_stack(frame): | ||
# type: (Frame) -> Sequence[FrameData] | ||
# type: (Optional[FrameType]) -> Sequence[FrameData] | ||
""" | ||
Extracts the stack starting the specified frame. The extracted stack | ||
assumes the specified frame is the top of the stack, and works back | ||
|
@@ -137,22 +135,52 @@ def _extract_stack(frame): | |
only the first `MAX_STACK_DEPTH` frames will be returned. | ||
""" | ||
|
||
stack = deque(maxlen=MAX_STACK_DEPTH) # type: Deque[FrameData] | ||
stack = deque(maxlen=MAX_STACK_DEPTH) # type: Deque[FrameType] | ||
|
||
while frame is not None: | ||
stack.append( | ||
( | ||
# co_name only contains the frame name. | ||
# If the frame was a class method, | ||
# the class name will NOT be included. | ||
frame.f_code.co_name, | ||
frame.f_code.co_filename, | ||
frame.f_code.co_firstlineno, | ||
) | ||
) | ||
stack.append(frame) | ||
frame = frame.f_back | ||
|
||
return stack | ||
return [ | ||
FrameData( | ||
name=get_frame_name(frame), | ||
file=frame.f_code.co_filename, | ||
line=frame.f_lineno, | ||
) | ||
for frame in stack | ||
] | ||
|
||
|
||
def get_frame_name(frame): | ||
# type: (FrameType) -> str | ||
|
||
# in 3.11+, there is a frame.f_code.co_qualname that | ||
# we should consider using instead where possible | ||
|
||
# co_name only contains the frame name. If the frame was a method, | ||
# the class name will NOT be included. | ||
name = frame.f_code.co_name | ||
|
||
# if it was a method, we can get the class name by inspecting | ||
# the f_locals for the `self` argument | ||
try: | ||
if "self" in frame.f_locals: | ||
return "{}.{}".format(frame.f_locals["self"].__class__.__name__, name) | ||
except AttributeError: | ||
pass | ||
|
||
# if it was a class method, (decorated with `@classmethod`) | ||
# we can get the class name by inspecting the f_locals for the `cls` argument | ||
try: | ||
if "cls" in frame.f_locals: | ||
return "{}.{}".format(frame.f_locals["cls"].__name__, name) | ||
except AttributeError: | ||
pass | ||
|
||
# nothing we can do if it is a staticmethod (decorated with @staticmethod) | ||
|
||
# we've done all we can, time to give up and return what we have | ||
return name | ||
|
||
|
||
class Profile(object): | ||
|
@@ -289,9 +317,9 @@ def slice_profile(self, start_ns, stop_ns): | |
frames[frame] = len(frames) | ||
frames_list.append( | ||
{ | ||
"name": frame[0], | ||
"file": frame[1], | ||
"line": frame[2], | ||
"name": frame.name, | ||
"file": frame.file, | ||
"line": frame.line, | ||
Comment on lines
+318
to
+320
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you don't mind using Also, we added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have this in a follow up 👍 |
||
} | ||
) | ||
current_stack.append(frames[frame]) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of extracting the data for every frame while we step through the stack, we step through the stack first and only extract the data from the frames within the
MAX_STACK_DEPTH
.Some quick benchmarks on 100,000 iterations
Illustration for convenience
Since the slowest part of this is the
get_frame_name
function and I figured it was worth the trade off here to optimize for the worst case and be a little slower on the common cases.