Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import {
import { FetchError, FetchResponse, SpanData } from './types';
import {
getFetchBodyLength,
isRequest,
normalizeHttpRequestMethod,
serverPortFromUrl,
} from './utils';
Expand Down Expand Up @@ -215,7 +216,7 @@ export class FetchInstrumentation extends InstrumentationBase<FetchInstrumentati
return;
}

if (options instanceof Request) {
if (isRequest(options)) {
propagation.inject(context.active(), options.headers, {
set: (h, k, v) => h.set(k, typeof v === 'string' ? v : String(v)),
});
Expand Down Expand Up @@ -395,10 +396,10 @@ export class FetchInstrumentation extends InstrumentationBase<FetchInstrumentati
): Promise<Response> {
const self = this;
const url = web.parseUrl(
args[0] instanceof Request ? args[0].url : String(args[0])
isRequest(args[0]) ? args[0].url : String(args[0])
).href;

const options = args[0] instanceof Request ? args[0] : args[1] || {};
const options = isRequest(args[0]) ? args[0] : args[1] || {};
const createdSpan = plugin._createSpan(url, options);
if (!createdSpan) {
return original.apply(this, args);
Expand Down Expand Up @@ -568,7 +569,7 @@ export class FetchInstrumentation extends InstrumentationBase<FetchInstrumentati
return original
.apply(
self,
options instanceof Request ? [options] : [url, options]
isRequest(options) ? [options] : [url, options]
)
.then(
onSuccess.bind(self, createdSpan, resolve),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,33 @@ const DIAG_LOGGER = diag.createComponentLogger({
namespace: '@opentelemetry/opentelemetry-instrumentation-fetch/utils',
});

export function isRequest(value: unknown): value is Request {
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to document that we duck-type for Request in this instrumentation as a result of this PR just to make it clear what the risk (even if minimal) of false positives.

if (value === null || typeof value !== 'object') {
return false;
}

const candidate = value as {
url?: unknown;
method?: unknown;
headers?: unknown;
clone?: unknown;
};

const headers = candidate.headers as
| { get?: unknown; set?: unknown }
| undefined;

return (
typeof candidate.url === 'string' &&
typeof candidate.method === 'string' &&
typeof candidate.clone === 'function' &&
typeof headers === 'object' &&
headers !== null &&
typeof headers.get === 'function' &&
typeof headers.set === 'function'
);
}

/**
* Helper function to determine payload content length for fetch requests
*
Expand Down Expand Up @@ -67,16 +94,18 @@ export function getFetchBodyLength(...args: Parameters<typeof fetch>) {
} else {
return Promise.resolve(getXHRBodyLength(requestInit.body));
}
} else {
} else if (isRequest(args[0])) {
const info = args[0];
if (!info?.body) {
if (!info.body) {
return Promise.resolve();
}

return info
.clone()
.text()
.then(t => getByteLength(t));
} else {
return Promise.resolve();
}
}

Expand Down Expand Up @@ -214,7 +243,7 @@ function getKnownMethods() {
);
if (cfgMethods && cfgMethods.length > 0) {
knownMethods = {};
cfgMethods.forEach(m => {
cfgMethods.forEach((m: string) => {
knownMethods[m] = true;
});
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
FetchInstrumentation,
FetchInstrumentationConfig,
} from '../src';
import { isRequest as isRequestLike } from '../src/utils';
import { AttributeNames } from '../src/enums/AttributeNames';
import {
ATTR_HTTP_HOST,
Expand Down Expand Up @@ -1750,7 +1751,7 @@ describe('fetch', () => {
config: {
requestHook: (span, request) => {
assert.ok(
request instanceof Request,
isRequestLike(request),
'`requestHook` should get the `Request` object passed to `fetch()`'
);

Expand Down Expand Up @@ -1779,6 +1780,53 @@ describe('fetch', () => {
);
});

it('treats a `Request` argument as a request even if global Request is overwritten', async () => {
const OriginalRequest = globalThis.Request;

try {
(globalThis as any).Request = function Request() {};

const { response } = await tracedFetch({
config: {
requestHook: (span, request) => {
assert.ok(
isRequestLike(request),
'`requestHook` should get the `Request` object passed to `fetch()`'
);

assert.ok(
!((request as any) instanceof Request),
'sanity check: `instanceof Request` should fail when global Request is overwritten'
);

(request as Request).headers.set('custom-foo', 'foo');
},
},
callback: () =>
fetch(
new OriginalRequest('/api/echo-headers.json', {
headers: new Headers({ 'custom-bar': 'bar' }),
})
),
});

const { request } = await response.json();

assert.strictEqual(
request.headers['custom-foo'],
'foo',
'header set from requestHook should be sent'
);
assert.strictEqual(
request.headers['custom-bar'],
'bar',
'header set from fetch() should be sent'
);
} finally {
globalThis.Request = OriginalRequest;
}
});

it('can modify headers when called with a `RequestInit` object', async () => {
const { response } = await tracedFetch({
config: {
Expand Down