Skip to content

feat: Add base config for JS loader with performance/replay - #46447

Merged
mydea merged 8 commits into
masterfrom
fn/loader-default-init
Mar 31, 2023
Merged

feat: Add base config for JS loader with performance/replay#46447
mydea merged 8 commits into
masterfrom
fn/loader-default-init

Conversation

@mydea

@mydeamydea commented Mar 28, 2023

Copy link
Copy Markdown
Member

This PR adjusts the loader to include default config when enabling debug/performance/replay.

We should probably do a bit more testing etc. for this, as this is not really covered well as of now.
I did some tests locally with different scenarios, where stuff worked well, as far as I can tell.

Lazy mode

Lazy mode is only available for errors-only config. Users can manually opt-out of this by adding data-lazy="no" to their script tag.

In lazy mode, nothing happens until an error occurs. Once an error occurs, we download the CDN bundle, and then...:

  • If onLoad is configured, trigger it
  • If SDK.init() has not been called, call it with defaults

Eager mode

If either performance or replay is enabled, we are always in eager (non-lazy) mode. In this mode, we'll automatically load the CDN bundle, and then...:

  • If onLoad is configured, trigger it
  • If SDK.init() has not been called, call it with defaults

Defaults

The following default config is defined:

  • dsn: Always set
  • debug: true is set when debug bundles are enabled
  • tracesSampleRate: 0.1 is set when performance is enabled
  • replaysSessionSampleRate: 0.1 & replaysOnErrorSampleRate: 1 are set when replay is enabled
  • integrations: Will add Sentry.BrowserTracing and/or Sentry.Replay if performance/replay is enabled

This config can be overwritten by the user, and is merged. So e.g. if they do:

Sentry.onLoad(function(){Sentry.init({tracesSampleRate: 0.5});});

For a full loader build with every option enabled, the actual config will be:

{dsn: 'your-dsn',debug: true,tracesSampleRate: 0.5,replaysSessionSampleRate: 0.1,replaysOnErrorSampleRate: 1,integrations: [newSentry.BrowserTracing(),newSentry.Replay()]}

Closes#46368

@mydea
mydea requested review from a team and AbhiPrasadMarch 28, 2023 14:31
@mydeamydea self-assigned this Mar 28, 2023
@github-actionsgithub-actionsBot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 28, 2023
@mydea

Copy link
Copy Markdown
MemberAuthor

So I updated this quite a bit:

image

When you switch your SDK version to <7, it automatically removes performance & tracing. This should ensure we do not get into a weird state.

  • When Replay is enabled, we show a message that this will result in the ES6 bundle:

image

ref #46436

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Not so sure about the wording here... maybe:

When using Replay, Sentry is not ES5 compatible.

Or

When using Replay, we use the ES6 bundle of Sentry.

@AbhiPrasadAbhiPrasad left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We'll need to merge in the backend changes first - and then do the frontend.

This is still fine since everything is gated behind a feature flag.

@mydea

Copy link
Copy Markdown
MemberAuthor

We'll need to merge in the backend changes first - and then do the frontend.

This is still fine since everything is gated behind a feature flag.

Ah, oops - I extracted the UI stuff out into #46604

@mydeamydea removed the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 30, 2023
@mydea

Copy link
Copy Markdown
MemberAuthor

So I don't think the getsentry test failures are related to this PR...?

@HazATHazAT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's change the tracesSampleRate to 1. The reason, it's a weird experience if you expect a transaction to show up when testing, but it doesn't.

Comment threadsrc/sentry/web/frontend/js_sdk_loader.py Outdated
Co-authored-by: Daniel Griesser <daniel.griesser.86@gmail.com>
@mydea
mydea merged commit bb7604e into masterMar 31, 2023
@mydea
mydea deleted the fn/loader-default-init branch March 31, 2023 09:44
@AbhiPrasadAbhiPrasad added the Trigger: Revert Add to a merged PR to revert it (skips CI) label Mar 31, 2023
@getsentry-bot

Copy link
Copy Markdown
Contributor

PR reverted: ed22b45

getsentry-bot added a commit that referenced this pull request Mar 31, 2023
…46447)"
This reverts commit bb7604e.
Co-authored-by: AbhiPrasad <18689448+AbhiPrasad@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 16, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: BackendAutomatically applied to PRs that change backend componentsTrigger: RevertAdd to a merged PR to revert it (skips CI)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JS Loader: Sentry.init should be called - with defaults from docs for Performance/Replay

4 participants

@mydea@getsentry-bot@HazAT@AbhiPrasad