Skip to content

Commit 1707d91

Browse files
armenzgameliahsu
authored andcommitted
ref(derived_code_mappings): Pass frame filename to find_roots (#85931)
This change is in preparation for a follow-up change.
1 parent 53a1c54 commit 1707d91

File tree

3 files changed

+69
-59
lines changed

3 files changed

+69
-59
lines changed

src/sentry/api/endpoints/project_repo_path_parsing.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from sentry.integrations.base import IntegrationFeatures
1313
from sentry.integrations.manager import default_manager as integrations
1414
from sentry.integrations.services.integration import RpcIntegration, integration_service
15-
from sentry.issues.auto_source_code_config.code_mapping import find_roots
15+
from sentry.issues.auto_source_code_config.code_mapping import FrameFilename, find_roots
1616
from sentry.models.repository import Repository
1717

1818

@@ -119,7 +119,7 @@ def post(self, request: Request, project) -> Response:
119119

120120
branch = installation.extract_branch_from_source_url(repo, source_url)
121121
source_path = installation.extract_source_path_from_source_url(repo, source_url)
122-
stack_root, source_root = find_roots(stack_path, source_path)
122+
stack_root, source_root = find_roots(FrameFilename({"filename": stack_path}), source_path)
123123

124124
return self.respond(
125125
{

src/sentry/issues/auto_source_code_config/code_mapping.py

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ class UnsupportedFrameFilename(Exception):
4747
pass
4848

4949

50+
class NeedsExtension(Exception):
51+
pass
52+
53+
5054
def derive_code_mappings(
5155
organization: Organization,
5256
frame: Mapping[str, Any],
@@ -86,7 +90,7 @@ def __init__(self, frame: Mapping[str, Any]) -> None:
8690
self.full_path = frame_file_path
8791
self.extension = get_extension(frame_file_path)
8892
if not self.extension:
89-
raise UnsupportedFrameFilename("It needs an extension.")
93+
raise NeedsExtension("It needs an extension.")
9094

9195
# Remove drive letter if it exists
9296
if is_windows_path and frame_file_path[1] == ":":
@@ -158,25 +162,19 @@ def list_file_matches(self, frame_filename: FrameFilename) -> list[dict[str, str
158162
for file in matches:
159163
stack_path = frame_filename.raw_path
160164
source_path = file
165+
extra = {"stack_path": stack_path, "source_path": source_path}
161166

162167
try:
163-
stack_root, source_root = find_roots(stack_path, source_path)
168+
stack_root, source_root = find_roots(frame_filename, source_path)
164169
except UnexpectedPathException:
165-
logger.info(
166-
"Unexpected format for stack_path or source_path",
167-
extra={"stack_path": stack_path, "source_path": source_path},
168-
)
170+
logger.warning("Unexpected format for stack_path or source_path", extra=extra)
169171
continue
170172

173+
extra.update({"stack_root": stack_root, "source_root": source_root})
171174
if stack_path.replace(stack_root, source_root, 1).replace("\\", "/") != source_path:
172-
logger.info(
175+
logger.warning(
173176
"Unexpected stack_path/source_path found. A code mapping was not generated.",
174-
extra={
175-
"stack_path": stack_path,
176-
"source_path": source_path,
177-
"stack_root": stack_root,
178-
"source_root": source_root,
179-
},
177+
extra=extra,
180178
)
181179
else:
182180
file_matches.append(
@@ -201,7 +199,7 @@ def _stacktrace_buckets(
201199
# Any files without a top directory will be grouped together
202200
buckets[frame_filename.root].append(frame_filename)
203201
except UnsupportedFrameFilename:
204-
logger.info("Frame's filepath not supported: %s", frame.get("filename"))
202+
logger.warning("Frame's filepath not supported: %s", frame.get("filename"))
205203
except Exception:
206204
logger.exception("Unable to split stacktrace path into buckets")
207205

@@ -270,24 +268,18 @@ def _generate_code_mapping_from_tree(
270268
stack_path = frame_filename.raw_path
271269
source_path = matched_files[0]
272270

271+
extra = {"stack_path": stack_path, "source_path": source_path}
273272
try:
274-
stack_root, source_root = find_roots(stack_path, source_path)
273+
stack_root, source_root = find_roots(frame_filename, source_path)
275274
except UnexpectedPathException:
276-
logger.info(
277-
"Unexpected format for stack_path or source_path",
278-
extra={"stack_path": stack_path, "source_path": source_path},
279-
)
275+
logger.warning("Unexpected format for stack_path or source_path", extra=extra)
280276
return []
281277

278+
extra.update({"stack_root": stack_root, "source_root": source_root})
282279
if stack_path.replace(stack_root, source_root, 1).replace("\\", "/") != source_path:
283-
logger.info(
280+
logger.warning(
284281
"Unexpected stack_path/source_path found. A code mapping was not generated.",
285-
extra={
286-
"stack_path": stack_path,
287-
"source_path": source_path,
288-
"stack_root": stack_root,
289-
"source_root": source_root,
290-
},
282+
extra=extra,
291283
)
292284
return []
293285

@@ -484,11 +476,12 @@ def get_straight_path_prefix_end_index(file_path: str) -> int:
484476
return index
485477

486478

487-
def find_roots(stack_path: str, source_path: str) -> tuple[str, str]:
479+
def find_roots(frame_filename: FrameFilename, source_path: str) -> tuple[str, str]:
488480
"""
489481
Returns a tuple containing the stack_root, and the source_root.
490482
If there is no overlap, raise an exception since this should not happen
491483
"""
484+
stack_path = frame_filename.raw_path
492485
stack_root = ""
493486
if stack_path[0] == "/" or stack_path[0] == "\\":
494487
stack_root += stack_path[0]

tests/sentry/issues/auto_source_code_config/test_code_mapping.py

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
CodeMapping,
1616
CodeMappingTreesHelper,
1717
FrameFilename,
18+
NeedsExtension,
1819
UnexpectedPathException,
1920
UnsupportedFrameFilename,
2021
convert_stacktrace_frame_path_to_source_path,
@@ -26,7 +27,7 @@
2627
from sentry.testutils.silo import assume_test_silo_mode
2728
from sentry.utils.event_frames import EventFrame
2829

29-
sentry_files = [
30+
SENTRY_FILES = [
3031
"bin/__init__.py",
3132
"bin/example1.py",
3233
"bin/example2.py",
@@ -37,8 +38,6 @@
3738
"src/sentry/wsgi.py",
3839
"src/sentry_plugins/slack/client.py",
3940
]
40-
41-
4241
UNSUPPORTED_FRAME_FILENAMES = [
4342
"async https://s1.sentry-cdn.com/_static/dist/sentry/entrypoints/app.js",
4443
"/gtm.js", # Top source; starts with backslash
@@ -47,20 +46,22 @@
4746
"[native code]",
4847
"O$t",
4948
"async https://s1.sentry-cdn.com/_static/dist/sentry/entrypoints/app.js",
50-
"/foo/bar/baz", # no extension
51-
"README", # no extension
49+
"README", # top level file
5250
"ssl.py",
5351
# XXX: The following will need to be supported
5452
"initialization.dart",
5553
"backburner.js",
5654
]
55+
NO_EXTENSION_FRAME_FILENAMES = [
56+
"/foo/bar/baz", # no extension
57+
]
5758

5859

5960
class TestRepoFiles(TestCase):
6061
"""These evaluate which files should be included as part of a repo."""
6162

6263
def test_filter_source_code_files(self) -> None:
63-
source_code_files = filter_source_code_files(sentry_files)
64+
source_code_files = filter_source_code_files(SENTRY_FILES)
6465

6566
assert source_code_files.index("bin/__init__.py") == 0
6667
assert source_code_files.index("docs-ui/.eslintrc.js") == 3
@@ -109,19 +110,6 @@ def test_buckets_logic() -> None:
109110

110111

111112
class TestFrameFilename:
112-
def test_frame_filename_package_and_more_than_one_level(self) -> None:
113-
pytest.skip("This test is outdated because of refactors have been made to code mappings")
114-
# ff = FrameFilename("getsentry/billing/tax/manager.py")
115-
# assert f"{ff.root}/{ff.dir_path}/{ff.file_name}" == "getsentry/billing/tax/manager.py"
116-
# assert f"{ff.dir_path}/{ff.file_name}" == ff.file_and_dir_path
117-
118-
def test_frame_filename_package_and_no_levels(self) -> None:
119-
pytest.skip("This test is outdated because of refactors have been made to code mappings")
120-
# ff = FrameFilename("root/bar.py")
121-
# assert f"{ff.root}/{ff.file_name}" == "root/bar.py"
122-
# assert f"{ff.root}/{ff.file_and_dir_path}" == "root/bar.py"
123-
# assert ff.dir_path == ""
124-
125113
def test_frame_filename_repr(self) -> None:
126114
path = "getsentry/billing/tax/manager.py"
127115
assert FrameFilename({"filename": path}).__repr__() == f"FrameFilename: {path}"
@@ -131,6 +119,11 @@ def test_raises_unsupported(self) -> None:
131119
with pytest.raises(UnsupportedFrameFilename):
132120
FrameFilename({"filename": filepath})
133121

122+
def test_raises_no_extension(self) -> None:
123+
for filepath in NO_EXTENSION_FRAME_FILENAMES:
124+
with pytest.raises(NeedsExtension):
125+
FrameFilename({"filename": filepath})
126+
134127
@pytest.mark.parametrize(
135128
"frame_filename, prefix",
136129
[
@@ -167,7 +160,7 @@ def setUp(self) -> None:
167160
self.bar_repo = RepoAndBranch("Test-Organization/bar", "main")
168161
self.code_mapping_helper = CodeMappingTreesHelper(
169162
{
170-
self.foo_repo.name: RepoTree(self.foo_repo, files=sentry_files),
163+
self.foo_repo.name: RepoTree(self.foo_repo, files=SENTRY_FILES),
171164
self.bar_repo.name: RepoTree(self.bar_repo, files=["sentry/web/urls.py"]),
172165
}
173166
)
@@ -303,52 +296,76 @@ def test_list_file_matches_multiple(self) -> None:
303296
assert matches == expected_matches
304297

305298
def test_find_roots_starts_with_period_slash(self) -> None:
306-
stacktrace_root, source_path = find_roots("./app/", "static/app/")
299+
stacktrace_root, source_path = find_roots(
300+
FrameFilename({"filename": "./app/foo.tsx"}), "static/app/foo.tsx"
301+
)
307302
assert stacktrace_root == "./"
308303
assert source_path == "static/"
309304

310305
def test_find_roots_starts_with_period_slash_no_containing_directory(self) -> None:
311-
stacktrace_root, source_path = find_roots("./app/", "app/")
306+
stacktrace_root, source_path = find_roots(
307+
FrameFilename({"filename": "./app/foo.tsx"}), "app/foo.tsx"
308+
)
312309
assert stacktrace_root == "./"
313310
assert source_path == ""
314311

315312
def test_find_roots_not_matching(self) -> None:
316-
stacktrace_root, source_path = find_roots("sentry/", "src/sentry/")
313+
stacktrace_root, source_path = find_roots(
314+
FrameFilename({"filename": "sentry/foo.py"}), "src/sentry/foo.py"
315+
)
317316
assert stacktrace_root == "sentry/"
318317
assert source_path == "src/sentry/"
319318

320319
def test_find_roots_equal(self) -> None:
321-
stacktrace_root, source_path = find_roots("source/", "source/")
320+
stacktrace_root, source_path = find_roots(
321+
FrameFilename({"filename": "source/foo.py"}), "source/foo.py"
322+
)
322323
assert stacktrace_root == ""
323324
assert source_path == ""
324325

325326
def test_find_roots_starts_with_period_slash_two_levels(self) -> None:
326-
stacktrace_root, source_path = find_roots("./app/", "app/foo/app/")
327+
stacktrace_root, source_path = find_roots(
328+
FrameFilename({"filename": "./app/foo.tsx"}), "app/foo/app/foo.tsx"
329+
)
327330
assert stacktrace_root == "./"
328331
assert source_path == "app/foo/"
329332

330333
def test_find_roots_starts_with_app(self) -> None:
331-
stacktrace_root, source_path = find_roots("app:///utils/", "utils/")
334+
stacktrace_root, source_path = find_roots(
335+
FrameFilename({"filename": "app:///utils/foo.tsx"}), "utils/foo.tsx"
336+
)
332337
assert stacktrace_root == "app:///"
333338
assert source_path == ""
334339

335340
def test_find_roots_starts_with_multiple_dot_dot_slash(self) -> None:
336-
stacktrace_root, source_path = find_roots("../../../../../../packages/", "packages/")
341+
stacktrace_root, source_path = find_roots(
342+
FrameFilename({"filename": "../../../../../../packages/foo.tsx"}),
343+
"packages/foo.tsx",
344+
)
337345
assert stacktrace_root == "../../../../../../"
338346
assert source_path == ""
339347

340348
def test_find_roots_starts_with_app_dot_dot_slash(self) -> None:
341-
stacktrace_root, source_path = find_roots("app:///../services/", "services/")
349+
stacktrace_root, source_path = find_roots(
350+
FrameFilename({"filename": "app:///../services/foo.tsx"}),
351+
"services/foo.tsx",
352+
)
342353
assert stacktrace_root == "app:///../"
343354
assert source_path == ""
344355

345356
def test_find_roots_bad_stack_path(self) -> None:
346357
with pytest.raises(UnexpectedPathException):
347-
find_roots("https://yrurlsinyourstackpath.com/", "sentry/something.py")
358+
find_roots(
359+
FrameFilename({"filename": "https://yrurlsinyourstackpath.com/"}),
360+
"sentry/something.py",
361+
)
348362

349363
def test_find_roots_bad_source_path(self) -> None:
350364
with pytest.raises(UnexpectedPathException):
351-
find_roots("sentry/random.py", "nothing/something.js")
365+
find_roots(
366+
FrameFilename({"filename": "sentry/random.py"}),
367+
"nothing/something.js",
368+
)
352369

353370

354371
class TestConvertStacktraceFramePathToSourcePath(TestCase):

0 commit comments

Comments
 (0)