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

Commit

Permalink
Instrumentation/HTTP2/HTTPS: Enforce strictNullChecks and noUnusedLoc…
Browse files Browse the repository at this point in the history 
…als (#406)

* Enforce/strictNullChecks: HTTP2 and HTTPS Plugins

* Add check for isNaN
  • Loading branch information
@mayurkale22
mayurkale22 committed Mar 9, 2019
1 parent 89cb441 commit 0d91323
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 57 deletions.
37 changes: 20 additions & 17 deletions packages/opencensus-instrumentation-http2/src/http2.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,10 @@
* limitations under the License.
*/

import {Func, HeaderGetter, HeaderSetter, MessageEventType, Span, SpanKind, TraceOptions, Tracer} from '@opencensus/core';
import {Func, HeaderGetter, HeaderSetter, MessageEventType, Span, SpanKind, TraceOptions} from '@opencensus/core';
import {HttpPlugin} from '@opencensus/instrumentation-http';
import * as http from 'http';
import * as http2 from 'http2';
import * as net from 'net';
import * as shimmer from 'shimmer';
import * as tls from 'tls';
import * as url from 'url';
import * as uuid from 'uuid';

Expand Down Expand Up @@ -138,19 +135,20 @@ export class Http2Plugin extends HttpPlugin {
}

request.on('response', (responseHeaders: http2.IncomingHttpHeaders) => {
const statusCode = responseHeaders[':status'] || 200;
span.addAttribute(
Http2Plugin.ATTRIBUTE_HTTP_STATUS_CODE,
`${responseHeaders[':status']}`);
span.setStatus(
Http2Plugin.parseResponseStatus(+responseHeaders[':status']));
Http2Plugin.ATTRIBUTE_HTTP_STATUS_CODE, `${statusCode}`);
span.setStatus(Http2Plugin.parseResponseStatus(+statusCode));
});

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

