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

feat(nuxt): Automatically add BrowserTracing #13005

Merged
merged 4 commits into from
Jul 23, 2024
Merged

Conversation

s1gr1d
Copy link
Member

@s1gr1d s1gr1d commented Jul 22, 2024

Add BrowserTracing when tracesSampleRate is set.

@s1gr1d s1gr1d requested a review from Lms24 July 22, 2024 15:30
// This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false",
// in which case everything inside will get tree-shaken away
if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {
if (hasTracingEnabled(options)) {
Copy link
Member

@Lms24 Lms24 Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I checked and this logic is identical to next, sveltekit and astro. So far so good but there is a flaw here that we probably were not aware of:

hasTracingEnabled returns false if neither tracesSampleRate, tracesSampler nor enableTracing are set. So in this case, we'd not add the integration here. However, the only way to enable "Tracing without Performance" in browser land is to add browserTracingIntegration but not set any of the three options.

Before we merge this PR, let's discuss if we change this behaviour in all SDKs or continue with it.

// This evaluates to true unless __SENTRY_TRACING__ is text-replaced with "false",
// in which case everything inside will get tree-shaken away
if (typeof __SENTRY_TRACING__ === 'undefined' || __SENTRY_TRACING__) {
if (hasTracingEnabled(options)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed today, let's remove this guard and later, once we have a vite plugin setup, add an easy-to-use option to configure the __SENTRY_TRACING__ flag :)

@s1gr1d s1gr1d requested a review from Lms24 July 23, 2024 08:57
@s1gr1d s1gr1d merged commit f867cc0 into develop Jul 23, 2024
92 checks passed
@s1gr1d s1gr1d deleted the sig/nuxt-browsertracing branch July 23, 2024 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants