Skip to content

Commit 6541785

Browse files
alexmojakialexmojaki
authored
Order variables by closeness to executing statement in pure_eval (getsentry#807)
Part of getsentry#805
1 parent 55f4645 commit 6541785

File tree

2 files changed

+100
-15
lines changed

2 files changed

+100
-15
lines changed

‎sentry_sdk/integrations/pure_eval.py

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@
22

33
import ast
44

5-
from sentry_sdk import Hub
5+
from sentry_sdk import Hub, serializer
66
from sentry_sdk._types import MYPY
77
from sentry_sdk.integrations import Integration, DidNotEnable
88
from sentry_sdk.scope import add_global_event_processor
99
from sentry_sdk.utils import walk_exception_chain, iter_stacks
1010

1111
if MYPY:
12-
from typing import Optional, Dict, Any
12+
from typing import Optional, Dict, Any, Tuple, List
1313
from types import FrameType
1414

1515
from sentry_sdk._types import Event, Hint
@@ -75,7 +75,9 @@ def add_executing_info(event, hint):
7575
continue
7676

7777
for sentry_frame, tb in zip(sentry_frames, tbs):
78-
sentry_frame["vars"].update(pure_eval_frame(tb.tb_frame))
78+
sentry_frame["vars"] = (
79+
pure_eval_frame(tb.tb_frame) or sentry_frame["vars"]
80+
)
7981
return event
8082

8183

@@ -89,16 +91,42 @@ def pure_eval_frame(frame):
8991
if not statements:
9092
return {}
9193

92-
stmt = list(statements)[0]
94+
scope = stmt = list(statements)[0]
9395
while True:
9496
# Get the parent first in case the original statement is already
9597
# a function definition, e.g. if we're calling a decorator
9698
# In that case we still want the surrounding scope, not that function
97-
stmt = stmt.parent
98-
if isinstance(stmt, (ast.FunctionDef, ast.ClassDef, ast.Module)):
99+
scope = scope.parent
100+
if isinstance(scope, (ast.FunctionDef, ast.ClassDef, ast.Module)):
99101
break
100102

101103
evaluator = pure_eval.Evaluator.from_frame(frame)
102-
expressions = evaluator.interesting_expressions_grouped(stmt)
104+
expressions = evaluator.interesting_expressions_grouped(scope)
105+
106+
def closeness(expression):
107+
# type: (Tuple[List[Any], Any]) -> int
108+
# Prioritise expressions with a node closer to the statement executed
109+
# without being after that statement
110+
# A higher return value is better - the expression will appear
111+
# earlier in the list of values and is less likely to be trimmed
112+
nodes, _value = expression
113+
nodes_before_stmt = [
114+
node for node in nodes if node.first_token.startpos < stmt.last_token.endpos
115+
]
116+
if nodes_before_stmt:
117+
# The position of the last node before or in the statement
118+
return max(node.first_token.startpos for node in nodes_before_stmt)
119+
else:
120+
# The position of the first node after the statement
121+
# Negative means it's always lower priority than nodes that come before
122+
# Less negative means closer to the statement and higher priority
123+
return -min(node.first_token.startpos for node in nodes)
124+
125+
# This adds the first_token and last_token attributes to nodes
103126
atok = source.asttokens()
104-
return {atok.get_text(nodes[0]): value for nodes, value in expressions}
127+
128+
expressions.sort(key=closeness, reverse=True)
129+
return {
130+
atok.get_text(nodes[0]): value
131+
for nodes, value in expressions[: serializer.MAX_DATABAG_BREADTH]
132+
}

‎tests/integrations/pure_eval/test_pure_eval.py

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import sys
2+
from types import SimpleNamespace
3+
14
import pytest
25

3-
from sentry_sdk import capture_exception
6+
from sentry_sdk import capture_exception, serializer
47
from sentry_sdk.integrations.pure_eval import PureEvalIntegration
58

69

@@ -10,8 +13,27 @@ def test_with_locals_enabled(sentry_init, capture_events, integrations):
1013
events = capture_events()
1114

1215
def foo():
13-
foo.d = {1: 2}
14-
print(foo.d[1] / 0)
16+
namespace = SimpleNamespace()
17+
q = 1
18+
w = 2
19+
e = 3
20+
r = 4
21+
t = 5
22+
y = 6
23+
u = 7
24+
i = 8
25+
o = 9
26+
p = 10
27+
a = 11
28+
s = 12
29+
str((q, w, e, r, t, y, u, i, o, p, a, s)) # use variables for linter
30+
namespace.d = {1: 2}
31+
print(namespace.d[1] / 0)
32+
33+
# Appearances of variables after the main statement don't affect order
34+
print(q)
35+
print(s)
36+
print(events)
1537

1638
try:
1739
foo()
@@ -28,8 +50,43 @@ def foo():
2850
frame_vars = event["exception"]["values"][0]["stacktrace"]["frames"][-1]["vars"]
2951

3052
if integrations:
31-
assert sorted(frame_vars.keys()) == ["foo", "foo.d", "foo.d[1]"]
32-
assert frame_vars["foo.d"] == {"1": "2"}
33-
assert frame_vars["foo.d[1]"] == "2"
53+
# Values closest to the exception line appear first
54+
# Test this order if possible given the Python version and dict order
55+
expected_keys = [
56+
"namespace",
57+
"namespace.d",
58+
"namespace.d[1]",
59+
"s",
60+
"a",
61+
"p",
62+
"o",
63+
"i",
64+
"u",
65+
"y",
66+
]
67+
if sys.version_info[:2] == (3, 5):
68+
assert frame_vars.keys() == set(expected_keys)
69+
else:
70+
assert list(frame_vars.keys()) == expected_keys
71+
assert frame_vars["namespace.d"] == {"1": "2"}
72+
assert frame_vars["namespace.d[1]"] == "2"
3473
else:
35-
assert sorted(frame_vars.keys()) == ["foo"]
74+
# Without pure_eval, the variables are unpredictable.
75+
# In later versions, those at the top appear first and are thus included
76+
assert frame_vars.keys() <= {
77+
"namespace",
78+
"q",
79+
"w",
80+
"e",
81+
"r",
82+
"t",
83+
"y",
84+
"u",
85+
"i",
86+
"o",
87+
"p",
88+
"a",
89+
"s",
90+
"events",
91+
}
92+
assert len(frame_vars) == serializer.MAX_DATABAG_BREADTH

0 commit comments

Comments
 (0)