Skip to content

adding seperate metrics server#54

Open
hsingh6764 wants to merge 4 commits into
roylines:masterfrom
hsingh6764:seperate-metrics-server
Open

adding seperate metrics server#54
hsingh6764 wants to merge 4 commits into
roylines:masterfrom
hsingh6764:seperate-metrics-server

Conversation

@hsingh6764

Copy link
Copy Markdown

No description provided.

@hsingh6764 hsingh6764 force-pushed the seperate-metrics-server branch from e739ae9 to 71e6891 Compare August 4, 2018 21:36
@hsingh6764 hsingh6764 force-pushed the seperate-metrics-server branch from 05bf71e to 41da576 Compare August 6, 2018 14:44

@roylines roylines left a comment

Copy link
Copy Markdown
Owner

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.

Comment thread lib/express.js Outdated
server.use(middleware)
server.get(options.url, (req, res) => {
let mertricsEndpointServer = options.metricsServer || server;
mertricsEndpointServer.get(options.url, (req, res) => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

please fix spelling

Comment thread lib/hapi.js
register: (server, o, done) => {
server.route({
let mertricsEndpointServer = options.metricsServer || server;
mertricsEndpointServer.route({

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

please fix spelling

Comment thread lib/http.js Outdated
function instrument (server, options) {
server.on('request', (request, response) => {
let mertricsEndpointServer = options.metricsServer || server;
mertricsEndpointServer.on('request', (request, response) => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

please fix spelling

Comment thread package.json Outdated
{
"name": "epimetheus",
"version": "0.0.1",
"version": "1.0.56",

Copy link
Copy Markdown
Owner

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

@roylines

Copy link
Copy Markdown
Owner

Happy to merge this pull request if you could please add a couple of tests and fix the spelling mistakes. Thanks for the contribution.

@hsingh6764 hsingh6764 force-pushed the seperate-metrics-server branch from 1f71bbb to b08c313 Compare August 21, 2018 17:32
@hsingh6764

Copy link
Copy Markdown
Author

@roylines please take a look again

@roylines

Copy link
Copy Markdown
Owner

@hsingh6764 there are conflicts to be resolved. Would you please be able to sort.

@brandonros

Copy link
Copy Markdown

Can this get merged + published to NPM please?

@hsingh6764

Copy link
Copy Markdown
Author

Can this be merged?

@yciabaud

Copy link
Copy Markdown

Please...

@guillomep

Copy link
Copy Markdown

@roylines please merged this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants