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

Replace cognician/dogstatsd with swirrl/dogstatsd #545

Open
RickMoynihan opened this issue Oct 14, 2021 · 2 comments
Open

Replace cognician/dogstatsd with swirrl/dogstatsd #545

RickMoynihan opened this issue Oct 14, 2021 · 2 comments

Comments

@RickMoynihan
Copy link
Member

I forked cognician/dogstatsd which we use in drafter to a new project https://github.com/Swirrl/dogstatsd

This fork was created when I wanted to integrate muttnik with dogstatsd (see this PR:
https://github.com/Swirrl/muttnik/pull/1339), and ran into the issue where cognician/dogstatsd brings its own implicit singleton client to the party.

The main problem this brings in drafter is that you need to hack it to work well with integrant config. The hack we use in drafter is to automatically inject it into every other integrant component to force it to be configured before all others. The ugly thing is, that the components then don't actually use the statsd client they've been given at all.

I didn't want to include a similar hack in muttnik; which has quite a lot of existing complexity around its integrant setup, so decided to do it properly by forking the project and making a breaking change to its API such that the "client" is an explicit parameter on all the reporters.

This is a low priority "tech debt" ticket, where we can update drafter to use the new improved dogstatsd client, and remove our integrant hack.

@ricroberts
Copy link
Contributor

@lkitching do you know if this is still applicable - I seem to recall rick saying his fork was redundant now (but i might have got the wrong end of the stick)

@lkitching
Copy link
Contributor

I can't see anything in the swirrl/dogstatsd project to suggest it's no longer relevant, and the APIs of the two projects are still different. Drafter still depends on cognician/dogstatsd so it looks like this issue is still required.

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