-
Notifications
You must be signed in to change notification settings - Fork 45
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
adding seperate metrics server #54
base: master
Are you sure you want to change the base?
Conversation
e739ae9
to
71e6891
Compare
05bf71e
to
41da576
Compare
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.
Could you please extend the tests to test for the additional server option, and I will gladly merge.
lib/express.js
Outdated
@@ -12,7 +12,8 @@ function middleware (request, response, done) { | |||
|
|||
function instrument (server, options) { | |||
server.use(middleware) | |||
server.get(options.url, (req, res) => { | |||
let mertricsEndpointServer = options.metricsServer || server; | |||
mertricsEndpointServer.get(options.url, (req, res) => { |
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.
please fix spelling
@@ -3,7 +3,8 @@ const metrics = require('./metrics') | |||
function plugin (options) { | |||
var plugin = { | |||
register: (server, o, done) => { | |||
server.route({ | |||
let mertricsEndpointServer = options.metricsServer || server; | |||
mertricsEndpointServer.route({ |
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.
please fix spelling
lib/http.js
Outdated
@@ -18,7 +18,8 @@ function observeMetrics (request, response) { | |||
} | |||
|
|||
function instrument (server, options) { | |||
server.on('request', (request, response) => { | |||
let mertricsEndpointServer = options.metricsServer || server; | |||
mertricsEndpointServer.on('request', (request, response) => { |
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.
please fix spelling
package.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "epimetheus", | |||
"version": "0.0.1", | |||
"version": "1.0.56", |
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.
please leave version as 0.0.1 as it is automagically set by CI
Happy to merge this pull request if you could please add a couple of tests and fix the spelling mistakes. Thanks for the contribution. |
1f71bbb
to
b08c313
Compare
@roylines please take a look again |
@hsingh6764 there are conflicts to be resolved. Would you please be able to sort. |
Can this get merged + published to NPM please? |
Can this be merged? |
Please... |
@roylines please merged this PR |
No description provided.