search

equinor / terraform-baseline

Best practices for creating reusable Terraform modules using the Azure provider
https://equinor.github.io/terraform-baseline/
MIT License
12 stars 5 forks source link

Implement baseline alerts #136

Open hknutsen opened 7 months ago

hknutsen commented 7 months ago

Implement baseline alerts according to: https://azure.github.io/azure-monitor-baseline-alerts/welcome/

This opens up the possibility of moving the app-service and aci modules out of archive, because by implementing alerts we'll have a reason to keep them 🙂

Example: https://github.com/equinor/terraform-azurerm-app-service/pull/10. Note though that alert rules can be reused for multiple resources of the same type, so it might be a better idea to create an alerts submodule for each module. Discuss! 🙂

Tasks

hknutsen commented 6 months ago

Alerts also have a cost, so reusing alerts could be a good idea with that in mind.

hknutsen commented 5 months ago

Interesting finding...

I was playing around with setting the scope of an alert rule to multiple resources, which gave me the following warning:

Metric and Log signals might not be available if the scope includes multiple resources.

Investigate this warning, as it might be important for the decision on whether or not alert rules should be created for each individual resource, or if alert rules should be reused across resources of the same type.

Update: overview of resource types that support multi-resource alert rules: https://learn.microsoft.com/en-us/azure/azure-monitor/alerts/alerts-metric-near-real-time#metrics-and-dimensions-supported

Seems like very few resource types support multi-resource alert rules. Maybe ignore for now?

hknutsen commented 2 months ago

Added task list with link to PR for SQL module.

hknutsen commented 2 months ago

Arguments against including alerts in baseline modules:

For comparison, including a diagnostic setting in our modules made sense since it only requires the creation of a single simple resource that can be modified as needed by using a single module input (diagnostic_setting_enabled_log_categories).

helenakallekleiv commented 2 months ago

An addition, if we created the alerts as a sub module, we would avoid a bloated main module. Although this would then mean that for each resource in the users configuration, they would need one alert sub-module block, for each resource. Doubling the amount of module blocks and cause the config to be bloated.

As this is not required per se, we could define the use of alerts as an example for now.

hknutsen commented 1 month ago

Another potential solution... Using the Azure Key Vault module as an example:

Current implementation

With the current implementation, the module equinor/key-vault/azurerm creates two resources:

  1. azurerm_key_vault.this
  2. azurerm_monitor_diagnostic_setting.this

Proposed implementation

A new submodule equinor/key-vault/azurerm//modules/vault creates 2 resources:

  1. azurerm_key_vault.this
  2. azurerm_monitor_diagnostic_setting.this

A new submodule equinor/key-vault/azurerm//modules/alerts creates 2 resources:

  1. azurerm_monitor_metric_alert.this
  2. azurerm_scheduled_query_alert_v2.this

The module equinor/key-vault/azurerm creates 4 resources:

  1. module.vault.azurerm_key_vault.this
  2. module.vault.azurerm_monitor_diagnostic_setting.this
  3. module.alerts.azurerm_monitor_metric_alert.this
  4. module.alerts.azurerm_scheduled_query_alert_v2.this

So by calling the main equinor/key-vault/azurerm module you get everything, however if you only need the Key Vault with a diagnostic setting your can call the equinor/key-vault/azurerm//modules/vault submodule directly.

ErlendT commented 4 weeks ago

Waiting for POC from @hknutsen before moving into todo

hknutsen commented 1 week ago

https://github.com/equinor/terraform-azurerm-key-vault/pull/98 has been updated with a POC of the proposed implementation.