-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(errors) - Handle client sample rate on error events #92703
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
Conversation
Added the ability to receive a client sampling rate on error events through the `contexts` field. This gets parsed and placed on the event as ["data"]["sample_rate"] as part of processing before saving the event. Only enabled for specific projects, defined via a new option.
Codecov ReportAll modified and coverable lines are covered by tests ✅ Additional details and impacted files@@ Coverage Diff @@
## master #92703 +/- ##
===========================================
+ Coverage 53.21% 87.90% +34.68%
===========================================
Files 10244 10252 +8
Lines 587669 588316 +647
Branches 22831 22831
===========================================
+ Hits 312753 517146 +204393
+ Misses 274470 70724 -203746
Partials 446 446 |
src/sentry/event_manager.py
Outdated
| .get("error_sampling", {}) | ||
| .get("client_sample_rate") | ||
| ) | ||
| # Only set sample_rate if it's a valid number between 0 and 1 |
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.
do we want to log or metric if its not valid?
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.
yea that makes sense, I think ill add a metric been waiting for a chance to add one :)
src/sentry/event_manager.py
Outdated
| def _derive_client_error_sampling_rate(jobs: Sequence[Job], projects: ProjectsMapping) -> None: | ||
| for job in jobs: | ||
| project = projects[job["project_id"]] | ||
| if project.id in options.get("issues.client_error_sampling.project_allowlist"): |
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.
Would you rather use a project feature flag?
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.
discussed this with Matt, he actually suggested using an option because I dont need the granularity of a feature flag, its going to be for a specific single project.
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.
I wonder about this since both of you seem to agree, what would the benefits be here of using a feature flag instead?
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.
Feature flag allows you to pick projects, organizations, EA, etc
An option is a system-wide variable. You would have to opt in specific deployments (e.g. s4s), but I assume that the project only exists in one of our deployments.
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.
yea we would basically start with creating a project in s4s to test it out and enabling only that, then enabling it only for the specific project we want to enable for the one customer (its only their backend project that is going to do error sampling). So yea I am not going to need the flexibility, I am ok with it being specific to regions... I think ill just go with it and change if I find a good reason
Added the ability to receive a client sampling rate on error events through the `contexts` field. This gets parsed and placed on the event as `["data"]["sample_rate"]` as part of processing before saving the event. Only enabled for specific projects, defined via a new option.
Added the ability to receive a client sampling rate on error events through the
contextsfield. This gets parsed and placed on the event as["data"]["sample_rate"]as part of processing before saving the event. Only enabled for specific projects, defined via a new option.