Skip to content

fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation#13907

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.

Sign up for GitHub

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

Merged
lforst merged 13 commits into from
Oct 16, 2024
Merged

fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation #13907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d86cca9
fix(core): `.set` the `sentry-trace` header instead of `.append`ing i…
Oct 8, 2024
5206c77
Remove old ones and append new sentry baggage values
Oct 8, 2024
596eda4
lint & size limit
Oct 8, 2024
1ad1b53
Simplify and add comment
Oct 8, 2024
c9e8f38
Override values for all the different variants of passing headers
Oct 8, 2024
5e01be3
smol rename
Oct 8, 2024
9bc4bc8
Merge branch 'develop' into lforst-set-headers-instead-of-append
Oct 8, 2024
6a08581
Merge branch 'develop' into lforst-set-headers-instead-of-append
Oct 15, 2024
e672a37
Boi am I glad I added tests
Oct 15, 2024
7d16405
size limit
Oct 15, 2024
66d1153
size limit
Oct 16, 2024
28ace04
size limit
Oct 16, 2024
ba16ef7
Merge branch 'develop' into lforst-set-headers-instead-of-append
Oct 16, 2024
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
4 changes: 2 additions & 2 deletions .size-limit.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -79,7 +79,7 @@ module.exports = [
path: 'packages/browser/build/npm/esm/index.js',
import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
gzip: true,
limit: '78 KB',
limit: '78.1 KB',
},
{
name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
Expand DownExpand Up@@ -138,7 +138,7 @@ module.exports = [
import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
ignore: ['react/jsx-runtime'],
gzip: true,
limit: '39 KB',
limit: '39.05 KB',
},
// Vue SDK (ESM)
{
Expand Down
10 changes: 10 additions & 0 deletions ...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
integrations: [Sentry.browserTracingIntegration()],
tracePropagationTargets: ['http://example.com'],
tracesSampleRate: 1,
});
53 changes: 53 additions & 0 deletions ...s/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
fetchPojo.addEventListener('click', () => {
const fetchOptions = {
headers: {
'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
baggage: 'sentry-release=4.2.0',
},
};

// Make two fetch requests that reuse the same fetch object
Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
fetch('http://example.com/fetch-pojo', fetchOptions)
.then(res => res.text())
.then(() =>
Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)),
),
);
});

fetchArray.addEventListener('click', () => {
const fetchOptions = {
headers: [
['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'],
['baggage', 'sentry-release=4.2.0'],
],
};

// Make two fetch requests that reuse the same fetch object
Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
fetch('http://example.com/fetch-array', fetchOptions)
.then(res => res.text())
.then(() =>
Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)),
),
);
});

fetchHeaders.addEventListener('click', () => {
const fetchOptions = {
headers: new Headers({
'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
baggage: 'sentry-release=4.2.0',
}),
};

// Make two fetch requests that reuse the same fetch object
Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
fetch('http://example.com/fetch-headers', fetchOptions)
.then(res => res.text())
.then(() =>
Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)),
),
);
});
11 changes: 11 additions & 0 deletions ...rowser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8" />
</head>
<body>
<button id="fetchPojo">Fetch POJO</button>
<button id="fetchArray">Fetch array</button>
<button id="fetchHeaders">Fetch Headers</button>
</body>
</html>
64 changes: 64 additions & 0 deletions ...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
import type { Page, Request } from '@playwright/test';
import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { shouldSkipTracingTest } from '../../../../utils/helpers';

async function assertRequests({
page,
buttonSelector,
requestMatcher,
}: { page: Page; buttonSelector: string; requestMatcher: string }) {
const requests = await new Promise<Request[]>(resolve => {
const requests: Request[] = [];
page
.route(requestMatcher, (route, request) => {
requests.push(request);
if (requests.length === 2) {
resolve(requests);
}

return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({}),
});
})
.then(() => {
page.click(buttonSelector);
});
});

requests.forEach(request => {
const headers = request.headers();

// No merged sentry trace headers
expect(headers['sentry-trace']).not.toContain(',');

// No multiple baggage entries
expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1);
});
}

sentryTest(
'Ensure the SDK does not infinitely append tracing headers to outgoing requests',
async ({ getLocalTestUrl, page }) => {
if (shouldSkipTracingTest()) {
sentryTest.skip();
}

const url = await getLocalTestUrl({ testDir: __dirname });
await page.goto(url);

await sentryTest.step('fetch with POJO', () =>
assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }),
);

await sentryTest.step('fetch with array', () =>
assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }),
);

await sentryTest.step('fetch with Headers instance', () =>
assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }),
);
},
);
63 changes: 53 additions & 10 deletions packages/core/src/fetch.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types';
import {
BAGGAGE_HEADER_NAME,
SENTRY_BAGGAGE_KEY_PREFIX,
dynamicSamplingContextToSentryBaggageHeader,
generateSentryTraceHeader,
isInstanceOf,
Expand DownExpand Up@@ -122,7 +123,7 @@ export function addTracingHeadersToFetchRequest(
request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package,
client: Client,
scope: Scope,
options: {
fetchOptionsObj: {
headers?:
| {
[key: string]: string[] | string | undefined;
Expand All@@ -145,25 +146,53 @@ export function addTracingHeadersToFetchRequest(
);

const headers =
options.headers ||
fetchOptionsObj.headers ||
(typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : undefined);

if (!headers) {
return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader };
} else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) {
const newHeaders = new Headers(headers as Headers);

newHeaders.append('sentry-trace', sentryTraceHeader);
newHeaders.set('sentry-trace', sentryTraceHeader);

if (sentryBaggageHeader) {
// If the same header is appended multiple times the browser will merge the values into a single request header.
// Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header.
newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
const prevBaggageHeader = newHeaders.get(BAGGAGE_HEADER_NAME);
if (prevBaggageHeader) {
const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader);
newHeaders.set(
BAGGAGE_HEADER_NAME,
// If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header
// otherwise just set the sentry baggage header
prevHeaderStrippedFromSentryBaggage
? `${prevHeaderStrippedFromSentryBaggage},${sentryBaggageHeader}`
: sentryBaggageHeader,
);
} else {
newHeaders.set(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
}
}

return newHeaders as PolymorphicRequestHeaders;
} else if (Array.isArray(headers)) {
const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]];
const newHeaders = [
...headers
// Remove any existing sentry-trace headers
.filter(header => {
return !(Array.isArray(header) && header[0] === 'sentry-trace');
})
// Get rid of previous sentry baggage values in baggage header
.map(header => {
if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') {
const [headerName, headerValue, ...rest] = header;
return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest];
} else {
return header;
}
}),
// Attach the new sentry-trace header
['sentry-trace', sentryTraceHeader],
];

if (sentryBaggageHeader) {
// If there are multiple entries with the same key, the browser will merge the values into a single request header.
Expand All@@ -174,12 +203,16 @@ export function addTracingHeadersToFetchRequest(
return newHeaders as PolymorphicRequestHeaders;
} else {
const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined;
const newBaggageHeaders: string[] = [];
let newBaggageHeaders: string[] = [];

if (Array.isArray(existingBaggageHeader)) {
newBaggageHeaders.push(...existingBaggageHeader);
newBaggageHeaders = existingBaggageHeader
.map(headerItem =>
typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem,
)
.filter(headerItem => headerItem === '');
} else if (existingBaggageHeader) {
newBaggageHeaders.push(existingBaggageHeader);
newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader));
}

if (sentryBaggageHeader) {
Expand DownExpand Up@@ -221,3 +254,13 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void {
}
span.end();
}

function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string {
return (
baggageHeader
.split(',')
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
.filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))
.join(',')
);
}
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(core): `.set` the `sentry-trace` header instead of `.append`ing in fetch instrumentation by lforst · Pull Request #13907 · getsentry/sentry-javascript · GitHub
Skip to content
fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation#13907

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.

 Sign up for GitHub

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

 Merged

lforst
merged 13 commits into

from

