Skip to content

Commit

Permalink
feat(tracing): Bring http timings out of experiment (#8563)
Browse files Browse the repository at this point in the history
Co-authored-by: Abhijeet Prasad <[email protected]>
  • Loading branch information
k-fish and AbhiPrasad committed Jul 17, 2023
1 parent c2bd091 commit 564af01
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 18 deletions.
7 changes: 3 additions & 4 deletions packages/tracing-internal/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
_experiments: Partial<{
enableLongTask: boolean;
enableInteractions: boolean;
enableHTTPTimings: boolean;
onStartRouteTransaction: (t: Transaction | undefined, ctx: TransactionContext, getCurrentHub: () => Hub) => void;
}>;

Expand Down Expand Up @@ -140,6 +139,7 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
startTransactionOnLocationChange: true,
startTransactionOnPageLoad: true,
enableLongTask: true,
_experiments: {},
...defaultRequestInstrumentationOptions,
};

Expand Down Expand Up @@ -230,6 +230,7 @@ export class BrowserTracing implements Integration {
traceFetch,
traceXHR,
shouldCreateSpanForRequest,
enableHTTPTimings,
_experiments,
} = this.options;

Expand Down Expand Up @@ -277,9 +278,7 @@ export class BrowserTracing implements Integration {
traceXHR,
tracePropagationTargets,
shouldCreateSpanForRequest,
_experiments: {
enableHTTPTimings: _experiments.enableHTTPTimings,
},
enableHTTPTimings,
});
}

Expand Down
31 changes: 19 additions & 12 deletions packages/tracing-internal/src/browser/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@ export const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^\/(?!\/)/];

/** Options for Request Instrumentation */
export interface RequestInstrumentationOptions {
/**
* Allow experiments for the request instrumentation.
*/
_experiments: Partial<{
enableHTTPTimings: boolean;
}>;

/**
* @deprecated Will be removed in v8.
* Use `shouldCreateSpanForRequest` to control span creation and `tracePropagationTargets` to control
Expand Down Expand Up @@ -52,6 +45,13 @@ export interface RequestInstrumentationOptions {
*/
traceXHR: boolean;

/**
* If true, Sentry will capture http timings and add them to the corresponding http spans.
*
* Default: true
*/
enableHTTPTimings: boolean;

/**
* This function will be called before creating a span for a request with the given url.
* Return false if you don't want a span for the given url.
Expand Down Expand Up @@ -114,16 +114,23 @@ type PolymorphicRequestHeaders =
export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions = {
traceFetch: true,
traceXHR: true,
enableHTTPTimings: true,
// TODO (v8): Remove this property
tracingOrigins: DEFAULT_TRACE_PROPAGATION_TARGETS,
tracePropagationTargets: DEFAULT_TRACE_PROPAGATION_TARGETS,
_experiments: {},
};

/** Registers span creators for xhr and fetch requests */
export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumentationOptions>): void {
// eslint-disable-next-line deprecation/deprecation
const { traceFetch, traceXHR, tracePropagationTargets, tracingOrigins, shouldCreateSpanForRequest, _experiments } = {
const {
traceFetch,
traceXHR,
tracePropagationTargets,
// eslint-disable-next-line deprecation/deprecation
tracingOrigins,
shouldCreateSpanForRequest,
enableHTTPTimings,
} = {
traceFetch: defaultRequestInstrumentationOptions.traceFetch,
traceXHR: defaultRequestInstrumentationOptions.traceXHR,
..._options,
Expand All @@ -143,7 +150,7 @@ export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumenta
if (traceFetch) {
addInstrumentationHandler('fetch', (handlerData: FetchData) => {
const createdSpan = fetchCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans);
if (_experiments?.enableHTTPTimings && createdSpan) {
if (enableHTTPTimings && createdSpan) {
addHTTPTimings(createdSpan);
}
});
Expand All @@ -152,7 +159,7 @@ export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumenta
if (traceXHR) {
addInstrumentationHandler('xhr', (handlerData: XHRData) => {
const createdSpan = xhrCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans);
if (_experiments?.enableHTTPTimings && createdSpan) {
if (enableHTTPTimings && createdSpan) {
addHTTPTimings(createdSpan);
}
});
Expand Down
6 changes: 4 additions & 2 deletions packages/tracing-internal/test/browser/browsertracing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ conditionalTest({ min: 10 })('BrowserTracing', () => {

expect(browserTracing.options).toEqual({
enableLongTask: true,
_experiments: {},
...TRACING_DEFAULTS,
markBackgroundTransactions: true,
routingInstrumentation: instrumentRoutingWithDefaults,
Expand Down Expand Up @@ -132,6 +133,7 @@ conditionalTest({ min: 10 })('BrowserTracing', () => {

expect(browserTracing.options).toEqual({
enableLongTask: false,
_experiments: {},
...TRACING_DEFAULTS,
markBackgroundTransactions: true,
routingInstrumentation: instrumentRoutingWithDefaults,
Expand Down Expand Up @@ -246,7 +248,7 @@ conditionalTest({ min: 10 })('BrowserTracing', () => {
traceFetch: true,
traceXHR: true,
tracePropagationTargets: ['something'],
_experiments: {},
enableHTTPTimings: true,
});
});

Expand All @@ -260,7 +262,7 @@ conditionalTest({ min: 10 })('BrowserTracing', () => {
});

expect(instrumentOutgoingRequestsMock).toHaveBeenCalledWith({
_experiments: {},
enableHTTPTimings: true,
traceFetch: true,
traceXHR: true,
tracePropagationTargets: ['something-else'],
Expand Down

0 comments on commit 564af01

Please sign in to comment.