feat(txnames): Mark project as ready after N clusterer runs #48993
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
the
Scope: Backend
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #48993 +/- ##
=======================================
Coverage 80.95% 80.95%
=======================================
Files 4819 4820 +1
Lines 202111 202132 +21
Branches 11365 11365
=======================================
+ Hits 163616 163635 +19
- Misses 38241 38243 +2
Partials 254 254
|
jjbayer
added a commit
to getsentry/relay
that referenced
this pull request
May 16, 2023
Add a flag to project configs which indicates whether URL transaction names should always be considered `sanitized`, even if there are no clustering rules. This PR does not yet _use_ the flag, it is just a prerequisite for merging getsentry/sentry#48993. ref: getsentry/team-ingest#124
olksdr
approved these changes
May 17, 2023
jjbayer
added a commit
that referenced
this pull request
May 17, 2023
Use newest Relay library version. Required for #48993.
untitaker
approved these changes
May 17, 2023
iker-barriocanal
approved these changes
May 19, 2023
Comment on lines
+13
to
+14
| first_run: int | ||
| last_run: int |
There was a problem hiding this comment.
Why do we need to keep track of the first and last runs?
There was a problem hiding this comment.
No reason, just for debugging.
jjbayer
added a commit
to getsentry/relay
that referenced
this pull request
May 23, 2023
Start using the flag introduced by getsentry/sentry#48993 and #2128. Once the clusterer has run ~10 times, trust that all the rules than can be discovered have been discovered, and mark all incoming URL transactions as sanitized. ref: getsentry/team-ingest#124
jjbayer
added a commit
to getsentry/relay
that referenced
this pull request
May 25, 2023
We now mark all URL transactions as sanitized after 10 clusterer runs (see getsentry/sentry#48993), except 404s. Tag those explicitly on the metric so we can confirm that non-404s are now sanitized at 100%. ref: getsentry/team-ingest#124
volokluev
pushed a commit
that referenced
this pull request
May 30, 2023
Use newest Relay library version. Required for #48993.
volokluev
pushed a commit
that referenced
this pull request
May 30, 2023
Previously, Relay would mark _any_ URL transaction as sanitized as long as there is at least one clusterer rule. The idea was to keep new projects in `<< unparameterized >>` mode until they had some rules, but this backfired because some projects regressed after their single rule had expired. To make sure that no project ever flips from "sanitize everything" back to `<< unparametrized >>`, this PR introduces a counter stored in project options. Once the cluster has run 10 times, set a flag in the project config so Relay knows it can mark everything as sanitized. After deploying this + the accompanying Relay change, we should closely monitor the following metrics: * `relay_pop.event.transaction_name_changes` with `source_in:url, source_out:sanitized` - we should see an increase here. * `sentry.sentry_metrics.indexer.process_messages.dropped_message`. The cardinality rate limiter might drop more messages because more transactions get marked as `sanitized`. Requires getsentry/relay#2128 ref: getsentry/team-ingest#124
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.
Previously, Relay would mark any URL transaction as sanitized as long as there is at least one clusterer rule. The idea was to keep new projects in
<< unparameterized >>mode until they had some rules, but this backfired because some projects regressed after their single rule had expired.To make sure that no project ever flips from "sanitize everything" back to
<< unparametrized >>, this PR introduces a counter stored in project options. Once the cluster has run 10 times, set a flag in the project config so Relay knows it can mark everything as sanitized.After deploying this + the accompanying Relay change, we should closely monitor the following metrics:
relay_pop.event.transaction_name_changeswithsource_in:url, source_out:sanitized- we should see an increase here.sentry.sentry_metrics.indexer.process_messages.dropped_message. The cardinality rate limiter might drop more messages because more transactions get marked assanitized.Requires getsentry/relay#2128
ref: https://github.com/getsentry/team-ingest/issues/124