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

fix(log): Honor SentryConfig.enabled and don't init SDK at all if it is false #1380

Merged
merged 1 commit into from
Aug 4, 2022

Conversation

sl0thentr0py
Copy link
Member

@sl0thentr0py sl0thentr0py commented Aug 4, 2022

Fixes edge case behavior where enabled is false but SENTRY_DSN is present on the machine and the SDK internally still picks it up.

This came up because relay in an AWS extension setting was reporting errors to the user's project and eating up their quota.
Note that in our AWS layer, we do have SENTRY_DSN set in the env to also be used by the language specific SDKs (node/python).

Local testing

I tested that stuff like relay_log::capture_error no-ops successfully and does not break stuff even when the SDK is not initialized. And the fact that stuff compiles.

References

Zendesk ticket

@sl0thentr0py sl0thentr0py changed the title fix(log): init SDK only when there's an enabled_dsn fix(log): Honor SentryConfig.enabled and don't init SDK at all if it is false. Aug 4, 2022
@sl0thentr0py sl0thentr0py changed the title fix(log): Honor SentryConfig.enabled and don't init SDK at all if it is false. fix(log): Honor SentryConfig.enabled and don't init SDK at all if it is false Aug 4, 2022
@sl0thentr0py sl0thentr0py force-pushed the neel/disable-reporting branch 2 times, most recently from fcabb2f to 382a528 Compare August 4, 2022 13:31
Fixes edge case behaviour where `enabled` is false but SENTRY_DSN is
present on the machine and the SDK internally still picks it up.
This came up because relay in an AWS extension setting was reporting
errors to the user's project and eating up their quota.
@sl0thentr0py sl0thentr0py marked this pull request as ready for review August 4, 2022 14:04
@sl0thentr0py sl0thentr0py requested review from a team and Swatinem August 4, 2022 14:04
@sl0thentr0py sl0thentr0py enabled auto-merge (squash) August 4, 2022 14:25
@sl0thentr0py sl0thentr0py merged commit 8d0524c into master Aug 4, 2022
@sl0thentr0py sl0thentr0py deleted the neel/disable-reporting branch August 4, 2022 14:33
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