Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
7 changes: 6 additions & 1 deletion packages/browser/src/tracing/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
startTrackingLongTasks,
startTrackingWebVitals,
} from '@sentry-internal/browser-utils';
import type { Client, IntegrationFn, Span, StartSpanOptions, TransactionSource } from '@sentry/core';
import type { Client, IntegrationFn, Span, StartSpanOptions, TransactionSource, WebFetchHeaders } from '@sentry/core';
import {
GLOBAL_OBJ,
SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON,
Expand Down Expand Up @@ -195,6 +195,11 @@ export interface BrowserTracingOptions {
* Default: (url: string) => true
*/
shouldCreateSpanForRequest?(this: void, url: string): boolean;

/**
* Is called when spans are started for outgoing requests.
*/
onRequestSpanStart(span: Span, requestInformation: { headers?: WebFetchHeaders }): void;
}

const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
Expand Down
37 changes: 28 additions & 9 deletions packages/browser/src/tracing/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
extractNetworkProtocol,
} from '@sentry-internal/browser-utils';
import type { XhrHint } from '@sentry-internal/browser-utils';
import type { Client, HandlerDataXhr, SentryWrappedXMLHttpRequest, Span } from '@sentry/core';
import type { Client, HandlerDataXhr, SentryWrappedXMLHttpRequest, Span, WebFetchHeaders } from '@sentry/core';
import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
Expand Down Expand Up @@ -98,6 +98,11 @@ export interface RequestInstrumentationOptions {
* Default: (url: string) => true
*/
shouldCreateSpanForRequest?(this: void, url: string): boolean;

/**
* Is called when spans are started for outgoing requests.
*/
onRequestSpanStart(span: Span, requestInformation: { headers?: WebFetchHeaders }): void;
}

const responseToSpanId = new WeakMap<object, string>();
Expand All @@ -108,6 +113,9 @@ export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions
traceXHR: true,
enableHTTPTimings: true,
trackFetchStreamPerformance: false,
onRequestSpanStart() {
// noop
},
};

/** Registers span creators for xhr and fetch requests */
Expand All @@ -119,10 +127,9 @@ export function instrumentOutgoingRequests(client: Client, _options?: Partial<Re
shouldCreateSpanForRequest,
enableHTTPTimings,
tracePropagationTargets,
onRequestSpanStart,
} = {
traceFetch: defaultRequestInstrumentationOptions.traceFetch,
traceXHR: defaultRequestInstrumentationOptions.traceXHR,
trackFetchStreamPerformance: defaultRequestInstrumentationOptions.trackFetchStreamPerformance,
...defaultRequestInstrumentationOptions,
..._options,
};

Expand Down Expand Up @@ -179,19 +186,31 @@ export function instrumentOutgoingRequests(client: Client, _options?: Partial<Re
'http.url': fullUrl,
'server.address': host,
});
}

if (enableHTTPTimings && createdSpan) {
addHTTPTimings(createdSpan);
if (enableHTTPTimings) {
addHTTPTimings(createdSpan);
}

onRequestSpanStart(createdSpan, { headers: handlerData.headers });
}
});
}

if (traceXHR) {
addXhrInstrumentationHandler(handlerData => {
const createdSpan = xhrCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans);
if (enableHTTPTimings && createdSpan) {
addHTTPTimings(createdSpan);
if (createdSpan) {
if (enableHTTPTimings) {
addHTTPTimings(createdSpan);
}

let headers;
try {
headers = new Headers(handlerData.xhr.__sentry_xhr_v3__?.request_headers);
} catch {
// noop
}
onRequestSpanStart(createdSpan, { headers });
}
});
}
Expand Down
6 changes: 1 addition & 5 deletions packages/core/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { SPAN_STATUS_ERROR, setHttpStatus, startInactiveSpan } from './tracing';
import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan';
import type { FetchBreadcrumbHint, HandlerDataFetch, Span, SpanAttributes, SpanOrigin } from './types-hoist';
import { SENTRY_BAGGAGE_KEY_PREFIX } from './utils-hoist/baggage';
import { isInstanceOf } from './utils-hoist/is';
import { isInstanceOf, isRequest } from './utils-hoist/is';
import { getSanitizedUrlStringFromUrlObject, isURLObjectRelative, parseStringToURLObject } from './utils-hoist/url';
import { hasSpansEnabled } from './utils/hasSpansEnabled';
import { getActiveSpan } from './utils/spanUtils';
Expand Down Expand Up @@ -227,10 +227,6 @@ function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string
);
}

