Skip to content

Commit b9ef116

Browse files
AbhiPrasadmydea
AbhiPrasad
and
mydea
authored
feat(core/v7): Add server.address to browser http.client spans (#11663)
Backport of #11634 to v7 --------- Co-authored-by: Francesco Novy <francesco.novy@sentry.io>
1 parent 705f919 commit b9ef116

File tree

12 files changed

+276
-17
lines changed

12 files changed

+276
-17
lines changed

‎dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/init.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
integrations: [Sentry.browserTracingIntegration()],
8+
tracesSampleRate: 1,
9+
});

‎dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/subject.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
fetch('/test-req/0').then(
2+
fetch('/test-req/1', { headers: { 'X-Test-Header': 'existing-header' } }).then(fetch('/test-req/2')),
3+
);

‎dev-packages/browser-integration-tests/suites/tracing/request/fetch-relative-url/test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { TEST_HOST, sentryTest } from '../../../../utils/fixtures';
4+
import {
5+
envelopeRequestParser,
6+
shouldSkipTracingTest,
7+
waitForTransactionRequestOnUrl,
8+
} from '../../../../utils/helpers';
9+
10+
sentryTest('should create spans for fetch requests', async ({ getLocalTestUrl, page }) => {
11+
if (shouldSkipTracingTest()) {
12+
sentryTest.skip();
13+
}
14+
15+
const url = await getLocalTestUrl({ testDir: __dirname });
16+
const req = await waitForTransactionRequestOnUrl(page, url);
17+
const tracingEvent = envelopeRequestParser(req);
18+
19+
// eslint-disable-next-line deprecation/deprecation
20+
const requestSpans = tracingEvent.spans?.filter(({ op }) => op === 'http.client');
21+
22+
expect(requestSpans).toHaveLength(3);
23+
24+
requestSpans?.forEach((span, index) =>
25+
expect(span).toMatchObject({
26+
description: `GET /test-req/${index}`,
27+
parent_span_id: tracingEvent.contexts?.trace?.span_id,
28+
span_id: expect.any(String),
29+
start_timestamp: expect.any(Number),
30+
timestamp: expect.any(Number),
31+
trace_id: tracingEvent.contexts?.trace?.trace_id,
32+
data: {
33+
'http.method': 'GET',
34+
'http.url': `${TEST_HOST}/test-req/${index}`,
35+
url: `/test-req/${index}`,
36+
'server.address': 'sentry-test.io',
37+
type: 'fetch',
38+
},
39+
}),
40+
);
41+
});
42+
43+
sentryTest('should attach `sentry-trace` header to fetch requests', async ({ getLocalTestUrl, page }) => {
44+
if (shouldSkipTracingTest()) {
45+
sentryTest.skip();
46+
}
47+
48+
const url = await getLocalTestUrl({ testDir: __dirname });
49+
50+
const requests = (
51+
await Promise.all([
52+
page.goto(url),
53+
Promise.all([0, 1, 2].map(idx => page.waitForRequest(`${TEST_HOST}/test-req/${idx}`))),
54+
])
55+
)[1];
56+
57+
expect(requests).toHaveLength(3);
58+
59+
const request1 = requests[0];
60+
const requestHeaders1 = request1.headers();
61+
expect(requestHeaders1).toMatchObject({
62+
'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/),
63+
baggage: expect.any(String),
64+
});
65+
66+
const request2 = requests[1];
67+
const requestHeaders2 = request2.headers();
68+
expect(requestHeaders2).toMatchObject({
69+
'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/),
70+
baggage: expect.any(String),
71+
'x-test-header': 'existing-header',
72+
});
73+
74+
const request3 = requests[2];
75+
const requestHeaders3 = request3.headers();
76+
expect(requestHeaders3).toMatchObject({
77+
'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/),
78+
baggage: expect.any(String),
79+
});
80+
});

‎dev-packages/browser-integration-tests/suites/tracing/request/fetch/test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ sentryTest('should create spans for multiple fetch requests', async ({ getLocalT
3737
start_timestamp: expect.any(Number),
3838
timestamp: expect.any(Number),
3939
trace_id: tracingEvent.contexts?.trace?.trace_id,
40+
data: {
41+
'http.method': 'GET',
42+
'http.url': `http://example.com/${index}`,
43+
url: `http://example.com/${index}`,
44+
'server.address': 'example.com',
45+
type: 'fetch',
46+
},
4047
}),
4148
);
4249
});

‎dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/init.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
integrations: [Sentry.browserTracingIntegration()],
8+
tracesSampleRate: 1,
9+
});

‎dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/subject.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
const xhr_1 = new XMLHttpRequest();
2+
xhr_1.open('GET', '/test-req/0');
3+
xhr_1.send();
4+
5+
const xhr_2 = new XMLHttpRequest();
6+
xhr_2.open('GET', '/test-req/1');
7+
xhr_2.setRequestHeader('X-Test-Header', 'existing-header');
8+
xhr_2.send();
9+
10+
const xhr_3 = new XMLHttpRequest();
11+
xhr_3.open('GET', '/test-req/2');
12+
xhr_3.send();