Oct 16, 2024
 Merged


fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation
#13907
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


13 commits


Select commit
Hold shift + click to select a range
d86cca9

fix(core): `.set` the `sentry-trace` header instead of `.append`ing i…

Oct 8, 2024
5206c77

Remove old ones and append new sentry baggage values

Oct 8, 2024
596eda4

lint & size limit

Oct 8, 2024
1ad1b53

Simplify and add comment

Oct 8, 2024
c9e8f38

Override values for all the different variants of passing headers

Oct 8, 2024
5e01be3

smol rename

Oct 8, 2024
9bc4bc8

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 8, 2024
6a08581

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 15, 2024
e672a37

Boi am I glad I added tests

Oct 15, 2024
7d16405

size limit

Oct 15, 2024
66d1153

size limit

Oct 16, 2024
28ace04

size limit

Oct 16, 2024
ba16ef7

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 16, 2024

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


4 changes: 2 additions & 2 deletions
.size-limit.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -79,7 +79,7 @@ module.exports = [
 path: 'packages/browser/build/npm/esm/index.js',
 import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
 gzip: true,
 limit: '78 KB',
 limit: '78.1 KB',
 },
 {
 name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
Expand DownExpand Up@@ -138,7 +138,7 @@ module.exports = [
 import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
 ignore: ['react/jsx-runtime'],
 gzip: true,
 limit: '39 KB',
 limit: '39.05 KB',
 },
 // Vue SDK (ESM)
 {
Expand Down

10 changes: 10 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
 dsn: 'https://public@dsn.ingest.sentry.io/1337',
 integrations: [Sentry.browserTracingIntegration()],
 tracePropagationTargets: ['http://example.com'],
 tracesSampleRate: 1,
});

53 changes: 53 additions & 0 deletions
...s/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
fetchPojo.addEventListener('click', () => {
 const fetchOptions = {
 headers: {
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 },
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-pojo', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)),
 ),
 );
});

fetchArray.addEventListener('click', () => {
 const fetchOptions = {
 headers: [
 ['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'],
 ['baggage', 'sentry-release=4.2.0'],
 ],
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-array', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)),
 ),
 );
});

fetchHeaders.addEventListener('click', () => {
 const fetchOptions = {
 headers: new Headers({
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 }),
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-headers', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)),
 ),
 );
});

11 changes: 11 additions & 0 deletions
...rowser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html>
 <head>
 <meta charset="utf-8" />
 </head>
 <body>
 <button id="fetchPojo">Fetch POJO</button>
 <button id="fetchArray">Fetch array</button>
 <button id="fetchHeaders">Fetch Headers</button>
 </body>
</html>

64 changes: 64 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
import type { Page, Request } from '@playwright/test';
import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { shouldSkipTracingTest } from '../../../../utils/helpers';

async function assertRequests({
 page,
 buttonSelector,
 requestMatcher,
}: { page: Page; buttonSelector: string; requestMatcher: string }) {
 const requests = await new Promise<Request[]>(resolve => {
 const requests: Request[] = [];
 page
 .route(requestMatcher, (route, request) => {
 requests.push(request);
 if (requests.length === 2) {
 resolve(requests);
 }

 return route.fulfill({
 status: 200,
 contentType: 'application/json',
 body: JSON.stringify({}),
 });
 })
 .then(() => {
 page.click(buttonSelector);
 });
 });

 requests.forEach(request => {
 const headers = request.headers();

 // No merged sentry trace headers
 expect(headers['sentry-trace']).not.toContain(',');

 // No multiple baggage entries
 expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1);
 });
}

sentryTest(
 'Ensure the SDK does not infinitely append tracing headers to outgoing requests',
 async ({ getLocalTestUrl, page }) => {
 if (shouldSkipTracingTest()) {
 sentryTest.skip();
 }

 const url = await getLocalTestUrl({ testDir: __dirname });
 await page.goto(url);

 await sentryTest.step('fetch with POJO', () =>
 assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }),
 );

 await sentryTest.step('fetch with array', () =>
 assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }),
 );

 await sentryTest.step('fetch with Headers instance', () =>
 assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }),
 );
 },
);

63 changes: 53 additions & 10 deletions
packages/core/src/fetch.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types';
import {
 BAGGAGE_HEADER_NAME,
 SENTRY_BAGGAGE_KEY_PREFIX,
 dynamicSamplingContextToSentryBaggageHeader,
 generateSentryTraceHeader,
 isInstanceOf,
Expand DownExpand Up@@ -122,7 +123,7 @@ export function addTracingHeadersToFetchRequest(
 request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package,
 client: Client,
 scope: Scope,
options: {
fetchOptionsObj: {
 headers?:
 | {
 [key: string]: string[] | string | undefined;
Expand All@@ -145,25 +146,53 @@ export function addTracingHeadersToFetchRequest(
 );

 const headers =
options.headers ||
fetchOptionsObj.headers ||
 (typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : undefined);

 if (!headers) {
 return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader };
 } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) {
 const newHeaders = new Headers(headers as Headers);

 newHeaders.append('sentry-trace', sentryTraceHeader);
 newHeaders.set('sentry-trace', sentryTraceHeader);

 if (sentryBaggageHeader) {
 // If the same header is appended multiple times the browser will merge the values into a single request header.
 // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header.
 newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 const prevBaggageHeader = newHeaders.get(BAGGAGE_HEADER_NAME);
 if (prevBaggageHeader) {
 const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader);
 newHeaders.set(
 BAGGAGE_HEADER_NAME,
 // If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header
 // otherwise just set the sentry baggage header
 prevHeaderStrippedFromSentryBaggage
 ? `${prevHeaderStrippedFromSentryBaggage},${sentryBaggageHeader}`
 : sentryBaggageHeader,
 );
 } else {
 newHeaders.set(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 }
 }

 return newHeaders as PolymorphicRequestHeaders;
 } else if (Array.isArray(headers)) {
 const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]];
 const newHeaders = [
 ...headers
 // Remove any existing sentry-trace headers
 .filter(header => {
 return !(Array.isArray(header) && header[0] === 'sentry-trace');
 })
 // Get rid of previous sentry baggage values in baggage header
 .map(header => {
 if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') {
 const [headerName, headerValue, ...rest] = header;
 return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest];
 } else {
 return header;
 }
 }),
 // Attach the new sentry-trace header
 ['sentry-trace', sentryTraceHeader],
 ];

 if (sentryBaggageHeader) {
 // If there are multiple entries with the same key, the browser will merge the values into a single request header.
Expand All@@ -174,12 +203,16 @@ export function addTracingHeadersToFetchRequest(
 return newHeaders as PolymorphicRequestHeaders;
 } else {
 const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined;
const newBaggageHeaders: string[] = [];
let newBaggageHeaders: string[] = [];

 if (Array.isArray(existingBaggageHeader)) {
 newBaggageHeaders.push(...existingBaggageHeader);
 newBaggageHeaders = existingBaggageHeader
 .map(headerItem =>
 typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem,
 )
 .filter(headerItem => headerItem === '');
 } else if (existingBaggageHeader) {
 newBaggageHeaders.push(existingBaggageHeader);
 newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader));
 }

 if (sentryBaggageHeader) {
Expand DownExpand Up@@ -221,3 +254,13 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void {
 }
 span.end();
}

function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string {
 return (
 baggageHeader
 .split(',')
 // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 .filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))
 .join(',')
 );
}

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
fix(core): `.set` the `sentry-trace` header instead of `.append`ing in fetch instrumentation by lforst · Pull Request #13907 · getsentry/sentry-javascript · GitHub
Skip to content
fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation#13907

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.

 Sign up for GitHub

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

 Merged

lforst
merged 13 commits into

from

Oct 16, 2024
 Merged


fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation
#13907
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


13 commits


Select commit
Hold shift + click to select a range
d86cca9

fix(core): `.set` the `sentry-trace` header instead of `.append`ing i…

Oct 8, 2024
5206c77

Remove old ones and append new sentry baggage values

Oct 8, 2024
596eda4

lint & size limit

Oct 8, 2024
1ad1b53

Simplify and add comment

Oct 8, 2024
c9e8f38

Override values for all the different variants of passing headers

Oct 8, 2024
5e01be3

smol rename

Oct 8, 2024
9bc4bc8

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 8, 2024
6a08581

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 15, 2024
e672a37

Boi am I glad I added tests

Oct 15, 2024
7d16405

size limit

Oct 15, 2024
66d1153

size limit

Oct 16, 2024
28ace04

size limit

Oct 16, 2024
ba16ef7

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 16, 2024

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


4 changes: 2 additions & 2 deletions
.size-limit.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -79,7 +79,7 @@ module.exports = [
 path: 'packages/browser/build/npm/esm/index.js',
 import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
 gzip: true,
 limit: '78 KB',
 limit: '78.1 KB',
 },
 {
 name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
Expand DownExpand Up@@ -138,7 +138,7 @@ module.exports = [
 import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
 ignore: ['react/jsx-runtime'],
 gzip: true,
 limit: '39 KB',
 limit: '39.05 KB',
 },
 // Vue SDK (ESM)
 {
Expand Down

10 changes: 10 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
 dsn: 'https://public@dsn.ingest.sentry.io/1337',
 integrations: [Sentry.browserTracingIntegration()],
 tracePropagationTargets: ['http://example.com'],
 tracesSampleRate: 1,
});

53 changes: 53 additions & 0 deletions
...s/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
fetchPojo.addEventListener('click', () => {
 const fetchOptions = {
 headers: {
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 },
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-pojo', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)),
 ),
 );
});

fetchArray.addEventListener('click', () => {
 const fetchOptions = {
 headers: [
 ['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'],
 ['baggage', 'sentry-release=4.2.0'],
 ],
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-array', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)),
 ),
 );
});

fetchHeaders.addEventListener('click', () => {
 const fetchOptions = {
 headers: new Headers({
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 }),
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-headers', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)),
 ),
 );
});

11 changes: 11 additions & 0 deletions
...rowser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html>
 <head>
 <meta charset="utf-8" />
 </head>
 <body>
 <button id="fetchPojo">Fetch POJO</button>
 <button id="fetchArray">Fetch array</button>
 <button id="fetchHeaders">Fetch Headers</button>
 </body>
</html>

64 changes: 64 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
import type { Page, Request } from '@playwright/test';
import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { shouldSkipTracingTest } from '../../../../utils/helpers';

async function assertRequests({
 page,
 buttonSelector,
 requestMatcher,
}: { page: Page; buttonSelector: string; requestMatcher: string }) {
 const requests = await new Promise<Request[]>(resolve => {
 const requests: Request[] = [];
 page
 .route(requestMatcher, (route, request) => {
 requests.push(request);
 if (requests.length === 2) {
 resolve(requests);
 }

 return route.fulfill({
 status: 200,
 contentType: 'application/json',
 body: JSON.stringify({}),
 });
 })
 .then(() => {
 page.click(buttonSelector);
 });
 });

 requests.forEach(request => {
 const headers = request.headers();

 // No merged sentry trace headers
 expect(headers['sentry-trace']).not.toContain(',');

 // No multiple baggage entries
 expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1);
 });
}

sentryTest(
 'Ensure the SDK does not infinitely append tracing headers to outgoing requests',
 async ({ getLocalTestUrl, page }) => {
 if (shouldSkipTracingTest()) {
 sentryTest.skip();
 }

 const url = await getLocalTestUrl({ testDir: __dirname });
 await page.goto(url);

 await sentryTest.step('fetch with POJO', () =>
 assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }),
 );

 await sentryTest.step('fetch with array', () =>
 assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }),
 );

 await sentryTest.step('fetch with Headers instance', () =>
 assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }),
 );
 },
);

63 changes: 53 additions & 10 deletions
packages/core/src/fetch.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types';
import {
 BAGGAGE_HEADER_NAME,
 SENTRY_BAGGAGE_KEY_PREFIX,
 dynamicSamplingContextToSentryBaggageHeader,
 generateSentryTraceHeader,
 isInstanceOf,
Expand DownExpand Up@@ -122,7 +123,7 @@ export function addTracingHeadersToFetchRequest(
 request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package,
 client: Client,
 scope: Scope,
options: {
fetchOptionsObj: {
 headers?:
 | {
 [key: string]: string[] | string | undefined;
Expand All@@ -145,25 +146,53 @@ export function addTracingHeadersToFetchRequest(
 );

 const headers =
options.headers ||
fetchOptionsObj.headers ||
 (typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : undefined);

 if (!headers) {
 return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader };
 } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) {
 const newHeaders = new Headers(headers as Headers);

 newHeaders.append('sentry-trace', sentryTraceHeader);
 newHeaders.set('sentry-trace', sentryTraceHeader);

 if (sentryBaggageHeader) {
 // If the same header is appended multiple times the browser will merge the values into a single request header.
 // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header.
 newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 const prevBaggageHeader = newHeaders.get(BAGGAGE_HEADER_NAME);
 if (prevBaggageHeader) {
 const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader);
 newHeaders.set(
 BAGGAGE_HEADER_NAME,
 // If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header
 // otherwise just set the sentry baggage header
 prevHeaderStrippedFromSentryBaggage
 ? `${prevHeaderStrippedFromSentryBaggage},${sentryBaggageHeader}`
 : sentryBaggageHeader,
 );
 } else {
 newHeaders.set(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 }
 }

 return newHeaders as PolymorphicRequestHeaders;
 } else if (Array.isArray(headers)) {
 const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]];
 const newHeaders = [
 ...headers
 // Remove any existing sentry-trace headers
 .filter(header => {
 return !(Array.isArray(header) && header[0] === 'sentry-trace');
 })
 // Get rid of previous sentry baggage values in baggage header
 .map(header => {
 if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') {
 const [headerName, headerValue, ...rest] = header;
 return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest];
 } else {
 return header;
 }
 }),
 // Attach the new sentry-trace header
 ['sentry-trace', sentryTraceHeader],
 ];

 if (sentryBaggageHeader) {
 // If there are multiple entries with the same key, the browser will merge the values into a single request header.
Expand All@@ -174,12 +203,16 @@ export function addTracingHeadersToFetchRequest(
 return newHeaders as PolymorphicRequestHeaders;
 } else {
 const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined;
const newBaggageHeaders: string[] = [];
let newBaggageHeaders: string[] = [];

 if (Array.isArray(existingBaggageHeader)) {
 newBaggageHeaders.push(...existingBaggageHeader);
 newBaggageHeaders = existingBaggageHeader
 .map(headerItem =>
 typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem,
 )
 .filter(headerItem => headerItem === '');
 } else if (existingBaggageHeader) {
 newBaggageHeaders.push(existingBaggageHeader);
 newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader));
 }

 if (sentryBaggageHeader) {
Expand DownExpand Up@@ -221,3 +254,13 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void {
 }
 span.end();
}

function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string {
 return (
 baggageHeader
 .split(',')
 // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 .filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))
 .join(',')
 );
}

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
fix(core): `.set` the `sentry-trace` header instead of `.append`ing in fetch instrumentation by lforst · Pull Request #13907 · getsentry/sentry-javascript · GitHub
Skip to content
fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation#13907

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.

 Sign up for GitHub

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

 Merged

lforst
merged 13 commits into

from

Oct 16, 2024
 Merged


fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation
#13907
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


13 commits


Select commit
Hold shift + click to select a range
d86cca9

fix(core): `.set` the `sentry-trace` header instead of `.append`ing i…

Oct 8, 2024
5206c77

Remove old ones and append new sentry baggage values

Oct 8, 2024
596eda4

lint & size limit

Oct 8, 2024
1ad1b53

Simplify and add comment

Oct 8, 2024
c9e8f38

Override values for all the different variants of passing headers

Oct 8, 2024
5e01be3

smol rename

Oct 8, 2024
9bc4bc8

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 8, 2024
6a08581

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 15, 2024
e672a37

Boi am I glad I added tests

Oct 15, 2024
7d16405

size limit

Oct 15, 2024
66d1153

size limit

Oct 16, 2024
28ace04

size limit

Oct 16, 2024
ba16ef7

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 16, 2024

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


4 changes: 2 additions & 2 deletions
.size-limit.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -79,7 +79,7 @@ module.exports = [
 path: 'packages/browser/build/npm/esm/index.js',
 import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
 gzip: true,
 limit: '78 KB',
 limit: '78.1 KB',
 },
 {
 name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
Expand DownExpand Up@@ -138,7 +138,7 @@ module.exports = [
 import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
 ignore: ['react/jsx-runtime'],
 gzip: true,
 limit: '39 KB',
 limit: '39.05 KB',
 },
 // Vue SDK (ESM)
 {
Expand Down

10 changes: 10 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
 dsn: 'https://public@dsn.ingest.sentry.io/1337',
 integrations: [Sentry.browserTracingIntegration()],
 tracePropagationTargets: ['http://example.com'],
 tracesSampleRate: 1,
});

53 changes: 53 additions & 0 deletions
...s/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
fetchPojo.addEventListener('click', () => {
 const fetchOptions = {
 headers: {
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 },
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-pojo', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)),
 ),
 );
});

fetchArray.addEventListener('click', () => {
 const fetchOptions = {
 headers: [
 ['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'],
 ['baggage', 'sentry-release=4.2.0'],
 ],
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-array', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)),
 ),
 );
});

fetchHeaders.addEventListener('click', () => {
 const fetchOptions = {
 headers: new Headers({
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 }),
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-headers', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)),
 ),
 );
});

11 changes: 11 additions & 0 deletions
...rowser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html>
 <head>
 <meta charset="utf-8" />
 </head>
 <body>
 <button id="fetchPojo">Fetch POJO</button>
 <button id="fetchArray">Fetch array</button>
 <button id="fetchHeaders">Fetch Headers</button>
 </body>
</html>

64 changes: 64 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
import type { Page, Request } from '@playwright/test';
import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { shouldSkipTracingTest } from '../../../../utils/helpers';

async function assertRequests({
 page,
 buttonSelector,
 requestMatcher,
}: { page: Page; buttonSelector: string; requestMatcher: string }) {
 const requests = await new Promise<Request[]>(resolve => {
 const requests: Request[] = [];
 page
 .route(requestMatcher, (route, request) => {
 requests.push(request);
 if (requests.length === 2) {
 resolve(requests);
 }

 return route.fulfill({
 status: 200,
 contentType: 'application/json',
 body: JSON.stringify({}),
 });
 })
 .then(() => {
 page.click(buttonSelector);
 });
 });

 requests.forEach(request => {
 const headers = request.headers();

 // No merged sentry trace headers
 expect(headers['sentry-trace']).not.toContain(',');

 // No multiple baggage entries
 expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1);
 });
}

sentryTest(
 'Ensure the SDK does not infinitely append tracing headers to outgoing requests',
 async ({ getLocalTestUrl, page }) => {
 if (shouldSkipTracingTest()) {
 sentryTest.skip();
 }

 const url = await getLocalTestUrl({ testDir: __dirname });
 await page.goto(url);

 await sentryTest.step('fetch with POJO', () =>
 assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }),
 );

 await sentryTest.step('fetch with array', () =>
 assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }),
 );

 await sentryTest.step('fetch with Headers instance', () =>
 assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }),
 );
 },
);

63 changes: 53 additions & 10 deletions
packages/core/src/fetch.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types';
import {
 BAGGAGE_HEADER_NAME,
 SENTRY_BAGGAGE_KEY_PREFIX,
 dynamicSamplingContextToSentryBaggageHeader,
 generateSentryTraceHeader,
 isInstanceOf,
Expand DownExpand Up@@ -122,7 +123,7 @@ export function addTracingHeadersToFetchRequest(
 request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package,
 client: Client,
 scope: Scope,
options: {
fetchOptionsObj: {
 headers?:
 | {
 [key: string]: string[] | string | undefined;
Expand All@@ -145,25 +146,53 @@ export function addTracingHeadersToFetchRequest(
 );

 const headers =
options.headers ||
fetchOptionsObj.headers ||
 (typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : undefined);

 if (!headers) {
 return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader };
 } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) {
 const newHeaders = new Headers(headers as Headers);

 newHeaders.append('sentry-trace', sentryTraceHeader);
 newHeaders.set('sentry-trace', sentryTraceHeader);

 if (sentryBaggageHeader) {
 // If the same header is appended multiple times the browser will merge the values into a single request header.
 // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header.
 newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 const prevBaggageHeader = newHeaders.get(BAGGAGE_HEADER_NAME);
 if (prevBaggageHeader) {
 const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader);
 newHeaders.set(
 BAGGAGE_HEADER_NAME,
 // If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header
 // otherwise just set the sentry baggage header
 prevHeaderStrippedFromSentryBaggage
 ? `${prevHeaderStrippedFromSentryBaggage},${sentryBaggageHeader}`
 : sentryBaggageHeader,
 );
 } else {
 newHeaders.set(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 }
 }

 return newHeaders as PolymorphicRequestHeaders;
 } else if (Array.isArray(headers)) {
 const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]];
 const newHeaders = [
 ...headers
 // Remove any existing sentry-trace headers
 .filter(header => {
 return !(Array.isArray(header) && header[0] === 'sentry-trace');
 })
 // Get rid of previous sentry baggage values in baggage header
 .map(header => {
 if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') {
 const [headerName, headerValue, ...rest] = header;
 return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest];
 } else {
 return header;
 }
 }),
 // Attach the new sentry-trace header
 ['sentry-trace', sentryTraceHeader],
 ];

 if (sentryBaggageHeader) {
 // If there are multiple entries with the same key, the browser will merge the values into a single request header.
Expand All@@ -174,12 +203,16 @@ export function addTracingHeadersToFetchRequest(
 return newHeaders as PolymorphicRequestHeaders;
 } else {
 const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined;
const newBaggageHeaders: string[] = [];
let newBaggageHeaders: string[] = [];

 if (Array.isArray(existingBaggageHeader)) {
 newBaggageHeaders.push(...existingBaggageHeader);
 newBaggageHeaders = existingBaggageHeader
 .map(headerItem =>
 typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem,
 )
 .filter(headerItem => headerItem === '');
 } else if (existingBaggageHeader) {
 newBaggageHeaders.push(existingBaggageHeader);
 newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader));
 }

 if (sentryBaggageHeader) {
Expand DownExpand Up@@ -221,3 +254,13 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void {
 }
 span.end();
}

function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string {
 return (
 baggageHeader
 .split(',')
 // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 .filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))
 .join(',')
 );
}

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
fix(core): `.set` the `sentry-trace` header instead of `.append`ing in fetch instrumentation by lforst · Pull Request #13907 · getsentry/sentry-javascript · GitHub
Skip to content
fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation#13907

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.

 Sign up for GitHub

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

 Merged

lforst
merged 13 commits into

from

Oct 16, 2024
 Merged


fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation
#13907
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


13 commits


Select commit
Hold shift + click to select a range
d86cca9

fix(core): `.set` the `sentry-trace` header instead of `.append`ing i…

Oct 8, 2024
5206c77

Remove old ones and append new sentry baggage values

Oct 8, 2024
596eda4

lint & size limit

Oct 8, 2024
1ad1b53

Simplify and add comment

Oct 8, 2024
c9e8f38

Override values for all the different variants of passing headers

Oct 8, 2024
5e01be3

smol rename

Oct 8, 2024
9bc4bc8

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 8, 2024
6a08581

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 15, 2024
e672a37

Boi am I glad I added tests

Oct 15, 2024
7d16405

size limit

Oct 15, 2024
66d1153

size limit

Oct 16, 2024
28ace04

size limit

Oct 16, 2024
ba16ef7

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 16, 2024

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


4 changes: 2 additions & 2 deletions
.size-limit.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -79,7 +79,7 @@ module.exports = [
 path: 'packages/browser/build/npm/esm/index.js',
 import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
 gzip: true,
 limit: '78 KB',
 limit: '78.1 KB',
 },
 {
 name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
Expand DownExpand Up@@ -138,7 +138,7 @@ module.exports = [
 import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
 ignore: ['react/jsx-runtime'],
 gzip: true,
 limit: '39 KB',
 limit: '39.05 KB',
 },
 // Vue SDK (ESM)
 {
Expand Down

10 changes: 10 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
 dsn: 'https://public@dsn.ingest.sentry.io/1337',
 integrations: [Sentry.browserTracingIntegration()],
 tracePropagationTargets: ['http://example.com'],
 tracesSampleRate: 1,
});

53 changes: 53 additions & 0 deletions
...s/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
fetchPojo.addEventListener('click', () => {
 const fetchOptions = {
 headers: {
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 },
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-pojo', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)),
 ),
 );
});

fetchArray.addEventListener('click', () => {
 const fetchOptions = {
 headers: [
 ['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'],
 ['baggage', 'sentry-release=4.2.0'],
 ],
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-array', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)),
 ),
 );
});

fetchHeaders.addEventListener('click', () => {
 const fetchOptions = {
 headers: new Headers({
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 }),
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-headers', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)),
 ),
 );
});

11 changes: 11 additions & 0 deletions
...rowser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html>
 <head>
 <meta charset="utf-8" />
 </head>
 <body>
 <button id="fetchPojo">Fetch POJO</button>
 <button id="fetchArray">Fetch array</button>
 <button id="fetchHeaders">Fetch Headers</button>
 </body>
</html>

64 changes: 64 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
import type { Page, Request } from '@playwright/test';
import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { shouldSkipTracingTest } from '../../../../utils/helpers';

async function assertRequests({
 page,
 buttonSelector,
 requestMatcher,
}: { page: Page; buttonSelector: string; requestMatcher: string }) {
 const requests = await new Promise<Request[]>(resolve => {
 const requests: Request[] = [];
 page
 .route(requestMatcher, (route, request) => {
 requests.push(request);
 if (requests.length === 2) {
 resolve(requests);
 }

 return route.fulfill({
 status: 200,
 contentType: 'application/json',
 body: JSON.stringify({}),
 });
 })
 .then(() => {
 page.click(buttonSelector);
 });
 });

 requests.forEach(request => {
 const headers = request.headers();

 // No merged sentry trace headers
 expect(headers['sentry-trace']).not.toContain(',');

 // No multiple baggage entries
 expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1);
 });
}

sentryTest(
 'Ensure the SDK does not infinitely append tracing headers to outgoing requests',
 async ({ getLocalTestUrl, page }) => {
 if (shouldSkipTracingTest()) {
 sentryTest.skip();
 }

 const url = await getLocalTestUrl({ testDir: __dirname });
 await page.goto(url);

 await sentryTest.step('fetch with POJO', () =>
 assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }),
 );

 await sentryTest.step('fetch with array', () =>
 assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }),
 );

 await sentryTest.step('fetch with Headers instance', () =>
 assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }),
 );
 },
);

63 changes: 53 additions & 10 deletions
packages/core/src/fetch.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types';
import {
 BAGGAGE_HEADER_NAME,
 SENTRY_BAGGAGE_KEY_PREFIX,
 dynamicSamplingContextToSentryBaggageHeader,
 generateSentryTraceHeader,
 isInstanceOf,
Expand DownExpand Up@@ -122,7 +123,7 @@ export function addTracingHeadersToFetchRequest(
 request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package,
 client: Client,
 scope: Scope,
options: {
fetchOptionsObj: {
 headers?:
 | {
 [key: string]: string[] | string | undefined;
Expand All@@ -145,25 +146,53 @@ export function addTracingHeadersToFetchRequest(
 );

 const headers =
options.headers ||
fetchOptionsObj.headers ||
 (typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : undefined);

 if (!headers) {
 return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader };
 } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) {
 const newHeaders = new Headers(headers as Headers);

 newHeaders.append('sentry-trace', sentryTraceHeader);
 newHeaders.set('sentry-trace', sentryTraceHeader);

 if (sentryBaggageHeader) {
 // If the same header is appended multiple times the browser will merge the values into a single request header.
 // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header.
 newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 const prevBaggageHeader = newHeaders.get(BAGGAGE_HEADER_NAME);
 if (prevBaggageHeader) {
 const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader);
 newHeaders.set(
 BAGGAGE_HEADER_NAME,
 // If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header
 // otherwise just set the sentry baggage header
 prevHeaderStrippedFromSentryBaggage
 ? `${prevHeaderStrippedFromSentryBaggage},${sentryBaggageHeader}`
 : sentryBaggageHeader,
 );
 } else {
 newHeaders.set(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 }
 }

 return newHeaders as PolymorphicRequestHeaders;
 } else if (Array.isArray(headers)) {
 const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]];
 const newHeaders = [
 ...headers
 // Remove any existing sentry-trace headers
 .filter(header => {
 return !(Array.isArray(header) && header[0] === 'sentry-trace');
 })
 // Get rid of previous sentry baggage values in baggage header
 .map(header => {
 if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') {
 const [headerName, headerValue, ...rest] = header;
 return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest];
 } else {
 return header;
 }
 }),
 // Attach the new sentry-trace header
 ['sentry-trace', sentryTraceHeader],
 ];

 if (sentryBaggageHeader) {
 // If there are multiple entries with the same key, the browser will merge the values into a single request header.
Expand All@@ -174,12 +203,16 @@ export function addTracingHeadersToFetchRequest(
 return newHeaders as PolymorphicRequestHeaders;
 } else {
 const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined;
const newBaggageHeaders: string[] = [];
let newBaggageHeaders: string[] = [];

 if (Array.isArray(existingBaggageHeader)) {
 newBaggageHeaders.push(...existingBaggageHeader);
 newBaggageHeaders = existingBaggageHeader
 .map(headerItem =>
 typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem,
 )
 .filter(headerItem => headerItem === '');
 } else if (existingBaggageHeader) {
 newBaggageHeaders.push(existingBaggageHeader);
 newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader));
 }

 if (sentryBaggageHeader) {
Expand DownExpand Up@@ -221,3 +254,13 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void {
 }
 span.end();
}

function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string {
 return (
 baggageHeader
 .split(',')
 // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 .filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))
 .join(',')
 );
}

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
fix(core): `.set` the `sentry-trace` header instead of `.append`ing in fetch instrumentation by lforst · Pull Request #13907 · getsentry/sentry-javascript · GitHub
Skip to content
fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation#13907

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.

 Sign up for GitHub

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

 Merged

lforst
merged 13 commits into

from

Oct 16, 2024
 Merged


fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation
#13907
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


13 commits


Select commit
Hold shift + click to select a range
d86cca9

fix(core): `.set` the `sentry-trace` header instead of `.append`ing i…

Oct 8, 2024
5206c77

Remove old ones and append new sentry baggage values

Oct 8, 2024
596eda4

lint & size limit

Oct 8, 2024
1ad1b53

Simplify and add comment

Oct 8, 2024
c9e8f38

Override values for all the different variants of passing headers

Oct 8, 2024
5e01be3

smol rename

Oct 8, 2024
9bc4bc8

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 8, 2024
6a08581

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 15, 2024
e672a37

Boi am I glad I added tests

Oct 15, 2024
7d16405

size limit

Oct 15, 2024
66d1153

size limit

Oct 16, 2024
28ace04

size limit

Oct 16, 2024
ba16ef7

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 16, 2024

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