function isRequest(request: unknown): request is Request {
return typeof Request !== 'undefined' && isInstanceOf(request, Request);
}

function isHeaders(headers: unknown): headers is Headers {
return typeof Headers !== 'undefined' && isInstanceOf(headers, Headers);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/types-hoist/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ export interface HandlerDataFetch {
error?: unknown;
// This is to be consumed by the HttpClient integration
virtualError?: unknown;
/** Headers that the user passed to the fetch request. */
headers?: WebFetchHeaders;
}

export interface HandlerDataDom {
Expand Down
28 changes: 26 additions & 2 deletions packages/core/src/utils-hoist/instrument/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import type { HandlerDataFetch } from '../../types-hoist';
import type { HandlerDataFetch, WebFetchHeaders } from '../../types-hoist';

import { isError } from '../is';
import { isError, isRequest } from '../is';
import { addNonEnumerableProperty, fill } from '../object';
import { supportsNativeFetch } from '../supports';
import { timestampInSeconds } from '../time';
Expand Down Expand Up @@ -67,6 +67,7 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNat
startTimestamp: timestampInSeconds() * 1000,
// // Adding the error to be able to fingerprint the failed fetch event in HttpClient instrumentation
virtualError,
headers: getHeadersFromFetchArgs(args),
};

// if there is no callback, fetch is instrumented directly
Expand Down Expand Up @@ -253,3 +254,26 @@ export function parseFetchArgs(fetchArgs: unknown[]): { method: string; url: str
method: hasProp(arg, 'method') ? String(arg.method).toUpperCase() : 'GET',
};
}

function getHeadersFromFetchArgs(fetchArgs: unknown[]): WebFetchHeaders | undefined {
const [requestArgument, optionsArgument] = fetchArgs;

try {
if (
typeof optionsArgument === 'object' &&
optionsArgument !== null &&
'headers' in optionsArgument &&
optionsArgument.headers
) {
return new Headers(optionsArgument.headers as any);
}

if (isRequest(requestArgument)) {
return new Headers(requestArgument.headers);
}
} catch {
// noop
}

return;
}
9 changes: 9 additions & 0 deletions packages/core/src/utils-hoist/is.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,3 +201,12 @@ export function isVueViewModel(wat: unknown): boolean {
// Not using Object.prototype.toString because in Vue 3 it would read the instance's Symbol(Symbol.toStringTag) property.
return !!(typeof wat === 'object' && wat !== null && ((wat as VueViewModel).__isVue || (wat as VueViewModel)._isVue));
}

/**
* Checks whether the given parameter is a Standard Web API Request instance.
*
* Returns false if Request is not available in the current runtime.
*/
export function isRequest(request: unknown): request is Request {
return typeof Request !== 'undefined' && isInstanceOf(request, Request);
}
11 changes: 11 additions & 0 deletions packages/nextjs/src/client/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Integration } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_OP } from '@sentry/core';
import { browserTracingIntegration as originalBrowserTracingIntegration } from '@sentry/react';
import { nextRouterInstrumentNavigation, nextRouterInstrumentPageLoad } from './routing/nextRoutingInstrumentation';

Expand All @@ -12,6 +13,16 @@ export function browserTracingIntegration(
...options,
instrumentNavigation: false,
instrumentPageLoad: false,
onRequestSpanStart(...args) {
Copy link
Member

Choose a reason for hiding this comment

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

l: do we care about cases where users register their own browserTracingIntegration()? As in should we merge a possible custom implementation with our logic? Not sure what our general approach to this is so feel free to declare undefined behaviour :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think user's would be always importing this browserTracingIntegration (the Next.js one) so the behavior should already be merged (see line 23). Unless I am misunderstanding.

const [span, { headers }] = args;

// Next.js prefetch requests have a `next-router-prefetch` header
if (headers?.get('next-router-prefetch')) {
span?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.client.prefetch');
}
Copy link
Member

@Lms24 Lms24 Apr 4, 2025

Choose a reason for hiding this comment

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

Q: Have we checked if this change plays well with the product? Specifically I'm thinking of the "Network Requests" insights module
image

wondering if http.client.prefetch spans would still count towards the http.client request spans tracked in this module.

Also, might be worth adding the op to dev docs, wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very good point getsentry/sentry-docs#13239

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on an internal discussion we have agreed to instead add a http.request.prefetch: true attribute to the requests.


return options.onRequestSpanStart?.(...args);
},
});

const { instrumentPageLoad = true, instrumentNavigation = true } = options;
Expand Down
Loading