-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(tracing): Add experiment to capture http timings #8371
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
Changes from 1 commit
feb3b91
d8ebd86
1abbe42
12da836
6312fc1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import * as Sentry from '@sentry/browser'; | ||
import { Integrations } from '@sentry/tracing'; | ||
|
||
window.Sentry = Sentry; | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
integrations: [ | ||
new Integrations.BrowserTracing({ | ||
idleTimeout: 1000, | ||
_experiments: { | ||
enableHTTPTimings: true, | ||
}, | ||
}), | ||
], | ||
tracesSampleRate: 1, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
fetch('http://example.com/0').then(fetch('http://example.com/1').then(fetch('http://example.com/2'))); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
import { expect } from '@playwright/test'; | ||
import type { Event } from '@sentry/types'; | ||
|
||
import { sentryTest } from '../../../../utils/fixtures'; | ||
import { getMultipleSentryEnvelopeRequests, shouldSkipTracingTest } from '../../../../utils/helpers'; | ||
|
||
sentryTest('should create fetch spans with http timing', async ({ getLocalTestPath, page }) => { | ||
if (shouldSkipTracingTest()) { | ||
sentryTest.skip(); | ||
} | ||
await page.route('https://example.com/*', async route => { | ||
const request = route.request(); | ||
const postData = await request.postDataJSON(); | ||
|
||
await route.fulfill({ | ||
status: 200, | ||
contentType: 'application/json', | ||
body: JSON.stringify(Object.assign({ id: 1 }, postData)), | ||
}); | ||
}); | ||
|
||
const url = await getLocalTestPath({ testDir: __dirname }); | ||
|
||
const envelopes = await getMultipleSentryEnvelopeRequests<Event>(page, 2, { url, timeout: 10000 }); | ||
const tracingEvent = envelopes[envelopes.length - 1]; // last envelope contains tracing data on all browsers | ||
|
||
const requestSpans = tracingEvent.spans?.filter(({ op }) => op === 'http.client'); | ||
|
||
expect(requestSpans).toHaveLength(3); | ||
|
||
requestSpans?.forEach((span, index) => | ||
expect(span).toMatchObject({ | ||
description: `GET http://example.com/${index}`, | ||
parent_span_id: tracingEvent.contexts?.trace?.span_id, | ||
span_id: expect.any(String), | ||
start_timestamp: expect.any(Number), | ||
timestamp: expect.any(Number), | ||
trace_id: tracingEvent.contexts?.trace?.trace_id, | ||
data: expect.objectContaining({ | ||
'http.client.connectStart': expect.any(Number), | ||
'http.client.requestStart': expect.any(Number), | ||
'http.client.responseStart': expect.any(Number), | ||
'network.protocol.version': expect.any(String), | ||
}), | ||
}), | ||
); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import type { DynamicSamplingContext, Span } from '@sentry/types'; | |
import { | ||
addInstrumentationHandler, | ||
BAGGAGE_HEADER_NAME, | ||
browserPerformanceTimeOrigin, | ||
dynamicSamplingContextToSentryBaggageHeader, | ||
isInstanceOf, | ||
SENTRY_XHR_DATA_KEY, | ||
|
@@ -14,6 +15,13 @@ 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 | ||
|
@@ -108,12 +116,13 @@ export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions | |
// 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 } = { | ||
const { traceFetch, traceXHR, tracePropagationTargets, tracingOrigins, shouldCreateSpanForRequest, _experiments } = { | ||
traceFetch: defaultRequestInstrumentationOptions.traceFetch, | ||
traceXHR: defaultRequestInstrumentationOptions.traceXHR, | ||
..._options, | ||
|
@@ -132,17 +141,58 @@ export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumenta | |
|
||
if (traceFetch) { | ||
addInstrumentationHandler('fetch', (handlerData: FetchData) => { | ||
fetchCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); | ||
const createdSpan = fetchCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); | ||
if (_experiments?.enableHTTPTimings && createdSpan) { | ||
addHTTPTimings(createdSpan); | ||
} | ||
}); | ||
} | ||
|
||
if (traceXHR) { | ||
addInstrumentationHandler('xhr', (handlerData: XHRData) => { | ||
xhrCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); | ||
const createdSpan = xhrCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans); | ||
if (_experiments?.enableHTTPTimings && createdSpan) { | ||
addHTTPTimings(createdSpan); | ||
} | ||
}); | ||
} | ||
} | ||
|
||
/** | ||
* Creates a temporary observer to listen to the next fetch/xhr resourcing timings, | ||
* so that when timings hit their per-browser limit they don't need to be removed. | ||
* | ||
* @param span A span that has yet to be finished, must contain `url` on data. | ||
*/ | ||
function addHTTPTimings(span: Span): void { | ||
const url = span.data.url; | ||
const observer = new PerformanceObserver(list => { | ||
const entries = list.getEntries() as PerformanceResourceTiming[]; | ||
entries.forEach(entry => { | ||
if ((entry.initiatorType === 'fetch' || entry.initiatorType === 'xmlhttprequest') && entry.name.endsWith(url)) { | ||
const spanData = resourceTimingEntryToSpanData(entry); | ||
spanData.forEach(data => span.setData(...data)); | ||
observer.disconnect(); | ||
} | ||
}); | ||
}); | ||
observer.observe({ | ||
entryTypes: ['resource'], | ||
}); | ||
} | ||
|
||
function resourceTimingEntryToSpanData(resourceTiming: PerformanceResourceTiming): [string, string | number][] { | ||
if (!browserPerformanceTimeOrigin) { | ||
return [['network.protocol.version', resourceTiming.nextHopProtocol]]; | ||
} | ||
return [ | ||
['network.protocol.version', resourceTiming.nextHopProtocol], | ||
['http.client.connectStart', (browserPerformanceTimeOrigin + resourceTiming.connectStart) / 1000], | ||
['http.client.requestStart', (browserPerformanceTimeOrigin + resourceTiming.requestStart) / 1000], | ||
['http.client.responseStart', (browserPerformanceTimeOrigin + resourceTiming.responseStart) / 1000], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keeping these in line with span start time, up for other representations though before we bring this out of experiment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wonder if we should call these There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, works for me. |
||
]; | ||
} | ||
|
||
/** | ||
* A function that determines whether to attach tracing headers to a request. | ||
* This was extracted from `instrumentOutgoingRequests` to make it easier to test shouldAttachHeaders. | ||
|
@@ -154,13 +204,15 @@ export function shouldAttachHeaders(url: string, tracePropagationTargets: (strin | |
|
||
/** | ||
* Create and track fetch request spans | ||
* | ||
* @returns Span if a span was created, otherwise void. | ||
*/ | ||
export function fetchCallback( | ||
function fetchCallback( | ||
handlerData: FetchData, | ||
shouldCreateSpan: (url: string) => boolean, | ||
shouldAttachHeaders: (url: string) => boolean, | ||
spans: Record<string, Span>, | ||
): void { | ||
): Span | void { | ||
if (!hasTracingEnabled() || !(handlerData.fetchData && shouldCreateSpan(handlerData.fetchData.url))) { | ||
return; | ||
} | ||
|
@@ -230,6 +282,7 @@ export function fetchCallback( | |
options, | ||
); | ||
} | ||
return span; | ||
} | ||
} | ||
|
||
|
@@ -302,13 +355,15 @@ export function addTracingHeadersToFetchRequest( | |
|
||
/** | ||
* Create and track xhr request spans | ||
* | ||
* @returns Span if a span was created, otherwise void. | ||
*/ | ||
export function xhrCallback( | ||
function xhrCallback( | ||
handlerData: XHRData, | ||
shouldCreateSpan: (url: string) => boolean, | ||
shouldAttachHeaders: (url: string) => boolean, | ||
spans: Record<string, Span>, | ||
): void { | ||
): Span | void { | ||
const xhr = handlerData.xhr; | ||
const sentryXhrData = xhr && xhr[SENTRY_XHR_DATA_KEY]; | ||
|
||
|
@@ -372,5 +427,7 @@ export function xhrCallback( | |
// Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED. | ||
} | ||
} | ||
|
||
return span; | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.