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.

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

feat: Pick up sentry-trace in JS <meta/> tag #2703

Merged
merged 3 commits into from Jun 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -4,6 +4,9 @@

- "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott

- [tracing] feat: Pick up sentry-trace in JS <meta/> tag (#2703)
- [tracing] fix: Respect sample decision when continuing trace from header in node (#2703)

## 5.18.1

- [react] feat: Update peer dependencies for `react` and `react-dom` (#2694)
Expand Down
43 changes: 39 additions & 4 deletions packages/apm/src/integrations/tracing.ts
Expand Up @@ -283,6 +283,44 @@ export class Tracing implements Integration {
});
}

/**
* Returns a new Transaction either continued from sentry-trace meta or a new one
*/
private static _getNewTransaction(hub: Hub, transactionContext: TransactionContext): Transaction {
let traceId;
let parentSpanId;
let sampled;

const header = Tracing._getMeta('sentry-trace');
Copy link
Member

Choose a reason for hiding this comment

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

So I guess now if someone sets this in an SPA, all transactions would use this meta header right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. For me this is opt-in behavior enough, this can't happen by accident so guarding this behind an option in the integration doesn't make a lot of sense to me.

if (header) {
const span = SpanClass.fromTraceparent(header);
if (span) {
traceId = span.traceId;
parentSpanId = span.parentSpanId;
sampled = span.sampled;
Tracing._log(
`[Tracing] found 'sentry-meta' '<meta />' continuing trace with: trace_id: ${traceId} span_id: ${parentSpanId}`,
);
}
}

return hub.startTransaction({
parentSpanId,
sampled,
traceId,
trimEnd: true,
...transactionContext,
}) as Transaction;
}

/**
* Returns the value of a meta tag
*/
private static _getMeta(metaName: string): string | null {
const el = document.querySelector(`meta[name=${metaName}]`);
return el ? el.getAttribute('content') : null;
}

/**
* Pings the heartbeat
*/
Expand Down Expand Up @@ -454,10 +492,7 @@ export class Tracing implements Integration {
return undefined;
}

Tracing._activeTransaction = hub.startTransaction({
trimEnd: true,
...transactionContext,
}) as Transaction;
Tracing._activeTransaction = Tracing._getNewTransaction(hub, transactionContext);

// We set the transaction here on the scope so error events pick up the trace context and attach it to the error
hub.configureScope(scope => scope.setSpan(Tracing._activeTransaction));
Expand Down
3 changes: 3 additions & 0 deletions packages/node/src/handlers.ts
Expand Up @@ -34,6 +34,7 @@ export function tracingHandler(): (

let traceId;
let parentSpanId;
let sampled;

// If there is a trace header set, we extract the data from it and set the span on the scope
// to be the origin an created transaction set the parent_span_id / trace_id
Expand All @@ -42,13 +43,15 @@ export function tracingHandler(): (
if (span) {
traceId = span.traceId;
parentSpanId = span.parentSpanId;
sampled = span.sampled;
Copy link
Member

Choose a reason for hiding this comment

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

This is a bug fix right? We should remember for the changelog then that there are two changes in this PR. a feat for the meta tag, and a fix for sampling from trace parent in node.

}
}

const transaction = startTransaction({
name: `${reqMethod} ${reqUrl}`,
op: 'http.server',
parentSpanId,
sampled,
traceId,
});

Expand Down