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.

Sign up for GitHub

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

Jump to bottom

Datadog provider #3611

Closed
wants to merge 24 commits into from
Closed

Datadog provider #3611

wants to merge 24 commits into from

Conversation

ojongerius
Copy link
Contributor

@phinze this PR is rough. I've pulled @vinceprignano's changes in, and copied my provider work over.
We've been using this provider at Atlassian for a little while and does what we need. To be useful to the general public it probably needs a little TLC.

Things worth noting/discussing:

  • Read resources are not in there yet, I've started in a branch https://github.com/ojongerius/terraform-provider-datadog/tree/38_read_resources but this needs work.
  • I've copied the pattern @vinceprignano used of 'faking' warning and critical levels. It keeps configurations small, but is a little inconvenient in the provider.
  • Graphs are not an autonomous resource in Datadog, I'm simulating this by sneaking an ID in the title. Not something I'm happy with. What are your thoughts? Keen to discuss.
  • There is a lot of duplication in the resources that are easy to abstract out. Happy to do that but wanted to get the PR our rather sooner than later.

Happy to refactor as we go 😄

@ojongerius ojongerius mentioned this pull request Nov 23, 2015
Closed
@ojongerius
Merge remote-tracking branch 'upstream/master' into datadog_provider
5f76554 
* upstream/master: (775 commits)
  docs/aws: Fix highlighting of ECR in sidebar
  docs/aws: Whitespaces removed
  provider/azure: Copy settings file into variable
  provider/azure: Allow AZURE_SETTINGS_FILE for tests
  provider/aws: mention us-east-1 in ECR docs and tests
  provider/azure: Fix vetting error
  Update CHANGELOG.md
  provider/azure: Remove obsolete tests
  provider/aws: Update Route53 Zone tests
  vpc vpn connection test fixes
  skip TestAccAWSVPCPeeringConnection_tags for now
  VPC Endpoint test updates
  Update CHANGELOG.md
  provider/aws: fix CheckDestroy for ProtocolPolicy tests
  provider/aws: Update tests destroy checks
  Add script for running tests in Travis
  provider/aws: Update ElastiCache tests to verify delete
  provider/aws: fix CheckDestroy on placement_group tests
  Check for ecr repository and policy removal
  provider/aws: fixes for Network ACL Rules
  ...
@ojongerius
* Remove monitor resource, it's been replaced by metric_alert, outlie…
8535656 
…r_alert and service_check

* Refactor to be closer to the API; each resource creates exactly *one* resource, not 2, this removes much unneeded complexity. A warning threshold is now supported by the API.
* Remove fuzzy resources like graph, and resources that used them -dashboard and screenboards. I'd welcome these resources, but the current state of Terraform and the Datadog API does not allow these to be implemented in a clean way.
@ojongerius
Copy link
Contributor Author

Have done some cleanup:

  • Remove monitor resource, its replaced by metric_alert, outlier_alert and service_check
  • Refactor to be closer to the API; each resource creates exactly one resource (unless count is used) instead of 2, this removes much unneeded complexity. A warning threshold is now supported by the API, which I'm working on.
  • Remove fuzzy resources like graph, and resources that used them -dashboard and screenboards. I'd welcome these resources, but the current state of Terraform and the Datadog API does not allow these to be implemented in a clean way.

ColinHebert and others added 3 commits January 6, 2016 08:34
@ColinHebert
Merge branch 'master' into datadog_provider
9ecc46e
@ojongerius
Merge remote-tracking branch 'origin/master' into datadog_provider
21de534 
* origin/master: (115 commits)
  Update CHANGELOG.md
  Update CHANGELOG.md
  provider/aws: Default false for VPC Classic Link
  Update CHANGELOG.md
  Update CHANGELOG.md
  Update CHANGELOG.md
  provider/aws: Limit SNS Topic Subscription protocols
  Update CHANGELOG.md
  Update CHANGELOG.md
  Update cloudwatch_metric_alarm.html.markdown
  provider/aws: Allow ap-northeast-2 (Seoul) as valid region
  Update CHANGELOG.md
  provider/aws: use random cert name in ELB test
  Add az_mode and availability_zones parameters
  Add availability_zone parameter.
  Add the option to specify a custom (AWS compatible) S3 endpoint
  Changes (inconsistently used) t2.micro back to t1.micro - t2.micro is VPC only and thus will cause problems for users with a default VPC (e.g. people who signed up for AWS a few years ago)
  provider/openstack: Add Instance Personality
  Update interpolation.html.md
  Update CHANGELOG.md
  ...
@ojongerius
Support multiple thresholds for metric alerts, remove notify argument.
2391f42
@ojongerius
Copy link
Contributor Author

  • Implemented support for multiple thresholds per monitor.
  • Removed notify attributes, setting it in the message is more flexible.

kelvl and others added 7 commits January 23, 2016 17:54
@kelvl @ojongerius
Add query parameter to metric_alert
1202da2 
`query` is used when it is specified by the user, if not
`metric`/`tags`/`keys`/`time_aggr`/`window` is used instead.
@kelvl @ojongerius
Add test for query metric alerts
60753de
@ojongerius
Merge remote-tracking branch 'origin/master' into datadog_provider
8ab236e 
* origin/master: (172 commits)
  Update CHANGELOG.md
  Update CHANGELOG.md
  heroku: randomize the remaining appnames in tests
  Policing up the new `tags` code from @Carles-Figuerola
  Add tests for Network Tags for provider/cloudstack and applied tips from PR#4742
  Update CHANGELOG.md
  provider/aws enable specifying aws s3 redirect protocol
  core: Add `make core-test` target
  Update CHANGELOG.md
  Fixing a flapping AzureRM PublicIP validation test
  Make the concurrence for applying rules configurable
  mailgun: fixup go vet problem in test
  Update CHANGELOG.md
  core: demote evaltree loglevel from INFO -> TRACE
  Specify that the vlan attribute in cloudstack is only usable for ROOT admins and stop reading it back
  mailgun: poll until domain destroy takes effect
  heroku: randomize names in acctests
  Update CHANGELOG.md
  Update CHANGELOG.md
  provider/digitalocean: allow reassignment of floating IPs
  ...
@ojongerius
Replace separate monitors with one generic monitor with read function…
40e8357 
…ality.
@ojongerius
Merge remote-tracking branch 'origin/master' into datadog_provider
92217d2 
* origin/master: (94 commits)
  provider/openstack Fix LB Member Errors
  Update CHANGELOG.md
  Update CHANGELOG.md
  provider/openstack: Per-network Floating IPs
  provider/openstack Add Acceptance Test for No Port IP
  Changing the default for ship_final_snapshot on redshift_cluster to be true so as to match the code
  Update CHANGELOG.md
  remove extra parenthesis
  Catch potential custom network errors in docker
  provider/docker: Update documentation
  Update CHANGELOG.md
  config: Add docs for new base64sha256 func
  config: Add base64sha256() function
  Update CHANGELOG.md
  Update the test file
  rename trim to trimspace
  Fix name from strip to trim
  Add the trim() interpolation function
  Vendor all dependencies w/ Godep
  Update CHANGELOG.md
  ...
@ojongerius
Gofmt.
dc03c3e
@ojongerius
Gofmt.
12ed4cd
@ojongerius
Copy link
Contributor Author

  • Revert back to one monitor resource that support all types
  • Add more options
  • Add read resource
  • Add tests for updates
  • Break Travis build fails -It looks like this is caused by vendoring, not sure how to fix that one. I'll wait for the documentation referred to in Vendor all dependencies w/ Godep #4909 has surfaced.

Happy to add documentation if there is willingness to merge this in to master.

@ojongerius ojongerius mentioned this pull request Feb 3, 2016
Closed
@pmoust pmoust mentioned this pull request Feb 7, 2016
Merged
@ojongerius
Copy link
Contributor Author

Fixed build and added some doco.

@ojongerius ojongerius mentioned this pull request Feb 9, 2016
Closed
@ojongerius
Merge remote-tracking branch 'origin/master' into datadog_provider
cb57dd3 
* origin/master: (77 commits)
  Update CHANGELOG.md
  Fixing some golint issues on the new validate command
  Update CHANGELOG.md
  Update CHANGELOG.md
  provider/azurerm: Add `azurerm_sql_firewall_rule` resource
  Update CHANGELOG.md
  Update CHANGELOG.md
  Update CHANGELOG.md
  Update CHANGELOG.md
  docs: Clarify use cases in docs for the validate cmd
  provider/aws: Document log file validation + KMS Key ID in CloudTrail
  provider/aws: Add support for CloudTrail log validation + KMS encryption
  Added verify command
  Fix load balancers read logic.
  Update CHANGELOG.md
  provider/cloudflare: Change CloudFlare record TTL property to be `computed`
  provider/azurerm: Add `azurerm_dns_mx_record` resource
  Check calculated availability zones.
  Make availability zones a computed attribute.
  Remove redundant metadata get.
  ...
@ojongerius ojongerius mentioned this pull request Feb 10, 2016
Closed
@jen20
Copy link
Contributor

jen20 commented Feb 22, 2016

Hi @ojongerius! This looks good and I think we'll be happy to merge it - could you squash the commits and force push back to the pull request branch though? I'll look at getting a test account set up so that we can run the acceptance tests nightly in Travis.

@jen20 jen20 added this to the Terraform 0.6.12 milestone Feb 22, 2016
@jen20 jen20 mentioned this pull request Feb 22, 2016
Merged
@jen20
Copy link
Contributor

jen20 commented Feb 22, 2016

I've gone ahead and rebased this as #5251. It should appear in Terraform 0.6.12. We also have a test account set up for acceptance testing, so the tests should be run nightly in due course.

@jen20 jen20 closed this Feb 22, 2016
@ojongerius ojongerius deleted the datadog_provider branch February 23, 2016 07:10
@ghost
Copy link

ghost commented Apr 28, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ojongerius @jen20 @apparentlymart @vincepri @ColinHebert @kelvl @pmoust