-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
chore(similarity): Do not send > 30 system frames to seer #81259
Conversation
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## master #81259 +/- ##
==========================================
- Coverage 80.35% 80.34% -0.01%
==========================================
Files 7221 7220 -1
Lines 319640 319595 -45
Branches 20782 20782
==========================================
- Hits 256832 256791 -41
+ Misses 62414 62410 -4
Partials 394 394 |
PR reverted: 104352c |
…1259)" This reverts commit 2e3a412. Co-authored-by: jangjodi <[email protected]>
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 looking at it again since I saw the error on Slack.
stacktrace_string = get_stacktrace_string(get_grouping_info_from_variants(variants)) | ||
stacktrace_string = get_stacktrace_string_with_metrics( | ||
get_grouping_info_from_variants(variants), event.platform, ReferrerOptions.INGEST | ||
) | ||
if stacktrace_string == "": |
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.
Maybe over here we need to check if it's None as well.
|
||
if stacktrace_string == "" or not latest_event: | ||
if not stacktrace_string or not latest_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.
Just because I always forget how Python behaves I test the following:
>>> def foo(b):
... if not b:
... print("hey")
...
>>> foo("")
hey
>>> foo(None)
hey
>>> foo("bar")
>>>
@@ -217,7 +220,10 @@ def get_seer_similar_issues( | |||
"hash": event_hash, | |||
"project_id": event.project.id, | |||
"stacktrace": event.data.get( | |||
"stacktrace_string", get_stacktrace_string(get_grouping_info_from_variants(variants)) |
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.
In this function, we must always prevent request_data["stacktrace"]
to be None
.
Do not send stacktraces with > 30 system frames and no in-app frames to seer
…1259)" This reverts commit 2e3a412. Co-authored-by: jangjodi <[email protected]>
Do not send stacktraces with > 30 system frames and no in-app frames to seer