sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

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

Open vctormb opened this issue 1 year ago • 4 comments

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

  1. Initialize Replay with replaysSessionSampleRate = 0 and replaysOnErrorSampleRate = 1.0
  2. 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.

vctormb avatar Jun 26 '24 17:06 vctormb

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.

mydea avatar Jun 27 '24 07:06 mydea

@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 avatar Jun 27 '24 18:06 ant1m4tt3r

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

ant1m4tt3r avatar Jun 27 '24 18:06 ant1m4tt3r

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();
}

mydea avatar Jun 28 '24 06:06 mydea

indeed, it worked for us (sorry it took so long to reply). thanks @mydea

ant1m4tt3r avatar Oct 15 '24 02:10 ant1m4tt3r

Ahh missed your ping @mydea, I think it's fine to expose recordingMode - I've made a ticket for it here and will close out this issue.

billyvg avatar Oct 15 '24 16:10 billyvg