Azure / bicep-registry-modules

Bicep registry modules
MIT License
505 stars 351 forks source link

[Bug Report]: diagnosticLogCategoriesToEnable for HostPools #2446

Closed maxricketts closed 4 months ago

maxricketts commented 1 year ago

Describe the bug

This param at the bottom that has a default of 'allLogs' doesn't apply all logs to the diagnostics settings.

It uses diagnosticsLogsSpecified instead.

in my module I only specify the following:

diagnosticWorkspaceId: law.id
diagnosticLogsRetentionInDays: 30
var diagnosticsLogsSpecified = [for category in filter(diagnosticLogCategoriesToEnable, item => item != 'allLogs'): {
  category: category
  enabled: true
  retentionPolicy: {
    enabled: true
    days: diagnosticLogsRetentionInDays
  }
}]

var diagnosticsLogs = contains(diagnosticLogCategoriesToEnable, 'allLogs') ? [
  {
    categoryGroup: 'allLogs'
    enabled: true
    retentionPolicy: {
      enabled: true
      days: diagnosticLogsRetentionInDays
    }
  }
] : diagnosticsLogsSpecified
@description('Optional. The name of logs that will be streamed. "allLogs" includes all possible logs for the resource.')
@allowed([
  'allLogs'
  'Checkpoint'
  'Error'
  'Management'
  'Connection'
  'HostRegistration'
  'AgentHealthStatus'
])
param diagnosticLogCategoriesToEnable array = [
  'allLogs'
]

To reproduce

deploy a host pool specifying only the below.

Code snippet

module hostpoolOffice 'br/exgRegistry:microsoft.desktopvirtualization.hostpools:0.4.1530' = {
  scope: resourceGroup(rg.name)
  name: '${hostpoolOfficeName}${deploy}'
  params: {
    name: hostpoolOfficeName
    hostpoolFriendlyName: hostpoolOfficeFriendlyName
    preferredAppGroupType: 'RailApplications'
    maxSessionLimit: 20
    tags: tags
    diagnosticWorkspaceId: law.id
    diagnosticLogsRetentionInDays: 30
  }
}

Relevant log output

No response

microsoft-github-policy-service[bot] commented 4 months ago

[!IMPORTANT] The "Needs: Triage :mag:" label must be removed once the triage process is complete!

[!TIP] For additional guidance on how to triage this issue/PR, see the BRM Issue Triage documentation.

AlexanderSehr commented 4 months ago

Hey @danycontre , I just migrated this issue over from CARML. Please take a look and triage if still relevant :)

danycontre commented 4 months ago

@AlexanderSehr this issue is no longer relevant, AVM defualt to AllLogs now also applies to host pools diagnostic settings: image