Skip to content

feat(performance): Add tracesSampleRate and tracesSampler to JS docs #2376

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

Merged
merged 13 commits into from
Sep 30, 2020

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented Sep 28, 2020

This PR adds docs for the tracesSampleRate and new tracesSampler option in the @sentry/browser family of SDKs.

In order to do this in a way that made sense, and in order not to create an enormously long page, some of the surrounding material needed to be reorganized and/or rewritten slightly. At a high level:

  • The tracesSampleRate and tracesSampler options were added to the common config options page. For the moment, I only listed "javascript" and "node" as supported platforms, even though support will be coming soon to both PHP and Python.

  • The Performance Monitoring index page was split into 4:

    • An index page, covering installation, enablement, and other miscellaneous topics which didn't fit elsewhere
    • A page on what's captured automatically, which basically means what's captured by integrations
    • A page on manual instrumentation
    • A page on sampling
  • The config page on filtering had material added about filtering transactions

In everything I did, I tried to do it in such a way that it could apply to both @sentry/browser and @sentry/node, using platform includes where the material would have differed for each. For the moment, until we figure out the node/browser JS platorm inheritance problem*, once everything is good to go I'll just copy the browser material to the node platform (in a separate PR).

*Specifically, that "javascript" (actually browser) and node are separate platforms, but really need to inherit most of their material from a common parent (but one that will never be common to any other platform, making the common folder not a great place for such stuff). A question for another day, however.

Because of limited time resources, I didn't do a full audit/edit/wordsmithing job on the pages I worked on, though I think at some point they could benefit from it. Instead, I tried to touch or not touch discrete sections, in hopes that it might be somewhat easier to do that full pass later (whether it's me or someone else who does it). Specifically:

  • Performance Monitoring index page

    • Added introductory section on enablement, since that's facilitated by the two options I was documenting
    • Moved the text about the BrowserTracing integration to the automatic capturing page
    • Moved the text about manual instrumentation to the manual capturing page
    • Reworked the Connecting Frontend to Backend section to include discussion of the sentry-trace header
    • Not edited:
      • Retrieving an Active Transaction
      • Adding Query Information and Parameters to Spans
      • Grouping Transactions
  • Automatic Capturing page

    • Moved content from index page
    • Otherwise unedited
  • Manual Capturing page

    • Moved content from index page
    • Otherwise unedited
  • Sampling page

    • Entirely new content
    • Two new includes, performance/default-sampling-context and performance/custom-sampling-context, along with the requisite snippets for both javascript (browser) and node
  • Filtering config page

    • Added Sampling to title
    • Split page into four sections:
      • Filtering Error Events - otherwise unedited
      • Sampling Error Events - rewritten slightly
      • Filtering Transaction events - new content, only for javascript (browser) and node, for the moment links straight to browser performance sampling docs (since node docs don't exist yet)
      • Sampling Transactions - unedited except to add a paragraph at the end telling the user how to do it

The only other thing that happened here (other than sporadic language tweaks) is that everywhere we include tracesSampleRate in a code snippet, with the comment Be sure to lower this in production, I added a mention of the tracesSampler option.

P.S. I tried to keep the commits pretty atomic, since I know there's a lot here.

@vercel
Copy link

vercel bot commented Sep 28, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sentry/sentry-docs/5jpx3er1z/sentry.dev
✅ Preview: https://sentry-docs-git-kmclb-tracessampler-js-docs.sentry.dev

@lobsterkatie lobsterkatie requested review from PeloWriter and a team September 28, 2020 19:29
Copy link
Contributor

@PeloWriter PeloWriter left a comment

Choose a reason for hiding this comment

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

Lots of edits, but primarily due to change around our language for lowering the rate in production ( which I'm very glad you captured all.of.them in this PR - makes the language update easier and more consistent.

I think the primary Performance Monitoring page here might be better if it was a primary page, with subpages of capturing xns manually and automatically. Also, that page is titled "Enabling Tracing" in the sidebar, but the file still includes the name "Performance Monitoring". I am uncertain that using the title "Enabling Tracing" is better than the current name, since we refer to the tracing package as enabling Performance Monitoring.

Thank you for all this work.

Copy link
Contributor

@rhcarvalho rhcarvalho left a comment

Choose a reason for hiding this comment

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

The aspects related to tracesSampleRate and tracesSampler LGTM. Thanks @lobsterkatie! And thanks for all the accompanying grammar fixes and restructuring.

@lobsterkatie lobsterkatie requested a review from a team as a code owner September 30, 2020 20:53
@vercel vercel bot temporarily deployed to Preview September 30, 2020 20:53 Inactive
@lobsterkatie lobsterkatie force-pushed the kmclb-tracessampler-js-docs branch from ec1a1fb to b5c826c Compare September 30, 2020 20:59
@lobsterkatie lobsterkatie merged commit 7e621c2 into master Sep 30, 2020
@lobsterkatie lobsterkatie deleted the kmclb-tracessampler-js-docs branch September 30, 2020 21:24
ajjindal pushed a commit that referenced this pull request Sep 30, 2020
…ocs (#2376)

Also a bunch of reorganization, and reworking of some (but not all) of the JS performance docs.
NisanthanNanthakumar pushed a commit that referenced this pull request Oct 1, 2020
…ocs (#2376)

Also a bunch of reorganization, and reworking of some (but not all) of the JS performance docs.
@github-actions github-actions bot locked and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants