-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(auto_source_config): Derive in-app stack trace rules #87842
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
Changes from all commits
4434146
4627f6e
c44bfe6
746441b
ed698e5
23a941a
e91b9b5
f52e04a
0d5a25a
a021092
6592b9b
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 |
---|---|---|
|
@@ -540,8 +540,8 @@ def find_roots(frame_filename: FrameInfo, source_path: str) -> tuple[str, str]: | |
elif source_path.endswith(stack_path): # "Packaged" logic | ||
source_prefix = source_path.rpartition(stack_path)[0] | ||
return ( | ||
f"{stack_root}{frame_filename.stack_root}/", | ||
f"{source_prefix}{frame_filename.stack_root}/", | ||
f"{stack_root}{frame_filename.stack_root}/".replace("//", "/"), | ||
f"{source_prefix}{frame_filename.stack_root}/".replace("//", "/"), | ||
) | ||
elif stack_path.endswith(source_path): | ||
stack_prefix = stack_path.rpartition(source_path)[0] | ||
|
@@ -593,9 +593,17 @@ def get_path_from_module(module: str, abs_path: str) -> tuple[str, str]: | |
if "." not in module: | ||
raise DoesNotFollowJavaPackageNamingConvention | ||
|
||
# If module has a dot, take everything before the last dot | ||
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. These changes simplify this code block. It also fixes a bug where we were not restricting the length of the stack root, thus, we would end up with There's a test down below that ensures we get the right number of levels. Inputs:
Expected:
|
||
# com.example.foo.Bar$InnerClass -> com/example/foo | ||
stack_root = module.rsplit(".", 1)[0].replace(".", "/") | ||
file_path = f"{stack_root}/{abs_path}" | ||
parts = module.split(".") | ||
|
||
if len(parts) > 2: | ||
# com.example.foo.bar.Baz$InnerClass, Baz.kt -> | ||
# stack_root: com/example/ | ||
# file_path: com/example/foo/bar/Baz.kt | ||
stack_root = "/".join(parts[:2]) | ||
file_path = "/".join(parts[:-1]) + "/" + abs_path | ||
else: | ||
# a.Bar, Bar.kt -> stack_root: a/, file_path: a/Bar.kt | ||
stack_root = parts[0] + "/" | ||
file_path = f"{stack_root}{abs_path}" | ||
|
||
return stack_root, file_path |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
import logging | ||
from collections.abc import Sequence | ||
|
||
from sentry.issues.auto_source_code_config.code_mapping import CodeMapping | ||
from sentry.issues.auto_source_code_config.utils import PlatformConfig | ||
from sentry.models.project import Project | ||
from sentry.utils import metrics | ||
|
||
from .constants import DERIVED_ENHANCEMENTS_OPTION_KEY, METRIC_PREFIX | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
def save_in_app_stack_trace_rules( | ||
project: Project, code_mappings: Sequence[CodeMapping], platform_config: PlatformConfig | ||
) -> list[str]: | ||
"""Save in app stack trace rules for a given project""" | ||
rules_from_code_mappings = set() | ||
for code_mapping in code_mappings: | ||
try: | ||
rules_from_code_mappings.add(generate_rule_for_code_mapping(code_mapping)) | ||
except ValueError: | ||
pass | ||
|
||
current_enhancements = project.get_option(DERIVED_ENHANCEMENTS_OPTION_KEY) | ||
current_rules = set(current_enhancements.split("\n")) if current_enhancements else set() | ||
|
||
united_rules = rules_from_code_mappings.union(current_rules) | ||
if not platform_config.is_dry_run_platform() and united_rules != current_rules: | ||
project.update_option(DERIVED_ENHANCEMENTS_OPTION_KEY, "\n".join(sorted(united_rules))) | ||
|
||
new_rules_added = united_rules - current_rules | ||
metrics.incr( | ||
key=f"{METRIC_PREFIX}.in_app_stack_trace_rules.created", | ||
amount=len(new_rules_added), | ||
tags={ | ||
"platform": platform_config.platform, | ||
"dry_run": platform_config.is_dry_run_platform(), | ||
}, | ||
sample_rate=1.0, | ||
) | ||
return list(new_rules_added) | ||
|
||
|
||
# XXX: This is very Java specific. If we want to support other languages, we need to | ||
# come up with a better way to generate the rule. | ||
def generate_rule_for_code_mapping(code_mapping: CodeMapping) -> str: | ||
"""Generate an in-app rule for a given code mapping""" | ||
stacktrace_root = code_mapping.stacktrace_root | ||
if stacktrace_root == "": | ||
raise ValueError("Stacktrace root is empty") | ||
|
||
parts = stacktrace_root.rstrip("/").split("/", 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. Do we need to account for backslash paths too? 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 for Java. |
||
# We only want the first two parts | ||
module = ".".join(parts[:2]) | ||
|
||
if module == "": | ||
raise ValueError("Module is empty") | ||
|
||
# a/ -> a.** | ||
# x/y/ -> x.y.** | ||
# com/example/foo/bar/ -> com.example.** | ||
# uk/co/example/foo/bar/ -> uk.co.** | ||
return f"stack.module:{module}.** +app" |
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.
Just curious - where/how is this enhancement string used?
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.
Primarily, it is used as part of
save_event
to determine which frames should be considered for app or system hash calculations.