-
-
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
feat(seer grouping): Add Seer fields to grouphash metadata table #78106
Merged
lobsterkatie
merged 3 commits into
master
from
kmclb-add-seer-fields-to-grouphash-metadata
Sep 30, 2024
Merged
feat(seer grouping): Add Seer fields to grouphash metadata table #78106
lobsterkatie
merged 3 commits into
master
from
kmclb-add-seer-fields-to-grouphash-metadata
Sep 30, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
Scope: Frontend
Automatically applied to PRs that change frontend components
Scope: Backend
Automatically applied to PRs that change backend components
labels
Sep 25, 2024
This comment was marked as off-topic.
This comment was marked as off-topic.
lobsterkatie
removed
the
Scope: Frontend
Automatically applied to PRs that change frontend components
label
Sep 25, 2024
This comment was marked as outdated.
This comment was marked as outdated.
This was referenced Sep 25, 2024
wedamija
approved these changes
Sep 25, 2024
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.
migration lgtm. The table has only 14k rows so adding the index is fine.
lobsterkatie
force-pushed
the
kmclb-add-seer-fields-to-grouphash-metadata
branch
from
September 25, 2024 20:22
68eaeab
to
7e84260
Compare
github-actions
bot
added
the
Scope: Frontend
Automatically applied to PRs that change frontend components
label
Sep 25, 2024
This comment was marked as outdated.
This comment was marked as outdated.
lobsterkatie
force-pushed
the
kmclb-add-seer-fields-to-grouphash-metadata
branch
from
September 25, 2024 20:30
ba3003d
to
806d5a5
Compare
lobsterkatie
force-pushed
the
kmclb-add-seer-fields-to-grouphash-metadata
branch
from
September 25, 2024 21:13
806d5a5
to
6ab3c0d
Compare
JoshFerge
approved these changes
Sep 26, 2024
lobsterkatie
force-pushed
the
kmclb-add-seer-fields-to-grouphash-metadata
branch
from
September 27, 2024 21:34
6ab3c0d
to
71a3939
Compare
This PR has a migration; here is the generated SQL for --
-- Add field seer_date_sent to grouphashmetadata
--
ALTER TABLE "sentry_grouphashmetadata" ADD COLUMN "seer_date_sent" timestamp with time zone NULL;
--
-- Add field seer_event_sent to grouphashmetadata
--
ALTER TABLE "sentry_grouphashmetadata" ADD COLUMN "seer_event_sent" varchar(32) NULL;
--
-- Add field seer_match_distance to grouphashmetadata
--
ALTER TABLE "sentry_grouphashmetadata" ADD COLUMN "seer_match_distance" double precision NULL;
--
-- Add field seer_matched_grouphash to grouphashmetadata
--
ALTER TABLE "sentry_grouphashmetadata" ADD COLUMN "seer_matched_grouphash_id" bigint NULL;
--
-- Add field seer_model to grouphashmetadata
--
ALTER TABLE "sentry_grouphashmetadata" ADD COLUMN "seer_model" varchar NULL;
ALTER TABLE "sentry_grouphashmetadata" ADD CONSTRAINT "sentry_grouphashmeta_seer_matched_groupha_c92b0107_fk_sentry_gr" FOREIGN KEY ("seer_matched_grouphash_id") REFERENCES "sentry_grouphash" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_grouphashmetadata" VALIDATE CONSTRAINT "sentry_grouphashmeta_seer_matched_groupha_c92b0107_fk_sentry_gr";
CREATE INDEX CONCURRENTLY "sentry_grouphashmetadata_seer_matched_grouphash_id_c92b0107" ON "sentry_grouphashmetadata" ("seer_matched_grouphash_id"); |
lobsterkatie
removed
the
Scope: Frontend
Automatically applied to PRs that change frontend components
label
Sep 30, 2024
lobsterkatie
deleted the
kmclb-add-seer-fields-to-grouphash-metadata
branch
September 30, 2024 17:16
lobsterkatie
added a commit
that referenced
this pull request
Sep 30, 2024
…78107) This is a follow up to #78106, which added seer-relevant fields to the `GroupHashMetadata` table, actually using those fields to store data about Seer calls during ingest. The following data is stored: - When the grouphash was sent to Seer - Which event's stacktrace was sent (stored as an event id) - The Seer model version used to analyze the stacktrace - The matched hash returned by Seer, if any (stored as a reference to that hash's `GroupHash` record) - The similarity distance returned by Seer, if any Notes: - As a result of this change, we're no longer storing Seer results in event data. This is better - before, to see the Seer results you had to find the first event to generate a given hash (which is the first event in a group in cases where Seer doesn't find a match, but is some random event among a group's full event list in cases where Seer does match to an existing group). Now, you can find Seer resuls from any event in a group, via the group's `GroupHash` records. (It did mean I had to pull out some temporary logs I had added to group creation, but it's unclear if they're still necessary. If the issue which prompted them comes up again, I'll add them back in a different way.) - As agreed offline, I'm updating the `GroupHashMetadata` records which are created elsewhere (rather than waiting to create them until the Seer results are available, or until we decide not to call Seer) because it's easier to reason about and because the cardinality here is low, given that more than 99% of events match to an existing group and therefore never hit this code. If, after the `GroupHashMetadata` MVP is done, we decide we need to optimize this, we can do so before GA. - Not done here: Updating the backfill code to also store Seer results in grouphash metadata rather than on the group. This should be done before the next time we run a backfill, but given that the current backfill is already half-completed, having them in one place for new groups and a different (single) place for backfilled groups - while not ideal - seemed better than having them in one place for new groups and sometimes the same place but sometimes a different place for backfilled groups.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds fields to the
GroupHashMetadata
table to track the following for hashes sent to Seer:GroupHash
record)Use of these fields is added in #78107.