Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

http-instrumentation: fix propagation errors when using Expect header #481 #482

Merged
merged 3 commits into from
Apr 11, 2019
Merged
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
28 changes: 27 additions & 1 deletion packages/opencensus-instrumentation-http/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,28 @@ export class HttpPlugin extends BasePlugin {

const setter: HeaderSetter = {
setHeader(name: string, value: string) {
request.setHeader(name, value);
// If outgoing request headers contain the "Expect" header, the
// returned ClientRequest will throw an error if any new headers are
// added. We need to set the header directly in the headers object
// which has been cloned earlier.
if (plugin.hasExpectHeader(options) && options.headers) {
options.headers[name] = value;
} else {
request.setHeader(name, value);
}
}
};

const propagation = plugin.tracer.propagation;
if (propagation) {
// If outgoing request headers contain the "Expect" header, the returned
// ClientRequest will throw an error if any new headers are added.
// So we need to clone the options object to be able to inject new
// header.
if (plugin.hasExpectHeader(options)) {
options = Object.assign({}, options) as RequestOptions;
options.headers = Object.assign({}, options.headers);
}
propagation.inject(setter, span.spanContext);
}

Expand Down Expand Up @@ -485,6 +501,16 @@ export class HttpPlugin extends BasePlugin {
} catch (ignore) {
}
}

/**
* Returns whether the Expect header is on the given options object.
* @param options Options for http.request.
*/
hasExpectHeader(options: RequestOptions|url.URL): boolean {
return !!(
(options as RequestOptions).headers &&
(options as RequestOptions).headers!.Expect);
}
}

const plugin = new HttpPlugin('http');
Expand Down
20 changes: 20 additions & 0 deletions packages/opencensus-instrumentation-http/test/test-http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import * as assert from 'assert';
import * as http from 'http';
import * as nock from 'nock';
import * as shimmer from 'shimmer';
import * as url from 'url';

import {HttpPlugin, plugin} from '../src/';
import * as stats from '../src/http-stats';
Expand Down Expand Up @@ -374,6 +375,25 @@ describe('HttpPlugin', () => {
});
nock.disableNetConnect();
});

it('should create a rootSpan for GET requests and add propagation headers with Expect headers',
async () => {
nock.enableNetConnect();
assert.strictEqual(rootSpanVerifier.endedRootSpans.length, 0);
const options = Object.assign(
{headers: {Expect: '100-continue'}},
url.parse('http://google.fr/'));
await httpRequest.get(options).then((result) => {
assert.strictEqual(rootSpanVerifier.endedRootSpans.length, 1);
assert.ok(
rootSpanVerifier.endedRootSpans[0].name.indexOf('GET /') >= 0);

const span = rootSpanVerifier.endedRootSpans[0];
assertSpanAttributes(span, 301, 'GET', 'google.fr', '/');
assertClientStats(testExporter, 301, 'GET');
});
nock.disableNetConnect();
});
});


Expand Down