4 changes: 2 additions & 2 deletions
.size-limit.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -79,7 +79,7 @@ module.exports = [
 path: 'packages/browser/build/npm/esm/index.js',
 import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
 gzip: true,
 limit: '78 KB',
 limit: '78.1 KB',
 },
 {
 name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
Expand DownExpand Up@@ -138,7 +138,7 @@ module.exports = [
 import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
 ignore: ['react/jsx-runtime'],
 gzip: true,
 limit: '39 KB',
 limit: '39.05 KB',
 },
 // Vue SDK (ESM)
 {
Expand Down

10 changes: 10 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
 dsn: 'https://public@dsn.ingest.sentry.io/1337',
 integrations: [Sentry.browserTracingIntegration()],
 tracePropagationTargets: ['http://example.com'],
 tracesSampleRate: 1,
});

53 changes: 53 additions & 0 deletions
...s/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
fetchPojo.addEventListener('click', () => {
 const fetchOptions = {
 headers: {
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 },
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-pojo', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)),
 ),
 );
});

fetchArray.addEventListener('click', () => {
 const fetchOptions = {
 headers: [
 ['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'],
 ['baggage', 'sentry-release=4.2.0'],
 ],
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-array', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)),
 ),
 );
});

fetchHeaders.addEventListener('click', () => {
 const fetchOptions = {
 headers: new Headers({
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 }),
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-headers', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)),
 ),
 );
});

11 changes: 11 additions & 0 deletions
...rowser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html>
 <head>
 <meta charset="utf-8" />
 </head>
 <body>
 <button id="fetchPojo">Fetch POJO</button>
 <button id="fetchArray">Fetch array</button>
 <button id="fetchHeaders">Fetch Headers</button>
 </body>
</html>

64 changes: 64 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
import type { Page, Request } from '@playwright/test';
import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { shouldSkipTracingTest } from '../../../../utils/helpers';

async function assertRequests({
 page,
 buttonSelector,
 requestMatcher,
}: { page: Page; buttonSelector: string; requestMatcher: string }) {
 const requests = await new Promise<Request[]>(resolve => {
 const requests: Request[] = [];
 page
 .route(requestMatcher, (route, request) => {
 requests.push(request);
 if (requests.length === 2) {
 resolve(requests);
 }

 return route.fulfill({
 status: 200,
 contentType: 'application/json',
 body: JSON.stringify({}),
 });
 })
 .then(() => {
 page.click(buttonSelector);
 });
 });

 requests.forEach(request => {
 const headers = request.headers();

 // No merged sentry trace headers
 expect(headers['sentry-trace']).not.toContain(',');

 // No multiple baggage entries
 expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1);
 });
}

sentryTest(
 'Ensure the SDK does not infinitely append tracing headers to outgoing requests',
 async ({ getLocalTestUrl, page }) => {
 if (shouldSkipTracingTest()) {
 sentryTest.skip();
 }

 const url = await getLocalTestUrl({ testDir: __dirname });
 await page.goto(url);

 await sentryTest.step('fetch with POJO', () =>
 assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }),
 );

 await sentryTest.step('fetch with array', () =>
 assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }),
 );

 await sentryTest.step('fetch with Headers instance', () =>
 assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }),
 );
 },
);

63 changes: 53 additions & 10 deletions
packages/core/src/fetch.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types';
import {
 BAGGAGE_HEADER_NAME,
 SENTRY_BAGGAGE_KEY_PREFIX,
 dynamicSamplingContextToSentryBaggageHeader,
 generateSentryTraceHeader,
 isInstanceOf,
Expand DownExpand Up@@ -122,7 +123,7 @@ export function addTracingHeadersToFetchRequest(
 request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package,
 client: Client,
 scope: Scope,
options: {
fetchOptionsObj: {
 headers?:
 | {
 [key: string]: string[] | string | undefined;
Expand All@@ -145,25 +146,53 @@ export function addTracingHeadersToFetchRequest(
 );

 const headers =
options.headers ||
fetchOptionsObj.headers ||
 (typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : undefined);

 if (!headers) {
 return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader };
 } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) {
 const newHeaders = new Headers(headers as Headers);

 newHeaders.append('sentry-trace', sentryTraceHeader);
 newHeaders.set('sentry-trace', sentryTraceHeader);

 if (sentryBaggageHeader) {
 // If the same header is appended multiple times the browser will merge the values into a single request header.
 // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header.
 newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 const prevBaggageHeader = newHeaders.get(BAGGAGE_HEADER_NAME);
 if (prevBaggageHeader) {
 const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader);
 newHeaders.set(
 BAGGAGE_HEADER_NAME,
 // If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header
 // otherwise just set the sentry baggage header
 prevHeaderStrippedFromSentryBaggage
 ? `${prevHeaderStrippedFromSentryBaggage},${sentryBaggageHeader}`
 : sentryBaggageHeader,
 );
 } else {
 newHeaders.set(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 }
 }

 return newHeaders as PolymorphicRequestHeaders;
 } else if (Array.isArray(headers)) {
 const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]];
 const newHeaders = [
 ...headers
 // Remove any existing sentry-trace headers
 .filter(header => {
 return !(Array.isArray(header) && header[0] === 'sentry-trace');
 })
 // Get rid of previous sentry baggage values in baggage header
 .map(header => {
 if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') {
 const [headerName, headerValue, ...rest] = header;
 return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest];
 } else {
 return header;
 }
 }),
 // Attach the new sentry-trace header
 ['sentry-trace', sentryTraceHeader],
 ];

 if (sentryBaggageHeader) {
 // If there are multiple entries with the same key, the browser will merge the values into a single request header.
Expand All@@ -174,12 +203,16 @@ export function addTracingHeadersToFetchRequest(
 return newHeaders as PolymorphicRequestHeaders;
 } else {
 const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined;
const newBaggageHeaders: string[] = [];
let newBaggageHeaders: string[] = [];

 if (Array.isArray(existingBaggageHeader)) {
 newBaggageHeaders.push(...existingBaggageHeader);
 newBaggageHeaders = existingBaggageHeader
 .map(headerItem =>
 typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem,
 )
 .filter(headerItem => headerItem === '');
 } else if (existingBaggageHeader) {
 newBaggageHeaders.push(existingBaggageHeader);
 newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader));
 }

 if (sentryBaggageHeader) {
Expand DownExpand Up@@ -221,3 +254,13 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void {
 }
 span.end();
}

function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string {
 return (
 baggageHeader
 .split(',')
 // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 .filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))
 .join(',')
 );
}

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
fix(core): `.set` the `sentry-trace` header instead of `.append`ing in fetch instrumentation by lforst · Pull Request #13907 · getsentry/sentry-javascript · GitHub
Skip to content
fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation#13907

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.

 Sign up for GitHub

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

 Merged

lforst
merged 13 commits into

from

Oct 16, 2024
 Merged


fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation
#13907
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


13 commits


Select commit
Hold shift + click to select a range
d86cca9

fix(core): `.set` the `sentry-trace` header instead of `.append`ing i…

Oct 8, 2024
5206c77

Remove old ones and append new sentry baggage values

Oct 8, 2024
596eda4

lint & size limit

Oct 8, 2024
1ad1b53

Simplify and add comment

Oct 8, 2024
c9e8f38

Override values for all the different variants of passing headers

Oct 8, 2024
5e01be3

smol rename

Oct 8, 2024
9bc4bc8

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 8, 2024
6a08581

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 15, 2024
e672a37

Boi am I glad I added tests

Oct 15, 2024
7d16405

size limit

Oct 15, 2024
66d1153

size limit

Oct 16, 2024
28ace04

size limit

Oct 16, 2024
ba16ef7

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 16, 2024

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


