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

expose the statsdclient to the request #529

Open
designfrontier opened this issue Jan 3, 2018 · 6 comments
Open

expose the statsdclient to the request #529

designfrontier opened this issue Jan 3, 2018 · 6 comments

Comments

@designfrontier
Copy link
Member

designfrontier commented Jan 3, 2018

This would facilitate users doing custom statsd stuff and building out there own stats for performance metrics etc.

The statsd client already exists in the router.js file and in the correct scope to be added to the connection object passed to route events. Just needs to be added.

@cursiv3
Copy link

cursiv3 commented Mar 20, 2018

Hi I've been looking for a project to get into open source, I think I could handle this. If I'm understanding correctly, you would simply like statsdClient added inside the connection object so it can be accessed in a monument project route file events callback like events.statsdClient?

@designfrontier
Copy link
Member Author

designfrontier commented Mar 23, 2018

Yeah!

As I am thinking about this more it might actually be best to just expose it at the same level as the events object. That way you could do require('monument').statsd to get it wherever you want to use it. Alternately if that ends up not being feasible you could just add it to the connection object in the router so that it is available to all routes.

@cursiv3
Copy link

cursiv3 commented Mar 27, 2018

I submitted a PR that allows access to statsd anywhere via require('monument').statsd:
#577

but as I commented there, I couldn't figure out how to test it

@designfrontier
Copy link
Member Author

Line 96-108 of index.test.js are a pretty simple approach to testing this.

Basically assert that the statsd client is returned on the app. The rest of the tests around statsd are already there so testing it in that location isn't needed.

@cursiv3
Copy link

cursiv3 commented Mar 30, 2018

Will do! And thanks for the heads up on the test stuff, I should have poked around in there more

@KazChe
Copy link

KazChe commented Oct 26, 2019

Did this change make it?

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

No branches or pull requests

3 participants