datadog integration for twemproxy, options for haproxy#326
datadog integration for twemproxy, options for haproxy#326truthbk merged 13 commits intoDataDog:masterfrom
Conversation
|
Nice, and big thanks for the spec tests. This looks promising, I'll review very soon! :) |
| # } | ||
| # | ||
| class datadog_agent::integrations::twemproxy( | ||
| $servers = [{'host' => 'localhost', 'port' => '22222'}] |
There was a problem hiding this comment.
I'd rather respect the pattern we have in similar checks where we have arguments host and port and maybe some basic options (for single instance) and then an instances argument for users who wish to specify multiple instances for the check. Don't get me wrong, this is perfectly fine, but it breaks a pattern used widely across the check.
I am aware that mongo and zk both follow this approach you have here, but I'd rather have that be the exception. Most integrations follow the pattern you can see haproxy. If you could fix that up, and the erb, which should be easy, that would be great.
|
@truthbk I did the necessary changes 😃 |
truthbk
left a comment
There was a problem hiding this comment.
This now looks good!! Merging! 🙇
* add instances parameter * building config from instances * spec test for instances set * adding datadog integration for twemproxy * add options to haproxy integration * following instances pattern for twemproxy integration * fix twemproxy template typo * fix twemproxy template typo * fix twemproxy param error * fix spec test
I added twemproxy integration, and needed a way to add optional parameters for the haproxy integration (e.g. set "collect_aggregates_only: False" for an instance)