feat: added support for prometheus metrics scraping #167
Conversation
|
/run pipeline |
ocofaigh
left a comment
ocofaigh
left a comment
There was a problem hiding this comment.
I left a few comments - I think this approach needs to be revised to make it easier for consumers to consume
variables.tf
Outdated
| description = "Prometheus configuration for the agent. If you want to enable Prometheus configuration, set `file` to true and provide the prometheus.yaml file content in `yaml`. [Learn more](https://github.com/terraform-ibm-modules/terraform-ibm-monitoring-agent/blob/main/solutions/fully-configurable/DA-types.md#prometheus)." | ||
| type = object({ | ||
| file = bool | ||
| yaml = map(any) |
There was a problem hiding this comment.
This is going to cause confusion. The attribute is called yaml but the consumers is actually going to have to pass HPC map syntax right? Is there a schema we could define for the config, or is it too large?
|
/run pipeline |
|
/run pipeline |
| **Tip:** | ||
| Use `metrics_filter` to optimize your monitoring setup by collecting only the metrics that matter most to your use case. This can help reduce costs and improve performance. | ||
|
|
||
| ## `prometheus` |
There was a problem hiding this comment.
prometheus -> prometheus_config
There was a problem hiding this comment.
@ocofaigh since we are not exposing variable in DA, should I remove this variable from DA-types.md or we can keep it for future.
There was a problem hiding this comment.
Module is also pointing to same place so I guess we can keep it.
…modules/terraform-ibm-monitoring-agent into prometheus-metrics-scrape
…modules/terraform-ibm-monitoring-agent into prometheus-metrics-scrape
variables.tf
Outdated
| } | ||
|
|
||
| variable "prometheus_config" { | ||
| description = "Prometheus configuration for the agent. If you want to enable Prometheus configuration provide the prometheus.yaml file content in `hcl` format. [Learn more](https://github.com/terraform-ibm-modules/terraform-ibm-monitoring-agent/blob/main/solutions/fully-configurable/DA-types.md#prometheus)." |
There was a problem hiding this comment.
Is this URL still valid after changing the title in the markdown?
examples/obs-agent-iks/variables.tf
Outdated
| default = "syd01" | ||
| } | ||
|
|
||
| variable "prometheus_config" { |
There was a problem hiding this comment.
Its probably better to hard code a sample value in the example itself instead of exposing a variable. That way people can see a sample value that can be passed. We don't usually expose variables like this in examples as we don't want the example to become flexible solution thats people will use. they are just for reference
examples/obs-agent-iks/variables.tf
Outdated
| } | ||
|
|
||
| variable "prometheus_config" { | ||
| description = "Prometheus configuration for the agent. If you want to enable Prometheus configuration provide the prometheus.yaml file content in `hcl` format. [Learn more](https://github.com/terraform-ibm-modules/terraform-ibm-monitoring-agent/blob/main/solutions/fully-configurable/DA-types.md#prometheus)." |
There was a problem hiding this comment.
same comment as below - url need to be updated due to title change in markdown?
|
/run pipeline |
|
/run pipeline |
|
/run pipeline |
|
🎉 This PR is included in version 1.6.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Description
added support for prometheus metrics scraping
Issue
Release required?
x.x.X)x.X.x)X.x.x)Release notes content
Run the pipeline
If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.
Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:
Checklist for reviewers
For mergers