-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
feat(derive_code_mapping): Support dry run mode #85065
Conversation
The dry run mode will allow testing the load of adding a new platform without creating the code mappings. It will also help collect the metrics and discover any errors.
@@ -13,7 +13,22 @@ | |||
|
|||
|
|||
# We only care about extensions of files which would show up in stacktraces after symbolication | |||
SUPPORTED_EXTENSIONS = ["js", "jsx", "tsx", "ts", "mjs", "py", "rb", "rake", "php", "go", "cs"] | |||
SUPPORTED_EXTENSIONS = [ |
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.
I'm not making any changes besides sorting it.
@@ -1,3 +1,13 @@ | |||
SUPPORTED_INTEGRATIONS = ["github"] | |||
# Any new languages should also require updating the stacktraceLink.tsx and repo_trees.py SUPPORTED_EXTENSIONS | |||
SUPPORTED_LANGUAGES = ["javascript", "python", "node", "ruby", "php", "go", "csharp"] | |||
SUPPORTED_LANGUAGES = [ |
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.
I'm not making any changes besides sorting it.
"ruby", | ||
] | ||
# These languages will run as dry-run mode by default | ||
DRY_RUN_PLATFORMS: list[str] = [] |
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.
We will add Java at a later point or even use an option to control it.
if frame and frame.get("in_app") and frame.get("filename") | ||
} | ||
stacktrace_paths.update(paths) | ||
for frame in frames: |
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.
This is equivalent, however, I'm going to add more to it in my next PR.
|
||
try: | ||
installation = get_installation(org) | ||
trees = get_trees_for_org(installation, org, extra) | ||
trees_helper = CodeMappingTreesHelper(trees) | ||
code_mappings = trees_helper.generate_code_mappings(stacktrace_paths) | ||
set_project_codemappings(code_mappings, installation, project) | ||
if event.platform not in DRY_RUN_PLATFORMS: | ||
set_project_codemappings(code_mappings, installation, project) |
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.
This is what creates code mappings.
def create_event( | ||
self, frames: Sequence[Mapping[str, str | bool]], platform: str | None = None | ||
) -> Event: | ||
def create_event(self, frames: Sequence[Mapping[str, str | bool]], platform: str) -> Event: |
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.
Removing | None = None
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #85065 +/- ##
========================================
Coverage 87.93% 87.94%
========================================
Files 9616 9611 -5
Lines 542926 542173 -753
Branches 20996 20950 -46
========================================
- Hits 477445 476799 -646
+ Misses 65172 65067 -105
+ Partials 309 307 -2 |
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.
Good idea! I wish I had this when I added php and go 👍
Heads up: I reran |
Thanks! It seems to still be there. I will merge master. |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
PR reverted: c841d5a |
This reverts commit 322dbe7. Co-authored-by: armenzg <[email protected]>
The dry run mode allows testing the load of adding a new platform without creating the code mappings. It also helps collect metrics and discover errors. This handles the issues found in #85065.
The dry run mode will allow testing the load of adding a new platform without creating the code mappings.
It will also help collect the metrics and discover any errors.