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

Commit

Permalink
enforce strictNullChecks on HTTP plugin (#364)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mayurkale22
mayurkale22 committed Feb 26, 2019
1 parent 8ba7367 commit d4f36b0
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 47 deletions.
75 changes: 42 additions & 33 deletions packages/opencensus-instrumentation-http/src/http.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,14 @@
* limitations under the License.
*/

import {BasePlugin, CanonicalCode, Func, HeaderGetter, HeaderSetter, MessageEventType, RootSpan, Span, SpanKind, Tracer} from '@opencensus/core';
import {logger, Logger} from '@opencensus/core';
import {BasePlugin, CanonicalCode, Func, HeaderGetter, HeaderSetter, MessageEventType, RootSpan, Span, SpanKind, TraceOptions} from '@opencensus/core';
import * as httpModule from 'http';
import * as semver from 'semver';
import * as shimmer from 'shimmer';
import * as url from 'url';
import * as uuid from 'uuid';

import {HttpPluginConfig, IgnoreMatcher} from './types';


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

protected options: HttpPluginConfig;

/** Constructs a new HttpPlugin instance. */
constructor(moduleName: string) {
super(moduleName);
Expand Down Expand Up @@ -172,8 +167,7 @@ export class HttpPlugin extends BasePlugin {

const request: httpModule.IncomingMessage = args[0];
const response: httpModule.ServerResponse = args[1];
const path = url.parse(request.url).pathname;

const path = request.url ? url.parse(request.url).pathname || '' : '';
plugin.logger.debug('%s plugin incomingRequest', plugin.moduleName);

if (plugin.isIgnored(
Expand All @@ -189,11 +183,13 @@ export class HttpPlugin extends BasePlugin {
}
};

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

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

const requestUrl = url.parse(request.url);
const requestUrl = request.url ? url.parse(request.url) : null;
const host = headers.host || 'localhost';
const userAgent =
(headers['user-agent'] || headers['User-Agent']) as string;
Expand All @@ -220,12 +216,17 @@ export class HttpPlugin extends BasePlugin {
/^(.*)(\:[0-9]{1,5})/,
'$1',
));

rootSpan.addAttribute(
HttpPlugin.ATTRIBUTE_HTTP_METHOD, request.method);
rootSpan.addAttribute(
HttpPlugin.ATTRIBUTE_HTTP_PATH, requestUrl.pathname);
rootSpan.addAttribute(
HttpPlugin.ATTRIBUTE_HTTP_ROUTE, requestUrl.path);
HttpPlugin.ATTRIBUTE_HTTP_METHOD, request.method || 'GET');

if (requestUrl) {
rootSpan.addAttribute(
HttpPlugin.ATTRIBUTE_HTTP_PATH, requestUrl.pathname || '');
rootSpan.addAttribute(
HttpPlugin.ATTRIBUTE_HTTP_ROUTE, requestUrl.path || '');
}

rootSpan.addAttribute(
HttpPlugin.ATTRIBUTE_HTTP_USER_AGENT, userAgent);

Expand Down Expand Up @@ -267,13 +268,13 @@ export class HttpPlugin extends BasePlugin {
}

// Makes sure the url is an url object
let pathname = '';
let method = 'GET';
let pathname;
let method;
let origin = '';
if (typeof (options) === 'string') {
const parsedUrl = url.parse(options);
options = parsedUrl;
pathname = parsedUrl.pathname;
pathname = parsedUrl.pathname || '';
origin = `${parsedUrl.protocol || 'http:'}//${parsedUrl.host}`;
} else {
// Do not trace ourselves
Expand All @@ -285,9 +286,11 @@ export class HttpPlugin extends BasePlugin {
}

try {
pathname = (options as url.URL).pathname ||
url.parse(options.path).pathname;
method = options.method;
pathname = (options as url.URL).pathname;
if (!pathname) {
pathname = options.path ? url.parse(options.path).pathname : '';
}
method = options.method || 'GET';
origin = `${options.protocol || 'http:'}//${options.host}`;
} catch (e) {
}
Expand Down Expand Up @@ -371,19 +374,25 @@ export class HttpPlugin extends BasePlugin {
const userAgent =
headers ? (headers['user-agent'] || headers['User-Agent']) : null;

span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_HOST, options.hostname);
if (options.hostname) {
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_HOST, options.hostname);
}
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_METHOD, method);
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_PATH, options.path);
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_ROUTE, options.path);
if (options.path) {
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_PATH, options.path);
span.addAttribute(HttpPlugin.ATTRIBUTE_HTTP_ROUTE, options.path);
}

