Add enhanced monitoring #6
Conversation
| Enhanced Monitoring example | ||
| =========================== | ||
|
|
||
| Configuration in this directory creates the additional resources required to use Enhanced Monitoring. |
There was a problem hiding this comment.
Remember that these examples should always be executed using just terraform init, plan, apply. Users should not be asked to update anything in the example to try them out. Please include "provider" section and "rest of params here as per complete example".
There was a problem hiding this comment.
Are there guidelines for examples?
There was a problem hiding this comment.
No, but there will be (probably during next week or two).
README.md
Outdated
|
|
||
| # Enhanced Monitoring | ||
| monitoring_interval = "${var.monitoring_interval} | ||
| monitoring_role_arn = "${var.monitoring_interval == "0" ? "" : aws_iam_role.rds_monitoring.arn}" |
There was a problem hiding this comment.
Similar should be in usage section. Most users want to just copy-paste this from readme file and put as little effort as possible to get it up, but line 44 will make them think that they need to create IAM role somehow.
Keep this section as easy as possible and make it look as end result:
monitoring_interval = "10"
monitoring_role_arn = "arn:aws:i-don't-know-but-you-know-what-to-write-here:12345678"
No need to use variables here - again, for simplicity.
There was a problem hiding this comment.
Yeah, I didn't mean to include that.
But, there's no magic role arn you can include - you have to create the role. Not sure how best to document it.
There was a problem hiding this comment.
I think it will be best to make an example, where you create everything what is needed for it. And link to this example on the main readme page.
Maybe soon we can also start section "Features", where we can highlight what this module can do. Something like:
Features:
* Create RDS resources with most of the options supported
* Support [enhanced monitoring](example/...). This module does not create IAM role for you. Check example code for more details.
There was a problem hiding this comment.
OK, I'll pull the params from the README and fix pu the example.
|
How's this? |
README.md
Outdated
| monitoring_role_arn = "${var.monitoring_interval == "0" ? "" : aws_iam_role.rds_monitoring.arn}" | ||
| # Enhanced Monitoring - see example for details on how to create the role | ||
| # monitoring_interval = "10" | ||
| # monitoring_role_arn = "aws_iam_role.rds_enhanced_monitoring.arn" |
There was a problem hiding this comment.
Write how the end result will have to look like - monitoring_role_arn = "arn:aws:iam::123456789012:role/RDSEnhancedMonitoring" and leave it to curious user to figure out how to get the value (maybe he will use data-source to find the one he has already make, maybe he will create resource himself, maybe he will hardcode it, maybe he passes it as variable, ...)
There was a problem hiding this comment.
Is it worth putting in a link to the relevant AWS page?
There was a problem hiding this comment.
I think so, a small note is usually a good idea. In general, I would love to hear from real users what do they miss.
|
Did you get chance to look at this? |
|
I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Re-working of terraform-community-modules/tf_aws_rds#37
(plus a
terraform fmt:) )