Skip to content
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

Terraform wants to delete metrics that intentionally are left out. #7235

Closed
EndureBlackout opened this issue Jun 5, 2020 · 10 comments
Closed
Assignees
Labels
question service/monitor upstream/terraform This issue is blocked on an upstream issue within Terraform (Terraform Core/CLI, The Plugin SDK etc)

Comments

@EndureBlackout
Copy link
Contributor

EndureBlackout commented Jun 5, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform (and AzureRM Provider) Version

Terraform: 0.12.26

Affected Resource(s)

  • azurerm_monitor_diagnostic_setting

Terraform Configuration Files

resource "azurerm_monitor_diagnostic_setting" "api-diagnostics" {
  name               = "${var.global.main-rg.name}-api-diagnostics"
  target_resource_id = azurerm_api_management.api-management.id

  log_analytics_workspace_id     = var.global.log_analytics.id
  log_analytics_destination_type = "Dedicated"

  log {
    category = "GatewayLogs"
    enabled  = true

    retention_policy {
      enabled = true
      days    = var.global.log_analytics.retention_in_days
    }
  }

  metric {
    category = "Gateway Requests"
    enabled  = true

    retention_policy {
      enabled = true
      days    = var.global.log_analytics.retention_in_days
    }
  }
}

Expected Behavior

Only show the included metrics in the terraform plan

 ~ resource "azurerm_monitor_diagnostic_setting" "api-diagnostics" {
        id                             = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/group/providers/Microsoft.ApiManagement/service/theservice"
        log_analytics_destination_type = "Dedicated"
        log_analytics_workspace_id     = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/group/providers/Microsoft.operationalinsights/workspaces/workspace"
        name                           = "api-diagnostics-name"
        target_resource_id             = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/group/providers/Microsoft.ApiManagement/service/theservice"

        log {
            category = "GatewayLogs"
            enabled  = true

            retention_policy {
                days    = 90
                enabled = true
            }
        }
        metric {
            category = "Gateway Requests"
            enabled  = true

            retention_policy {
                days    = 90
                enabled = true
            }
        }
    }

Actual Behavior

Shows metrics that weren't included in the configuration for destroy

 ~ resource "azurerm_monitor_diagnostic_setting" "api-diagnostics" {
        id                             = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/group/providers/Microsoft.ApiManagement/service/theservice"
        log_analytics_destination_type = "Dedicated"
        log_analytics_workspace_id     = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/group/providers/Microsoft.operationalinsights/workspaces/workspace"
        name                           = "api-diagnostics-name"
        target_resource_id             = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/group/providers/Microsoft.ApiManagement/service/theservice"

        log {
            category = "GatewayLogs"
            enabled  = true

            retention_policy {
                days    = 90
                enabled = true
            }
        }
      - metric {
          - category = "Capacity" -> null
          - enabled  = false -> null

          - retention_policy {
              - days    = 0 -> null
              - enabled = false -> null
            }
        }
      - metric {
          - category = "EventHub Events" -> null
          - enabled  = false -> null

          - retention_policy {
              - days    = 0 -> null
              - enabled = false -> null
            }
        }
        metric {
            category = "Gateway Requests"
            enabled  = true

            retention_policy {
                days    = 90
                enabled = true
            }
        }
      - metric {
          - category = "Network Status" -> null
          - enabled  = false -> null

          - retention_policy {
              - days    = 0 -> null
              - enabled = false -> null
            }
        }
    }

Steps to Reproduce

  1. Create an azurerm_monitor_diagnostic_setting
  2. Include metrics that you want to monitor
  3. terraform plan
@davidtom
Copy link
Contributor

davidtom commented Jun 18, 2020

I'm seeing this for logs too, and have tried to fix it by explicitly disabling them:

  log {
    category = "SSISIntegrationRuntimeLogs"
    enabled  = false
  }

Not ideal, but better than seeing these in all my plans. However, this just exacerbates the problem, as now my plans looks like this:

      - log {
          - category = "SSISIntegrationRuntimeLogs" -> null
          - enabled  = false -> null

          - retention_policy {
              - days    = 0 -> null
              - enabled = false -> null
            }
        }
      + log {
          + category = "SSISIntegrationRuntimeLogs"
          + enabled  = false
        }

@magodo magodo self-assigned this Jun 23, 2020
@magodo
Copy link
Collaborator

magodo commented Jun 23, 2020

Hi @EndureBlackout Thank you for submitting this!

As you know, terraform config is describing the final state of the representation of the resource. This resource (azurerm_monitor_diagnostic_setting) is a bit special. Different resource (specified by target_resource_id) has a different set of metric categories and log categories. In which case, if you didn't specify the complete available metric/log categories, then the absent ones are regarded as to be absent in the resource representation. But the case is that each resource has a default setting to those categories, which means a refresh will bring these defaults in, and if you didn't specify those in terraform config, you are telling terraform you want to remove them (while actually they can't be removed).

I can tell that is not the intended behavior, users just want to prefer convention over configuration in this case. I think in this case we can leverage the SuppressDiffFunc to suppress terraform from detecting diff if the only diff is the absence of the default setting. I'll submit a PR to fix this.

@EndureBlackout
Copy link
Contributor Author

@magodo Hey! I appreciate it! Can be closed with PR when merged :)

@jonmaestas
Copy link

Just dropping this here for others. I ran into this some time back and it was very annoying when it showed an update every time.

In our case we want logs to go to a Storage Account, and metrics to go to Log Analytics Workspace. This is for cost reasons since Log Analytics is pricey for ingesting logs, but we still want metrics to go here so we can create dashboards and alerts from these.

This is what I did to fix this, which lines up to what @magodo had explained as expected behavior above in his #7235 (comment).

  1. Pull a dynamic list of Logs and Metrics available (using the azurerm_monitor_diagnostic_categories data) for the resource. Each resource type has different values so we need to use a dynamic block to iterate through the values.
  2. In the first azurerm_monitor_diagnostic_setting resource I set each Metric (using the dynamic data.azurerm_monitor_diagnostic_categories.default.metrics values) to point to a Log Analytics Workspace and set the Logs (using the dynamic data.azurerm_monitor_diagnostic_categories.default.logs values) to the default values when they are disabled.
  3. In the second azurerm_monitor_diagnostic_setting resource I am doing the same as the previous, but setting Logs to go to a Storage Account and setting the Metrics to their default values when disabled.

This is what my diagnostic setting module looks like:

variable resource_id {
  description = "ID of Resource to Enable"
}

variable log_analytics_workspace_id {
  description = "Log Analytics Workspace ID to store Diagnostic Metrics"
}

variable storage_account_id {
  description = "Storage Account ID to store Diagnostic Logs"
}

# https://www.terraform.io/docs/providers/azurerm/d/monitor_diagnostic_categories.html
data azurerm_monitor_diagnostic_categories default {
  resource_id = var.resource_id
}

# https://www.terraform.io/docs/providers/azurerm/r/monitor_diagnostic_setting.html
resource azurerm_monitor_diagnostic_setting metrics {
  name               = "diagnostic_metrics"
  target_resource_id = var.resource_id

  log_analytics_workspace_id = var.log_analytics_workspace_id
  
  dynamic metric {
    for_each = sort(data.azurerm_monitor_diagnostic_categories.default.metrics)
    content {
      category = metric.value
      enabled  = true

      retention_policy {
        enabled = true
        days    = 180
      }
    }
  }

  # this needs to be here with enabled = false to prevent TF from showing changes happening with each plan/apply
  dynamic log {
    for_each = sort(data.azurerm_monitor_diagnostic_categories.default.logs)
    content {
      category = log.value
      enabled  = false

      retention_policy {
        enabled = false
        days    = 0
      }
    }
  }
}

