-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add diagnostic channels #4429
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
base: master
Are you sure you want to change the base?
Add diagnostic channels #4429
Changes from 1 commit
6680441
2606bb5
890062d
e046e26
cf9c82d
5eb9907
35ea6ef
f21651d
a246101
34f3f35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'); | ||
|
@@ -510,6 +511,8 @@ exports = module.exports = internals.Core = class { | |
|
||
const request = Request.generate(this.root, req, res, options); | ||
|
||
Channels.onRequestChannel.publish(request); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would probably move the Or maybe move the publish to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this assignment is in the I would vote to keep the |
||
|
||
// Track socket request processing state | ||
|
||
if (this.settings.operations.cleanStop && | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
'use strict'; | ||
|
||
const DC = require('diagnostics_channel'); | ||
|
||
const Code = require('@hapi/code'); | ||
const Lab = require('@hapi/lab'); | ||
YoannMa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const Hapi = require('..'); | ||
|
||
const { describe, it } = exports.lab = Lab.script(); | ||
const expect = Code.expect; | ||
YoannMa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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(); | ||
|
||
YoannMa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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(); | ||
|
||
server.route({ method: 'GET', path: '/', handler: () => 'ok' }); | ||
|
||
let responseExposed; | ||
YoannMa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const eventPromise = new Promise((resolve) => { | ||
|
||
channel.subscribe((res) => { | ||
|
||
responseExposed = res; | ||
|
||
resolve(); | ||
YoannMa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
}); | ||
|
||
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(); | ||
|
||
server.route({ method: 'GET', path: '/', handler: () => 'ok' }); | ||
|
||
let requestExposed; | ||
YoannMa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const eventPromise = new Promise((resolve) => { | ||
|
||
channel.subscribe((req) => { | ||
|
||
requestExposed = req; | ||
|
||
resolve(); | ||
YoannMa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
}); | ||
|
||
const response = await server.inject('/'); | ||
await eventPromise; | ||
|
||
expect(response.request).to.equal(requestExposed); | ||
YoannMa marked this conversation as resolved.
Show resolved
Hide resolved
YoannMa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of linking to the Node docs, can you provide a small example. If you still want to link to the Node docs, I would recommend linking to latest so we don't have to keep updating the link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed the URL, added a bit of explaining and examples, do you feel like it's enough ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with it. Let's give other folks a chance to weigh in.