-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add new resource, google_logging_metric #1531
Add new resource, google_logging_metric #1531
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
Hello! I am a robot who works on Magic Modules PRs. I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review. They will authorize it to run through our CI pipeline, which will generate downstream PRs. Thanks for your contribution! A human will be with you soon. |
Just signed the CLA. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JoeStanton Thanks for contributing! I'm not sure where you are on this PR so I haven't done a full review yet. If you are ready please move this out of draft status and I'll look at it in more depth.
Just FYI, at a quick glance this PR will need more test coverage before we're able to merge it in. The example you added will generate 1 small test, but with a more complicated object like this I'd like to see a few more that cover more of the fields. Additionally since this is an updatable resource one of the tests should also cover updating the resource. Tests at this point are still hand written and added in the third_party/terraform
directory.
Logs-based metric can also be used to extract values from logs and create a a distribution of the values. The distribution records the statistics of the extracted values along with an optional histogram of the values as specified by the bucket options. | ||
references: !ruby/object:Api::Resource::ReferenceLinks | ||
guides: | ||
"Official Documentation": "https://cloud.google.com/logging/docs/reference/v2/rest/v2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This link 404's. We usually point this link to the overview for the product, such as https://cloud.google.com/logging/docs/apis
Hi @chrisst thanks for getting back to me. We are using this code to provision and update custom metrics in the project I'm working on (success!), but I've been super busy so have not been able to tidy this up yet so it's ready for merge. Definitely needs more test coverage. Could you point me in the right direction for the following:
I think in the next week or two I should be able to finish this up. Beyond tests, do I need to consider all of the Ansible/Inspec use-cases to make this mergeable? |
To ignore ordering you can look at using a Terraform TypeSet https://www.terraform.io/docs/extend/schemas/schema-types.html#typeset which in MagicModules is created with Our general rule of thumb is to mirror the api as closely as possible. The API is committed to not making breaking changes whereas the UI doesn't have that guarantee. As for Ansible, we often add new resources here just for a single provider (Terraform) and will |
I double checked on the ordering, and MM will turn the |
What's next? |
@JoeStanton it looks like there is a lot of excitement for this to land so we'd love to get this in soon. If you have time this week to finish the PR please let us know, otherwise we might build off of what you have started so that we can get this out. Thanks! |
Opening this as a draft PR, though I hope it won't kick off the build robot. I wanted to validate my approach early, and will finish over the next couple of days.
Usage as follows
[all]
[terraform]
Logs-based metrics support
[terraform-beta]
[ansible]
[inspec]