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

Add diagnostic channels #4429

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
65 changes: 65 additions & 0 deletions API.md
Original file line number Diff line number Diff line change
Expand Up @@ -5030,3 +5030,68 @@ const plugin = {
}
};
```

## Diagnostic Channels

Diagnostic Channels allows Hapi to report events to other modules. This is useful for APM vendors to collect data from Hapi for diagnostics purposes.
YoannMa marked this conversation as resolved.
Show resolved Hide resolved

See [the official documentation](https://nodejs.org/docs/latest/api/diagnostics_channel.html) for more information.

### `hapi.onServer`

This event is sent when a new server is created. The [`server`](#server) object is passed as the message.

```js
const DC = require('diagnostics_channel');
const channel = DC.channel('hapi.onServer');

channel.subscribe((server) => {
// Do something with the server
YoannMa marked this conversation as resolved.
Show resolved Hide resolved
console.log(server.version);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Hapi itself doesn't actually use any deprecated API, only these examples and the tests. I'm not a fan of using deprecated API to illustrate how to use, but since this is targeted at APM vendors they should known what they are doing anyway.

Maybe just have a single example at the top?

Copy link
Author

Choose a reason for hiding this comment

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

I don't mind doing another PR when Hapi drops support for Node 14 to replace those deprecated usage

```

### `hapi.onRoute`

This event is sent when a new route is added to the server. The [`route`](#request.route) object is passed as the message.
Similar to `server.events.on('route', (route) => {})`.

```js
const DC = require('diagnostics_channel');
const channel = DC.channel('hapi.onRoute');

channel.subscribe((route) => {
// Do something with the route
console.log(route.path);
});
```

### `hapi.onRequest`

This event is sent when a request is generated by the framework. The [`request`](#request) object is passed as the message.
Similar to `server.events.on('request', (request) => {})`.

```js
const DC = require('diagnostics_channel');
const channel = DC.channel('hapi.onRequest');

channel.subscribe((request) => {
// Do something with the request
console.log(request.info.id);
});
```

### `hapi.onResponse`

This event is sent after the response is sent back to the client. The [`response`](#request.response) object is passed as the message.
Similar to `server.events.on('response', ({ response }) => {})`.

```js
const DC = require('diagnostics_channel');
const channel = DC.channel('hapi.onResponse');

channel.subscribe((response) => {
// Do something with the response
console.log(response.statusCode);
});
```
10 changes: 10 additions & 0 deletions lib/channels.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
'use strict';

const DC = require('diagnostics_channel');

module.exports = {
onServerChannel: DC.channel('hapi.onServer'),
onRouteChannel: DC.channel('hapi.onRoute'),
onResponseChannel: DC.channel('hapi.onResponse'),
onRequestChannel: DC.channel('hapi.onRequest')
};
YoannMa marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions lib/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const Podium = require('@hapi/podium');
const Statehood = require('@hapi/statehood');

const Auth = require('./auth');
const Channels = require('./channels');
const Compression = require('./compression');
const Config = require('./config');
const Cors = require('./cors');
Expand Down Expand Up @@ -510,6 +511,8 @@ exports = module.exports = internals.Core = class {

const request = Request.generate(this.root, req, res, options);

Channels.onRequestChannel.publish(request);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably move the request._execute() info assignment to the request constructor to make this more representative of what the onRequest ext sees.

Or maybe move the publish to the _execute() method? (though that would mean it doesn't see the ones that are denied due to load).

Copy link
Author

Choose a reason for hiding this comment

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

If this assignment is in the request._execute(), perhaps it is to avoid executing it in case of heavy load ?

I would vote to keep the publish() here given we need another event when a request is routed. I could add an event early in the request._lifecycle() method ? Something like hapi.onRequestLifecycle ?


// Track socket request processing state

if (this.settings.operations.cleanStop &&
Expand Down
3 changes: 3 additions & 0 deletions lib/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const Bounce = require('@hapi/bounce');
const Hoek = require('@hapi/hoek');
const Podium = require('@hapi/podium');

const Channels = require('./channels');
const Cors = require('./cors');
const Toolkit = require('./toolkit');
const Transmit = require('./transmit');
Expand Down Expand Up @@ -510,6 +511,8 @@ exports = module.exports = internals.Request = class {

this._core.events.emit('response', this);

Channels.onResponseChannel.publish(this.response);

if (this._route._extensions.onPostResponse.nodes) {
this._invoke(this._route._extensions.onPostResponse, { ignoreResponse: true });
}
Expand Down
6 changes: 6 additions & 0 deletions lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const Hoek = require('@hapi/hoek');
const Shot = require('@hapi/shot');
const Teamwork = require('@hapi/teamwork');

const Channels = require('./channels');
const Config = require('./config');
const Core = require('./core');
const Cors = require('./cors');
Expand Down Expand Up @@ -83,6 +84,8 @@ internals.Server = class {
}

core.registerServer(this);

Channels.onServerChannel.publish(this);
}

_clone(name) {
Expand Down Expand Up @@ -532,6 +535,9 @@ internals.Server = class {
}

this.events.emit('route', route.public);

Channels.onRouteChannel.publish(route.public);

Cors.options(route.public, server);
}

Expand Down
121 changes: 121 additions & 0 deletions test/channels.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
'use strict';

const DC = require('diagnostics_channel');

const Code = require('@hapi/code');
const Lab = require('@hapi/lab');

const Hapi = require('..');

const { describe, it } = exports.lab = Lab.script();
const expect = Code.expect;

describe('DiagnosticChannel', () => {

describe('onServerChannel', () => {
YoannMa marked this conversation as resolved.
Show resolved Hide resolved

const channel = DC.channel('hapi.onServer');

it('server should be exposed on creation through the channel hapi.onServer', async () => {

let exposedServer;
let server;

await new Promise((resolve) => {

channel.subscribe((srv) => {

exposedServer = srv;
resolve();
YoannMa marked this conversation as resolved.
Show resolved Hide resolved
});

server = Hapi.server();
});

expect(exposedServer).to.equal(server);
YoannMa marked this conversation as resolved.
Show resolved Hide resolved
});
});

describe('onRouteChannel', () => {

const channel = DC.channel('hapi.onRoute');

it('route should be exposed on creation through the channel hapi.onRoute', async () => {

const server = Hapi.server();
let route;

await new Promise((resolve) => {

channel.subscribe((rte) => {

route = rte;
resolve();
});

server.route({
method: 'GET',
path: '/',
options: { app: { x: 'o' } },
handler: () => 'ok'
});
});

expect(route).to.be.an.object();
expect(route.settings.app.x).to.equal('o');
});
});

describe('onResponseChannel', () => {

const channel = DC.channel('hapi.onResponse');

it('response should be exposed on creation through the channel hapi.onResponse', async () => {

const server = Hapi.server();
let responseExposed;

server.route({ method: 'GET', path: '/', handler: () => 'ok' });

const eventPromise = new Promise((resolve) => {

channel.subscribe((res) => {

responseExposed = res;
resolve();
});
});

const response = await server.inject('/');
await eventPromise;

expect(response.request.response).to.equal(responseExposed);
YoannMa marked this conversation as resolved.
Show resolved Hide resolved
});
});

describe('onRequestChannel', () => {

const channel = DC.channel('hapi.onRequest');

it('request should be exposed on creation through the channel hapi.onRequest', async () => {

const server = Hapi.server();
let requestExposed;

server.route({ method: 'GET', path: '/', handler: () => 'ok' });

const eventPromise = new Promise((resolve) => {

channel.subscribe((req) => {

requestExposed = req;
resolve();
});
});

const response = await server.inject('/');
await eventPromise;
expect(response.request).to.equal(requestExposed);
YoannMa marked this conversation as resolved.
Show resolved Hide resolved
});
});
});