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

[metrics] Handling transaction:<< unparameterized >> better for new users #2186

Closed
k-fish opened this issue Jun 5, 2023 · 1 comment · Fixed by #2210
Closed

[metrics] Handling transaction:<< unparameterized >> better for new users #2186

k-fish opened this issue Jun 5, 2023 · 1 comment · Fixed by #2210
Assignees

Comments

@k-fish
Copy link
Member

k-fish commented Jun 5, 2023

Problem statement

We currently let through unparameterized still especially for new users since there is a warmup period before rules are built.

at least 99% (TBD) of transaction names are parameterized when a customer first sets up Performance on all platforms

While we are at a very high % for overall unparameterized sanitized, the majority of the remaining % unparameterized resides in our users first experience sending in transactions. We don't track user 'newness' in our metrics charts for transaction source transformation, but since the mechanism needs warming time it's likely that at that many of our browser frontends are seeing << unparameterized >> as their first experience. Part of this is due to lack of integrations creating better transaction names in the sdk themselves, we also need to mitigate the current legitimate use case to improve the initial user experience since sdks adoption is quite slow and we don't have a perfect solution there either.

Current state

This only applies for transaction.source:url and when the clusterer does not have rules built.

start = [unparameterized]  // wait until clusterer is warmed up
[unparameterized] --brought cardinality down with clusterer --> [let_through]  

Before we make any decisions here, the first step should probably be improving the metrics. Adding txNameReady to the existing metrics so we can track what % are being let through before the 10h mark gets hit might be helpful since it's closer to 'new user' since it applies only before clusterer has run multiple times.

Potential solutions

Sourced from discussion in slack.

  • Add a set to allow through the first X transaction names. Doesn't need to be exact just needs to let through
start = [let_through]   // Allow all new transactions in for accounts never seen by the clusterer
[let_through] --high cardinality detected--> [unparameterized]      // Add a check to limit an explosion of cardinality
[unparameterized] --brought cardinality down--> [let_through]
  • Lower time to txNameReady (less clusterer runs required) or add more fine grain controls based on throughput.
  • Improve adoption of routers vs. raw urls (not relevant to this ticket but will have an impact on this %)
@jjbayer
Copy link
Member

jjbayer commented Jun 13, 2023

@k-fish discussed this with the team. Instead of tracking a new set of "allowed" transactions, we will go for the simple solution of letting everything through (which we already did for projects older than 10 hours), and rely on the cardinality limiter to protect our infrastructure against peaks in cardinality.

See #2210

jjbayer added a commit that referenced this issue Jun 13, 2023
In #2139, we transitioned to
marking all URL transactions as "sanitized" once sentry provides the
"ready" flag. However, this results in a bad onboarding experience for
new projects because they still see `<< unparameterized >>` for the
first ~10 hours.

This PR removes the "ready" condition, such that _all_ transactions with
source URL will now get their transaction name added as a metric tag.
Since we were already doing this for all but the newest projects, this
should not add much cardinality after all. But we have to keep an eye on
the cardinality limiter in the first few hours / days after deploying
this.

Fixes #2186

---------

Co-authored-by: Iker Barriocanal <32816711+iker-barriocanal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants