chore(similarity): Add logging for over 30 system frames

[jangjodi]

Add logging for events with stacktraces with over 30 system-only frames

[JoshFerge]

if we follow the pattern where we raise the exception we can catch and log then, so no need to pass anything extra into the function

[jangjodi]

if we follow the pattern where we raise the exception we can catch and log then, so no need to pass anything extra into the function

if we raise the exception, would we still be able to return the stacktrace string? I chose to do it this way so the grouping logic would not change yet.

also this is a temporary metric that we're gathering so we can look at some examples

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #81130      +/-   ##
==========================================
- Coverage   80.34%   80.30%   -0.04%     
==========================================
  Files        7225     7220       -5     
  Lines      320221   319346     -875     
  Branches    20781    20779       -2     
==========================================
- Hits       257268   256445     -823     
+ Misses      62559    62508      -51     
+ Partials      394      393       -1     

[lobsterkatie]

Nit:

Suggested change

	is_frames_truncated = False
	are_frames_truncated = False

or

Suggested change

	is_frames_truncated = False
	were_frames_truncated = False

or

Suggested change

	is_frames_truncated = False
	is_truncated_stacktrace = False

etc.

[lobsterkatie]

It might just be unclear variable naming (and not having read through the entire algorithm) which is getting me, but why are we adding contributing frames to all frames?

Also, nit:

Comparisons return a bool, so we can just use that directly.

Suggested change

	if len(contributing_frames) + frame_count > MAX_FRAME_COUNT:
	is_frames_truncated = True
	is_frames_truncated = len(contributing_frames) + frame_count > MAX_FRAME_COUNT

[jangjodi]

we're doing this to see if the length of all the contributing frames we've accumulated so far (either from this current exception or some previous ones) is greater than 30

[lobsterkatie]

Okay, so basially contributing_frames is ones we've already counted and frame_count is ones we're about to add?

[jangjodi]

other way around, but yes that's the idea

[lobsterkatie]

Got it, thanks for the explanation!

[lobsterkatie]

Nit:

Suggested change

	grouping_info = get_grouping_info_from_variants(variants)
	mock_logger.info.assert_called_with(
	"grouping.similarity.over_threshold_system_only_frames",
	extra={
	"project_id": "",
	"event_id": "",
	"hash": grouping_info["system"]["hash"],
	},
	)
	grouping_info = get_grouping_info_from_variants(variants)
	grouping_info["event_id"] = 11212012123120120415201309082013
	grouping_info["project_id"] = 12311121
	mock_logger.info.assert_called_with(
	"grouping.similarity.over_threshold_system_only_frames",
	extra={
	"project_id": 11212012123120120415201309082013,
	"event_id": 12311121,
	"hash": grouping_info["system"]["hash"],
	},
	)

[jangjodi]

the case we're testing here is calling get_seer_similar_issues without modifying grouping_info first. so the case when this line is called.

this also doesn't work since grouping info is calculated in the function. we could mock it, but I think the other tests cover this.

[lobsterkatie]

Ah, I see. Okay, great. If we were doing this for real I might suggest moving adding the info into the grouping info helper itself (then you only have to do it in one place, I think?), but again, since this is temporary, I say 🚢 .

[getsentry-bot]

PR reverted: 7b7e795