-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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: Implement instruction_addr_adjustment #42533
Changes from 9 commits
3a5d729
1ead86b
f376786
a8f0acc
bc7b217
77eb49e
29f0db3
db956b4
3bfd147
18a892b
ab7d86b
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 |
---|---|---|
@@ -1,5 +1,6 @@ | ||
from __future__ import annotations | ||
|
||
from copy import deepcopy | ||
from datetime import datetime | ||
from time import sleep, time | ||
from typing import Any, List, Mapping, MutableMapping, Optional, Tuple | ||
|
@@ -225,18 +226,41 @@ def _normalize(profile: Profile, organization: Organization) -> None: | |
def _prepare_frames_from_profile(profile: Profile) -> Tuple[List[Any], List[Any]]: | ||
modules = profile["debug_meta"]["images"] | ||
|
||
# NOTE: the usage of `adjust_instruction_addr` assumes that all | ||
# the profilers on all the platforms are walking stacks right from a | ||
# suspended threads cpu context | ||
|
||
# in the sample format, we have a frames key containing all the frames | ||
if "version" in profile: | ||
stacktraces = [{"registers": {}, "frames": profile["profile"]["frames"]}] | ||
frames = profile["profile"]["frames"] | ||
|
||
for stack in profile["profile"]["stacks"]: | ||
if len(stack) > 0: | ||
# Make a deep copy of the leaf frame with adjust_instruction_addr = False | ||
# and append it to the list. This ensures correct behavior | ||
# if the leaf frame also shows up in the middle of another stack. | ||
first_frame_idx = stack[0] | ||
frame = deepcopy(frames[first_frame_idx]) | ||
frame["adjust_instruction_addr"] = False | ||
frames.append(frame) | ||
stack[0] = len(frames) - 1 | ||
|
||
stacktraces = [{"registers": {}, "frames": [dict(frame) for frame in frames]}] | ||
# in the original format, we need to gather frames from all samples | ||
else: | ||
stacktraces = [ | ||
{ | ||
"registers": {}, | ||
"frames": s["frames"], | ||
} | ||
for s in profile["sampled_profile"]["samples"] | ||
] | ||
stacktraces = [] | ||
for s in profile["sampled_profile"]["samples"]: | ||
frames = [dict(frame) for frame in s["frames"]] | ||
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. Not sure why you're casting as a |
||
|
||
if len(frames) > 0: | ||
frames[0]["adjust_instruction_addr"] = False | ||
|
||
stacktraces.append( | ||
{ | ||
"registers": {}, | ||
"frames": frames, | ||
} | ||
) | ||
return (modules, stacktraces) | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
from copy import copy | ||
from datetime import datetime | ||
from unittest.mock import Mock, patch | ||
|
||
|
@@ -6,6 +7,7 @@ | |
from arroyo.types import BrokerValue, Message, Partition, Topic | ||
|
||
from sentry.profiles.consumers.process.factory import ProcessProfileStrategyFactory | ||
from sentry.profiles.task import _prepare_frames_from_profile | ||
from sentry.testutils.cases import TestCase | ||
from sentry.utils import json | ||
|
||
|
@@ -56,3 +58,51 @@ def test_basic_profile_to_celery(self, process_profile_task): | |
) | ||
|
||
process_profile_task.assert_called_with(profile=profile) | ||
|
||
|
||
def test_adjust_instruction_addr_sample_format(): | ||
original_frames = [ | ||
{"instruction_addr": "0xdeadbeef"}, | ||
{"instruction_addr": "0xbeefdead"}, | ||
{"instruction_addr": "0xfeedface"}, | ||
] | ||
profile = { | ||
"version": "1", | ||
"profile": { | ||
"frames": copy(original_frames), | ||
"stacks": [[1, 0], [0, 1, 2]], | ||
}, | ||
"debug_meta": {"images": []}, | ||
} | ||
|
||
_, stacktraces = _prepare_frames_from_profile(profile) | ||
assert profile["profile"]["stacks"] == [[3, 0], [4, 1, 2]] | ||
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. You are correctly asserting that |
||
frames = stacktraces[0]["frames"] | ||
|
||
for i in range(3): | ||
assert frames[i] == original_frames[i] | ||
|
||
assert frames[3] == {"instruction_addr": "0xbeefdead", "adjust_instruction_addr": False} | ||
assert frames[4] == {"instruction_addr": "0xdeadbeef", "adjust_instruction_addr": False} | ||
|
||
|
||
def test_adjust_instruction_addr_original_format(): | ||
profile = { | ||
"sampled_profile": { | ||
"samples": [ | ||
{ | ||
"frames": [ | ||
{"instruction_addr": "0xdeadbeef", "platform": "native"}, | ||
{"instruction_addr": "0xbeefdead", "platform": "native"}, | ||
], | ||
} | ||
] | ||
}, | ||
"debug_meta": {"images": []}, | ||
} | ||
|
||
_, stacktraces = _prepare_frames_from_profile(profile) | ||
frames = stacktraces[0]["frames"] | ||
|
||
assert not frames[0]["adjust_instruction_addr"] | ||
assert "adjust_instruction_addr" not in frames[1] |
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.
Why not transform the frame to a
dict
in theappend
above so you don't have to loop once more?