ojongerius / terraform-provider-datadog

A Terraform plugin to manage Datadog resources.
Mozilla Public License 2.0
29 stars 6 forks source link

Unable to build monitors that combines metrics #69

Closed kelvl closed 8 years ago

kelvl commented 8 years ago

Because the query param on the monitor is built by combining various parameters from the resource data, we can't create monitors that uses multiple metrics, ie:

min:aws.rds.free_storage_space / min:aws.rds.total_storage_space

for monitoring the % of disk used

current implementation: https://github.com/ojongerius/terraform-provider-datadog/blob/master/datadog/resource_datadog_metric_alert.go#L130

I suggest to make another parameter on the resource data called query so that the user can override the actual query for the monitor if needed.

The original parameters can still take precedence and query is only used if none of the original parameters are specified.

I can try and work on a PR if this is a way you want to go

ojongerius commented 8 years ago

Interesting, had not run into this scenario! Thanks for raising this issue and thinking along.

How do you feel about the current implementation? If you, as a user, could define query, would you still use the old combination of metric, tags, keys, time_aggr and time_window or would you switch to alway/often set query directly?

I'm leaning towards lettings users write the query themselves, it allows more freedom, and is closer to the API, removing complexity from the provider.

What are your thoughts?

kelvl commented 8 years ago

I second the opinion to allow people to specify query directly as well.

My workflow is that I usually go to datadog and add an monitor manually to look at what queries work for me and what those settings are before committing them into a terraform template and using that to create more monitors for other resources, it's good to be able to see the exact query used in the monitor and use that in my terraform template if query is directly available.

Attempted at an implementation #70, please take a look and let me know if you would like any changes!

ojongerius commented 8 years ago

Thanks!

ojongerius commented 8 years ago

@kelvl could you be convinced to update the README and add this to the outlier resource too?

kelvl commented 8 years ago

71 made for documentation updates!

ojongerius commented 8 years ago

Nice one @kelvl.