Flush saves the recorded session even if the sample rate is zero

vctormb commented 1 week ago

Is there an existing issue for this?

[X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
[X] I have reviewed the documentation https://docs.sentry.io/
[X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/nextjs

SDK Version

8.10.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

const client = SentrySDK.getClient()
const options = client.getOptions()

options.replaysSessionSampleRate = 0
options.replaysOnErrorSampleRate = 1.0

const replay = SentrySDK.replayIntegration({
 minReplayDuration: 3000,
})

client.addIntegration(replay)

Steps to Reproduce

Initialize Replay with replaysSessionSampleRate = 0 and replaysOnErrorSampleRate = 1.0
Call Sentry.getReplay().flush()and it will upload a new video recording even if replaysSessionSampleRate is 0

Expected Result

It should not record a video when calling flush if replaysSessionSampleRate = 0

Actual Result

It is recording a video when calling flush when replaysSessionSampleRate = 0.

mydea commented 1 week ago

Hey, this is expected behavior for flush(). flush will always convert the session and start sending if it hasn't so far. See: https://docs.sentry.io/platforms/javascript/session-replay/understanding-sessions/#manually-flushing-recording-data

What's your use case here? We could expose the recording mode, if that's important.

ant1m4tt3r commented 1 week ago

@mydea let me give you some context on our use case.

we run an application in an iframe that is embeded in mulitple customer websites. this iframe holds a checkout process and once the checkout is done, we close the iframe so the user can keep the navigation in the store.

if we don't call the flush() method what happens is that sentry replay will loose the final seconds of recording, as I believe it did not had the time to send the information to sentry services given some debounce or throttling. when we call flush() it sends the final sends to the server and we can see the end of the session, which is quite important for us.

the main issue arose when we decided to only record sessions for merchants based on a dynamic runtime discovered sample rate, cause we don't need to record every session, but what happens if that if this session was not suposed to be sampled and we call flush, it still keeps a recording of it.

do we have a way to determine if a session was sampled or not so we can call flush conditionaly? if not, there is a way to prevent this "extra session" being recorded even when calling flush?

ant1m4tt3r commented 1 week ago

oh, forgot to mention, me and @vctormb work at the same place :)

mydea commented 1 week ago

OK, I see. This makes sense I think, IMHO we can expose recordingMode on the replay integration - WDYT @billyvg ?

For now, you should be able to make this work like this - this is internals, but you can try it:

// Only flush if recordingMode is session
// This would be `buffer` otherwise
if (replay._replay.recordingMode === 'session') {
  replay.flush();
}

getsentry / sentry-javascript