span.addAttribute(
Http2Plugin.ATTRIBUTE_HTTP_HOST, url.parse(authority).host);
const host = url.parse(authority).host;
if (host) {
span.addAttribute(Http2Plugin.ATTRIBUTE_HTTP_HOST, host);
}
span.addAttribute(
Http2Plugin.ATTRIBUTE_HTTP_METHOD, `${headers[':method']}`);
span.addAttribute(
Expand Down Expand Up @@ -209,21 +207,26 @@ export class Http2Plugin extends HttpPlugin {
}

const propagation = plugin.tracer.propagation;
const getter = {
const getter: HeaderGetter = {
getHeader(name: string) {
return headers[name];
}
} as HeaderGetter;
};

const traceOptions = {
name: headers[':path'],
const traceOptions: TraceOptions = {
name: headers[':path'] || '',
kind: SpanKind.SERVER,
spanContext: propagation ? propagation.extract(getter) : null
} as TraceOptions;
};
if (propagation) {
const spanContext = propagation.extract(getter);
if (spanContext) {
traceOptions.spanContext = spanContext;
}
}

// Respond is called in a stream event. We wrap it to get the sent
// status code.
let statusCode: number = null;
let statusCode: number;
const originalRespond = stream.respond;
stream.respond = function(this: http2.Http2Stream) {
// Unwrap it since respond is not allowed to be called more than once
Expand Down
34 changes: 16 additions & 18 deletions packages/opencensus-instrumentation-http2/test/test-http2.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,10 @@
*/


import {CoreTracer, RootSpan, Span, SpanEventListener, TracerConfig} from '@opencensus/core';
import {logger} from '@opencensus/core';
import {CoreTracer, logger, RootSpan, Span, SpanEventListener} from '@opencensus/core';
import * as assert from 'assert';
import * as http2 from 'http2';
import * as mocha from 'mocha';
import * as semver from 'semver';
import * as shimmer from 'shimmer';

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

function assertSpanAttributes(
span: Span, httpStatusCode: number, httpMethod: string, hostName: string,
path: string, userAgent: string) {
path: string, userAgent?: string) {
assert.strictEqual(
span.status.code, Http2Plugin.parseResponseStatus(httpStatusCode));
assert.strictEqual(
Expand All @@ -48,8 +45,10 @@ function assertSpanAttributes(
span.attributes[Http2Plugin.ATTRIBUTE_HTTP_METHOD], httpMethod);
assert.strictEqual(span.attributes[Http2Plugin.ATTRIBUTE_HTTP_PATH], path);
assert.strictEqual(span.attributes[Http2Plugin.ATTRIBUTE_HTTP_ROUTE], path);
assert.strictEqual(
span.attributes[Http2Plugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
if (userAgent) {
assert.strictEqual(
span.attributes[Http2Plugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
}
assert.strictEqual(
span.attributes[Http2Plugin.ATTRIBUTE_HTTP_STATUS_CODE],
`${httpStatusCode}`);
Expand Down Expand Up @@ -100,12 +99,14 @@ describe('Http2Plugin', () => {
before(() => {
tracer.registerSpanEventListener(rootSpanVerifier);

plugin.enable(http2, tracer, VERSION, {}, null);
plugin.enable(http2, tracer, VERSION, {}, '');
server = http2.createServer();
server.on('stream', (stream, requestHeaders) => {
const statusCode = requestHeaders[':path'].length > 1 ?
+requestHeaders[':path'].slice(1) :
200;
const path = requestHeaders[':path'];
let statusCode = 200;
if (path && path.length > 1) {
statusCode = isNaN(Number(path.slice(1))) ? 200 : Number(path.slice(1));
}
stream.respond({':status': statusCode, 'content-type': 'text/plain'});
stream.end(`${statusCode}`);
});
Expand Down Expand Up @@ -139,7 +140,7 @@ describe('Http2Plugin', () => {
rootSpanVerifier.endedRootSpans[1].name.indexOf(testPath) >= 0);

const span = rootSpanVerifier.endedRootSpans[1];
assertSpanAttributes(span, 200, 'GET', host, testPath, undefined);
assertSpanAttributes(span, 200, 'GET', host, testPath);
});
});

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

const span = rootSpanVerifier.endedRootSpans[1];
assertSpanAttributes(
span, errorCode, 'GET', host, testPath, undefined);
assertSpanAttributes(span, errorCode, 'GET', host, testPath);
});
});
});
Expand All @@ -179,8 +179,7 @@ describe('Http2Plugin', () => {
assert.ok(root.spans[0].name.indexOf(testPath) >= 0);
assert.strictEqual(root.traceId, root.spans[0].traceId);
const span = root.spans[0];
assertSpanAttributes(
span, statusCode, 'GET', host, testPath, undefined);
assertSpanAttributes(span, statusCode, 'GET', host, testPath);
});
});
});
Expand All @@ -201,8 +200,7 @@ describe('Http2Plugin', () => {
assert.strictEqual(root.traceId, root.spans[0].traceId);

const span = root.spans[0];
assertSpanAttributes(
span, errorCode, 'GET', host, testPath, undefined);
assertSpanAttributes(span, errorCode, 'GET', host, testPath);
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions packages/opencensus-instrumentation-http2/tsconfig.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"pretty": true,
"module": "commonjs",
"target": "es6",
"strictNullChecks": false,
"strictNullChecks": true,
"noUnusedLocals": true
},
"include": [
"src/**/*.ts",
Expand All @@ -16,4 +17,3 @@
"node_modules"
]
}

1 change: 0 additions & 1 deletion packages/opencensus-instrumentation-https/src/https.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import * as http from 'http';
import * as https from 'https';
import * as semver from 'semver';
import * as shimmer from 'shimmer';
import * as url from 'url';