4 changes: 2 additions & 2 deletions
.size-limit.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -79,7 +79,7 @@ module.exports = [
 path: 'packages/browser/build/npm/esm/index.js',
 import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
 gzip: true,
 limit: '78 KB',
 limit: '78.1 KB',
 },
 {
 name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
Expand DownExpand Up@@ -138,7 +138,7 @@ module.exports = [
 import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
 ignore: ['react/jsx-runtime'],
 gzip: true,
 limit: '39 KB',
 limit: '39.05 KB',
 },
 // Vue SDK (ESM)
 {
Expand Down

10 changes: 10 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
 dsn: 'https://public@dsn.ingest.sentry.io/1337',
 integrations: [Sentry.browserTracingIntegration()],
 tracePropagationTargets: ['http://example.com'],
 tracesSampleRate: 1,
});

53 changes: 53 additions & 0 deletions
...s/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
fetchPojo.addEventListener('click', () => {
 const fetchOptions = {
 headers: {
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 },
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-pojo', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)),
 ),
 );
});

fetchArray.addEventListener('click', () => {
 const fetchOptions = {
 headers: [
 ['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'],
 ['baggage', 'sentry-release=4.2.0'],
 ],
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-array', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)),
 ),
 );
});

fetchHeaders.addEventListener('click', () => {
 const fetchOptions = {
 headers: new Headers({
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 }),
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-headers', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)),
 ),
 );
});

11 changes: 11 additions & 0 deletions
...rowser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html>
 <head>
 <meta charset="utf-8" />
 </head>
 <body>
 <button id="fetchPojo">Fetch POJO</button>
 <button id="fetchArray">Fetch array</button>
 <button id="fetchHeaders">Fetch Headers</button>
 </body>
</html>

64 changes: 64 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
import type { Page, Request } from '@playwright/test';
import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { shouldSkipTracingTest } from '../../../../utils/helpers';

async function assertRequests({
 page,
 buttonSelector,
 requestMatcher,
}: { page: Page; buttonSelector: string; requestMatcher: string }) {
 const requests = await new Promise<Request[]>(resolve => {
 const requests: Request[] = [];
 page
 .route(requestMatcher, (route, request) => {
 requests.push(request);
 if (requests.length === 2) {
 resolve(requests);
 }

 return route.fulfill({
 status: 200,
 contentType: 'application/json',
 body: JSON.stringify({}),
 });
 })
 .then(() => {
 page.click(buttonSelector);
 });
 });

 requests.forEach(request => {
 const headers = request.headers();

 // No merged sentry trace headers
 expect(headers['sentry-trace']).not.toContain(',');

 // No multiple baggage entries
 expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1);
 });
}

sentryTest(
 'Ensure the SDK does not infinitely append tracing headers to outgoing requests',
 async ({ getLocalTestUrl, page }) => {
 if (shouldSkipTracingTest()) {
 sentryTest.skip();
 }

 const url = await getLocalTestUrl({ testDir: __dirname });
 await page.goto(url);

 await sentryTest.step('fetch with POJO', () =>
 assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }),
 );

 await sentryTest.step('fetch with array', () =>
 assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }),
 );

 await sentryTest.step('fetch with Headers instance', () =>
 assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }),
 );
 },
);

63 changes: 53 additions & 10 deletions
packages/core/src/fetch.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types';
import {
 BAGGAGE_HEADER_NAME,
 SENTRY_BAGGAGE_KEY_PREFIX,
 dynamicSamplingContextToSentryBaggageHeader,
 generateSentryTraceHeader,
 isInstanceOf,
Expand DownExpand Up@@ -122,7 +123,7 @@ export function addTracingHeadersToFetchRequest(
 request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package,
 client: Client,
 scope: Scope,
options: {
fetchOptionsObj: {
 headers?:
 | {
 [key: string]: string[] | string | undefined;
Expand All@@ -145,25 +146,53 @@ export function addTracingHeadersToFetchRequest(
 );

 const headers =
options.headers ||
fetchOptionsObj.headers ||
 (typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : undefined);

 if (!headers) {
 return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader };
 } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) {
 const newHeaders = new Headers(headers as Headers);

 newHeaders.append('sentry-trace', sentryTraceHeader);
 newHeaders.set('sentry-trace', sentryTraceHeader);

 if (sentryBaggageHeader) {
 // If the same header is appended multiple times the browser will merge the values into a single request header.
 // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header.
 newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 const prevBaggageHeader = newHeaders.get(BAGGAGE_HEADER_NAME);
 if (prevBaggageHeader) {
 const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader);
 newHeaders.set(
 BAGGAGE_HEADER_NAME,
 // If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header
 // otherwise just set the sentry baggage header
 prevHeaderStrippedFromSentryBaggage
 ? `${prevHeaderStrippedFromSentryBaggage},${sentryBaggageHeader}`
 : sentryBaggageHeader,
 );
 } else {
 newHeaders.set(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 }
 }

 return newHeaders as PolymorphicRequestHeaders;
 } else if (Array.isArray(headers)) {
 const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]];
 const newHeaders = [
 ...headers
 // Remove any existing sentry-trace headers
 .filter(header => {
 return !(Array.isArray(header) && header[0] === 'sentry-trace');
 })
 // Get rid of previous sentry baggage values in baggage header
 .map(header => {
 if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') {
 const [headerName, headerValue, ...rest] = header;
 return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest];
 } else {
 return header;
 }
 }),
 // Attach the new sentry-trace header
 ['sentry-trace', sentryTraceHeader],
 ];

 if (sentryBaggageHeader) {
 // If there are multiple entries with the same key, the browser will merge the values into a single request header.
Expand All@@ -174,12 +203,16 @@ export function addTracingHeadersToFetchRequest(
 return newHeaders as PolymorphicRequestHeaders;
 } else {
 const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined;
const newBaggageHeaders: string[] = [];
let newBaggageHeaders: string[] = [];

 if (Array.isArray(existingBaggageHeader)) {
 newBaggageHeaders.push(...existingBaggageHeader);
 newBaggageHeaders = existingBaggageHeader
 .map(headerItem =>
 typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem,
 )
 .filter(headerItem => headerItem === '');
 } else if (existingBaggageHeader) {
 newBaggageHeaders.push(existingBaggageHeader);
 newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader));
 }

 if (sentryBaggageHeader) {
Expand DownExpand Up@@ -221,3 +254,13 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void {
 }
 span.end();
}

function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string {
 return (
 baggageHeader
 .split(',')
 // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 .filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))
 .join(',')
 );
}

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
fix(core): `.set` the `sentry-trace` header instead of `.append`ing in fetch instrumentation by lforst · Pull Request #13907 · getsentry/sentry-javascript · GitHub
Skip to content
fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation#13907

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.

 Sign up for GitHub

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

 Merged

lforst
merged 13 commits into

from

Oct 16, 2024
 Merged


fix(core): .set the sentry-trace header instead of .appending in fetch instrumentation
#13907
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


13 commits


Select commit
Hold shift + click to select a range
d86cca9

fix(core): `.set` the `sentry-trace` header instead of `.append`ing i…

Oct 8, 2024
5206c77

Remove old ones and append new sentry baggage values

Oct 8, 2024
596eda4

lint & size limit

Oct 8, 2024
1ad1b53

Simplify and add comment

Oct 8, 2024
c9e8f38

Override values for all the different variants of passing headers

Oct 8, 2024
5e01be3

smol rename

Oct 8, 2024
9bc4bc8

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 8, 2024
6a08581

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 15, 2024
e672a37

Boi am I glad I added tests

Oct 15, 2024
7d16405

size limit

Oct 15, 2024
66d1153

size limit

Oct 16, 2024
28ace04

size limit

Oct 16, 2024
ba16ef7

Merge branch 'develop' into lforst-set-headers-instead-of-append

Oct 16, 2024

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


