-
Notifications
You must be signed in to change notification settings - Fork 62
coder_monitoring module #362
Conversation
monitoring/main.tf
Outdated
| required_providers { | ||
| coder = { | ||
| source = "coder/coder" | ||
| version = ">= 0.11" |
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.
Why 0.11? We're on v2.0.2 now.
You'd need to add the coder_monitoring datasource to the terraform provider before we can merge this, do you have a PR in the works for this?
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.
Also is there an RFC design on how coder_monitoring will work.?
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.
Yes, I'll open it in a few minutes - I was just willing to open this one to have feedback too as I was not really sure if I was going on the right direction with writing the module this way. 🙏
| ```tf | ||
| module "monitoring" { | ||
| source = "registry.coder.com/modules/monitoring/coder" | ||
| version = "1.0.0" |
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.
The versioning of modules right now follows the whole repo I believe, so it'd be v1.0.25.
| default = ["/"] | ||
| } | ||
|
|
||
| variable "enabled" { |
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.
Why do we have a variable for this?
Can use the terraform count to control if the module is added or not.
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.
I imagined we can use this enabled parameter to let users keep the block but just change the enabled variable if they want, for some specific cases, disable it quickly without loosing their configuration, does it make sense ? Otherwise I can delete it and indeed rely on the terraform count (I need to read about it.)
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.
That's the same purpose the count argument can serve.
It can be set to 0 or 1 to enable or disable the whole module.
If someone is not using the module they can use the count on the data "coder_monitoring" too.
see: https://developer.hashicorp.com/terraform/language/meta-arguments/count
|
Hey , thanks for the fast comments. @matifali I resolved most of your points - just asking a question on the enabled one. ✅ |
|
Closing until approvals on the design ✅ Still some points to validate together first. |
This PR aims to add a new coder module - named
coder_monitoringwhich can be used to enable and customize monitoring of resources over the coder workspaces.