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

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

Closed
3 tasks done
vctormb opened this issue Jun 26, 2024 · 6 comments
Closed
3 tasks done

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

vctormb opened this issue Jun 26, 2024 · 6 comments
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug

Comments

@vctormb
Copy link

vctormb commented Jun 26, 2024

Is there an existing issue for this?

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.

@github-actions github-actions bot added the Package: nextjs Issues related to the Sentry Nextjs SDK label Jun 26, 2024
@mydea
Copy link
Member

mydea commented Jun 27, 2024

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
Copy link

@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
Copy link

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

@mydea
Copy link
Member

mydea commented Jun 28, 2024

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

@ant1m4tt3r
Copy link

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

@billyvg
Copy link
Member

billyvg commented Oct 15, 2024

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 billyvg closed this as completed Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug
Projects
Archived in project
Development

No branches or pull requests

5 participants