‎dev-packages/browser-integration-tests/suites/tracing/request/xhr-relative-url/test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { TEST_HOST, sentryTest } from '../../../../utils/fixtures';
4+
import {
5+
envelopeRequestParser,
6+
shouldSkipTracingTest,
7+
waitForTransactionRequestOnUrl,
8+
} from '../../../../utils/helpers';
9+
10+
sentryTest('should create spans for xhr requests', async ({ getLocalTestUrl, page }) => {
11+
if (shouldSkipTracingTest()) {
12+
sentryTest.skip();
13+
}
14+
15+
const url = await getLocalTestUrl({ testDir: __dirname });
16+
const req = await waitForTransactionRequestOnUrl(page, url);
17+
const tracingEvent = envelopeRequestParser(req);
18+
19+
// eslint-disable-next-line deprecation/deprecation
20+
const requestSpans = tracingEvent.spans?.filter(({ op }) => op === 'http.client');
21+
22+
expect(requestSpans).toHaveLength(3);
23+
24+
requestSpans?.forEach((span, index) =>
25+
expect(span).toMatchObject({
26+
description: `GET /test-req/${index}`,
27+
parent_span_id: tracingEvent.contexts?.trace?.span_id,
28+
span_id: expect.any(String),
29+
start_timestamp: expect.any(Number),
30+
timestamp: expect.any(Number),
31+
trace_id: tracingEvent.contexts?.trace?.trace_id,
32+
data: {
33+
'http.method': 'GET',
34+
'http.url': `${TEST_HOST}/test-req/${index}`,
35+
url: `/test-req/${index}`,
36+
'server.address': 'sentry-test.io',
37+
type: 'xhr',
38+
},
39+
}),
40+
);
41+
});
42+
43+
sentryTest('should attach `sentry-trace` header to xhr requests', async ({ getLocalTestUrl, page }) => {
44+
if (shouldSkipTracingTest()) {
45+
sentryTest.skip();
46+
}
47+
48+
const url = await getLocalTestUrl({ testDir: __dirname });
49+
50+
const requests = (
51+
await Promise.all([
52+
page.goto(url),
53+
Promise.all([0, 1, 2].map(idx => page.waitForRequest(`${TEST_HOST}/test-req/${idx}`))),
54+
])
55+
)[1];
56+
57+
expect(requests).toHaveLength(3);
58+
59+
const request1 = requests[0];
60+
const requestHeaders1 = request1.headers();
61+
expect(requestHeaders1).toMatchObject({
62+
'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/),
63+
baggage: expect.any(String),
64+
});
65+
66+
const request2 = requests[1];
67+
const requestHeaders2 = request2.headers();
68+
expect(requestHeaders2).toMatchObject({
69+
'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/),
70+
baggage: expect.any(String),
71+
'x-test-header': 'existing-header',
72+
});
73+
74+
const request3 = requests[2];
75+
const requestHeaders3 = request3.headers();
76+
expect(requestHeaders3).toMatchObject({
77+
'sentry-trace': expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/),
78+
baggage: expect.any(String),
79+
});
80+
});

‎dev-packages/browser-integration-tests/suites/tracing/request/xhr/test.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ sentryTest('should create spans for multiple XHR requests', async ({ getLocalTes
2525
start_timestamp: expect.any(Number),
2626
timestamp: expect.any(Number),
2727
trace_id: eventData.contexts?.trace?.trace_id,
28+
data: {
29+
'http.method': 'GET',
30+
'http.url': `http://example.com/${index}`,
31+
url: `http://example.com/${index}`,
32+
'server.address': 'example.com',
33+
type: 'xhr',
34+
},
2835
}),
2936
);
3037
});

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ test('Should trace outgoing fetch requests inside middleware and create breadcru
6868
'http.response.status_code': 200,
6969
type: 'fetch',
7070
url: 'http://localhost:3030/',
71+
'http.url': 'http://localhost:3030/',
72+
'server.address': 'localhost:3030',
7173
'sentry.op': 'http.client',
7274
'sentry.origin': 'auto.http.wintercg_fetch',
7375
},

