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

Commit 80db31f

Browse files
loneparadoxmayurkale22
authored andcommitted
Fix propagation breaking for https.get (#657)
shamelful copy from the http plugin code, to fix the `https.get` same as `http.get`.
1 parent 67aca05 commit 80db31f

File tree

2 files changed

+63
-6
lines changed

2 files changed

+63
-6
lines changed

packages/opencensus-instrumentation-https/src/https.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,26 @@ export class HttpsPlugin extends HttpPlugin {
5959
this.getPatchHttpsOutgoingRequest()
6060
);
6161
if (semver.satisfies(this.version, '>=8.0.0')) {
62-
shimmer.wrap(
63-
this.moduleExports,
64-
'get',
65-
this.getPatchHttpsOutgoingRequest()
66-
);
62+
shimmer.wrap(this.moduleExports, 'get', () => {
63+
// Re-implement http.get. This needs to be done (instead of using
64+
// getPatchOutgoingRequestFunction to patch it) because we need to
65+
// set the trace context header before the returned ClientRequest is
66+
// ended. The Node.js docs state that the only differences between
67+
// request and get are that (1) get defaults to the HTTP GET method and
68+
// (2) the returned request object is ended immediately. The former is
69+
// already true (at least in supported Node versions up to v9), so we
70+
// simply follow the latter. Ref:
71+
// https://nodejs.org/dist/latest/docs/api/http.html#http_http_get_options_callback
72+
// https://github.com/googleapis/cloud-trace-nodejs/blob/master/src/plugins/plugin-http.ts#L198
73+
return function getTrace(
74+
options: https.RequestOptions | string,
75+
callback: ((res: http.IncomingMessage) => void) | undefined
76+
) {
77+
const req = https.request(options, callback);
78+
req.end();
79+
return req;
80+
};
81+
});
6782
}
6883

6984
return this.moduleExports;

packages/opencensus-instrumentation-https/test/test-https.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ import {
2020
Span,
2121
SpanEventListener,
2222
SpanKind,
23+
HeaderGetter,
24+
HeaderSetter,
25+
Propagation,
26+
SpanContext,
2327
} from '@opencensus/core';
2428
import * as assert from 'assert';
2529
import * as fs from 'fs';
@@ -57,6 +61,27 @@ function customAttributeFunction(
5761

5862
type RequestFunction = typeof https.request | typeof https.get;
5963

64+
class DummyPropagation implements Propagation {
65+
extract(getter: HeaderGetter): SpanContext {
66+
return {
67+
traceId: 'dummy-trace-id',
68+
spanId: 'dummy-span-id',
69+
} as SpanContext;
70+
}
71+
72+
inject(setter: HeaderSetter, spanContext: SpanContext): void {
73+
setter.setHeader('x-dummy-trace-id', spanContext.traceId || 'undefined');
74+
setter.setHeader('x-dummy-span-id', spanContext.spanId || 'undefined');
75+
}
76+
77+
generate(): SpanContext {
78+
return {
79+
traceId: 'dummy-trace-id',
80+
spanId: 'dummy-span-id',
81+
} as SpanContext;
82+
}
83+
}
84+
6085
const httpRequest = {
6186
request: (options: {} | string) => {
6287
return httpRequest.make(options, https.request);
@@ -150,7 +175,11 @@ describe('HttpsPlugin', () => {
150175
const log = logger.logger();
151176
const tracer = new CoreTracer();
152177
const spanVerifier = new SpanVerifier();
153-
tracer.start({ samplingRate: 1, logger: log });
178+
tracer.start({
179+
samplingRate: 1,
180+
logger: log,
181+
propagation: new DummyPropagation(),
182+
});
154183

155184
it('should return a plugin', () => {
156185
assert.ok(plugin instanceof HttpsPlugin);
@@ -369,6 +398,19 @@ describe('HttpsPlugin', () => {
369398
}
370399
});
371400
});
401+
402+
it('should create a rootSpan for GET requests and add propagation headers', async () => {
403+
nock.enableNetConnect();
404+
assert.strictEqual(spanVerifier.endedSpans.length, 0);
405+
await httpRequest.get(`https://google.fr/`).then(result => {
406+
assert.strictEqual(spanVerifier.endedSpans.length, 1);
407+
assert.ok(spanVerifier.endedSpans[0].name.indexOf('GET /') >= 0);
408+
409+
const span = spanVerifier.endedSpans[0];
410+
assertSpanAttributes(span, 301, 'GET', 'google.fr', '/');
411+
});
412+
nock.disableNetConnect();
413+
});
372414
});
373415

374416
/** Should intercept incoming requests */

0 commit comments

Comments
 (0)