Skip to content
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

Merged
merged 8 commits into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 53 additions & 25 deletions sentry_sdk/profiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -115,7 +113,7 @@ def _sample_stack(*args, **kwargs):
(
nanosecond_time(),
[
(tid, _extract_stack(frame))
(tid, extract_stack(frame))
for tid, frame in sys._current_frames().items()
],
)
Expand All @@ -126,8 +124,8 @@ def _sample_stack(*args, **kwargs):
MAX_STACK_DEPTH = 128


def _extract_stack(frame):
# type: (Frame) -> Sequence[FrameData]
def extract_stack(frame, max_stack_depth=MAX_STACK_DEPTH):
# type: (Optional[FrameType], int) -> 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
Expand All @@ -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
]
Comment on lines +144 to +151
Copy link
Member Author

@Zylphrex Zylphrex Oct 7, 2022

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

  • stack depth of 100: 4.22s vs 4.55s (a little slower)
  • stack depth of 500: 21.76s vs 8.32 (a lot faster)

Illustration for convenience

Figure_1

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.



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):
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't mind using function/filename/lineno/colno, that'd be great. We renamed them to align with the Frame object in the protocol but still provide compatibility though.

Also, we added colno in case you want/can report the column.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have this in a follow up 👍

}
)
current_stack.append(frames[frame])
Expand Down
95 changes: 95 additions & 0 deletions tests/test_profiler.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import inspect

import pytest

from sentry_sdk.profiler import extract_stack, get_frame_name


def get_frame(depth=1):
"""
This function is not exactly true to its name. Depending on
how it is called, the true depth of the stack can be deeper
than the argument implies.
"""
if depth <= 0:
raise ValueError("only positive integers allowed")
if depth > 1:
return get_frame(depth=depth - 1)
return inspect.currentframe()


class GetFrame:
def instance_method(self):
return inspect.currentframe()

@classmethod
def class_method(cls):
return inspect.currentframe()

@staticmethod
def static_method():
return inspect.currentframe()


@pytest.mark.parametrize(
("frame", "frame_name"),
[
pytest.param(
get_frame(),
"get_frame",
id="function",
),
pytest.param(
(lambda: inspect.currentframe())(),
"<lambda>",
id="lambda",
),
pytest.param(
GetFrame().instance_method(),
"GetFrame.instance_method",
id="instance_method",
),
pytest.param(
GetFrame().class_method(),
"GetFrame.class_method",
id="class_method",
),
pytest.param(
GetFrame().static_method(),
"GetFrame.static_method",
id="static_method",
marks=pytest.mark.skip(reason="unsupported"),
),
],
)
def test_get_frame_name(frame, frame_name):
assert get_frame_name(frame) == frame_name


@pytest.mark.parametrize(
("depth", "max_stack_depth", "actual_depth"),
[
pytest.param(1, 128, 1, id="less than"),
pytest.param(256, 128, 128, id="greater than"),
pytest.param(128, 128, 128, id="equals"),
],
)
def test_extract_stack_with_max_depth(depth, max_stack_depth, actual_depth):
# introduce a lambda that we'll be looking for in the stack
frame = (lambda: get_frame(depth=depth))()

# plus 1 because we introduced a lambda intentionally that we'll
# look for in the final stack to make sure its in the right position
base_stack_depth = len(inspect.stack()) + 1

# increase the max_depth by the `base_stack_depth` to account
# for the extra frames pytest will add
stack = extract_stack(frame, max_stack_depth + base_stack_depth)
assert len(stack) == base_stack_depth + actual_depth

for i in range(actual_depth):
assert stack[i].name == "get_frame", i

# index 0 contains the inner most frame on the stack, so the lamdba
# should be at index `actual_depth`
assert stack[actual_depth].name == "<lambda>", actual_depth