/** Https instrumentation plugin for Opencensus */
export class HttpsPlugin extends HttpPlugin {
Expand Down
28 changes: 11 additions & 17 deletions packages/opencensus-instrumentation-https/test/test-https.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,12 @@
* limitations under the License.
*/

import {CoreTracer, RootSpan, Span, SpanEventListener, TracerConfig} from '@opencensus/core';
import {logger} from '@opencensus/core';
import {CoreTracer, logger, RootSpan, Span, SpanEventListener} from '@opencensus/core';
import * as assert from 'assert';
import * as fs from 'fs';
import {IncomingMessage} from 'http';
import * as https from 'https';
import * as mocha from 'mocha';
import * as nock from 'nock';
import * as shimmer from 'shimmer';
import * as url from 'url';

import {plugin} from '../src/';
import {HttpsPlugin} from '../src/';
Expand Down Expand Up @@ -83,7 +79,7 @@ const httpsOptions = {

function assertSpanAttributes(
span: Span, httpStatusCode: number, httpMethod: string, hostName: string,
path: string, userAgent: string) {
path: string, userAgent?: string) {
assert.strictEqual(
span.status.code, HttpsPlugin.parseResponseStatus(httpStatusCode));
assert.strictEqual(
Expand All @@ -92,8 +88,10 @@ function assertSpanAttributes(
span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_METHOD], httpMethod);
assert.strictEqual(span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_PATH], path);
assert.strictEqual(span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_ROUTE], path);
assert.strictEqual(
span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
if (userAgent) {
assert.strictEqual(
span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
}
assert.strictEqual(
span.attributes[HttpsPlugin.ATTRIBUTE_HTTP_STATUS_CODE],
`${httpStatusCode}`);
Expand Down Expand Up @@ -127,7 +125,7 @@ describe('HttpsPlugin', () => {
(url: string) => url === `${urlHost}/ignored/function`
]
},
null);
'');
tracer.registerSpanEventListener(rootSpanVerifier);
server = https.createServer(httpsOptions, (request, response) => {
response.end('Test Server Response');
Expand Down Expand Up @@ -170,8 +168,7 @@ describe('HttpsPlugin', () => {
assert.strictEqual(rootSpanVerifier.endedRootSpans.length, 1);

const span = rootSpanVerifier.endedRootSpans[0];
assertSpanAttributes(
span, 200, 'GET', hostName, testPath, undefined);
assertSpanAttributes(span, 200, 'GET', hostName, testPath);
});
});

Expand All @@ -194,8 +191,7 @@ describe('HttpsPlugin', () => {
assert.strictEqual(rootSpanVerifier.endedRootSpans.length, 1);
const span = rootSpanVerifier.endedRootSpans[0];
assertSpanAttributes(
span, httpErrorCodes[i], 'GET', hostName, testPath,
undefined);
span, httpErrorCodes[i], 'GET', hostName, testPath);
});
});
}
Expand All @@ -212,8 +208,7 @@ describe('HttpsPlugin', () => {
assert.ok(root.spans[0].name.indexOf(testPath) >= 0);
assert.strictEqual(root.traceId, root.spans[0].traceId);
const span = root.spans[0];
assertSpanAttributes(
span, 200, 'GET', hostName, testPath, undefined);
assertSpanAttributes(span, 200, 'GET', hostName, testPath);
});
});
});
Expand All @@ -236,8 +231,7 @@ describe('HttpsPlugin', () => {

const span = root.spans[0];
assertSpanAttributes(
span, httpErrorCodes[i], 'GET', hostName, testPath,
undefined);
span, httpErrorCodes[i], 'GET', hostName, testPath);
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions packages/opencensus-instrumentation-https/tsconfig.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"pretty": true,
"module": "commonjs",
"target": "es6",
"strictNullChecks": false
"strictNullChecks": true,
"noUnusedLocals": true
},
"include": [
"src/**/*.ts",
Expand All @@ -16,4 +17,3 @@
"node_modules"
]
}

0 comments on commit 0d91323

Please sign in to comment.