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

feat(symbolicator): Automatically add and remove projects to the LPQ based on their historical perf #28714

Merged
merged 37 commits into from
Oct 2, 2021

Conversation

relaxolotl
Copy link
Contributor

@relaxolotl relaxolotl commented Sep 21, 2021

This is a bit of a bloated PR, but this fills in basically everything else left besides the logic to determine whether a project belongs in the LPQ or not.

This does not include any logging to datadog; If that is needed and can be done, a follow-up PR can be opened to add that in.

More than half of the changes in this are simply tests, which will be broken up into classes for readability and modularity in a future PR. A significant number of lines are dedicated to simply declaring the API on a base class.

added = cluster.sadd(name=LPQ_MEMBERS_KEY, values=[project_id])

if added:
# cry at sentry
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think we need to do anything here? sentry will read the redis key and that's that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc we do want to tell sentry something when projects are added and removed from the LPQ list, as a starting step to tracking the general behaviour of these tasks and when/which projects are matching the LPQ criteria. i assume that means sending something over via capture_message(), but could i be misinterpreting one of the goals or tasks related to that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, that way. yes we need this probably both as datadog metrics as well as some capture_message() probably. i misunderstood what you meant, the danger of working both on the product and using the product at the same time... 😄

@relaxolotl
Copy link
Contributor Author

I've pulled out all of the code related to manipulating the list of projects into #28757 and rebased this PR on top of that one.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants