fix(similarity): Add formatted stacktrace null check

[jangjodi]

Add an empty stacktrace check after stacktrace formatting is done in ingest
If the stacktrace is empty, do not send data to seer

[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   #80796      +/-   ##
==========================================
+ Coverage   77.34%   80.33%   +2.98%     
==========================================
  Files        7223     7223              
  Lines      320123   320132       +9     
  Branches    20779    20779              
==========================================
+ Hits       247612   257189    +9577     
+ Misses      72118    62550    -9568     
  Partials      393      393              

[lobsterkatie]

Overall looks good! Just some nits and suggestions.

[lobsterkatie]

Do we need to use .get here? If we make it to this point, we know we'll have stored the string on the event.

[jangjodi]

we don't need it, but considering that it's in ingest I thought we'd play it safe just in case someone calls this function again later and forgets this caveat

[lobsterkatie]

That fair. Actually, here's an idea: Assuming my tallying-of-frames business works, I'm realizing I'm actually still going to need the variants to be passed into this function. So what if we were to do it like this?

Suggested change

	"stacktrace": event.data.get("stacktrace_string", ""),
	"stacktrace": event.data.get(
	"stacktrace_string", get_stacktrace_string(get_grouping_info_from_variants(variants))
	),

Then you also wouldn't need to add a call to should_call_seer_for_grouping in test_sends_expected_data_to_seer.

[lobsterkatie]

That fair. Actually, here's an idea: Assuming my tallying-of-frames business works, I'm realizing I'm actually still going to need the variants to be passed into this function. So what if we were to do it like this?

Suggested change

	"stacktrace": event.data.get("stacktrace_string", ""),
	"stacktrace": event.data.get(
	"stacktrace_string", get_stacktrace_string(get_grouping_info_from_variants(variants))
	),

Then you also wouldn't need to add a call to should_call_seer_for_grouping in test_sends_expected_data_to_seer.

[lobsterkatie]

I don't think in-app and contributes are things which show up in actual event data (only in variants). Also, an empty frames list also wouldn't get past event_content_is_seer_eligible... But here's maybe a simpler solution: Can we just mock get_stacktrace_string to return an empty string?

[jangjodi]

The empty frame list actually passes event_content_is_seer_eligible, hence how the empty stacktrace metric is called after

[jangjodi]

@lobsterkatie regarding your first suggestion, we don't want to have to call get_stacktrace_string again since we need to call it in should_call_seer_for_grouping to figure out if the stacktrace is empty

[lobsterkatie]

@lobsterkatie regarding your first suggestion, we don't want to have to call get_stacktrace_string again since we need to call it in should_call_seer_for_grouping to figure out if the stacktrace is empty

Right - that's why we're still getting it out of the event, where it should be, given that fact. It's just that if somehow it's not there, we're recalculating it as backup.

[jangjodi]

@lobsterkatie regarding your first suggestion, we don't want to have to call get_stacktrace_string again since we need to call it in should_call_seer_for_grouping to figure out if the stacktrace is empty

Right - that's why we're still getting it out of the event, where it should be, given that fact. It's just that if somehow it's not there, we're recalculating it as backup.

ah sorry, I missed that. thank you!