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

Commit d4f36b0

Browse files
mayurkale22mayurkale22
authored
enforce strictNullChecks on HTTP plugin (#364)
1 parent 8ba7367 commit d4f36b0

3 files changed

Lines changed: 55 additions & 47 deletions

File tree

‎packages/opencensus-instrumentation-http/src/http.ts‎

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,14 @@
1414
* limitations under the License.
1515
*/
1616

17-
import {BasePlugin, CanonicalCode, Func, HeaderGetter, HeaderSetter, MessageEventType, RootSpan, Span, SpanKind, Tracer} from '@opencensus/core';
18-
import {logger, Logger} from '@opencensus/core';
17+
import {BasePlugin, CanonicalCode, Func, HeaderGetter, HeaderSetter, MessageEventType, RootSpan, Span, SpanKind, TraceOptions} from '@opencensus/core';
1918
import * as httpModule from 'http';
2019
import * as semver from 'semver';
2120
import * as shimmer from 'shimmer';
2221
import * as url from 'url';
2322
import * as uuid from 'uuid';
24-
2523
import {HttpPluginConfig, IgnoreMatcher} from './types';
2624

27-
2825
export type HttpGetCallback = (res: httpModule.IncomingMessage) => void;
2926
export type HttpModule = typeof httpModule;
3027
export type RequestFunction = typeof httpModule.request;
@@ -45,8 +42,6 @@ export class HttpPlugin extends BasePlugin {
4542
static ATTRIBUTE_HTTP_ERROR_NAME = 'http.error_name';
4643
static ATTRIBUTE_HTTP_ERROR_MESSAGE = 'http.error_message';
4744

48-
protected options: HttpPluginConfig;
49-
5045
/** Constructs a new HttpPlugin instance. */
5146
constructor(moduleName: string) {
5247
super(moduleName);
@@ -172,8 +167,7 @@ export class HttpPlugin extends BasePlugin {
172167

173168
const request: httpModule.IncomingMessage = args[0];
174169
const response: httpModule.ServerResponse = args[1];
175-
const path = url.parse(request.url).pathname;
176-
170+
const path = request.url ? url.parse(request.url).pathname || '' : '';
177171
plugin.logger.debug('%s plugin incomingRequest', plugin.moduleName);
178172

179173
if (plugin.isIgnored(
@@ -189,11 +183,13 @@ export class HttpPlugin extends BasePlugin {
189183
}
190184
};
191185

192-
const traceOptions = {
193-
name: path,
194-
kind: SpanKind.SERVER,
195-
spanContext: propagation ? propagation.extract(getter) : null
196-
};
186+
const traceOptions: TraceOptions = {name: path, kind: SpanKind.SERVER};
187+
if (propagation) {
188+
const spanContext = propagation.extract(getter);
189+
if (spanContext) {
190+
traceOptions.spanContext = spanContext;
191+
}
192+
}
197193

198194
return plugin.tracer.startRootSpan(traceOptions, rootSpan => {
199195
if (!rootSpan) return original.apply(this, arguments);
@@ -209,7 +205,7 @@ export class HttpPlugin extends BasePlugin {
209205
response.end = originalEnd;
210206
const returned = response.end.apply(this, arguments);
211207

212-
const requestUrl = url.parse(request.url);
208+
const requestUrl = request.url ? url.parse(request.url) : null;
213209
const host = headers.host || 'localhost';
214210
const userAgent =
215211
(headers['user-agent'] || headers['User-Agent']) as string;
@@ -220,12 +216,17 @@ export class HttpPlugin extends BasePlugin {
220216
/^(.*)(\:[0-9]{1,5})/,
221217
'$1',
222218
));
219+
223220
rootSpan.addAttribute(
224-
HttpPlugin.ATTRIBUTE_HTTP_METHOD, request.method);
225-
rootSpan.addAttribute(
226-
HttpPlugin.ATTRIBUTE_HTTP_PATH, requestUrl.pathname);
227-
rootSpan.addAttribute(
228-
HttpPlugin.ATTRIBUTE_HTTP_ROUTE, requestUrl.path);
221+
HttpPlugin.ATTRIBUTE_HTTP_METHOD, request.method || 'GET');
222+
223+
if (requestUrl) {
224+
rootSpan.addAttribute(
225+
HttpPlugin.ATTRIBUTE_HTTP_PATH, requestUrl.pathname || '');
226+
rootSpan.addAttribute(
227+
HttpPlugin.ATTRIBUTE_HTTP_ROUTE, requestUrl.path || '');
228+
}
229+
229230
rootSpan.addAttribute(
230231
HttpPlugin.ATTRIBUTE_HTTP_USER_AGENT, userAgent);
231232

@@ -267,13 +268,13 @@ export class HttpPlugin extends BasePlugin {
267268
}
268269

269270
// Makes sure the url is an url object
270-
let pathname = '';
271-
let method = 'GET';
271+
let pathname;
272+
let method;
272273
let origin = '';
273274
if (typeof (options) === 'string') {
274275
const parsedUrl = url.parse(options);
275276
options = parsedUrl;
276-
pathname = parsedUrl.pathname;
277+
pathname = parsedUrl.pathname || '';
277278
origin = `${parsedUrl.protocol || 'http:'}//${parsedUrl.host}`;
278279
} else {
279280
// Do not trace ourselves
@@ -285,9 +286,11 @@ export class HttpPlugin extends BasePlugin {
285286
}
286287

287288
try {
288-
pathname = (options as url.URL).pathname ||
289-
url.parse(options.path).pathname;
290-
method = options.method;
289+
pathname = (options as url.URL).pathname;
290+
if (!pathname) {
291+
pathname = options.path ? url.parse(options.path).pathname : '';
292+
}
293+
method = options.method || 'GET';
291294
origin = `${options.protocol || 'http:'}//${options.host}`;
292295
} catch (e) {
293296
}
@@ -371,19 +374,25 @@ export class HttpPlugin extends BasePlugin {
371374
const userAgent =
372375
headers ? (headers['user-agent'] || headers['User-Agent']) : null;
373376

374-
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_HOST, options.hostname);
377+
if (options.hostname) {
378+
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_HOST, options.hostname);
379+
}
375380
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_METHOD, method);
376-
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_PATH, options.path);
377-
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_ROUTE, options.path);
381+
if (options.path) {
382+
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_PATH, options.path);
383+
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_ROUTE, options.path);
384+
}
385+
378386
if (userAgent) {
379387
span.addAttribute(
380388
HttpPlugin.ATTRIBUTE_HTTP_USER_AGENT, userAgent.toString());
381389
}
382-
span.addAttribute(
383-
HttpPlugin.ATTRIBUTE_HTTP_STATUS_CODE,
384-
response.statusCode.toString());
385-
386-
span.setStatus(HttpPlugin.parseResponseStatus(response.statusCode));
390+
if (response.statusCode) {
391+
span.addAttribute(
392+
HttpPlugin.ATTRIBUTE_HTTP_STATUS_CODE,
393+
response.statusCode.toString());
394+
span.setStatus(HttpPlugin.parseResponseStatus(response.statusCode));
395+
}
387396

388397
// Message Event ID is not defined
389398
span.addMessageEvent(

‎packages/opencensus-instrumentation-http/test/test-http.ts‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,8 @@ import {CoreTracer, HeaderGetter, HeaderSetter, Propagation, RootSpan, Span, Spa
1818
import {logger} from '@opencensus/core';
1919
import * as assert from 'assert';
2020
import * as http from 'http';
21-
import * as mocha from 'mocha';
2221
import * as nock from 'nock';
2322
import * as shimmer from 'shimmer';
24-
2523
import {plugin} from '../src/';
2624
import {HttpPlugin} from '../src/';
2725

@@ -81,7 +79,7 @@ class RootSpanVerifier implements SpanEventListener {
8179

8280
function assertSpanAttributes(
8381
span: Span, httpStatusCode: number, httpMethod: string, hostName: string,
84-
path: string, userAgent: string) {
82+
path: string, userAgent?: string) {
8583
assert.strictEqual(
8684
span.status.code, HttpPlugin.parseResponseStatus(httpStatusCode));
8785
assert.strictEqual(
@@ -92,8 +90,10 @@ function assertSpanAttributes(
9290
span.attributes[HttpPlugin.ATTRIBUTE_HTTP_METHOD], httpMethod);
9391
assert.strictEqual(span.attributes[HttpPlugin.ATTRIBUTE_HTTP_PATH], path);
9492
assert.strictEqual(span.attributes[HttpPlugin.ATTRIBUTE_HTTP_ROUTE], path);
95-
assert.strictEqual(
96-
span.attributes[HttpPlugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
93+
if (userAgent) {
94+
assert.strictEqual(
95+
span.attributes[HttpPlugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
96+
}
9797
assert.strictEqual(
9898
span.attributes[HttpPlugin.ATTRIBUTE_HTTP_STATUS_CODE],
9999
`${httpStatusCode}`);
@@ -130,7 +130,7 @@ describe('HttpPlugin', () => {
130130
(url: string) => url === `${urlHost}/ignored/function`
131131
]
132132
},
133-
null);
133+
'');
134134
tracer.registerSpanEventListener(rootSpanVerifier);
135135
server = http.createServer((request, response) => {
136136
response.end('Test Server Response');
@@ -169,7 +169,7 @@ describe('HttpPlugin', () => {
169169
assert.strictEqual(rootSpanVerifier.endedRootSpans.length, 1);
170170

171171
const span = rootSpanVerifier.endedRootSpans[0];
172-
assertSpanAttributes(span, 200, 'GET', hostName, testPath, undefined);
172+
assertSpanAttributes(span, 200, 'GET', hostName, testPath);
173173
});
174174
});
175175

@@ -192,7 +192,7 @@ describe('HttpPlugin', () => {
192192
assert.strictEqual(rootSpanVerifier.endedRootSpans.length, 1);
193193
const span = rootSpanVerifier.endedRootSpans[0];
194194
assertSpanAttributes(
195-
span, httpErrorCodes[i], 'GET', hostName, testPath, undefined);
195+
span, httpErrorCodes[i], 'GET', hostName, testPath);
196196
});
197197
});
198198
}
@@ -209,7 +209,7 @@ describe('HttpPlugin', () => {
209209
assert.ok(root.spans[0].name.indexOf(testPath) >= 0);
210210
assert.strictEqual(root.traceId, root.spans[0].traceId);
211211
const span = root.spans[0];
212-
assertSpanAttributes(span, 200, 'GET', hostName, testPath, undefined);
212+
assertSpanAttributes(span, 200, 'GET', hostName, testPath);
213213
});
214214
});
215215
});
@@ -232,8 +232,7 @@ describe('HttpPlugin', () => {
232232

233233
const span = root.spans[0];
234234
assertSpanAttributes(
235-
span, httpErrorCodes[i], 'GET', hostName, testPath,
236-
undefined);
235+
span, httpErrorCodes[i], 'GET', hostName, testPath);
237236
});
238237
});
239238
});
@@ -303,7 +302,7 @@ describe('HttpPlugin', () => {
303302
rootSpanVerifier.endedRootSpans[0].name.indexOf('GET /') >= 0);
304303

305304
const span = rootSpanVerifier.endedRootSpans[0];
306-
assertSpanAttributes(span, 301, 'GET', 'google.fr', '/', undefined);
305+
assertSpanAttributes(span, 301, 'GET', 'google.fr', '/');
307306
});
308307
nock.disableNetConnect();
309308
});

‎packages/opencensus-instrumentation-http/tsconfig.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
"pretty": true,
99
"module": "commonjs",
1010
"target": "es6",
11-
"strictNullChecks": false
11+
"strictNullChecks": true,
12+
"noUnusedLocals": false
1213
},
1314
"include": [
1415
"src/**/*.ts",
@@ -18,4 +19,3 @@
1819
"node_modules"
1920
]
2021
}
21-

0 commit comments

Comments
 (0)