chef-datadog
chef-datadog copied to clipboard
DataDog
Add support for JBoss/WildFly
Hi @miketheman, I should have put this in the initial PR, but here is a link to the DD docs page where there are instructions for JBoss/WildFly monitoring, about 3/4 down. This PR just adds the capability to put that additional config in place. I removed the version change. I think this would be helpful for anyone wanting to monitor JBoss or Wildfly with Datadog. Let me know if you have any more feedback and I'll be happy to make the changes.Hi @miketheman, I should have put this in the initial PR, but here is a link to the DD docs page where there are instructions for JBoss/WildFly monitoring, about 3/4 down. This PR just adds the capability to put that additional config in place. I removed the version change. I think this would be helpful for anyone wanting to monitor JBoss or Wildfly with Datadog. Let me know if you have any more feedback and I'll be happy to make the changes.
Hi @ianrossi, thanks for opening this PR!
Instead of creating a new recipe/template, is there any reason you didn't update the existing jmx recipe and template with the additional init_config field? (for instance the template would look like the docker_daemon template)
Hi @olivielpeau, thanks for replying to my PR. I like the separation of the implementation of the two use cases. In one, someone wanting to monitor jmx can use the existing template that is straightforward that does not require any init_config. In the second use case, where someone wants to monitor JBoss's or Wildfly's implementation of jmx--where init_config is required--they can straightforwardly use the separate template. Looking back, with the two use cases, I guess I may not have needed the conditionals in the new template since they always apply to JBoss.
All that being said, if you want me to change the PR before it's merged, I'll be happy to do so. I just have this question: Are you saying I can use the exact docker_daemon template or that it would just look similar? Can you provide example code? I'm assuming you would need a conditional in the template to incorporate init_config. You could control that with a ['datadog']['jmx']['jboss'] = true attribute, but, again I think writing conditional code for two different use cases is needlessly complex. I also feel that having a separate recipe/template will help users to read the self-documenting code in that, "Yes, there is support for JBoss!" when they see the recipe/template, instead of having to figure out how to use the conditionals.
Let me know, thanks.
Thanks for your quick reply!
Even with one common jmx template, you could let the users not define node['datadog']['jmx']['init_config'] at all, and the init_config section in the generated yaml file would simply be empty.
So I think the way to go is to have one common recipe (the jmx.rb recipe, with an init_config parameter passed to datadog_monitor) and template (an updated jmx.yaml.erb template that could be exactly like docker_daemon's template).
Does that make sense? Let me know if you have any other questions.
OK, thanks. I think I've got it. I'll put this on my to-do list and let you know when I've pushed an update to the PR.
Ian
On Fri, Jul 8, 2016 at 2:30 PM Olivier Vielpeau [email protected] wrote:
Thanks for your quick reply!
Even with one common jmx template, you could let the users not define node['datadog']['jmx']['init_config'] at all, and the init_config section in the generated yaml file would simply be empty.
So I think the way to go is to have one common recipe (the jmx.rb https://github.com/DataDog/chef-datadog/blob/master/recipes/jmx.rb recipe, with an init_config parameter passed to datadog_monitor) and template (an updated jmx.yaml.erb template that could be exactly like docker_daemon's template).
Does that make sense? Let me know if you have any other questions.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DataDog/chef-datadog/pull/303#issuecomment-231437455, or mute the thread https://github.com/notifications/unsubscribe/AAGTT2up5SGDmFyeHTXbTQRKDG4QP6Siks5qTpdWgaJpZM4IVaZH .