Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wrapper function for hapi route & plugins #221

Merged
merged 6 commits into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@
"internal": ":house: Internal",
"documentation": ":memo: Documentation"
},
"ignoreCommitters": ["renovate-bot", "dependabot"]
"ignoreCommitters": [
jk1z marked this conversation as resolved.
Show resolved Hide resolved
"renovate-bot",
"dependabot"
]
}
}
32 changes: 25 additions & 7 deletions plugins/node/opentelemetry-hapi-instrumentation/src/hapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,18 @@ export class HapiInstrumentation extends BasePlugin<typeof Hapi> {
) {
if (Array.isArray(pluginInput)) {
for (const pluginObj of pluginInput) {
instrumentation._wrapRegisterHandler(pluginObj.plugin);
instrumentation._wrapRegisterHandler(
pluginObj.plugin && pluginObj.plugin.plugin
jk1z marked this conversation as resolved.
Show resolved Hide resolved
? pluginObj.plugin.plugin
: pluginObj.plugin
);
}
} else {
instrumentation._wrapRegisterHandler(pluginInput.plugin);
instrumentation._wrapRegisterHandler(
pluginInput.plugin && pluginInput.plugin.plugin
jk1z marked this conversation as resolved.
Show resolved Hide resolved
? pluginInput.plugin.plugin
: pluginInput.plugin
);
}
return original.apply(this, [pluginInput, options]);
};
Expand Down Expand Up @@ -357,26 +365,36 @@ export class HapiInstrumentation extends BasePlugin<typeof Hapi> {
const instrumentation: HapiInstrumentation = this;
if (route[handlerPatched] === true) return route;
route[handlerPatched] = true;
if (typeof route.handler === 'function') {
const handler = route.handler as Hapi.Lifecycle.Method;
let handler;
if (route.handler) {
handler = route.handler;
} else if (route.options && route.options.handler) {
jk1z marked this conversation as resolved.
Show resolved Hide resolved
handler = route.options.handler;
}
if (typeof handler === 'function') {
const oldHandler = handler as Hapi.Lifecycle.Method;
jk1z marked this conversation as resolved.
Show resolved Hide resolved
const newHandler: Hapi.Lifecycle.Method = async function (
request: Hapi.Request,
h: Hapi.ResponseToolkit,
err?: Error
) {
if (instrumentation._tracer.getCurrentSpan() === undefined) {
return await handler(request, h, err);
return await oldHandler(request, h, err);
}
const metadata = getRouteMetadata(route, pluginName);
const span = instrumentation._tracer.startSpan(metadata.name, {
attributes: metadata.attributes,
});
const res = await handler(request, h, err);
const res = await oldHandler(request, h, err);
span.end();

return res;
};
route.handler = newHandler;
if (route.handler) {
jk1z marked this conversation as resolved.
Show resolved Hide resolved
route.handler = newHandler;
} else if (route.options && route.options.handler) {
jk1z marked this conversation as resolved.
Show resolved Hide resolved
route.options.handler = newHandler;
}
}
return route;
}
Expand Down
12 changes: 10 additions & 2 deletions plugins/node/opentelemetry-hapi-instrumentation/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/
import type * as Hapi from '@hapi/hapi';
import { Lifecycle } from '@hapi/hapi';

export const HapiComponentName = '@hapi/hapi';

Expand All @@ -33,11 +34,18 @@ export type HapiServerRouteInput =

export type PatchableServerRoute = Hapi.ServerRoute & {
[handlerPatched]?: boolean;
options?: {
handler?: Lifecycle.Method;
};
};

export type HapiPluginObject<T> = Hapi.ServerRegisterPluginObject<T> & {
plugin: Hapi.ServerRegisterPluginObject<T>;
};

export type HapiPluginInput<T> =
| Hapi.ServerRegisterPluginObject<T>
| Array<Hapi.ServerRegisterPluginObject<T>>;
| HapiPluginObject<T>
| Array<HapiPluginObject<T>>;

export type RegisterFunction<T> = (
plugin: HapiPluginInput<T>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ describe('Hapi Instrumentation - Hapi.Plugin Tests', () => {
},
};

const nestedPackagePlugin = {
plugin: packagePlugin,
};

describe('Instrumenting Hapi Plugins', () => {
it('should create spans for routes within single plugins', async () => {
const rootSpan = tracer.startSpan('rootSpan');
Expand Down Expand Up @@ -287,7 +291,7 @@ describe('Hapi Instrumentation - Hapi.Plugin Tests', () => {
});
});

it('should instrument package-based plugins', async () => {
it('should instrument package-based plugins (direct exports)', async () => {
const rootSpan = tracer.startSpan('rootSpan');

await server.register({
Expand Down Expand Up @@ -325,5 +329,46 @@ describe('Hapi Instrumentation - Hapi.Plugin Tests', () => {
assert.notStrictEqual(exportedRootSpan, undefined);
});
});

it('should instrument package-based plugins (with exports.plugin)', async () => {
jk1z marked this conversation as resolved.
Show resolved Hide resolved
const rootSpan = tracer.startSpan('rootSpan');

// Suppress this ts error due the Hapi plugin type definition is incomplete. server.register can accept nested plugin. See reference https://hapi.dev/api/?v=20.0.0#-routeoptionshandler
await server.register({
// @ts-ignore
plugin: nestedPackagePlugin,
});
await server.start();
assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);

await tracer.withSpan(rootSpan, async () => {
const res = await server.inject({
method: 'GET',
url: '/package',
});
assert.strictEqual(res.statusCode, 200);

rootSpan.end();
assert.deepStrictEqual(memoryExporter.getFinishedSpans().length, 2);

const requestHandlerSpan = memoryExporter
.getFinishedSpans()
.find(span => span.name === 'plugin-by-package: route - /package');
assert.notStrictEqual(requestHandlerSpan, undefined);
assert.strictEqual(
requestHandlerSpan?.attributes[AttributeNames.HAPI_TYPE],
HapiLayerType.PLUGIN
);
assert.strictEqual(
requestHandlerSpan?.attributes[AttributeNames.PLUGIN_NAME],
'plugin-by-package'
);

const exportedRootSpan = memoryExporter
.getFinishedSpans()
.find(span => span.name === 'rootSpan');
assert.notStrictEqual(exportedRootSpan, undefined);
});
});
});
});
41 changes: 41 additions & 0 deletions plugins/node/opentelemetry-hapi-instrumentation/test/hapi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,47 @@ describe('Hapi Instrumentation - Core Tests', () => {
});
});

it('should create a child span for single routes with handler method in the options', async () => {
const rootSpan = tracer.startSpan('rootSpan');
server.route({
method: 'GET',
path: '/',
options: {
handler: (request, h) => {
return 'Hello World!';
},
},
});

await server.start();
assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);

await tracer.withSpan(rootSpan, async () => {
const res = await server.inject({
method: 'GET',
url: '/',
});
assert.strictEqual(res.statusCode, 200);

rootSpan.end();
assert.deepStrictEqual(memoryExporter.getFinishedSpans().length, 2);

const requestHandlerSpan = memoryExporter
.getFinishedSpans()
.find(span => span.name === 'route - /');
assert.notStrictEqual(requestHandlerSpan, undefined);
assert.strictEqual(
requestHandlerSpan?.attributes[AttributeNames.HAPI_TYPE],
HapiLayerType.ROUTER
);

const exportedRootSpan = memoryExporter
.getFinishedSpans()
.find(span => span.name === 'rootSpan');
assert.notStrictEqual(exportedRootSpan, undefined);
});
});

it('should instrument the Hapi.Server (note: uppercase) method', async () => {
const rootSpan = tracer.startSpan('rootSpan');
server = new hapi.Server({
Expand Down