feat(profiling): Extract qualified name for each frame (#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

sentry_sdk/profiler.py

@@ -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]
@@ -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()
             ],
         )
@@ -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
@@ -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):
@@ -287,9 +315,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,
                             }
                         )
                     current_stack.append(frames[frame])

tests/test_profiler.py

@@ -1,10 +1,11 @@
+import inspect
 import platform
 import sys
 import threading
 
 import pytest
 
-from sentry_sdk.profiler import setup_profiler
+from sentry_sdk.profiler import extract_stack, get_frame_name, setup_profiler
 
 
 minimum_python_33 = pytest.mark.skipif(
@@ -20,7 +21,6 @@
 def test_profiler_invalid_mode(teardown_profiling):
     with pytest.raises(ValueError):
         setup_profiler({"_experiments": {"profiler_mode": "magic"}})
-    # make sure to clean up at the end of the test
 
 
 @unix_only
@@ -52,10 +52,99 @@ def join(self, timeout=None):
         thread.start()
         thread.join()
 
-    # make sure to clean up at the end of the test
-
 
+@unix_only
 @pytest.mark.parametrize("mode", ["sleep", "event", "sigprof", "sigalrm"])
 def test_profiler_valid_mode(mode, teardown_profiling):
     # should not raise any exceptions
     setup_profiler({"_experiments": {"profiler_mode": mode}})
+
+
+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