Skip to content

fix(stacktrace-link): Use new Java logic #90415

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 28, 2025
Merged

fix(stacktrace-link): Use new Java logic #90415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/sentry/features/temporary.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ def register_temporary_features(manager: FeatureManager):
manager.add("organizations:ds-org-recalibration", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, api_expose=False)
# Enable custom dynamic sampling rates
manager.add("organizations:dynamic-sampling-custom", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
# Enable new munging logic for java frames
manager.add("organizations:java-frame-munging-new-logic", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
# Enable issue platform deletion
manager.add("organizations:issue-platform-deletion", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
# Enable archive/escalating issue workflow features in v2
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/integrations/utils/stacktrace_link.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from sentry.issues.auto_source_code_config.code_mapping import (
convert_stacktrace_frame_path_to_source_path,
)
from sentry.models.organization import Organization
from sentry.models.repository import Repository
from sentry.shared_integrations.exceptions import ApiError
from sentry.utils.event_frames import EventFrame
Expand Down Expand Up @@ -83,6 +84,7 @@ class StacktraceLinkOutcome(TypedDict):
def get_stacktrace_config(
configs: list[RepositoryProjectPathConfig],
ctx: StacktraceLinkContext,
organization: Organization | None = None,
) -> StacktraceLinkOutcome:
result: StacktraceLinkOutcome = {
"source_url": None,
Expand All @@ -97,6 +99,7 @@ def get_stacktrace_config(
platform=ctx["platform"],
sdk_name=ctx["sdk_name"],
code_mapping=config,
organization=organization,
)
result["src_path"] = src_path
if not src_path:
Expand Down
13 changes: 13 additions & 0 deletions src/sentry/issues/auto_source_code_config/code_mapping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from collections.abc import Mapping, Sequence
from typing import Any, NamedTuple

from sentry import features
from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig
from sentry.integrations.source_code_management.repo_trees import (
RepoAndBranch,
Expand Down Expand Up @@ -386,6 +387,7 @@ def convert_stacktrace_frame_path_to_source_path(
code_mapping: RepositoryProjectPathConfig,
platform: str | None,
sdk_name: str | None,
organization: Organization | None = None,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we are satisfied with the new feature, we won't need to pass the organization.

) -> str | None:
"""
Applies the given code mapping to the given stacktrace frame and returns the source path.
Expand All @@ -403,6 +405,17 @@ def convert_stacktrace_frame_path_to_source_path(
try_munge_frame_path(frame=frame, platform=platform, sdk_name=sdk_name) or frame.filename
)

has_new_logic = (
features.has("organizations:java-frame-munging-new-logic", organization, actor=None)
if organization
else False
)
if has_new_logic and platform == "java" and frame.module and frame.abs_path:
try:
_, stacktrace_path = get_path_from_module(frame.module, frame.abs_path)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_path_from_module is the new logic we're using and closer to what symbolication does:

sentry/src/sentry/issues/auto_source_code_config/code_mapping.py

Lines 578 to 614 in c86f666

# Based on # https://github.com/getsentry/symbolicator/blob/450f1d6a8c346405454505ed9ca87e08a6ff34b7/crates/symbolicator-proguard/src/symbolication.rs#L450-L485
def get_path_from_module(module: str, abs_path: str) -> tuple[str, str]:
"""This attempts to generate a modified module and a real path from a Java module name and filename.
Returns a tuple of (stack_root, source_path).
"""
# An `abs_path` is valid if it contains a `.` and doesn't contain a `$`.
if "$" in abs_path or "." not in abs_path:
# Split the module at the first '$' character and take the part before it
# If there's no '$', use the entire module
file_path = module.split("$", 1)[0] if "$" in module else module
stack_root = module.rsplit(".", 1)[0].replace(".", "/") + "/"
return stack_root, file_path.replace(".", "/")
if "." not in module:
raise DoesNotFollowJavaPackageNamingConvention
# Gets rid of the class name
parts = module.rsplit(".", 1)[0].split(".")
dirpath = "/".join(parts)
# a.Bar, Bar.kt -> stack_root: a/, file_path: a/Bar.kt
granularity = 1
if len(parts) > 1:
# com.example.foo.bar.Baz$InnerClass, Baz.kt ->
# stack_root: com/example/foo/
# file_path: com/example/foo/bar/Baz.kt
granularity = STACK_ROOT_MAX_LEVEL - 1
if parts[1] in SECOND_LEVEL_TLDS:
# uk.co.example.foo.bar.Baz$InnerClass, Baz.kt ->
# stack_root: uk/co/example/foo/
# file_path: uk/co/example/foo/bar/Baz.kt
granularity = STACK_ROOT_MAX_LEVEL
stack_root = "/".join(parts[:granularity]) + "/"
file_path = f"{dirpath}/{abs_path}"
return stack_root, file_path

except Exception:
logger.warning("Investigate. Falling back to old logic.", exc_info=True)

if stacktrace_path and stacktrace_path.startswith(code_mapping.stack_root):
return (
stacktrace_path.replace(stack_root, code_mapping.source_root, 1)
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/issues/endpoints/project_stacktrace_link.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def get(self, request: Request, project: Project) -> Response:
scope = Scope.get_isolation_scope()

set_top_tags(scope, project, ctx, len(configs) > 0)
result = get_stacktrace_config(configs, ctx)
result = get_stacktrace_config(configs, ctx, project.organization)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible for a project to not have an organization?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, small nit — no need for the Organization | None types right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MichaelSun48 there's another for the function in commit_context, thus, made it optional.

I will patch it up in the next PR (I tried to focus the test changes).

error = result["error"]
src_path = result["src_path"]
# Post-processing before exiting scope context
Expand Down
31 changes: 31 additions & 0 deletions tests/sentry/issues/auto_source_code_config/test_code_mapping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
)
from sentry.silo.base import SiloMode
from sentry.testutils.cases import TestCase
from sentry.testutils.helpers import Feature
from sentry.testutils.silo import assume_test_silo_mode
from sentry.utils.event_frames import EventFrame

Expand Down Expand Up @@ -533,6 +534,11 @@ def test_convert_stacktrace_frame_path_to_source_path_java_no_source_context(sel
"MainFragment.java",
"src/com/example/vu/android/empowerplant/MainFragment.java",
),
(
"com.example.foo.BarImpl$invoke$bazFetch$2",
"Bar.kt", # Notice "Impl" is not included in the module above
"src/com/example/foo/BarImpl.kt", # This is incorrect; the new logic will fix this
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will remove this test case once we have switched over to the new logic.

),
]:
assert (
convert_stacktrace_frame_path_to_source_path(
Expand All @@ -544,6 +550,31 @@ def test_convert_stacktrace_frame_path_to_source_path_java_no_source_context(sel
== expected_path
)

def test_convert_stacktrace_frame_path_to_source_path_java_new_logic(self) -> None:
code_mapping = self.create_code_mapping(
organization_integration=self.oi,
project=self.project,
repo=self.repo,
stack_root="com/example/",
source_root="src/com/example/",
automatically_generated=False,
)
with Feature({"organizations:java-frame-munging-new-logic": True}):
assert (
convert_stacktrace_frame_path_to_source_path(
frame=EventFrame(
filename="Baz.java",
abs_path="Baz.java", # Notice "Impl" is not included in the module below
module="com.example.foo.BazImpl$invoke$bazFetch$2",
),
code_mapping=code_mapping,
platform="java",
sdk_name="sentry.java.android",
organization=self.organization,
)
== "src/com/example/foo/Baz.java"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice that Impl is not included in the file path.

)

def test_convert_stacktrace_frame_path_to_source_path_backslashes(self) -> None:
assert (
convert_stacktrace_frame_path_to_source_path(
Expand Down
Loading