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

Release 1.0.0 with integration recipes (was: First stab at recipe-based configuration.) #43

Merged
merged 50 commits into from
May 6, 2013

Conversation

alq666
Copy link
Member

@alq666 alq666 commented Jan 30, 2013

Cassandra is in.

Role example:

role "cassandra"

run_list "cassandra", "datadog::cassandra"

If this is a viable option, the rest of the recipes will follow.

@ghost ghost assigned miketheman Jan 30, 2013
@alq666
Copy link
Member Author

alq666 commented Jan 30, 2013

No need to merge yet, but curious about your thoughts.

@miketheman
Copy link
Contributor

  1. I like this
  2. I suspect that having the yaml file actually as a template with attributes (port, username, etc) having sane defaults that can be expected to "always work", and overridable from elsewhere (role, env, et al) makes a little more sense
  3. This kind of "migration" can also support namespacing the attributes to live under a 'datadog' node namespace - and not conflict with others, as we've seen before.
  4. We could potentially include an 'auto-detect' recipe once we have 'component' recipes - it looks for the default attributes of a widely-used and well-known cookbook namespace and includes the correct recipe for that component. Maybe not immediately, but I suspect that adoption of 'it just works!' monitoring is benefiting somewhat from automatic config generation today.

@miketheman
Copy link
Contributor

Some thoughts:

  1. Use modern notification syntax FC043 (old style is deprecated)
  2. Consider enclosing ERB conditionals with closing syntax to reduce rendered whitespace (see Datadog agent template renders a lot of whitespace #56)
  3. LWRP name should be monitor.rb => datadog_monitor, cleaner than datadog_ddmonitor. Or possibly datadog_agentmonitor - but that's very long...
  4. Currently, the :add action doesn't notify. Why not? Presumably something could subscribe to a datadog_monitor :add resource.
  5. There seems to be inconsistency with the order to init_config vs instances - it probably doesn't matter to the agent, but we should choose one over the other for consistency.
  6. This still doesn't cover how we would accomplish multiple instances per host, but that might be something we can have the agent handle. Potentially attempt to interrogate the filename first for the plugin name, if not immediately discernable, have an integration or plugin attribute at the top of the .yaml file that tells the agent what this is.
    This might be a simpler route than trying to write the same file each run and compare the generated output.
    We can speak on this via other channels if this isn't 100% clear.

Other:

  • Current attributes.rb probably deserves a full sweep of cleanup
  • Do we want to continue to use EPEL (which may surprise some people) or should we package up any dependencies we need for a given integration? Most recently seen was Redis 2.0 package vs pypi version for Slave monitoring.

@ghost ghost assigned alq666 Apr 23, 2013
@alq666
Copy link
Member Author

alq666 commented Apr 29, 2013

@miketheman what's triggered thus far in Vagrantfile passes a vagrant provision run. I'm going to push and finish the rest on the plane or in Austin.

Resolved Conflicts:
	.gitignore
	README.md
	attributes/default.rb
	metadata.rb
	recipes/dd-agent.rb
	templates/default/datadog.conf.erb
@miketheman
Copy link
Contributor

@alq666 @MisterRayCo Please review the README and CHANGELOG - let me know if something eneds to be updated before I ship this code out to the world.

@MisterRayCo
Copy link

@miketheman looking good to me

miketheman added a commit that referenced this pull request May 6, 2013
Release 1.0.0 with integration recipes (was: First stab at recipe-based configuration.)
@miketheman miketheman merged commit 9639b69 into master May 6, 2013
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.

3 participants