4 changes: 2 additions & 2 deletions
.size-limit.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -79,7 +79,7 @@ module.exports = [
 path: 'packages/browser/build/npm/esm/index.js',
 import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
 gzip: true,
 limit: '78 KB',
 limit: '78.1 KB',
 },
 {
 name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
Expand DownExpand Up@@ -138,7 +138,7 @@ module.exports = [
 import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
 ignore: ['react/jsx-runtime'],
 gzip: true,
 limit: '39 KB',
 limit: '39.05 KB',
 },
 // Vue SDK (ESM)
 {
Expand Down

10 changes: 10 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/browser';

window.Sentry = Sentry;

Sentry.init({
 dsn: 'https://public@dsn.ingest.sentry.io/1337',
 integrations: [Sentry.browserTracingIntegration()],
 tracePropagationTargets: ['http://example.com'],
 tracesSampleRate: 1,
});

53 changes: 53 additions & 0 deletions
...s/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
fetchPojo.addEventListener('click', () => {
 const fetchOptions = {
 headers: {
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 },
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-pojo', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-pojo', fetchOptions)),
 ),
 );
});

fetchArray.addEventListener('click', () => {
 const fetchOptions = {
 headers: [
 ['sentry-trace', '12312012123120121231201212312012-1121201211212012-1'],
 ['baggage', 'sentry-release=4.2.0'],
 ],
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-array', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-array', fetchOptions)),
 ),
 );
});

fetchHeaders.addEventListener('click', () => {
 const fetchOptions = {
 headers: new Headers({
 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
 baggage: 'sentry-release=4.2.0',
 }),
 };

 // Make two fetch requests that reuse the same fetch object
 Sentry.startSpan({ name: 'does-not-matter-1' }, () =>
 fetch('http://example.com/fetch-headers', fetchOptions)
 .then(res => res.text())
 .then(() =>
 Sentry.startSpan({ name: 'does-not-matter-2' }, () => fetch('http://example.com/fetch-headers', fetchOptions)),
 ),
 );
});

11 changes: 11 additions & 0 deletions
...rowser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/template.html

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html>
 <head>
 <meta charset="utf-8" />
 </head>
 <body>
 <button id="fetchPojo">Fetch POJO</button>
 <button id="fetchArray">Fetch array</button>
 <button id="fetchHeaders">Fetch Headers</button>
 </body>
</html>

64 changes: 64 additions & 0 deletions
...ages/browser-integration-tests/suites/tracing/trace-lifetime/trace-header-merging/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
import type { Page, Request } from '@playwright/test';
import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { shouldSkipTracingTest } from '../../../../utils/helpers';

async function assertRequests({
 page,
 buttonSelector,
 requestMatcher,
}: { page: Page; buttonSelector: string; requestMatcher: string }) {
 const requests = await new Promise<Request[]>(resolve => {
 const requests: Request[] = [];
 page
 .route(requestMatcher, (route, request) => {
 requests.push(request);
 if (requests.length === 2) {
 resolve(requests);
 }

 return route.fulfill({
 status: 200,
 contentType: 'application/json',
 body: JSON.stringify({}),
 });
 })
 .then(() => {
 page.click(buttonSelector);
 });
 });

 requests.forEach(request => {
 const headers = request.headers();

 // No merged sentry trace headers
 expect(headers['sentry-trace']).not.toContain(',');

 // No multiple baggage entries
 expect(headers['baggage'].match(/sentry-trace_id/g) ?? []).toHaveLength(1);
 });
}

sentryTest(
 'Ensure the SDK does not infinitely append tracing headers to outgoing requests',
 async ({ getLocalTestUrl, page }) => {
 if (shouldSkipTracingTest()) {
 sentryTest.skip();
 }

 const url = await getLocalTestUrl({ testDir: __dirname });
 await page.goto(url);

 await sentryTest.step('fetch with POJO', () =>
 assertRequests({ page, buttonSelector: '#fetchPojo', requestMatcher: 'http://example.com/fetch-pojo' }),
 );

 await sentryTest.step('fetch with array', () =>
 assertRequests({ page, buttonSelector: '#fetchArray', requestMatcher: 'http://example.com/fetch-array' }),
 );

 await sentryTest.step('fetch with Headers instance', () =>
 assertRequests({ page, buttonSelector: '#fetchHeaders', requestMatcher: 'http://example.com/fetch-headers' }),
 );
 },
);

63 changes: 53 additions & 10 deletions
packages/core/src/fetch.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import type { Client, HandlerDataFetch, Scope, Span, SpanOrigin } from '@sentry/types';
import {
 BAGGAGE_HEADER_NAME,
 SENTRY_BAGGAGE_KEY_PREFIX,
 dynamicSamplingContextToSentryBaggageHeader,
 generateSentryTraceHeader,
 isInstanceOf,
Expand DownExpand Up@@ -122,7 +123,7 @@ export function addTracingHeadersToFetchRequest(
 request: string | unknown, // unknown is actually type Request but we can't export DOM types from this package,
 client: Client,
 scope: Scope,
options: {
fetchOptionsObj: {
 headers?:
 | {
 [key: string]: string[] | string | undefined;
Expand All@@ -145,25 +146,53 @@ export function addTracingHeadersToFetchRequest(
 );

 const headers =
options.headers ||
fetchOptionsObj.headers ||
 (typeof Request !== 'undefined' && isInstanceOf(request, Request) ? (request as Request).headers : undefined);

 if (!headers) {
 return { 'sentry-trace': sentryTraceHeader, baggage: sentryBaggageHeader };
 } else if (typeof Headers !== 'undefined' && isInstanceOf(headers, Headers)) {
 const newHeaders = new Headers(headers as Headers);

 newHeaders.append('sentry-trace', sentryTraceHeader);
 newHeaders.set('sentry-trace', sentryTraceHeader);

 if (sentryBaggageHeader) {
 // If the same header is appended multiple times the browser will merge the values into a single request header.
 // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header.
 newHeaders.append(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 const prevBaggageHeader = newHeaders.get(BAGGAGE_HEADER_NAME);
 if (prevBaggageHeader) {
 const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader);
 newHeaders.set(
 BAGGAGE_HEADER_NAME,
 // If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header
 // otherwise just set the sentry baggage header
 prevHeaderStrippedFromSentryBaggage
 ? `${prevHeaderStrippedFromSentryBaggage},${sentryBaggageHeader}`
 : sentryBaggageHeader,
 );
 } else {
 newHeaders.set(BAGGAGE_HEADER_NAME, sentryBaggageHeader);
 }
 }

 return newHeaders as PolymorphicRequestHeaders;
 } else if (Array.isArray(headers)) {
 const newHeaders = [...headers, ['sentry-trace', sentryTraceHeader]];
 const newHeaders = [
 ...headers
 // Remove any existing sentry-trace headers
 .filter(header => {
 return !(Array.isArray(header) && header[0] === 'sentry-trace');
 })
 // Get rid of previous sentry baggage values in baggage header
 .map(header => {
 if (Array.isArray(header) && header[0] === BAGGAGE_HEADER_NAME && typeof header[1] === 'string') {
 const [headerName, headerValue, ...rest] = header;
 return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest];
 } else {
 return header;
 }
 }),
 // Attach the new sentry-trace header
 ['sentry-trace', sentryTraceHeader],
 ];

 if (sentryBaggageHeader) {
 // If there are multiple entries with the same key, the browser will merge the values into a single request header.
Expand All@@ -174,12 +203,16 @@ export function addTracingHeadersToFetchRequest(
 return newHeaders as PolymorphicRequestHeaders;
 } else {
 const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined;
const newBaggageHeaders: string[] = [];
let newBaggageHeaders: string[] = [];

 if (Array.isArray(existingBaggageHeader)) {
 newBaggageHeaders.push(...existingBaggageHeader);
 newBaggageHeaders = existingBaggageHeader
 .map(headerItem =>
 typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem,
 )
 .filter(headerItem => headerItem === '');
 } else if (existingBaggageHeader) {
 newBaggageHeaders.push(existingBaggageHeader);
 newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader));
 }

 if (sentryBaggageHeader) {
Expand DownExpand Up@@ -221,3 +254,13 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void {
 }
 span.end();
}

function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string {
 return (
 baggageHeader
 .split(',')
 // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 .filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))
 .join(',')
 );
}