From 9e4987de66ef792ffb08de600780ab38a3cb1799 Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Wed, 28 Mar 2018 11:09:05 -0700 Subject: [PATCH] feat: add hapi 17 tracing support --- src/plugins/plugin-hapi.ts | 202 +++++++++++-------- src/plugins/types/index.d.ts | 2 + test/fixtures/plugin-fixtures.json | 28 +-- test/test-mysql-pool.ts | 2 +- test/test-trace-web-frameworks.ts | 22 +- test/web-frameworks/hapi17.ts | 85 ++++++++ test/web-frameworks/{hapi.ts => hapi8_16.ts} | 2 +- 7 files changed, 227 insertions(+), 116 deletions(-) create mode 100644 test/web-frameworks/hapi17.ts rename test/web-frameworks/{hapi.ts => hapi8_16.ts} (98%) diff --git a/src/plugins/plugin-hapi.ts b/src/plugins/plugin-hapi.ts index 0e6b33f28..56f8430ab 100644 --- a/src/plugins/plugin-hapi.ts +++ b/src/plugins/plugin-hapi.ts @@ -20,11 +20,19 @@ import {parse as urlParse} from 'url'; import {PluginTypes} from '..'; -import {hapi_16} from './types'; +import {hapi_16, hapi_17} from './types'; -type Hapi16Module = typeof hapi_16; +// Used when patching Hapi 17. +const ORIGINAL = Symbol(); -const SUPPORTED_VERSIONS = '8 - 16'; +type Hapi16Module = typeof hapi_16; +type Hapi17RequestExecutePrivate = { + (this: hapi_17.Request): Promise; + [ORIGINAL]?: Hapi17RequestExecutePrivate; +}; +type Hapi17Request = hapi_17.Request&{ + _execute: Hapi17RequestExecutePrivate; +}; function getFirstHeader(req: IncomingMessage, key: string): string|null { let headerValue = req.headers[key] || null; @@ -34,87 +42,121 @@ function getFirstHeader(req: IncomingMessage, key: string): string|null { return headerValue; } -function createMiddleware(api: PluginTypes.TraceAgent): - hapi_16.ServerExtRequestHandler { - return function middleware(request, reply) { - const req = request.raw.req; - const res = request.raw.res; - const originalEnd = res.end; - const options: PluginTypes.RootSpanOptions = { - name: req.url ? (urlParse(req.url).pathname || '') : '', - url: req.url, - traceContext: - getFirstHeader(req, api.constants.TRACE_CONTEXT_HEADER_NAME), - skipFrames: 3 - }; - api.runInRootSpan(options, (root) => { - // Set response trace context. - const responseTraceContext = api.getResponseTraceContext( - options.traceContext || null, api.isRealSpan(root)); - if (responseTraceContext) { - res.setHeader( - api.constants.TRACE_CONTEXT_HEADER_NAME, responseTraceContext); - } - - if (!api.isRealSpan(root)) { - return reply.continue(); +function instrument( + api: PluginTypes.TraceAgent, request: hapi_16.Request|hapi_17.Request, + continueCb: () => T): T { + const req = request.raw.req; + const res = request.raw.res; + const originalEnd = res.end; + const options: PluginTypes.RootSpanOptions = { + name: req.url ? (urlParse(req.url).pathname || '') : '', + url: req.url, + traceContext: getFirstHeader(req, api.constants.TRACE_CONTEXT_HEADER_NAME), + skipFrames: 4 + }; + return api.runInRootSpan(options, (root) => { + // Set response trace context. + const responseTraceContext = api.getResponseTraceContext( + options.traceContext || null, api.isRealSpan(root)); + if (responseTraceContext) { + res.setHeader( + api.constants.TRACE_CONTEXT_HEADER_NAME, responseTraceContext); + } + + if (!api.isRealSpan(root)) { + return continueCb(); + } + + api.wrapEmitter(req); + api.wrapEmitter(res); + + const url = `${req.headers['X-Forwarded-Proto'] || 'http'}://${ + req.headers.host}${req.url}`; + + // we use the path part of the url as the span name and add the full + // url as a label + // req.path would be more desirable but is not set at the time our + // middleware runs. + root.addLabel(api.labels.HTTP_METHOD_LABEL_KEY, req.method); + root.addLabel(api.labels.HTTP_URL_LABEL_KEY, url); + root.addLabel(api.labels.HTTP_SOURCE_IP, req.connection.remoteAddress); + + // wrap end + res.end = function(this: ServerResponse) { + res.end = originalEnd; + const returned = res.end.apply(this, arguments); + + if (request.route && request.route.path) { + root.addLabel('hapi/request.route.path', request.route.path); } + root.addLabel(api.labels.HTTP_RESPONSE_CODE_LABEL_KEY, res.statusCode); + root.endSpan(); - api.wrapEmitter(req); - api.wrapEmitter(res); - - const url = `${req.headers['X-Forwarded-Proto'] || 'http'}://${ - req.headers.host}${req.url}`; - - // we use the path part of the url as the span name and add the full - // url as a label - // req.path would be more desirable but is not set at the time our - // middleware runs. - root.addLabel(api.labels.HTTP_METHOD_LABEL_KEY, req.method); - root.addLabel(api.labels.HTTP_URL_LABEL_KEY, url); - root.addLabel(api.labels.HTTP_SOURCE_IP, req.connection.remoteAddress); - - // wrap end - res.end = function(this: ServerResponse) { - res.end = originalEnd; - const returned = res.end.apply(this, arguments); - - if (request.route && request.route.path) { - root.addLabel('hapi/request.route.path', request.route.path); - } - root.addLabel(api.labels.HTTP_RESPONSE_CODE_LABEL_KEY, res.statusCode); - root.endSpan(); - - return returned; - }; - - // if the event is aborted, end the span (as res.end will not be called) - req.once('aborted', () => { - root.addLabel(api.labels.ERROR_DETAILS_NAME, 'aborted'); - root.addLabel( - api.labels.ERROR_DETAILS_MESSAGE, 'client aborted the request'); - root.endSpan(); - }); + return returned; + }; - return reply.continue(); + // if the event is aborted, end the span (as res.end will not be called) + req.once('aborted', () => { + root.addLabel(api.labels.ERROR_DETAILS_NAME, 'aborted'); + root.addLabel( + api.labels.ERROR_DETAILS_MESSAGE, 'client aborted the request'); + root.endSpan(); }); - }; + + return continueCb(); + }); } -const plugin: PluginTypes.Plugin = [{ - file: '', - versions: SUPPORTED_VERSIONS, - patch: (hapi, api) => { - shimmer.wrap(hapi.Server.prototype, 'connection', (connection) => { - return function connectionTrace(this: {}) { - const server: hapi_16.Server = connection.apply(this, arguments); - server.ext('onRequest', createMiddleware(api)); - return server; - }; - }); - }, - unpatch: (hapi) => { - shimmer.unwrap(hapi.Server.prototype, 'connection'); - } -} as PluginTypes.Patch]; +const plugin: PluginTypes.Plugin = [ + { + versions: '8 - 16', + patch(hapi, api) { + function createMiddleware(): hapi_16.ServerExtRequestHandler { + return function handler(request, reply) { + return instrument(api, request, () => reply.continue()); + }; + } + + shimmer.wrap(hapi.Server.prototype, 'connection', (connection) => { + return function connectionTrace(this: hapi_16.Server) { + const server = connection.apply(this, arguments); + server.ext('onRequest', createMiddleware()); + return server; + }; + }); + }, + unpatch(hapi) { + shimmer.unwrap(hapi.Server.prototype, 'connection'); + } + } as PluginTypes.Patch, + /** + * In Hapi 17, the work that is done on behalf of a request stems from + * Request#_execute. We patch that function to ensure that context is + * available in every handler. + */ + { + versions: '17', + file: 'lib/request.js', + // Request is a class name. + // tslint:disable-next-line:variable-name + patch: (Request, api) => { + // TODO(kjin): shimmer cannot wrap AsyncFunction objects. + // Once shimmer introduces this functionality, change this code to use it. + const origExecute = Request.prototype._execute; + Request.prototype._execute = + Object.assign(function _executeWrap(this: hapi_17.Request) { + return instrument(api, this, () => { + return origExecute.apply(this, arguments); + }); + }, {[ORIGINAL]: origExecute}); + }, + // Request is a class name. + // tslint:disable-next-line:variable-name + unpatch: (Request) => { + if (Request.prototype._execute[ORIGINAL]) { + Request.prototype._execute = Request.prototype._execute[ORIGINAL]!; + } + } + } as PluginTypes.Patch<{prototype: Hapi17Request}> +]; export = plugin; diff --git a/src/plugins/types/index.d.ts b/src/plugins/types/index.d.ts index 27810b3a0..36d4e3add 100644 --- a/src/plugins/types/index.d.ts +++ b/src/plugins/types/index.d.ts @@ -30,6 +30,7 @@ import * as connect_3 from './connect_3'; // connect@3 import * as express_4 from './express_4'; // express@4 import * as hapi_16 from './hapi_16'; // hapi@16 +import * as hapi_17 from './hapi_17'; // hapi@17 import * as koa_2 from './koa_2'; // koa@2 import * as pg_7 from './pg_7'; // pg@7 import * as restify_5 from './restify_5'; // restify@5 @@ -88,6 +89,7 @@ export { connect_3, express_4, hapi_16, + hapi_17, koa_1, koa_2, pg_6, diff --git a/test/fixtures/plugin-fixtures.json b/test/fixtures/plugin-fixtures.json index b8c02c235..662eb25a1 100644 --- a/test/fixtures/plugin-fixtures.json +++ b/test/fixtures/plugin-fixtures.json @@ -39,31 +39,11 @@ "hapi-plugin-mysql": "^3.1.3" } }, - "hapi10": { - "dependencies": { - "hapi": "^10.5.0" - } - }, - "hapi11": { - "dependencies": { - "hapi": "^11.1.4" - } - }, "hapi12": { "dependencies": { "hapi": "^12.1.0" } }, - "hapi13": { - "dependencies": { - "hapi": "^13.2.1" - } - }, - "hapi14": { - "dependencies": { - "hapi": "^14.0.0" - } - }, "hapi15": { "dependencies": { "hapi": "^15.0.0" @@ -74,14 +54,14 @@ "hapi": "^16.0.0" } }, - "hapi8": { + "hapi17": { "dependencies": { - "hapi": "^8.8.1" + "hapi": "^17.0.0" } }, - "hapi9": { + "hapi8": { "dependencies": { - "hapi": "^9.5.1" + "hapi": "^8.8.1" } }, "knex0.10": { diff --git a/test/test-mysql-pool.ts b/test/test-mysql-pool.ts index 8bd8e1eb1..7e0bb0cc5 100644 --- a/test/test-mysql-pool.ts +++ b/test/test-mysql-pool.ts @@ -31,7 +31,7 @@ if (semver.satisfies(process.version, '>=4')) { samplingRate: 0, enhancedDatabaseReporting: true }); - Hapi = require('./plugins/fixtures/hapi13'); + Hapi = require('./plugins/fixtures/hapi16'); }); it('should work with connection pool access', function(done) { diff --git a/test/test-trace-web-frameworks.ts b/test/test-trace-web-frameworks.ts index e6d4a6bc4..f2d8ac3fb 100644 --- a/test/test-trace-web-frameworks.ts +++ b/test/test-trace-web-frameworks.ts @@ -29,7 +29,8 @@ import {assertSpanDuration, DEFAULT_SPAN_DURATION, isServerSpan, wait} from './u import {WebFramework, WebFrameworkConstructor} from './web-frameworks/base'; import {Connect3} from './web-frameworks/connect'; import {Express4} from './web-frameworks/express'; -import {Hapi12, Hapi15, Hapi16, Hapi8} from './web-frameworks/hapi'; +import {Hapi17} from './web-frameworks/hapi17'; +import {Hapi12, Hapi15, Hapi16, Hapi8} from './web-frameworks/hapi8_16'; import {Koa1} from './web-frameworks/koa1'; import {Koa2} from './web-frameworks/koa2'; import {Restify3, Restify4, Restify5, Restify6} from './web-frameworks/restify'; @@ -44,8 +45,9 @@ type TraceSpanStackFrames = { const ABORTED_SPAN_RETRIES = 3; // The list of web frameworks to test. const FRAMEWORKS: WebFrameworkConstructor[] = [ - Connect3, Express4, Hapi8, Hapi12, Hapi15, Hapi16, Koa1, Koa2, Restify3, - Restify4, Restify5, Restify6 + // Connect3, Express4, Hapi8, Hapi12, Hapi15, Hapi16, Hapi17, Koa1, Koa2, + // Restify3, Restify4, Restify5, Restify6 + Hapi17 ]; /** @@ -146,7 +148,7 @@ describe('Web framework tracing', () => { it('accurately measures get time (1 handler)', async () => { let recordedTime = 0; await trace.get().runInRootSpan({name: 'outer'}, async (span) => { - assert.ok(span); + assert.ok(trace.get().isRealSpan(span)); recordedTime = Date.now(); await axios.get(`http://localhost:${port}/one-handler`); recordedTime = Date.now() - recordedTime; @@ -160,7 +162,7 @@ describe('Web framework tracing', () => { it('accurately measures get time (2 handlers)', async () => { let recordedTime = 0; await trace.get().runInRootSpan({name: 'outer'}, async (span) => { - assert.ok(span); + assert.ok(trace.get().isRealSpan(span)); recordedTime = Date.now(); // Hit endpoint with two middlewares/handlers. await axios.get(`http://localhost:${port}/two-handlers`); @@ -174,7 +176,7 @@ describe('Web framework tracing', () => { it('handles errors', async () => { await trace.get().runInRootSpan({name: 'outer'}, async (span) => { - assert.ok(span); + assert.ok(trace.get().isRealSpan(span)); // Hit endpoint which always throws an error. await axios.get(`http://localhost:${port}/error`, { validateStatus: () => true // Obviates try/catch. @@ -189,7 +191,7 @@ describe('Web framework tracing', () => { it('doesn\'t trace ignored urls', async () => { await trace.get().runInRootSpan({name: 'outer'}, async (span) => { - assert.ok(span); + assert.ok(trace.get().isRealSpan(span)); // Hit endpoint that always gets ignored. await axios.get(`http://localhost:${port}/ignore-me`); span!.endSpan(); @@ -200,7 +202,7 @@ describe('Web framework tracing', () => { it('ends span upon client abort', async () => { await trace.get().runInRootSpan({name: 'outer'}, async (span) => { - assert.ok(span); + assert.ok(trace.get().isRealSpan(span)); // Hit endpoint, but time out before it has a chance to respond. // To ensure that a trace is written, also waits await axios @@ -250,7 +252,7 @@ describe('Web framework tracing', () => { it('propagates trace context', async () => { await trace.get().runInRootSpan({name: 'outer'}, async (span) => { - assert.ok(span); + assert.ok(trace.get().isRealSpan(span)); // Hits endpoint that will make an additional outgoing HTTP request // (to another endpoint on the same server). await axios.get(`http://localhost:${port}/propagate-hello`); @@ -290,7 +292,7 @@ describe('Web framework tracing', () => { beforeEach(async () => { await trace.get().runInRootSpan({name: 'outer'}, async (span) => { - assert.ok(span); + assert.ok(trace.get().isRealSpan(span)); // Hit an endpoint with a query parameter. await axios.get(`http://localhost:${port}/hello?this-is=dog`); span!.endSpan(); diff --git a/test/web-frameworks/hapi17.ts b/test/web-frameworks/hapi17.ts new file mode 100644 index 000000000..a4deaf1d2 --- /dev/null +++ b/test/web-frameworks/hapi17.ts @@ -0,0 +1,85 @@ +/** + * Copyright 2018 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {hapi_17} from '../../src/plugins/types'; + +import {WebFramework, WebFrameworkAddHandlerOptions, WebFrameworkResponse} from './base'; + +export class Hapi17 implements WebFramework { + static commonName = `hapi@17`; + static expectedTopStackFrame = '_executeWrap'; + static versionRange = '>=7.5'; + + private server: hapi_17.Server; + // We can't add two routes on the same path. + // So instead of registering a new Hapi plugin per path, + // register only the first time -- passing a function that will iterate + // through a list of routes keyed under the path. + private routes = + new Map Promise>>(); + private registering = Promise.resolve(); + + constructor() { + const hapi = require('../plugins/fixtures/hapi17') as typeof hapi_17; + this.server = new hapi.Server(); + } + + addHandler(options: WebFrameworkAddHandlerOptions): void { + let shouldRegister = false; + if (!this.routes.has(options.path)) { + this.routes.set(options.path, [options.fn]); + shouldRegister = true; + } else { + this.routes.get(options.path)!.push(options.fn); + } + + // Only register a new plugin for the first occurrence of this path. + if (shouldRegister) { + this.registering = this.registering.then(() => this.server.register({ + plugin: { + name: options.path, + register: async (server, registerOpts) => { + server.route({ + method: 'GET', + path: options.path, + handler: async (request, h) => { + let result; + for (const handler of this.routes.get(options.path)!) { + result = await handler(); + if (result) { + return result; + } + } + return h.continue; + } + }); + } + } + })); + } + } + + async listen(port: number): Promise { + await this.registering; + this.server.settings.port = port; + await this.server.start(); + return Number(this.server.info!.port); + } + + shutdown(): void { + this.server.stop(); + } +} diff --git a/test/web-frameworks/hapi.ts b/test/web-frameworks/hapi8_16.ts similarity index 98% rename from test/web-frameworks/hapi.ts rename to test/web-frameworks/hapi8_16.ts index 821901db6..fc32953c1 100644 --- a/test/web-frameworks/hapi.ts +++ b/test/web-frameworks/hapi8_16.ts @@ -85,7 +85,7 @@ export class Hapi implements WebFramework { const makeHapiClass = (version: number) => class extends Hapi { static commonName = `hapi@${version}`; - static expectedTopStackFrame = 'middleware'; + static expectedTopStackFrame = 'handler'; static versionRange = '*'; constructor() {