Skip to content

Conversation

@mbudde
Copy link
Contributor

@mbudde mbudde commented Oct 14, 2024

No description provided.

@pmb0
Copy link
Member

pmb0 commented Nov 18, 2024

Thanks!

@pmb0 pmb0 merged commit 4175742 into heiseonline:master Nov 18, 2024
@rabbiveesh
Copy link
Contributor

hey, the change to sample breaks existing code really badly - anything that created a transaction now suddenly gets undef during testing. One of the commits in #30 fixes this.
Unless i'm not understanding something, it never tried reporting anything when there's no dsn; i've been using this for years and in tests i have no DSN and everything is hunky-dory.

@rabbiveesh
Copy link
Contributor

In fact, i had already written this behavior in #6 ; what wasn't working about that?

@mbudde
Copy link
Contributor Author

mbudde commented Nov 28, 2024

In fact, i had already written this behavior in #6 ; what wasn't working about that?

The motivation is having a way to completely disable Sentry::SDK (integrations should not be initialized) such that Sentry::SDK->capture_exeception et al are just no-ops and there are no potential side-effects (performance or bugs) from the integrations. But looking at the Python SDK it looks like it is handled by having a default no-op client before initialization and if an empty DSN is set the SDK is initialized but doesn't send events. That might be a better way to do it.

@rabbiveesh
Copy link
Contributor

Oh, so then this is the wrong thing to do b/c this breaks any integration that was starting a transaction.
If you wanna disable the integrations, then just don't always load them. in #31 I implement this (it matches better to other SDKs where there's a defaultIntegrations function exported)

@mbudde mbudde deleted the disable-reporting branch January 23, 2025 13:49
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.

3 participants