-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
chore(sentry-sdk): add traces #10745
Conversation
environment=os.getenv("SENTRY_ENVIRONMENT", "production"), | ||
sample_rate=1.0, # Configures the sample rate for error events, in the range of 0.0 to 1.0. The default is 1.0 which means that 100% of error events are sent. If set to 0.1 only 10% of error events will be sent. Events are picked randomly. |
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.
The SDK default was already 1.0 (see code comment)
57588df
to
6653fac
Compare
6653fac
to
7f574f2
Compare
posthog/settings/sentry.py
Outdated
send_default_pii=True, | ||
environment=os.getenv("SENTRY_ENVIRONMENT", "production"), | ||
traces_sample_rate=0.1, # A number between 0 and 1, controlling the percentage chance a given transaction will be sent to Sentry. (0 represents 0% while 1 represents 100%.) Applies equally to all transactions created in the app. |
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.
This is 10% not 0.1% as advertised in the PR.
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.
True brainfart 😄 thanks for catching this
I'm lacking context why we care about traces here? :) What is the problem we're solving? |
Yesterday @timgl was trying to investigate a performance issue reported by a customer. As we unfortunately do not have a tracing solution available (yet) the task became more difficult than expected. Mid-long term plan is to have a tracing solution available via our Helm chart but in the meantime let's leverage what Sentry can provide us. |
Co-authored-by: Karl-Aksel Puulmann <macobo@users.noreply.github.com>
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 don't think this will be of any use debugging performance issues we typically run into but also not going to block this as you think it does :)
Approved pending fixing the percentage.
Problem
While working on #10743 I've realised we are running without traces enabled.
Changes
Set the
traces_sample_rate
to 0.1% (we will bump the latter little by little after monitoring the usage).How did you test this code?
I didn't.