‎packages/nextjs/test/integration/test/client/tracingFetch.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ test('should correctly instrument `fetch` for performance tracing', async ({ pag
3333
data: {
3434
'http.method': 'GET',
3535
url: 'http://example.com',
36+
'http.url': 'http://example.com/',
37+
'server.address': 'example.com',
3638
type: 'fetch',
3739
'http.response_content_length': expect.any(Number),
3840
'http.response.status_code': 200,

‎packages/tracing-internal/src/browser/request.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@ import {
2121
browserPerformanceTimeOrigin,
2222
dynamicSamplingContextToSentryBaggageHeader,
2323
generateSentryTraceHeader,
24+
parseUrl,
2425
stringMatchesSomePattern,
2526
} from '@sentry/utils';
2627

2728
import { instrumentFetchRequest } from '../common/fetch';
2829
import { addPerformanceInstrumentationHandler } from './instrument';
30+
import { WINDOW } from './types';
2931

3032
export const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^\/(?!\/)/];
3133

@@ -119,6 +121,18 @@ export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumenta
119121
if (traceFetch) {
120122
addFetchInstrumentationHandler(handlerData => {
121123
const createdSpan = instrumentFetchRequest(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans);
124+
// We cannot use `window.location` in the generic fetch instrumentation,
125+
// but we need it for reliable `server.address` attribute.
126+
// so we extend this in here
127+
if (createdSpan) {
128+
const fullUrl = getFullURL(handlerData.fetchData.url);
129+
const host = fullUrl ? parseUrl(fullUrl).host : undefined;
130+
createdSpan.setAttributes({
131+
'http.url': fullUrl,
132+
'server.address': host,
133+
});
134+
}
135+
122136
if (enableHTTPTimings && createdSpan) {
123137
addHTTPTimings(createdSpan);
124138
}
@@ -279,14 +293,19 @@ export function xhrCallback(
279293
const scope = getCurrentScope();
280294
const isolationScope = getIsolationScope();
281295

296+
const fullUrl = getFullURL(sentryXhrData.url);
297+
const host = fullUrl ? parseUrl(fullUrl).host : undefined;
298+
282299
const span = shouldCreateSpanResult
283300
? startInactiveSpan({
284301
name: `${sentryXhrData.method} ${sentryXhrData.url}`,
285302
onlyIfParent: true,
286303
attributes: {
287304
type: 'xhr',
288305
'http.method': sentryXhrData.method,
306+
'http.url': fullUrl,
289307
url: sentryXhrData.url,
308+
'server.address': host,
290309
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser',
291310
},
292311
op: 'http.client',
@@ -338,3 +357,14 @@ function setHeaderOnXhr(
338357
// Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED.
339358
}
340359
}
360+
361+
function getFullURL(url: string): string | undefined {
362+
try {
363+
// By adding a base URL to new URL(), this will also work for relative urls
364+
// If `url` is a full URL, the base URL is ignored anyhow
365+
const parsed = new URL(url, WINDOW.location.origin);
366+
return parsed.href;
367+
} catch {
368+
return undefined;
369+
}
370+
}

‎packages/tracing-internal/src/common/fetch.ts

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
dynamicSamplingContextToSentryBaggageHeader,
1717
generateSentryTraceHeader,
1818
isInstanceOf,
19+
parseUrl,
1920
} from '@sentry/utils';
2021

2122
type PolymorphicRequestHeaders =
@@ -53,23 +54,7 @@ export function instrumentFetchRequest(
5354

5455
const span = spans[spanId];
5556
if (span) {
56-
if (handlerData.response) {
57-
setHttpStatus(span, handlerData.response.status);
58-
59-
const contentLength =
60-
handlerData.response && handlerData.response.headers && handlerData.response.headers.get('content-length');
61-
62-
if (contentLength) {
63-
const contentLengthNum = parseInt(contentLength);
64-
if (contentLengthNum > 0) {
65-
span.setAttribute('http.response_content_length', contentLengthNum);
66-
}
67-
}
68-
} else if (handlerData.error) {
69-
span.setStatus('internal_error');
70-
}
71-
span.end();
72-
57+
endSpan(span, handlerData);
7358
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
7459
delete spans[spanId];
7560
}
@@ -81,6 +66,9 @@ export function instrumentFetchRequest(
8166

8267
const { method, url } = handlerData.fetchData;
8368

69+
const fullUrl = getFullURL(url);
70+
const host = fullUrl ? parseUrl(fullUrl).host : undefined;
71+
8472
const span = shouldCreateSpanResult
8573
? startInactiveSpan({
8674
name: `${method} ${url}`,
@@ -89,6 +77,8 @@ export function instrumentFetchRequest(
8977
url,
9078
type: 'fetch',
9179
'http.method': method,
80+
'http.url': fullUrl,
81+
'server.address': host,
9282
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanOrigin,
9383
},
9484
op: 'http.client',
@@ -198,3 +188,31 @@ export function addTracingHeadersToFetchRequest(
198188
};
199189
}
200190
}
191+
192+
function getFullURL(url: string): string | undefined {
193+
try {
194+
const parsed = new URL(url);
195+
return parsed.href;
196+
} catch {
197+
return undefined;
198+
}
199+
}
200+
201+
function endSpan(span: Span, handlerData: HandlerDataFetch): void {
202+
if (handlerData.response) {
203+
setHttpStatus(span, handlerData.response.status);
204+
205+
const contentLength =
206+
handlerData.response && handlerData.response.headers && handlerData.response.headers.get('content-length');
207+
208+
if (contentLength) {
209+
const contentLengthNum = parseInt(contentLength);
210+
if (contentLengthNum > 0) {
211+
span.setAttribute('http.response_content_length', contentLengthNum);
212+
}
213+
}
214+
} else if (handlerData.error) {
215+
span.setStatus('internal_error');
216+
}
217+
span.end();
218+
}

0 commit comments

Comments
 (0)