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

enforce strictNullChecks on HTTP plugin #364

Merged
merged 1 commit into from
Feb 26, 2019
Merged

enforce strictNullChecks on HTTP plugin #364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 42 additions & 33 deletions packages/opencensus-instrumentation-http/src/http.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will enable noUnusedLocals once stats are recorded for http plugin.

},
"include": [
"src/**/*.ts",
Expand All @@ -18,4 +19,3 @@
"node_modules"
]
}