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

interactionsSampleRate removed in v8 #12006

Closed
3 tasks done
mfb opened this issue May 13, 2024 · 5 comments · Fixed by #12023 or #12064
Closed
3 tasks done

interactionsSampleRate removed in v8 #12006

mfb opened this issue May 13, 2024 · 5 comments · Fixed by #12023 or #12064

Comments

@mfb
Copy link

mfb commented May 13, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

8.0.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

On v7 I was able to configure an interactionsSampleRate; on v8 I didn't see evidence of this being applied, nor is present in the codebase, so I believe it was removed?

Expected Result

If interactionsSampleRate was removed, should be documented in the changelog/migration guide.

Actual Result

No documentation about the option being removed.

@HazAT
Copy link
Member

HazAT commented May 13, 2024

Hey, thanks for raising this.
It might have slipped through the cracks, but we will add this tomorrow.

@Lms24
Copy link
Member

Lms24 commented May 14, 2024

Hey @mfb thanks for reporting! I opened #12023 to fix this. Expect it to be released with the next version in the coming days. Sorry for the inconvenience!

Lms24 added a commit that referenced this issue May 14, 2024
…tion` options (#12023)

This patch forward-ports the `interactionsSampleRate` option for
`browserTracingIntegration` introduced in 7.110.0 to v8. Looks like we
missed this when forward-porting the INP implementation as reported in
#12006, probably because these two things happened in parallel.
@Lms24 Lms24 reopened this May 15, 2024
@Lms24
Copy link
Member

Lms24 commented May 15, 2024

Hi @mfb apologies for the apologies, I have to reopen this issues. We have serious concerns about the interactionsSampleRate in its current form. We're gonna revert #12023 and revisit this decision.

For the moment, I'd recommend to remove the interactionsSampleRate. It's worth noting that you're not getting billed for INP spans at the moment, so these should not deplete your quota.

Sorry for the back and forth!

@mfb
Copy link
Author

mfb commented May 15, 2024

Yes, I'm not sure it was necessary (although it allowed me to do weird things, like set interactionsSampleRate higher than 1 so that I could increase the rate above tracesSampleRate for purposes of quickly seeing what it looked like in the Sentry UI :) I just wanted to mention so it could be documented

@Lms24
Copy link
Member

Lms24 commented May 16, 2024

Good point about raising the sample rate :) To be fair, I think that's also a valid use case, depending on what you want to achieve 😅 (and as long as the value stays below 1.0 fwiw).

Anyway, I opened #12064 to update the migration guide as we decided internally that we'll keep the option removed in v8. Concrete reasons:

  • This sample rate was only ever applied to INP spans which are
    1. only used to collect INP values and not shown as spans in the Sentry UI
    2. not billed for at the moment; meaning your quota does not deplete when these spans are sent to Sentry
  • There is already an SDK option to apply different sample rates to spans: the tracesSampler function (see example below)
  • In a coming soon (TM) minor release, we’ll add a beforeSendSpan callback that can also be used to filter out INP spans (completely, by rolling the dice again or whatever condition you decide)

To migrate off, you probably can just remove the option for the reasons outlined above, or use tracesSampler:

// v7
Sentry.init({
  integrations: [new BrowserTracing({ interactionsSampleRate: 0.1 })],
});
// v8 - please read the text above, you most likely don't need this :)
Sentry.init({
  tracesSampler: (ctx) => {
    if (ctx.attributes?['sentry.op']?.startsWith('ui.interaction')) {
      return 0.1;
    }
    return 0.5;
  }
})

If interaction/INP spans become more powerful in the future, we might revisit this decision.

andreiborza pushed a commit that referenced this issue May 16, 2024
…tion` options (#12023)

This patch forward-ports the `interactionsSampleRate` option for
`browserTracingIntegration` introduced in 7.110.0 to v8. Looks like we
missed this when forward-porting the INP implementation as reported in
#12006, probably because these two things happened in parallel.
Lms24 added a commit that referenced this issue May 16, 2024
As discussed internally in Slack and #12006, we decided to remove
`interactionsSampleRate` from v8 for good (it never was added in a
publicly released `8.0.0*` version). This PR now updates the migration
guide to retroactively reflect this change.

closes #12006
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment