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

[AVM Module Issue]: logAnalyticsDestinationType is not set correctly for Diagnostic settings #55

Closed
1 task done
acauret opened this issue Jul 29, 2024 · 3 comments
Closed
1 task done
Labels
Language: Terraform 🌐 This is related to the Terraform IaC language Needs: Immediate Attention ‼️ Immediate attention of module owner / AVM team is needed Needs: Triage 🔍 Maintainers need to triage still Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days Type: Bug 🐛 Something isn't working

Comments

@acauret
Copy link

acauret commented Jul 29, 2024

Check for previous/existing GitHub issues

  • I have checked for previous/existing GitHub issues

Issue Type?

Bug

(Optional) Module Version

No response

(Optional) Correlation Id

No response

Description

The logAnalyticsDestinationType should not be Dedicated but allow seeting of null value
image

as everytime terrform runs it thinks there is a change.
image

@acauret acauret added Language: Terraform 🌐 This is related to the Terraform IaC language Needs: Triage 🔍 Maintainers need to triage still labels Jul 29, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Type: Bug 🐛 Something isn't working label Jul 29, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days label Aug 29, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Immediate Attention ‼️ Immediate attention of module owner / AVM team is needed label Sep 11, 2024
@lonegunmanb
Copy link
Member

Thanks for opening this pr to us @acauret and sorry for the late reply, this diagnostic setting is required by Azure Verified Module Specification so we won't fix it.

@tonyskidmore
Copy link

@lonegunmanb does this need more of a review? The code below will always want to make changes on each run, which is certainly not ideal. Setting the azurerm_monitor_diagnostic_setting outside of the module is the useable workaround.

terraform {
  required_providers {
    azurerm = {
      source  = "hashicorp/azurerm"
      version = "~> 3.0"
    }
  }
}

provider "azurerm" {
  features {}
}

resource "random_string" "example" {
  length  = 5
  special = false
  upper   = false
  lower   = true
}

resource "azurerm_resource_group" "rg" {
  name     = "rg-openai-avm"
  location = "uksouth"
}

resource "azurerm_log_analytics_workspace" "law" {
  name                = "law-testing-${random_string.example.result}"
  location            = azurerm_resource_group.rg.location
  resource_group_name = azurerm_resource_group.rg.name
  sku                 = "PerGB2018"
  retention_in_days   = 30
}

module "openai" {
  source  = "Azure/avm-res-cognitiveservices-account/azurerm"
  version = "0.3.0"

  kind                = "OpenAI"
  resource_group_name = azurerm_resource_group.rg.name
  location            = azurerm_resource_group.rg.location

  name                          = "oai-testing-${random_string.example.result}"
  sku_name                      = "S0"
  public_network_access_enabled = true

  diagnostic_settings = {
    sendToLogAnalytics = {
      name = "sendToLogAnalytics"
      workspace_resource_id = azurerm_log_analytics_workspace.law.id
    }
  }

  cognitive_deployments = {
    "gpt-35-turbo" = {
      name = "gpt-35-turbo"
      model = {
        format  = "OpenAI"
        name    = "gpt-35-turbo"
        version = "0301"
      }
      scale = {
        type = "Standard"
      }
    }
  }
}

@acauret
Copy link
Author

acauret commented Dec 13, 2024

@lonegunmanb - Please reopen this and fix - Just because its defined in the Azure Verified Module Specification does not mean its correct. its very annoying as this happening on quite a few modules in AVM

This does not have the option for dedicated or azure diagnostics
Image

Azure Data factory does have this so it works correctly

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language: Terraform 🌐 This is related to the Terraform IaC language Needs: Immediate Attention ‼️ Immediate attention of module owner / AVM team is needed Needs: Triage 🔍 Maintainers need to triage still Status: Response Overdue 🚩 When an issue/PR has not been responded to for X amount of days Type: Bug 🐛 Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants