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

Commit 0d91323

Browse files
mayurkale22mayurkale22
authored
Instrumentation/HTTP2/HTTPS: Enforce strictNullChecks and noUnusedLocals (#406)
* Enforce/strictNullChecks: HTTP2 and HTTPS Plugins * Add check for isNaN
1 parent 89cb441 commit 0d91323

6 files changed

Lines changed: 51 additions & 57 deletions

File tree

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

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

17-
import {Func, HeaderGetter, HeaderSetter, MessageEventType, Span, SpanKind, TraceOptions, Tracer} from '@opencensus/core';
17+
import {Func, HeaderGetter, HeaderSetter, MessageEventType, Span, SpanKind, TraceOptions} from '@opencensus/core';
1818
import {HttpPlugin} from '@opencensus/instrumentation-http';
19-
import * as http from 'http';
2019
import * as http2 from 'http2';
21-
import * as net from 'net';
2220
import * as shimmer from 'shimmer';
23-
import * as tls from 'tls';
2421
import * as url from 'url';
2522
import * as uuid from 'uuid';
2623

@@ -138,19 +135,20 @@ export class Http2Plugin extends HttpPlugin {
138135
}
139136

140137
request.on('response', (responseHeaders: http2.IncomingHttpHeaders) => {
138+
const statusCode = responseHeaders[':status'] || 200;
141139
span.addAttribute(
142-
Http2Plugin.ATTRIBUTE_HTTP_STATUS_CODE,
143-
`${responseHeaders[':status']}`);
144-
span.setStatus(
145-
Http2Plugin.parseResponseStatus(+responseHeaders[':status']));
140+
Http2Plugin.ATTRIBUTE_HTTP_STATUS_CODE, `${statusCode}`);
141+
span.setStatus(Http2Plugin.parseResponseStatus(+statusCode));
146142
});
147143

148144
request.on('end', () => {
149145
const userAgent =
150146
headers['user-agent'] || headers['User-Agent'] || null;
151147

152-
span.addAttribute(
153-
Http2Plugin.ATTRIBUTE_HTTP_HOST, url.parse(authority).host);
148+
const host = url.parse(authority).host;
149+
if (host) {
150+
span.addAttribute(Http2Plugin.ATTRIBUTE_HTTP_HOST, host);
151+
}
154152
span.addAttribute(
155153
Http2Plugin.ATTRIBUTE_HTTP_METHOD, `${headers[':method']}`);
156154
span.addAttribute(
@@ -209,21 +207,26 @@ export class Http2Plugin extends HttpPlugin {
209207
}
210208

211209
const propagation = plugin.tracer.propagation;
212-
const getter = {
210+
const getter: HeaderGetter = {
213211
getHeader(name: string) {
214212
return headers[name];
215213
}
216-
} as HeaderGetter;
214+
};
217215

218-
const traceOptions = {
219-
name: headers[':path'],
216+
const traceOptions: TraceOptions = {
217+
name: headers[':path'] || '',
220218
kind: SpanKind.SERVER,
221-
spanContext: propagation ? propagation.extract(getter) : null
222-
} as TraceOptions;
219+
};
220+
if (propagation) {
221+
const spanContext = propagation.extract(getter);
222+
if (spanContext) {
223+
traceOptions.spanContext = spanContext;
224+
}
225+
}
223226

224227
// Respond is called in a stream event. We wrap it to get the sent
225228
// status code.
226-
let statusCode: number = null;
229+
let statusCode: number;
227230
const originalRespond = stream.respond;
228231
stream.respond = function(this: http2.Http2Stream) {
229232
// Unwrap it since respond is not allowed to be called more than once

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

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,10 @@
1515
*/
1616

1717

18-
import {CoreTracer, RootSpan, Span, SpanEventListener, TracerConfig} from '@opencensus/core';
19-
import {logger} from '@opencensus/core';
18+
import {CoreTracer, logger, RootSpan, Span, SpanEventListener} from '@opencensus/core';
2019
import * as assert from 'assert';
2120
import * as http2 from 'http2';
22-
import * as mocha from 'mocha';
2321
import * as semver from 'semver';
24-
import * as shimmer from 'shimmer';
2522

2623
import {plugin} from '../src/';
2724
import {Http2Plugin} from '../src/';
@@ -39,7 +36,7 @@ class RootSpanVerifier implements SpanEventListener {
3936

4037
function assertSpanAttributes(
4138
span: Span, httpStatusCode: number, httpMethod: string, hostName: string,
42-
path: string, userAgent: string) {
39+
path: string, userAgent?: string) {
4340
assert.strictEqual(
4441
span.status.code, Http2Plugin.parseResponseStatus(httpStatusCode));
4542
assert.strictEqual(
@@ -48,8 +45,10 @@ function assertSpanAttributes(
4845
span.attributes[Http2Plugin.ATTRIBUTE_HTTP_METHOD], httpMethod);
4946
assert.strictEqual(span.attributes[Http2Plugin.ATTRIBUTE_HTTP_PATH], path);
5047
assert.strictEqual(span.attributes[Http2Plugin.ATTRIBUTE_HTTP_ROUTE], path);
51-
assert.strictEqual(
52-
span.attributes[Http2Plugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
48+
if (userAgent) {
49+
assert.strictEqual(
50+
span.attributes[Http2Plugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
51+
}
5352
assert.strictEqual(
5453
span.attributes[Http2Plugin.ATTRIBUTE_HTTP_STATUS_CODE],
5554
`${httpStatusCode}`);
@@ -100,12 +99,14 @@ describe('Http2Plugin', () => {
10099
before(() => {
101100
tracer.registerSpanEventListener(rootSpanVerifier);
102101

103-
plugin.enable(http2, tracer, VERSION, {}, null);
102+
plugin.enable(http2, tracer, VERSION, {}, '');
104103
server = http2.createServer();
105104
server.on('stream', (stream, requestHeaders) => {
106-
const statusCode = requestHeaders[':path'].length > 1 ?
107-
+requestHeaders[':path'].slice(1) :
108-
200;
105+
const path = requestHeaders[':path'];
106+
let statusCode = 200;
107+
if (path && path.length > 1) {
108+
statusCode = isNaN(Number(path.slice(1))) ? 200 : Number(path.slice(1));
109+
}
109110
stream.respond({':status': statusCode, 'content-type': 'text/plain'});
110111
stream.end(`${statusCode}`);
111112
});
@@ -139,7 +140,7 @@ describe('Http2Plugin', () => {
139140
rootSpanVerifier.endedRootSpans[1].name.indexOf(testPath) >= 0);
140141

141142
const span = rootSpanVerifier.endedRootSpans[1];
142-
assertSpanAttributes(span, 200, 'GET', host, testPath, undefined);
143+
assertSpanAttributes(span, 200, 'GET', host, testPath);
143144
});
144145
});
145146

@@ -160,8 +161,7 @@ describe('Http2Plugin', () => {
160161
0);
161162

162163
const span = rootSpanVerifier.endedRootSpans[1];
163-
assertSpanAttributes(
164-
span, errorCode, 'GET', host, testPath, undefined);
164+
assertSpanAttributes(span, errorCode, 'GET', host, testPath);
165165
});
166166
});
167167
});
@@ -179,8 +179,7 @@ describe('Http2Plugin', () => {
179179
assert.ok(root.spans[0].name.indexOf(testPath) >= 0);
180180
assert.strictEqual(root.traceId, root.spans[0].traceId);
181181
const span = root.spans[0];
182-
assertSpanAttributes(
183-
span, statusCode, 'GET', host, testPath, undefined);
182+
assertSpanAttributes(span, statusCode, 'GET', host, testPath);
184183
});
185184
});
186185
});
@@ -201,8 +200,7 @@ describe('Http2Plugin', () => {
201200
assert.strictEqual(root.traceId, root.spans[0].traceId);
202201

203202
const span = root.spans[0];
204-
assertSpanAttributes(
205-
span, errorCode, 'GET', host, testPath, undefined);
203+
assertSpanAttributes(span, errorCode, 'GET', host, testPath);
206204
});
207205
});
208206
});

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
"pretty": true,
77
"module": "commonjs",
88
"target": "es6",
9-
"strictNullChecks": false,
9+
"strictNullChecks": true,
10+
"noUnusedLocals": true
1011
},
1112
"include": [
1213
"src/**/*.ts",
@@ -16,4 +17,3 @@
1617
"node_modules"
1718
]
1819
}
19-

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import * as http from 'http';
2020
import * as https from 'https';
2121
import * as semver from 'semver';
2222
import * as shimmer from 'shimmer';
23-
import * as url from 'url';
2423

2524
/** Https instrumentation plugin for Opencensus */
2625
export class HttpsPlugin extends HttpPlugin {

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

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

17-
import {CoreTracer, RootSpan, Span, SpanEventListener, TracerConfig} from '@opencensus/core';
18-
import {logger} from '@opencensus/core';
17+
import {CoreTracer, logger, RootSpan, Span, SpanEventListener} from '@opencensus/core';
1918
import * as assert from 'assert';
2019
import * as fs from 'fs';
21-
import {IncomingMessage} from 'http';
2220
import * as https from 'https';
23-
import * as mocha from 'mocha';
2421
import * as nock from 'nock';
2522
import * as shimmer from 'shimmer';
26-
import * as url from 'url';
2723

2824
import {plugin} from '../src/';
2925
import {HttpsPlugin} from '../src/';
@@ -83,7 +79,7 @@ const httpsOptions = {
8379

8480
function assertSpanAttributes(
8581
span: Span, httpStatusCode: number, httpMethod: string, hostName: string,
86-
path: string, userAgent: string) {
82+
path: string, userAgent?: string) {
8783
assert.strictEqual(
8884
span.status.code, HttpsPlugin.parseResponseStatus(httpStatusCode));
8985
assert.strictEqual(
@@ -92,8 +88,10 @@ function assertSpanAttributes(
9288
span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_METHOD], httpMethod);
9389
assert.strictEqual(span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_PATH], path);
9490
assert.strictEqual(span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_ROUTE], path);
95-
assert.strictEqual(
96-
span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
91+
if (userAgent) {
92+
assert.strictEqual(
93+
span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
94+
}
9795
assert.strictEqual(
9896
span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_STATUS_CODE],
9997
`${httpStatusCode}`);
@@ -127,7 +125,7 @@ describe('HttpsPlugin', () => {
127125
(url: string) => url === `${urlHost}/ignored/function`
128126
]
129127
},
130-
null);
128+
'');
131129
tracer.registerSpanEventListener(rootSpanVerifier);
132130
server = https.createServer(httpsOptions, (request, response) => {
133131
response.end('Test Server Response');
@@ -170,8 +168,7 @@ describe('HttpsPlugin', () => {
170168
assert.strictEqual(rootSpanVerifier.endedRootSpans.length, 1);
171169

172170
const span = rootSpanVerifier.endedRootSpans[0];
173-
assertSpanAttributes(
174-
span, 200, 'GET', hostName, testPath, undefined);
171+
assertSpanAttributes(span, 200, 'GET', hostName, testPath);
175172
});
176173
});
177174

@@ -194,8 +191,7 @@ describe('HttpsPlugin', () => {
194191
assert.strictEqual(rootSpanVerifier.endedRootSpans.length, 1);
195192
const span = rootSpanVerifier.endedRootSpans[0];
196193
assertSpanAttributes(
197-
span, httpErrorCodes[i], 'GET', hostName, testPath,
198-
undefined);
194+
span, httpErrorCodes[i], 'GET', hostName, testPath);
199195
});
200196
});
201197
}
@@ -212,8 +208,7 @@ describe('HttpsPlugin', () => {
212208
assert.ok(root.spans[0].name.indexOf(testPath) >= 0);
213209
assert.strictEqual(root.traceId, root.spans[0].traceId);
214210
const span = root.spans[0];
215-
assertSpanAttributes(
216-
span, 200, 'GET', hostName, testPath, undefined);
211+
assertSpanAttributes(span, 200, 'GET', hostName, testPath);
217212
});
218213
});
219214
});
@@ -236,8 +231,7 @@ describe('HttpsPlugin', () => {
236231

237232
const span = root.spans[0];
238233
assertSpanAttributes(
239-
span, httpErrorCodes[i], 'GET', hostName, testPath,
240-
undefined);
234+
span, httpErrorCodes[i], 'GET', hostName, testPath);
241235
});
242236
});
243237
});

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
"pretty": true,
77
"module": "commonjs",
88
"target": "es6",
9-
"strictNullChecks": false
9+
"strictNullChecks": true,
10+
"noUnusedLocals": true
1011
},
1112
"include": [
1213
"src/**/*.ts",
@@ -16,4 +17,3 @@
1617
"node_modules"
1718
]
1819
}
19-

0 commit comments

Comments
 (0)