feat(derived_code_mappings): Support Java #86280

armenzg · 2025-03-04T13:56:33Z

It will run in dry run mode until it's ready.

For now, it will run as dry run until we're ready to enable it for a few organizations.

… section When doing local development I noticed some funkiness and it was due to this.

armenzg · 2025-03-04T13:57:25Z

src/sentry/integrations/source_code_management/repo_trees.py

@@ -15,17 +15,24 @@
 # We only care about extensions of files which would show up in stacktraces after symbolication
 SUPPORTED_EXTENSIONS = [
    "clj",
+    "cljc",


@AbhiPrasad @lcian does it look good to you?

Yep.
I've found that for Groovy there's also a few more possible extensions: .gvy, .gy, .gsh even though this is not very important considering the usage of groovy etc.
Sorce: https://blog.mrhaki.com/2011/10/groovy-goodness-default-groovy-script.html

we should split up this constant to categorize by language, makes it a bit easier to review.

I intend to refactor this.

armenzg · 2025-03-04T13:58:59Z

src/sentry/issues/auto_source_code_config/code_mapping.py

@@ -214,8 +238,12 @@ def _stacktrace_buckets(
                buckets[frame_filename.stack_root].append(frame_filename)
            except UnsupportedFrameInfo:
                logger.warning("Frame's filepath not supported: %s", frame.get("filename"))
+            except MissingModuleOrAbsPath:
+                logger.warning("Do not panic. I'm collecting this data.")


Once I review the data I will remove reporting it to Sentry.

armenzg · 2025-03-04T14:01:35Z

src/sentry/issues/auto_source_code_config/code_mapping.py

@@ -507,8 +535,10 @@ def find_roots(frame_filename: FrameInfo, source_path: str) -> tuple[str, str]:
        return (stack_root, "")
    elif source_path.endswith(stack_path):  # "Packaged" logic
        source_prefix = source_path.rpartition(stack_path)[0]
-        package_dir = stack_path.split("/")[0]


In Python, it has always been a single word representing the package name (e.g. sentry) while in Java it is multiple words (e.g. io.sentry.some_app_name), thus, we need to change it.

codecov · 2025-03-04T14:25:50Z

Codecov Report

Attention: Patch coverage is 94.52055% with 4 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...try/issues/auto_source_code_config/code_mapping.py	86.66%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #86280    +/-   ##
========================================
  Coverage   87.88%   87.89%            
========================================
  Files        9721     9723     +2     
  Lines      551119   551254   +135     
  Branches    21478    21478            
========================================
+ Hits       484372   484503   +131     
- Misses      66394    66398     +4     
  Partials      353      353

adinauer

Left some comments about differences to the source context implementation (https://github.com/getsentry/symbolicator/blob/450f1d6a8c346405454505ed9ca87e08a6ff34b7/crates/symbolicator-proguard/src/symbolication.rs#L450-L485).

Doesn't mean they have to align 100% but some of them do make sense IMO.

Could we simply err on the side of trying too many strings that could work and worst case not find anything in the repo vs. not trying for something that could be found?

src/sentry/issues/auto_source_code_config/code_mapping.py

adinauer · 2025-03-04T14:11:49Z

src/sentry/issues/auto_source_code_config/code_mapping.py

+    extension = parts[1]
+
+    parts = module.split(".")
+    if len(parts) <= 1:


For source context afair we simply don't use module if it does not contain a .

adinauer · 2025-03-04T14:15:16Z

src/sentry/issues/auto_source_code_config/code_mapping.py

+    temp_path = None
+    # Find the first uppercase letter after a period to identify class name
+    for i, part in enumerate(parts):
+        if part and part[0].isupper():


For source context we simply use everything before the last . instead of looking for upper case letters. You can also use lowercase letters for classes - not sure how common that is tho. You can also use upper case letters for package names.

AbhiPrasad

✅ on the newly added extensions on my side!

AbhiPrasad · 2025-03-04T16:36:12Z

src/sentry/integrations/source_code_management/repo_trees.py

@@ -15,17 +15,24 @@
 # We only care about extensions of files which would show up in stacktraces after symbolication
 SUPPORTED_EXTENSIONS = [
    "clj",
+    "cljc",


we should split up this constant to categorize by language, makes it a bit easier to review.

src/sentry/issues/auto_source_code_config/utils.py

src/sentry/options/defaults.py

MichaelSun48 · 2025-03-04T19:20:18Z

src/sentry/issues/auto_source_code_config/code_mapping.py

+            f"{stack_root}{frame_filename.stack_root}/",
+            f"{source_prefix}{frame_filename.stack_root}/",
+        )


Any change to find_roots makes me nervous since this function effectively calculates the code mapping and was very sensitive to small changes, at least in my experience.

If it passes the tests, I think we should be fine, but I would keep a very close eye on related metrics after this PR lands.

I have been making sure the tests are sound. It has not required any changes to the Python packages.

Great! Apologies if I'm being a little overly paranoid 😅

adinauer

Mostly LGTM.

I'm uncertain how you're handling file extensions and the lack thereof.

If abs_path is used we trust the extension that is there.

But if only module is used, I would assume there's no file extension. Not sure how well that works with the rest of the code here. Just leaving this for you to decide.

adinauer · 2025-03-05T10:40:49Z

src/sentry/issues/auto_source_code_config/code_mapping.py

+        raise DoesNotFollowJavaPackageNamingConvention
+
+    # If module has a dot, take everything before the last dot
+    # com.example.foo.Bar$InnerClass -> com/example/foo/


Looks like the code below doesn't have a trailing /. Maybe we should update the comment to avoid confusion?

Good call; thanks!

adinauer · 2025-03-05T10:41:39Z

src/sentry/issues/auto_source_code_config/code_mapping.py

+    # If module has a dot, take everything before the last dot
+    # com.example.foo.Bar$InnerClass -> com/example/foo/
+    stack_root = module.rsplit(".", 1)[0].replace(".", "/")
+    file_path = f"{stack_root}/{abs_path}"


In symbolicator we use only the part of abs_path before the last .. But there we always append .jvm as fake file extension. I'm not entirely sure what works best here.

It would not work for us either.

I made up these two cases which are not going to help in deriving code mappings. However, we have many other cases that would help, thus, it's okay if these don't work.

pytest.param( {"module": "foo.bar.Baz", "abs_path": "no_extension"}, "foo/bar", "foo/bar/Baz", # The path does not use the abs_path id="invalid_abs_path_no_extension", ), pytest.param( {"module": "foo.bar.Baz", "abs_path": "foo$bar"}, "foo/bar", "foo/bar/Baz", # The path does not use the abs_path id="invalid_abs_path_dollar_sign", ),

It will run in dry run mode until it's ready.

armenzg added 18 commits March 3, 2025 08:09

feat(derived_code_mappings): Support Java

796467c

For now, it will run as dry run until we're ready to enable it for a few organizations.

Add option

8128f38

tests(derived_code_mappings): Do not create object in parametrization…

a52edee

… section When doing local development I noticed some funkiness and it was due to this.

Typing and another

7306adb

Minor refactor

06b4756

Correctly read dry run platforms from option

2c8d6a6

Update test

92fead9

Update extensions

06af5f1

Fix dry run test

1ad50f0

Fix typing

2c0f4ae

Test case

cf4b3ad

Merge branch 'master' into feat/java-support/auto_source/armenzg

74457f5

Fix test

78b2829

Handle $ sign

038c67d

Update test cases

60023e9

Another test case

9f2020f

Merge branch 'master' into feat/java-support/auto_source/armenzg

09708e9

Clean up code & tests

7246cbd

armenzg self-assigned this Mar 4, 2025

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 4, 2025

armenzg commented Mar 4, 2025

View reviewed changes

vercel bot deployed to Preview March 4, 2025 13:59 View deployment

armenzg commented Mar 4, 2025

View reviewed changes

armenzg marked this pull request as ready for review March 4, 2025 14:03

armenzg requested review from a team as code owners March 4, 2025 14:03

adinauer reviewed Mar 4, 2025

View reviewed changes

Adjust code and tests

f5f0352

vercel bot deployed to Preview March 4, 2025 15:35 View deployment

armenzg added 3 commits March 4, 2025 11:13

Fix invalid abs_path cases

e642e72

Fix tests

df769cf

Simplify logic

7e0de9c

vercel bot deployed to Preview March 4, 2025 16:29 View deployment

AbhiPrasad reviewed Mar 4, 2025

View reviewed changes

MichaelSun48 reviewed Mar 4, 2025

View reviewed changes

src/sentry/issues/auto_source_code_config/utils.py Show resolved Hide resolved

MichaelSun48 reviewed Mar 4, 2025

View reviewed changes

src/sentry/options/defaults.py Show resolved Hide resolved

armenzg requested a review from MichaelSun48 March 4, 2025 18:38

MichaelSun48 approved these changes Mar 4, 2025

View reviewed changes

adinauer approved these changes Mar 5, 2025

View reviewed changes

adinauer mentioned this pull request Mar 5, 2025

fix(code_mappings): Do not append extension #86283

Merged

armenzg merged commit b00b520 into master Mar 5, 2025
49 checks passed

armenzg deleted the feat/java-support/auto_source/armenzg branch March 5, 2025 12:32

philipphofmann pushed a commit that referenced this pull request Mar 6, 2025

feat(derived_code_mappings): Support Java (#86280)

1d672a1

It will run in dry run mode until it's ready.

armenzg added this to the Automate Code Mappings & In-App Stack Trace Rules for Java projects milestone Mar 6, 2025

github-actions bot locked and limited conversation to collaborators Mar 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(derived_code_mappings): Support Java #86280

feat(derived_code_mappings): Support Java #86280

armenzg commented Mar 4, 2025

armenzg Mar 4, 2025

lcian Mar 4, 2025

AbhiPrasad Mar 4, 2025

armenzg Mar 4, 2025

armenzg Mar 4, 2025

armenzg Mar 4, 2025

codecov bot commented Mar 4, 2025 •

edited

Loading

adinauer left a comment

adinauer Mar 4, 2025

adinauer Mar 4, 2025

AbhiPrasad left a comment

AbhiPrasad Mar 4, 2025

MichaelSun48 Mar 4, 2025 •

edited

Loading

armenzg Mar 4, 2025

MichaelSun48 Mar 4, 2025

adinauer left a comment

adinauer Mar 5, 2025

armenzg Mar 5, 2025

adinauer Mar 5, 2025

armenzg Mar 5, 2025

feat(derived_code_mappings): Support Java #86280

feat(derived_code_mappings): Support Java #86280

Conversation

armenzg commented Mar 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 4, 2025 • edited Loading

Codecov Report

adinauer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AbhiPrasad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelSun48 Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adinauer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 4, 2025 •

edited

Loading

MichaelSun48 Mar 4, 2025 •

edited

Loading