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.

Sign up for GitHub

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

Jump to bottom

Undocumented Limitation of Lazy-Loading SDK #4722

Closed
ArmoredCavalry opened this issue Feb 13, 2022 · 3 comments · Fixed by #4775
Closed

Undocumented Limitation of Lazy-Loading SDK #4722

ArmoredCavalry opened this issue Feb 13, 2022 · 3 comments · Fixed by #4775

Comments

@ArmoredCavalry
Copy link

Core or SDK?

Platform/SDK

Which part? Which one?

Javascript

Description

I recently added version for release health / sessions to my JS SDK, but was only seeing sessions with errors. I ended up tracing this back to use of the lazy-loading script. Since it will only initialize the SDK if there is an error / exception, 100% of reported sessions were 'crashes'.

Suggested Solution

I believe this should be added as a note under the Limitations section of the Lazy Loading docs -

https://docs.sentry.io/platforms/javascript/install/lazy-load-sentry/#limitations

This was when searching the docs for "sessions" there is a better chance of discovering why this behavior is happening.
@getsentry-release
Copy link
Contributor

Routing to @getsentry/{"message":"Not Found","documentation_url":"https://docs.github.com/rest/reference/teams#get-a-team-by-name"} for triage. ⏲️

@lobsterkatie lobsterkatie mentioned this issue Feb 14, 2022
Merged
@lobsterkatie
Copy link
Member

lobsterkatie commented Feb 14, 2022 •
edited
Loading

Sorry 'bout that! We changed how BrowserTracing is accessed, so the docs are right, or at least, describe what should be right. Unfortunately, the CDN bundle was missed in that PR. I've pushed a new PR to fix this.

Gah - that fix was was linked to the wrong issue. Sorry, folks. Reopening.

lobsterkatie added a commit to getsentry/sentry-javascript that referenced this issue Feb 14, 2022
@lobsterkatie
fix(tracing): Export BrowserTracing directly in CDN bundle (#4570)
9e87bf9 
When we made the changes in #4204 (to export `BrowserTracing` directly rather than as part of `Sentry.Integrations`), we didn't make the change in the tracing CDN bundle. While It's true that there's no bundle size issue there (mongo and friends never were getting included in the CDN bundle, so there was nothing to fix), it did mean that when we changed the docs to match the above PR's changes, we ended up breaking CDN folks' usage of `BrowserTracing`.

This fixes that by making the same change in the tracing CDN bundle as was made for the npm package.

Fixes getsentry/sentry-docs#4722.
@lobsterkatie lobsterkatie reopened this Feb 14, 2022
@chadwhitacre chadwhitacre mentioned this issue Feb 14, 2022
Closed
@lobsterkatie lobsterkatie reopened this Feb 28, 2022
@lobsterkatie
Copy link
Member

I'm pushing a fix now - thanks for pointing this out!

@lobsterkatie lobsterkatie mentioned this issue Feb 28, 2022
Merged
lobsterkatie added a commit that referenced this issue Mar 1, 2022
@lobsterkatie
fix(browser): Mention release health in loader limitations section (#…
5481b4e 
…4775)

In our current implementation of the feature, a session and a pageload transaction are essentially one and the same, which means that release health doesn't work any better than tracing does when you're using the loader. This clarifies that in the loader's Limitations section.

Fixes #4722.
@github-actions github-actions bot locked and limited conversation to collaborators Mar 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(browser): Mention release health in loader limitations section getsentry/sentry-docs
5 participants
@lobsterkatie @ArmoredCavalry @masinette @getsentry-release @imatwawana