Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
feat(tracing): Add tracing without performance to browser and client
SvelteKit
  • Loading branch information
AbhiPrasad committed Jul 5, 2023
commit 605825b0149ef1ac4ccc676ee9f5d2b4bd2f3f30
26 changes: 13 additions & 13 deletions packages/sveltekit/src/client/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch
return originalFetch;
}

const options = client.getOptions();

const browserTracingIntegration = client.getIntegrationById('BrowserTracing') as BrowserTracing | undefined;
const breadcrumbsIntegration = client.getIntegrationById('Breadcrumbs') as Breadcrumbs | undefined;

Expand All @@ -147,7 +149,10 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch
const shouldAttachHeaders: (url: string) => boolean = url => {
return (
!!shouldTraceFetch &&
stringMatchesSomePattern(url, browserTracingOptions.tracePropagationTargets || ['localhost', /^\//])
stringMatchesSomePattern(
url,
options.tracePropagationTargets || browserTracingOptions.tracePropagationTargets || ['localhost', /^\//],
)
);
};

Expand Down Expand Up @@ -177,20 +182,15 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch
};

const patchedInit: RequestInit = { ...init };
const activeSpan = getCurrentHub().getScope().getSpan();
const activeTransaction = activeSpan && activeSpan.transaction;

const createSpan = activeTransaction && shouldCreateSpan(rawUrl);
const attachHeaders = createSpan && activeTransaction && shouldAttachHeaders(rawUrl);

// only attach headers if we should create a span
if (attachHeaders) {
const dsc = activeTransaction.getDynamicSamplingContext();
const hub = getCurrentHub();
const scope = hub.getScope();
const client = hub.getClient();

if (client && shouldAttachHeaders(rawUrl)) {
const headers = addTracingHeadersToFetchRequest(
input as string | Request,
dsc,
activeSpan,
client,
scope,
patchedInit as {
headers:
| {
Expand All @@ -207,7 +207,7 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch

const patchedFetchArgs = [input, patchedInit];

if (createSpan) {
if (shouldCreateSpan(rawUrl)) {
fetchPromise = trace(
{
name: `${method} ${requestData.url}`, // this will become the description of the span
Expand Down
12 changes: 7 additions & 5 deletions packages/sveltekit/test/client/load.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const mockedGetIntegrationById = vi.fn(id => {
const mockedGetClient = vi.fn(() => {
return {
getIntegrationById: mockedGetIntegrationById,
getOptions: () => ({}),
};
});

Expand All @@ -77,6 +78,11 @@ vi.mock('@sentry/core', async () => {
getClient: mockedGetClient,
getScope: () => {
return {
getPropagationContext: () => ({
traceId: '1234567890abcdef1234567890abcdef',
spanId: '1234567890abcdef',
sampled: false,
}),
getSpan: () => {
return {
transaction: {
Expand Down Expand Up @@ -371,7 +377,7 @@ describe('wrapLoadWithSentry', () => {
mockedBrowserTracing.options.traceFetch = true;
});

it("doesn't create a span nor propagate headers, if `shouldCreateSpanForRequest` returns false", async () => {
it("doesn't create a span if `shouldCreateSpanForRequest` returns false", async () => {
mockedBrowserTracing.options.shouldCreateSpanForRequest = () => false;

const wrappedLoad = wrapLoadWithSentry(load);
Expand All @@ -391,10 +397,6 @@ describe('wrapLoadWithSentry', () => {
expect.any(Function),
);

expect(mockedSveltekitFetch).toHaveBeenCalledWith(
...[originalFetchArgs[0], originalFetchArgs.length === 2 ? originalFetchArgs[1] : {}],
);

mockedBrowserTracing.options.shouldCreateSpanForRequest = () => true;
});

Expand Down
88 changes: 53 additions & 35 deletions packages/tracing-internal/src/browser/request.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
/* eslint-disable max-lines */
import { getCurrentHub, hasTracingEnabled } from '@sentry/core';
import type { DynamicSamplingContext, Span } from '@sentry/types';
import { getCurrentHub, getDynamicSamplingContextFromClient, hasTracingEnabled } from '@sentry/core';
import type { Client, DynamicSamplingContext, Hub, Scope, Span } from '@sentry/types';
import {
addInstrumentationHandler,
BAGGAGE_HEADER_NAME,
browserPerformanceTimeOrigin,
dynamicSamplingContextToSentryBaggageHeader,
generateSentryTraceHeader,
isInstanceOf,
SENTRY_XHR_DATA_KEY,
stringMatchesSomePattern,
Expand Down Expand Up @@ -219,12 +220,14 @@ export function fetchCallback(
shouldCreateSpan: (url: string) => boolean,
shouldAttachHeaders: (url: string) => boolean,
spans: Record<string, Span>,
): Span | void {
if (!hasTracingEnabled() || !(handlerData.fetchData && shouldCreateSpan(handlerData.fetchData.url))) {
return;
): Span | undefined {
if (!hasTracingEnabled() || !handlerData.fetchData) {
return undefined;
}

if (handlerData.endTimestamp) {
const shouldCreateSpanResult = shouldCreateSpan(handlerData.fetchData.url);

if (handlerData.endTimestamp && shouldCreateSpanResult) {
const spanId = handlerData.fetchData.__span;
if (!spanId) return;

Expand All @@ -251,27 +254,35 @@ export function fetchCallback(
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
delete spans[spanId];
}
return;
return undefined;
}

const currentSpan = getCurrentHub().getScope().getSpan();
const activeTransaction = currentSpan && currentSpan.transaction;

if (currentSpan && activeTransaction) {
const { method, url } = handlerData.fetchData;
const span = currentSpan.startChild({
data: {
url,
type: 'fetch',
'http.method': method,
},
description: `${method} ${url}`,
op: 'http.client',
});

const hub = getCurrentHub();
const scope = hub.getScope();
const client = hub.getClient();
const parentSpan = scope.getSpan();

const { method, url } = handlerData.fetchData;

const span =
shouldCreateSpanResult && parentSpan
? parentSpan.startChild({
data: {
url,
type: 'fetch',
'http.method': method,
},
description: `${method} ${url}`,
op: 'http.client',
})
: undefined;

if (span) {
handlerData.fetchData.__span = span.spanId;
spans[span.spanId] = span;
}

if (shouldAttachHeaders(handlerData.fetchData.url) && client) {
const request: string | Request = handlerData.args[0];

// In case the user hasn't set the second argument of a fetch call we default it to `{}`.
Expand All @@ -280,35 +291,42 @@ export function fetchCallback(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const options: { [key: string]: any } = handlerData.args[1];

if (shouldAttachHeaders(handlerData.fetchData.url)) {
options.headers = addTracingHeadersToFetchRequest(
request,
activeTransaction.getDynamicSamplingContext(),
span,
options,
);
}
return span;
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
options.headers = addTracingHeadersToFetchRequest(request, client, scope, options);
}

return span;
}

/**
* Adds sentry-trace and baggage headers to the various forms of fetch headers
*/
export function addTracingHeadersToFetchRequest(
request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package,
dynamicSamplingContext: Partial<DynamicSamplingContext>,
span: Span,
client: Client,
scope: Scope,
options: {
headers?:
| {
[key: string]: string[] | string | undefined;
}
| PolymorphicRequestHeaders;
},
): PolymorphicRequestHeaders {
): PolymorphicRequestHeaders | undefined {
const span = scope.getSpan();

const transaction = span && span.transaction;

const { traceId, sampled, dsc } = scope.getPropagationContext();

const sentryTraceHeader = span ? span.toTraceparent() : generateSentryTraceHeader(traceId, undefined, sampled);
const dynamicSamplingContext = transaction
? transaction.getDynamicSamplingContext()
: dsc
? dsc
: getDynamicSamplingContextFromClient(traceId, client, scope);

const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext);
const sentryTraceHeader = span.toTraceparent();

const headers =
typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : options.headers;
Expand Down
4 changes: 1 addition & 3 deletions packages/tracing-internal/test/browser/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,7 @@ describe('callbacks', () => {
// each case is [shouldCreateSpanReturnValue, shouldAttachHeadersReturnValue, expectedSpan, expectedHeaderKeys]
[true, true, expect.objectContaining(fetchSpan), ['sentry-trace', 'baggage']],
[true, false, expect.objectContaining(fetchSpan), []],
// If there's no span then there's no parent span id to stick into a header, so no headers, even if there's a
// `tracingOrigins` match
[false, true, undefined, []],
[false, true, undefined, ['sentry-trace', 'baggage']],
[false, false, undefined, []],
])(
'span creation/header attachment interaction - shouldCreateSpan: %s, shouldAttachHeaders: %s',
Expand Down