-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Initial Session Replay SDK Docs #5899
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/platform-includes/performance/configure-sample-rate/javascript.electron.mdx
Outdated
Show resolved
Hide resolved
--- | ||
title: "Session Replay" | ||
sidebar_order: 3 | ||
redirect_from: | ||
- /session-replay/ | ||
description: "Session Replay. TODO" | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--- | ||
title: Set Up | ||
sidebar_order: 1 | ||
redirect_from: | ||
- /session-replay/setup/ | ||
- /session-replay/getting-started/ | ||
description: "Get started with Session Replay, TODO." | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine Set Up is mainly a client-side/SDK thing? If so, we could probably get by without the sub page to start? Basically add all content on the landing Session Replay page for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah just took a look at the "Performance Monitoring" section again, it looks like Ryan is following the pattern of what's done there. The "Set Up" just includes a list of links to the SDK docs and that makes sense to me. I wouldn't want to clutter product docs w/ installation instructions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup! copy+pasted from Perf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will pull these Product pages out of this PR and do them in a separate PR
Based on #5903 we don't have a specific product pages drafted yet (maybe faq?)
<DynamicNav | ||
root={`/${pathRoot}/session-replay`} | ||
title="Session Replay" | ||
prependLinks={[ | ||
[`/${pathRoot}/session-replay/`, "Set Up Session Replay"], | ||
]} | ||
suppressMissing | ||
tree={tree} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sidebar_order: 1 | ||
redirect_from: | ||
- /session-replay/setup/ | ||
- /session-replay/getting-started/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To replace:
- /platforms/javascript/integrations/rrweb/ |
Would require removing this page from the docs on this PR:
- /platforms/javascript/integrations/rrweb/ |
/platforms/javascript/configuration/integrations/rrweb/
being the current path
and /platforms/javascript/integrations/rrweb/
the old one
- /session-replay/getting-started/ | |
- /session-replay/getting-started/ | |
- /platforms/javascript/integrations/rrweb/ | |
- /platforms/javascript/configuration/integrations/rrweb/ |
--- | ||
title: Set Up | ||
sidebar_order: 1 | ||
redirect_from: | ||
- /session-replay/setup/ | ||
- /session-replay/getting-started/ | ||
description: "Get started with Session Replay, TODO." | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine Set Up is mainly a client-side/SDK thing? If so, we could probably get by without the sub page to start? Basically add all content on the landing Session Replay page for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM so far, thanks for writing all of this up!
<!-- | ||
Note that `bundle.replay.min.js` contains both `@sentry/browser` AND | ||
`@sentry/replay`, and should therefore be used in place of | ||
`@sentry/browser`'s bundle rather than in addition to it. | ||
--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This bundle doesn't exist and the bundle that's going to be released is an addon bundle, containing only the replay integration. So to install the SDK plus Replay, users have to add the follwing bundles:
bundle(.tracing)?(.min)?.js
replay(.min)?.js
So, the setup is identical to how to use our other pluggable integrations.
I was going to add instructions to the README today. I can also update this example if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update - Here are the readme docs for the CDN bundle: https://github.com/getsentry/sentry-javascript/blob/master/packages/replay/README.md#loading-replay-as-a-cdn-bundle
|
||
## Supported SDKs | ||
|
||
- JavaScript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add the plain Browser SDK (@sentry/browser) here for users who don't rely on a framework
|
||
// This enables automatic instrumentation (highly recommended), but is not | ||
// necessary for purely manual usage | ||
integrations: [new Replay()], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll also need to add a CDN tab here, as the setup isn't 100% identical (the namespace for the integration is Sentry.Integrations.Replay
). As already said above, I can add this later if you want.
|
||
With [session replay](/product/session-replay/), TODO. | ||
|
||
<PlatformSection supported={["javascript", "javascript.vue", "java.spring", "react-native"]} notSupported={["java", "go", "android", "ruby", "python", "dotnet", "dart", "native"]}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have way more context here but just out of curioisity, java.spring
really supports Replay? Are they using the Browser SDK?
Also, do we need to explicilty add javascript.vue
or is it automatically included by javascript
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, this is all copy+paste that i missed. none of these are really supported
## Enable Session Replay | ||
|
||
<PlatformContent includePath="session-replay/enable-session-replay" /> | ||
|
||
</PlatformSection> | ||
|
||
## Configure the Sample Rate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should change the order here a little in the sense that it's most important for users to understand how to set up Replay. So under "Enable Session Replay" I think we should already show both code examples (installing the package and the Sentry.init
call).
Below then, we can describe the impact of the sample rates and (maybe later on) the other configuration options.
Side note (no relevant for this PR): I just saw that our docs for Performance setup have the same structure so we might wanna refactor it a bit there as well.
While you're testing, set <PlatformIdentifier name="replays-session-sample-rate" /> to `1.0`, as that ensures that every user session will be sent to Sentry. | ||
|
||
Once testing is complete, **we recommend lowering this value in production** by either lowering your <PlatformIdentifier name="replays-session-sample-rate" /> value. We still recommend keeping <PlatformIdentifier name="replays-on-error-sample-rate" /> set to `1.0`. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add another subsection here (e.g. ## Advanced Configuration
or something similar) and link to the Readme for the time being? This way, users can be made aware that there are additional configuration options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// This enables automatic instrumentation (highly recommended), but is not | ||
// necessary for purely manual usage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// This enables automatic instrumentation (highly recommended), but is not | |
// necessary for purely manual usage |
I would honestly just delete this to keep it as simple as possible for the majority of our users here. (see my comment about "Advanced Configuration", as we're describing more manual usage in the Readme).
5238924
to
c2b1d9c
Compare
### Ignoring | ||
Ignoring only applies to form inputs. Events will be ignored on the input element so that the replay does not show what occurs inside of the input. In the below example, notice how the results in the table below the input changes, but no text is visible in the input. | ||
|
||
 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't playing, it was originally a .mov
file, i renamed it because some googling suggested that... it didn't work
Next approach: convert it to gif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks for addressing my previous feedback. I just have two more comments but nothing blocking.
src/platform-includes/session-replay/install-session-replay/javascript.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/session-replay/setup-session-replay/javascript.mdx
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,24 @@ | |||
--- | |||
title: Custom Instrumentation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit (feel free to ignore): Is "Custom Instrumentation" the correct title here? To me, the content here seems to be more around additional advanced configuration than doing custom things (maybe apart from the manual Replay
start/stop section). No strong opinion here, though. We can also revisit this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Craft uses the release artifacts zip generated by our "Upload Artifacts" job to search for files to upload. Our bundle path (`packages/replay/build/bundles`) wasn't added to this job's config so it wasn't included in the release. This patch fixes that and it also adds an entry to our release checklist. It furthermore bumps the minimum version for the CDN bundles in the Replay README. Docs should automatically pick up the newest version, so we don't need to bump anything in getsentry/sentry-docs#5899 Co-authored-by: Abhijeet Prasad <[email protected]>
…script.mdx Co-authored-by: Lukas Stracke <[email protected]>
…vascript.mdx Co-authored-by: Lukas Stracke <[email protected]>
Resolves getsentry/sentry-replay#174
Relates to #5894
Relates to #5903