# https://www.terraform.io/docs/providers/azurerm/r/monitor_diagnostic_setting.html
resource azurerm_monitor_diagnostic_setting logs {
  name               = "diagnostic_logs"
  target_resource_id = var.resource_id

  storage_account_id = var.storage_account_id

  dynamic log {
    for_each = sort(data.azurerm_monitor_diagnostic_categories.default.logs)
    content {
      category = log.value
      enabled  = true

      retention_policy {
        enabled = true
        days    = 30
      }
    }
  }

  # this needs to be here with enabled = false to prevent TF from showing changes happening with each plan/apply
  dynamic metric {
    for_each = sort(data.azurerm_monitor_diagnostic_categories.default.metrics)
    content {
      category = metric.value
      enabled  = false

      retention_policy {
        enabled = false
        days    = 0
      }
    }
  }
}

Hope this helps.

InsulaVentus added a commit to InsulaVentus/terraform-azurerm-cosmosdb-mongodb that referenced this issue Mar 18, 2021
InsulaVentus added a commit to InsulaVentus/terraform-azurerm-relay-hybrid-connection that referenced this issue Mar 18, 2021
InsulaVentus added a commit to InsulaVentus/terraform-azurerm-postgresql that referenced this issue Mar 18, 2021
InsulaVentus added a commit to InsulaVentus/terraform-azurerm-key-vault that referenced this issue Mar 18, 2021
yngveh pushed a commit to avinor/terraform-azurerm-key-vault that referenced this issue Mar 19, 2021
* Add each available diagnostics category, but enable only those we want

This is a workaround for a know issue: hashicorp/terraform-provider-azurerm#7235

* Use azurerm v2.51.0

* Run terraform fmt

* Don't rename resource name

* Use required_providers block to set azurerm version
@magodo
Copy link
Collaborator

magodo commented Apr 16, 2021

@jonmaestas There is one limitation in your solution, the resource that assigned to azurerm_monitor_diagnostic_categories data source should be provisioned prior to this provision. This should be fixed in #11342.

@jonmaestas
Copy link

jonmaestas commented Apr 17, 2021

@magodo just replace var.resource_id with azurerm_whatever.your_resource_is.id.

resource azurerm_key_vault example {...}

# https://www.terraform.io/docs/providers/azurerm/d/monitor_diagnostic_categories.html
data azurerm_monitor_diagnostic_categories default {
  resource_id = azurerm_key_vault.example.id
}
...

Edit: The above is a module I created to reuse this config. I pass in the resource id, ..., etc to that module.

@andyr8939
Copy link

I have been able to fix this by filling out all the fields even if they are not used. For example with Log Analytics I would see this everytime.

      - log {
          - category = "AuditEvent" -> null
          - enabled  = true -> null

          - retention_policy {
              - days    = 0 -> null
              - enabled = false -> null
            }

Now the Terraform for this would be

  log {
    category = "AuditEvent"
  }

It was the null that was catching my eye. So in my Terraform code I added in the items like retention policy that I wasn't using anyway so it can't null it out

  log {
    category = "AuditEvent"
    retention_policy {
      days = 0
      enabled = false
    }
  }

Now when I do a plan/apply it says no changes made. Hope that helps!

@vijred
Copy link

vijred commented Aug 5, 2021

I wanted to collect logs for only a few categories,

variable "diag_log_list" {
  type        = list(string)
  default     = ["PartitionKeyStatistics", "DataPlaneRequests"]
  description = "List of CosmosDB Diagnostic settings for logs - Only these log categories are enabled"
}
# ...

  dynamic "log" {
    for_each = sort(data.azurerm_monitor_diagnostic_categories.default.logs)
    content {
      category = log.value
      enabled  = contains(var.diag_log_list, log.value) ? true  : false
      retention_policy {
        enabled = false
        days    = 0
      }
    }
  }

@favoretti
Copy link
Collaborator

Thanks for opening this issue! Since this issue seems to have been addressed in the latest versions of the provider (or a valid workaround was provided) - I'm going to close it. Please open a new updated bug report if this is still relevant. Thank you.

@github-actions
Copy link

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question service/monitor upstream/terraform This issue is blocked on an upstream issue within Terraform (Terraform Core/CLI, The Plugin SDK etc)
Projects
None yet
9 participants