Skip to content

fix: Update JS Docs #1556

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 10 commits into from
Mar 23, 2020
70 changes: 60 additions & 10 deletions src/collections/_documentation/performance/distributed-tracing.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,16 +154,16 @@ def process_item(item):

### JavaScript

To access our tracing features, you will need to install our Tracing integration:
To access our tracing features, you will need to install our Tracing package `@sentry/apm`:

```bash
$ npm install @sentry/browser
$ npm install @sentry/apm
```

**Sending Traces**
**Sending Traces/Transactions/Spans**

Tracing resides in the `@sentry/apm` package. You can add it to your `Sentry.init` call:
The `Tracing` integration resides in the `@sentry/apm` package. You can add it to your `Sentry.init` call:

```javascript
import * as Sentry from '@sentry/browser';
Expand All @@ -174,6 +174,7 @@ Sentry.init({
integrations: [
new ApmIntegrations.Tracing(),
],
tracesSampleRate: 0.1,
});
```

Expand All @@ -186,10 +187,9 @@ import { Integrations as ApmIntegrations } from '@sentry/apm';
Sentry.init({
dsn: '___PUBLIC_DSN___',
integrations: [
new Integrations.Tracing({
tracesSampleRate: 0.1,
}),
new ApmIntegrations.Tracing(),
],
tracesSampleRate: 0.1,
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

This will be fixed in the next update, should have never been there in the first place.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. Thanks for clarifying 👍

});
```

Expand All @@ -198,6 +198,22 @@ You can pass many different options to tracing, but it comes with reasonable def

- A [transaction]({%- link _documentation/performance/performance-glossary.md -%}#transaction) for every page load
- All XHR/fetch requests as spans
- If available: Browser Resources (Scripts, CSS, Images ...)
- If available: Browser Performance API Marks

*tracingOrigins Option*

The default value of `tracingOrigins` is : `new ApmIntegrations.Tracing({tracingOrigins: ['localhost', /^\//]})`. The JavaScript SDK will attach the `sentry-trace` header to all outgoing XHR/fetch requests that contain a string in the list or match a regex. In case your frontend is making requests to a different domain you might want to add it there in order to propagate the `sentry-trace` header to the backend services, required to link transactions together as part of a single trace.

*Example:*

- A frontend application is served from `example.com`
- A backend service is served from `api.example.com`
- The frontend application makes API calls to the backend
- Then the option need to be configured like this `new ApmIntegrations.Tracing({tracingOrigins: ['api.example.com']})`
- Outgoing XHR/fetch requests to `api.example.com` would get the `sentry-trace` header attached

*NOTE:* You need to make sure your web server CORS is configured to allow the `sentry-trace` header. The configuration might look like this: `"Access-Control-Allow-Headers: sentry-trace"`, this depends a lot on your configuration. But if you are not whitelisting the `sentry-trace` header the request might be blocked.

**Using Tracing Integration for Manual Instrumentation**

Expand All @@ -210,24 +226,58 @@ The tracing integration will create a [transaction]({%- link _documentation/perf
const activity = ApmIntegrations.Tracing.pushActivity(displayName, {
data: {},
op: 'react',
description: `<${displayName}>`,
description: `${displayName}`,
});

// Sometime later ...
// When we pop the activity, the Integration will finish the span and after the timeout finish the transaction and send it to Sentry
Integrations.ApmIntegrations.popActivity(activity);
// Keep in mind, as long as you do not pop the activity, the transaction will be kept alive and not sent to Sentry
Comment on lines 233 to +234
Copy link
Contributor

@rhcarvalho rhcarvalho Mar 20, 2020

Choose a reason for hiding this comment

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

We ended up with two sentences that perhaps could be combined together to better convey the important information? ("When we pop... send it to Sentry" & "Keep in mind, ... sent to Sentry")

ApmIntegrations.Tracing.popActivity(activity);
```

Keep in mind, if there is no active transaction, you need to create one before pushing an activity otherwise nothing will happen.
So given a different example where you want to create an Transaction for a user interaction on you page you need to do the following:


```javascript
// Let's say this function is invoked when a user clicks on the checkout button of your shop
shopCheckout() {
// This will create a new Transaction for you
ApmIntegrations.Tracing.startIdleTransaction('shopCheckout');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why here we use ApmIntegrations.Tracing.startIdleTransaction instead of hub.startSpan?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because we prefer ppl using the Integration over the startSpan
The integration also adds spans for XHR/fetch


const result = validateShoppingCartOnServer(); // Consider this function making an xhr/fetch call
const activity = ApmIntegrations.Tracing.pushActivity('task', {
data: {
result
},
op: 'task',
Comment on lines +249 to +253
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need a name (first arg) for an Activity? Why would people need to write an activity name and then op (which is also some sort of activity/operation name)?!

Copy link
Member Author

Choose a reason for hiding this comment

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

It's there if you want to have an activity that is "blocking" the idle transaction but you do not want to create a span out of it.

description: `processing shopping cart result`,
});
processAndValidateShoppingCart(result);
ApmIntegrations.Tracing.popActivity(activity);

ApmIntegrations.Tracing.finishIdleTransaction(); // This is optional
}
```

This example will send a [transaction]({%- link _documentation/performance/performance-glossary.md -%}#transaction) `shopCheckout` to Sentry, containing all outgoing requests that might have happened in `validateShoppingCartOnServer`. It also contains a `task` span that measured how long the processing took. Finally the call to `ApmIntegrations.Tracing.finshIdleTransaction()` will finish the [transaction]({%- link _documentation/performance/performance-glossary.md -%}#transaction) and send it to Sentry. Calling this is optional since the Integration would send the [transaction]({%- link _documentation/performance/performance-glossary.md -%}#transaction) after the defined `idleTimeout` (default 500ms) itself.

**What is an activity?**

The concept of an activity only exists in the `Tracing` Integration in JavaScript Browser. It's a helper that tells the Integration for how long it should keep the `IdleTransaction` alive. An activity can be pushed and popped. Once all activities of an `IdleTransaction` have been popped, the `IdleTransaction` will be sent to Sentry.

**Manual Transactions**

To manually instrument certain regions of your code, you can create a [transaction]({%- link _documentation/performance/performance-glossary.md -%}#transaction) to capture them.
This is both valid for JavaScript Browser and Node and works besides the `Tracing` integration.

The following example creates a transaction for a scope that contains an expensive operation (for example, `process_item`), and sends the result to Sentry:

```javascript
const transaction = Sentry.getCurrentHub().startSpan({
description: 'My optional description',
op: "task",
transaction: item.getTransaction()
transaction: item.getTransaction() // A name describing the operation
})

// processItem may create more spans internally (see next example)
Expand Down Expand Up @@ -280,7 +330,7 @@ const Apm = require("@sentry/apm"); // This is required since it patches functio

Sentry.init({
dsn: "___PUBLIC_DSN___",
tracesSampleRate: 0.1
tracesSampleRate: 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Why change it to 1?

Copy link
Member

Choose a reason for hiding this comment

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

Daniel, we cannot allow anybody to send 100% of their traffic to us. This burns immediately through their quota as long as the new billing is not live.

Copy link
Member Author

Choose a reason for hiding this comment

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

I leave it in the responsibility of the user to set the correct value here.
1 = they see it works
0.1 = they write into support it's not working

Copy link
Member

Choose a reason for hiding this comment

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

Okay, can we then add a comment here that they should be careful in increasing this towards 1 in prod? 1 makes total sense in dev.

Copy link
Member

Choose a reason for hiding this comment

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

FWIW this is also inconsistent with other code examples.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we then add a comment here that they should be careful in increasing this towards 1 in prod? 1 makes total sense in dev.

+1

Maybe nobody cares, but I would also find 1.0 more indicative that this is not a 0 or 1 setting, but that typical values are floats in the [0.0, 1.0] range: 0.0, 0.1, 0.25...

Copy link
Contributor

Choose a reason for hiding this comment

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

@HazAT
maybe keep it at 1.0 but then put a comment before or after the code snippet that says,
"We recommend 0.1 for when you put this to production [because...]"

maybe keep it at 0.1 but then put a comment before or after the code snippet that says,
"You may want this on 1.0 if you're testing it for the first time"
"You may want this on 1.0 if you're testing it for the first time or else you might think your transactions aren't coming through"

you're 100% correct about them writing into support, as I tricked myself on this the first time I set it up, and then again a month later when I re-visisted it. So that makes me prefer to see it at 1.0 by default.

});
```

Expand Down