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(txnames): Use txNameReady flag #2139

Merged
merged 4 commits into from
May 23, 2023
Merged

feat(txnames): Use txNameReady flag #2139

merged 4 commits into from
May 23, 2023

Conversation

jjbayer
Copy link
Member

@jjbayer jjbayer commented May 22, 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: https://github.com/getsentry/team-ingest/issues/124

@jjbayer jjbayer changed the title Feat/txname ready flag feat(txnames): Use txNameReady flag May 22, 2023
@jjbayer jjbayer marked this pull request as ready for review May 22, 2023 09:04
@jjbayer jjbayer requested a review from a team May 22, 2023 09:04
Copy link
Contributor

@olksdr olksdr left a comment

Choose a reason for hiding this comment

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

lgtm

@jjbayer jjbayer merged commit 13a6141 into master May 23, 2023
17 checks passed
@jjbayer jjbayer deleted the feat/txname-ready-flag branch May 23, 2023 07:41
jjbayer added a commit that referenced this pull request 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 this pull request may close these issues.

None yet

3 participants