if (userAgent) {
span.addAttribute(
HttpPlugin.ATTRIBUTE_HTTP_USER_AGENT, userAgent.toString());
}
span.addAttribute(
HttpPlugin.ATTRIBUTE_HTTP_STATUS_CODE,
response.statusCode.toString());

span.setStatus(HttpPlugin.parseResponseStatus(response.statusCode));
if (response.statusCode) {
span.addAttribute(
HttpPlugin.ATTRIBUTE_HTTP_STATUS_CODE,
response.statusCode.toString());
span.setStatus(HttpPlugin.parseResponseStatus(response.statusCode));
}

// Message Event ID is not defined
span.addMessageEvent(
Expand Down
23 changes: 11 additions & 12 deletions packages/opencensus-instrumentation-http/test/test-http.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ import {CoreTracer, HeaderGetter, HeaderSetter, Propagation, RootSpan, Span, Spa
import {logger} from '@opencensus/core';
import * as assert from 'assert';
import * as http from 'http';
import * as mocha from 'mocha';
import * as nock from 'nock';
import * as shimmer from 'shimmer';

import {plugin} from '../src/';
import {HttpPlugin} from '../src/';

Expand Down Expand Up @@ -81,7 +79,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, HttpPlugin.parseResponseStatus(httpStatusCode));
assert.strictEqual(
Expand All @@ -92,8 +90,10 @@ function assertSpanAttributes(
span.attributes[HttpPlugin.ATTRIBUTE_HTTP_METHOD], httpMethod);
assert.strictEqual(span.attributes[HttpPlugin.ATTRIBUTE_HTTP_PATH], path);
assert.strictEqual(span.attributes[HttpPlugin.ATTRIBUTE_HTTP_ROUTE], path);
assert.strictEqual(
span.attributes[HttpPlugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
if (userAgent) {
assert.strictEqual(
span.attributes[HttpPlugin.ATTRIBUTE_HTTP_USER_AGENT], userAgent);
}
assert.strictEqual(
span.attributes[HttpPlugin.ATTRIBUTE_HTTP_STATUS_CODE],
`${httpStatusCode}`);
Expand Down Expand Up @@ -130,7 +130,7 @@ describe('HttpPlugin', () => {
(url: string) => url === `${urlHost}/ignored/function`
]
},
null);
'');
tracer.registerSpanEventListener(rootSpanVerifier);
server = http.createServer((request, response) => {
response.end('Test Server Response');
Expand Down Expand Up @@ -169,7 +169,7 @@ describe('HttpPlugin', () => {
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 @@ -192,7 +192,7 @@ describe('HttpPlugin', () => {
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 @@ -209,7 +209,7 @@ describe('HttpPlugin', () => {
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 @@ -232,8 +232,7 @@ describe('HttpPlugin', () => {

const span = root.spans[0];
assertSpanAttributes(
span, httpErrorCodes[i], 'GET', hostName, testPath,
undefined);
span, httpErrorCodes[i], 'GET', hostName, testPath);
});
});
});
Expand Down Expand Up @@ -303,7 +302,7 @@ describe('HttpPlugin', () => {
rootSpanVerifier.endedRootSpans[0].name.indexOf('GET /') >= 0);

const span = rootSpanVerifier.endedRootSpans[0];
assertSpanAttributes(span, 301, 'GET', 'google.fr', '/', undefined);
assertSpanAttributes(span, 301, 'GET', 'google.fr', '/');
});
nock.disableNetConnect();
});
Expand Down
4 changes: 2 additions & 2 deletions packages/opencensus-instrumentation-http/tsconfig.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"pretty": true,
"module": "commonjs",
"target": "es6",
"strictNullChecks": false
"strictNullChecks": true,
"noUnusedLocals": false
},
"include": [
"src/**/*.ts",
Expand All @@ -18,4 +19,3 @@
"node_modules"
]
}

0 comments on commit d4f36b0

Please sign in to comment.