Skip to content
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

ref(seer grouping): Stop sending/receiving group id when communicating with Seer #70396

Merged
merged 10 commits into from
May 20, 2024

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented May 6, 2024

Now that Seer has stopped expecting group ids (we started sending the hash in addition to group id in #70244, and seer switched to handling hash rather than group id in https://github.com/getsentry/seer/pull/625), and has stopped sending them back (also changed in https://github.com/getsentry/seer/pull/625), we can close the loop and stop sending or expecting them on the Sentry side, too.

This PR makes that change, which allows for some simplification in three places:

  • In the SimilarIssuesEmbeddingsRequest and RawSeerSimilarIssueData types, group_id and parent_group_id (respectively) are removed and hash and parent_hash (respectively) are no longer optional.

  • In SeerSimilarIssueData.from_raw, we no longer have to play the "Did Seer send back parent hash or parent group id?" game, and can instead just trust that the parent hash will be there and process it.

  • In GroupSimilarIssuesEmbeddingsEndpoint.get_formatted_results, we no longer have to handle the case of there being parent group ids in the passed data which correspond to non-existent groups, since the group ids there are no longer coming from Seer but from us, which gives us the opportunity to make sure they're real before passing them on. (Technically this could/should have happened in ref(seer-grouping): Automatically convert RawSeerSimilarIssueData into SeerSimilarIssueData #70240, but better late than never.)

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label May 6, 2024
@lobsterkatie lobsterkatie force-pushed the kmclb-remove-group-id-handling-with-seer branch from 68d698e to 9e73117 Compare May 6, 2024 23:38
@lobsterkatie lobsterkatie force-pushed the kmclb-remove-group-id-handling-with-seer branch from 9e73117 to 1a45b4f Compare May 7, 2024 01:36
@lobsterkatie lobsterkatie force-pushed the kmclb-rename-seer-hash-values branch from 32f707e to 5567d64 Compare May 7, 2024 16:04
@lobsterkatie lobsterkatie force-pushed the kmclb-remove-group-id-handling-with-seer branch from 1a45b4f to 513a02c Compare May 7, 2024 16:09
@lobsterkatie lobsterkatie requested a review from jangjodi May 7, 2024 16:11
@lobsterkatie lobsterkatie force-pushed the kmclb-rename-seer-hash-values branch from 5567d64 to c75b67d Compare May 7, 2024 17:39
@lobsterkatie lobsterkatie force-pushed the kmclb-remove-group-id-handling-with-seer branch from 513a02c to 17518ab Compare May 7, 2024 17:40
@lobsterkatie lobsterkatie force-pushed the kmclb-rename-seer-hash-values branch from c75b67d to 4b274bb Compare May 7, 2024 21:27
@lobsterkatie lobsterkatie force-pushed the kmclb-remove-group-id-handling-with-seer branch from 17518ab to c609237 Compare May 7, 2024 21:28
Base automatically changed from kmclb-rename-seer-hash-values to master May 7, 2024 23:41
@lobsterkatie lobsterkatie force-pushed the kmclb-remove-group-id-handling-with-seer branch from c609237 to 67d1494 Compare May 8, 2024 02:22
@lobsterkatie lobsterkatie force-pushed the kmclb-remove-group-id-handling-with-seer branch from 67d1494 to a045038 Compare May 8, 2024 21:05
@lobsterkatie lobsterkatie force-pushed the kmclb-remove-group-id-handling-with-seer branch from a045038 to bbce292 Compare May 9, 2024 18:48
Copy link

codecov bot commented May 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.88%. Comparing base (da94b69) to head (e4f5880).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #70396   +/-   ##
=======================================
  Coverage   77.88%   77.88%           
=======================================
  Files        6528     6528           
  Lines      290835   290824   -11     
  Branches    50338    50335    -3     
=======================================
- Hits       226518   226510    -8     
+ Misses      58070    58065    -5     
- Partials     6247     6249    +2     
Files Coverage Δ
...y/api/endpoints/group_similar_issues_embeddings.py 100.00% <ø> (ø)
src/sentry/seer/utils.py 93.15% <100.00%> (-0.48%) ⬇️

... and 6 files with indirect coverage changes

@lobsterkatie lobsterkatie force-pushed the kmclb-remove-group-id-handling-with-seer branch from b8a7c06 to e4f5880 Compare May 17, 2024 21:54
@lobsterkatie lobsterkatie marked this pull request as ready for review May 20, 2024 18:06
@lobsterkatie lobsterkatie requested a review from a team as a code owner May 20, 2024 18:06
@lobsterkatie lobsterkatie merged commit 7ddc69c into master May 20, 2024
50 checks passed
@lobsterkatie lobsterkatie deleted the kmclb-remove-group-id-handling-with-seer branch May 20, 2024 18:06
cmanallen pushed a commit that referenced this pull request May 21, 2024
…g with Seer (#70396)

Now that Seer has stopped expecting group ids (we started sending the hash in addition to group id in #70244, and seer switched to handling hash rather than group id in getsentry/seer#625), and has stopped sending them back (also changed in getsentry/seer#625), we can close the loop and stop sending or expecting them on the Sentry side, too.

This PR makes that change, which allows for some simplification in three places:

- In the `SimilarIssuesEmbeddingsRequest` and `RawSeerSimilarIssueData` types, `group_id` and `parent_group_id` (respectively) are  removed and `hash` and `parent_hash` (respectively) are no longer optional.

- In `SeerSimilarIssueData.from_raw`, we no longer have to play the "Did Seer send back parent hash or parent group id?" game, and can instead just trust that the parent hash will be there and process it.

- In `GroupSimilarIssuesEmbeddingsEndpoint.get_formatted_results`, we no longer have to handle the case of there being parent group ids in the passed data which correspond to non-existent groups, since the group ids there are no longer coming from Seer but from us, which gives us the opportunity to make sure they're real before passing them on. (Technically this could/should have happened in #70240, but better late than never.)
@github-actions github-actions bot locked and limited conversation to collaborators Jun 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants