ref(replay): Log warning if sample rates are all undefined #6959
Conversation
size-limit report 📦
|
Lms24
force-pushed
the
lms-replay-sample-rate-warning
branch
from
6b5a8c8
to
095dd92
Compare
Lms24
changed the title
packages/replay/src/integration.ts
Outdated
| const finalOptions = { sessionSampleRate: 0, errorSampleRate: 0, ...dropUndefinedKeys(initialOptions) }; | ||
|
|
||
| if (!opt) { | ||
| return finalOptions; |
There was a problem hiding this comment.
m: Hmm in this case we'll also want to check for the warning, I guess?
There was a problem hiding this comment.
I'd argue that in this case, something more fundamental is wrong with the SDK than anything Replay specific. We should always get defined options from the client.
Thinking about it, the typecast above unnecessarily includes | undefined. I think it should be good to be removed.
There was a problem hiding this comment.
argh my jet lag isn't helping here - the client could be undefined which is what we're actually checking for here 😅
So yes, we probably should emit another warning there but I'd still argue that it is more a fundamental SDK problem than just the sample rates.
There was a problem hiding this comment.
jup, right! Maybe we just add a more generic client not found type message there?
Lms24
force-pushed
the
lms-replay-sample-rate-warning
branch
from
8eef3df
to
7314883
Compare
Since we decided to set both replay sample rates to 0 by default, we should log a warning that replay will be disabled if no sample rates were passed to Sentry.init. Since we're still supporting the deprecated sample rates in the integration options, this warning will only be emitted if these rates aren't set either.
Had to do some refactoring to nail down the logic as previously, default values were applied too early