-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
ref(stacktrace-link): Drop usage of mobile_frame and fix tag #41733
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
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 |
|---|---|---|
|
|
@@ -51,20 +51,18 @@ def get_link( | |
| return result | ||
|
|
||
|
|
||
| # This is to support mobile languages with non-fully-qualified file pathing. | ||
| # We attempt to 'munge' the proper source-relative filepath based on the stackframe data. | ||
| def generate_mobile_frame(parameters: Dict[str, Optional[str]]) -> Dict[str, str]: | ||
| abs_path = parameters.get("absPath") | ||
| module = parameters.get("module") | ||
| package = parameters.get("package") | ||
| frame = {} | ||
| if abs_path: | ||
| frame["abs_path"] = abs_path | ||
| if module: | ||
| frame["module"] = module | ||
| if package: | ||
| frame["package"] = package | ||
| return frame | ||
| def generate_context(parameters: Dict[str, Optional[str]]) -> Dict[str, Optional[str]]: | ||
| return { | ||
| "file": parameters.get("file"), | ||
| # XXX: Temp change to support try_path_munging until refactored | ||
| "filename": parameters.get("file"), | ||
| "commit_id": parameters.get("commitId"), | ||
| "platform": parameters.get("platform"), | ||
| "sdk_name": parameters.get("sdkName"), | ||
| "abs_path": parameters.get("absPath"), | ||
| "module": parameters.get("module"), | ||
| "package": parameters.get("package"), | ||
| } | ||
|
|
||
|
|
||
| def set_top_tags( | ||
|
|
@@ -80,10 +78,10 @@ def set_top_tags( | |
| "organization.early_adopter", bool(project.organization.flags.early_adopter.is_set) | ||
| ) | ||
| scope.set_tag("stacktrace_link.platform", ctx["platform"]) | ||
| scope.set_tag("stacktrace_link.has_code_mappings", has_code_mappings) | ||
| scope.set_tag("stacktrace_link.code_mappings", has_code_mappings) | ||
| if ctx["platform"] == "python": | ||
| # This allows detecting a file that belongs to Python's 3rd party modules | ||
| scope.set_tag("stacktrace_link.in_app", "site-packages" in str(ctx["file"])) | ||
| scope.set_tag("stacktrace_link.in_app", "site-packages" not in str(ctx["file"])) | ||
|
Member
Author
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. This is the bug I mentioned on the title of the PR. |
||
| except Exception: | ||
| # If errors arises we can still proceed | ||
| logger.exception("We failed to set a tag.") | ||
|
|
@@ -92,13 +90,11 @@ def set_top_tags( | |
| def try_path_munging( | ||
| config: RepositoryProjectPathConfig, | ||
| filepath: str, | ||
| mobile_frame: Mapping[str, Optional[str]], | ||
| ctx: Mapping[str, Optional[str]], | ||
|
Member
Author
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. All the contents of |
||
| ) -> Dict[str, str]: | ||
| result: Dict[str, str] = {} | ||
| mobile_frame["filename"] = filepath # type: ignore | ||
|
Member
Author
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. This is now part of |
||
| munged_frames = munged_filename_and_frames( | ||
| str(ctx["platform"]), [mobile_frame], "munged_filename", sdk_name=str(ctx["sdk_name"]) | ||
| str(ctx["platform"]), [ctx], "munged_filename", sdk_name=str(ctx["sdk_name"]) | ||
| ) | ||
| if munged_frames: | ||
| munged_frame: Mapping[str, Mapping[str, str]] = munged_frames[1][0] | ||
|
|
@@ -132,18 +128,11 @@ class ProjectStacktraceLinkEndpoint(ProjectEndpoint): # type: ignore | |
| """ | ||
|
|
||
| def get(self, request: Request, project: Project) -> Response: | ||
| # should probably feature gate | ||
| filepath = request.GET.get("file") | ||
| ctx = generate_context(request.GET) | ||
| filepath = ctx.get("file") | ||
| if not filepath: | ||
| return Response({"detail": "Filepath is required"}, status=400) | ||
|
|
||
| ctx = { | ||
| "file": request.GET.get("file"), | ||
| "commit_id": request.GET.get("commitId"), | ||
| "platform": request.GET.get("platform"), | ||
| "sdk_name": request.GET.get("sdkName"), | ||
| } | ||
| mobile_frame = generate_mobile_frame(request.GET) | ||
|
Member
Author
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. All of this is now returned as part of |
||
| result: JSONData = {"config": None, "sourceUrl": None} | ||
|
|
||
| integrations = Integration.objects.filter(organizations=project.organization_id) | ||
|
|
@@ -162,6 +151,7 @@ def get(self, request: Request, project: Project) -> Response: | |
| configs = RepositoryProjectPathConfig.objects.filter( | ||
| project=project, organization_integration__isnull=False | ||
| ) | ||
| derived = False | ||
| matched_code_mappings = [] | ||
| with configure_scope() as scope: | ||
| set_top_tags(scope, project, ctx, len(configs) > 0) | ||
|
|
@@ -176,13 +166,13 @@ def get(self, request: Request, project: Project) -> Response: | |
| filepath.startswith(config.stack_root) | ||
| and config.automatically_generated is True | ||
| ): | ||
| scope.set_tag("stacktrace_link.automatically_generated", True) | ||
| derived = True | ||
|
|
||
| outcome = get_link(config, filepath, ctx["commit_id"]) | ||
| # In some cases the stack root matches and it can either be that we have | ||
| # an invalid code mapping or that munging is expect it to work | ||
| if not outcome.get("sourceUrl"): | ||
| munging_outcome = try_path_munging(config, filepath, mobile_frame, ctx) | ||
| munging_outcome = try_path_munging(config, filepath, ctx) | ||
| # If we failed to munge we should keep the original outcome | ||
| if munging_outcome: | ||
| outcome = munging_outcome | ||
|
|
@@ -202,7 +192,7 @@ def get(self, request: Request, project: Project) -> Response: | |
| # Post-processing before exiting scope context | ||
| found: bool = result["sourceUrl"] is not None | ||
| scope.set_tag("stacktrace_link.found", found) | ||
|
|
||
| scope.set_tag("stacktrace_link.auto_derived", derived) | ||
|
Member
Author
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. Shortening from |
||
| if matched_code_mappings: | ||
| # Any code mapping that matches and its results will be returned | ||
| result["matched_code_mappings"] = matched_code_mappings | ||
|
|
||
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.
Shortening since there's a limit of 32 chars.