Skip to content
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

Jump to bottom

fix(instrumentation-fetch): fetch(string, Request) silently drops request body #2411

Merged
merged 3 commits into from Aug 14, 2021
Merged

fix(instrumentation-fetch): fetch(string, Request) silently drops request body #2411

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e8bba80
fix(instrumentation-fetch): pass request object as the only parameter
t2t2 Aug 12, 2021
1109df8
Pass arguments as-is when no span is created
t2t2 Aug 12, 2021
a6250b4
fix: typescript not liking arguments
t2t2 Aug 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/opentelemetry-instrumentation-fetch/src/fetch.ts
View file
Open in desktop
Expand Up @@ -300,7 +300,7 @@ export class FetchInstrumentation extends InstrumentationBase<
const options = input instanceof Request ? input : init || {};
const createdSpan = plugin._createSpan(url, options);
if (!createdSpan) {
return original.apply(this, [url, options]);
return original.apply(this, options instanceof Request ? [options] : [url, options]);
t2t2 marked this conversation as resolved.
Show resolved Hide resolved
}
const spanData = plugin._prepareSpanData(url);

Expand Down Expand Up @@ -380,7 +380,7 @@ export class FetchInstrumentation extends InstrumentationBase<
plugin._addHeaders(options, url);
plugin._tasksCount++;
return original
.apply(this, [url, options])
.apply(this, options instanceof Request ? [options] : [url, options])
dyladan marked this conversation as resolved.
Show resolved Hide resolved
.then(
(onSuccess as any).bind(this, createdSpan, resolve),
onError.bind(this, createdSpan, reject)
Expand Down
26 changes: 24 additions & 2 deletions packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts
View file
Open in desktop
Expand Up @@ -176,11 +176,16 @@ describe('fetch', () => {
};
response.headers = Object.assign({}, init.headers);

if (init.method === 'DELETE') {
if (init instanceof Request) {
// Passing request as 2nd argument causes missing body bug (#2411)
response.status = 400;
response.statusText = 'Bad Request (Request object as 2nd argument)';
reject(new window.Response(JSON.stringify(response), response));
} else if (init.method === 'DELETE') {
response.status = 405;
response.statusText = 'OK';
resolve(new window.Response('foo', response));
} else if (input === url) {
} else if ((input instanceof Request && input.url === url) || input === url) {
response.status = 200;
response.statusText = 'OK';
resolve(new window.Response(JSON.stringify(response), response));
Expand Down Expand Up @@ -530,6 +535,15 @@ describe('fetch', () => {
assert.ok(typeof r.headers.get(X_B3_TRACE_ID) === 'string');
});

it('should pass request object as first parameter to the original function (#2411)', () => {
const r = new Request(url);
return window.fetch(r).then(() => {
assert.ok(true);
}, (response: Response) => {
assert.fail(response.statusText);
});
});

it('should NOT clear the resources', () => {
assert.strictEqual(
clearResourceTimingsSpy.args.length,
Expand Down Expand Up @@ -659,6 +673,14 @@ describe('fetch', () => {
it('should NOT create any span', () => {
assert.strictEqual(exportSpy.args.length, 0, "span shouldn't b exported");
});
it('should pass request object as the first parameter to the original function (#2411)', () => {
const r = new Request(url);
return window.fetch(r).then(() => {
assert.ok(true);
}, (response: Response) => {
assert.fail(response.statusText);
});
});
});

describe('when clearTimingResources is TRUE', () => {
Expand Down