-
Notifications
You must be signed in to change notification settings - Fork 535
feat: Sample everything 100% w/ Spotlight & no DSN set #4207
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
This patch makes Spotlight easier to setup by turning all sampling to 100% when no DSN is set and Spotlight is enabled. I consider this a non-breaking and a safe change as these only apply when no DSN is set so it should have no production or billing implications.
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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.
Nice! I have two suggestions, but since you'll likely have to implement them anyways in order to get the tests to pass, I'll approve
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.
lgtm, see suggestion
I'm not quite sure why the GraphQL tests are failing here |
Ah @sentrivana just pointed out in Slack that graphql tests are failing on |
Co-authored-by: Daniel Szoke <[email protected]>
This patch makes Spotlight easier to setup by turning all sampling to 100% when no DSN is set and Spotlight is enabled. I consider this a non-breaking and a safe change as these only apply when no DSN is set so it should have no production or billing implications.