Bug Report / Feature Request - Lack of consistency in policy initiative Deploy Diagnostic Settings to Azure Services

[stalejohnsen]

Describe the bug

The policy initiative 'Deploy Diagnostic Settings to Azure Services' is an important part of ALZ and the policy-driven governance concept but the current state is lacking some consistency in how it deploys diagnostic settings and more specifically the value for AllMetrics . The custom policy initiative consists of 52 custom policies and 17 built-in policies where the custom policies set AllMetrics to True. Example for VPN gateway:

"metricsEnabled": {
        "defaultValue": "True",
}

On the other hand most of the built-in policies for setting diagnostic settings for example Key Vault have AllMetrics set to False:

"metricsEnabled": {
        "defaultValue": "False",
}

An expection to this is the builtin policy for Public IP which is hardcoded to True in the assignment, at least this is true for ALZ Bicep (I have not checked for ARM or ALZ terraform). https://github.com/Azure/ALZ-Bicep/blob/main/infra-as-code/bicep/modules/policy/definitions/lib/policy_set_definitions/policy_set_definition_es_Deploy-Diagnostics-LogAnalytics.json#L1619

Is there a reason behind this inconsistency?

Feature Request I added this issue as a combination of bug report and feature request because it would be beneficial to have the flexibility to override the AllMetrics according to individual customer requirements. In the current version of the diagnostic settings custom initiative AllMetrics is not a parameter. The request for AllMetrics flexibility is also backed by this statement:

You might also not want to collect platform metrics from Azure resources because this data is already being collected in Metrics. Only configure your diagnostic data to collect metrics if you need metric data in the workspace for more complex analysis with log queries. https://learn.microsoft.com/en-us/azure/azure-monitor/essentials/diagnostic-settings?WT.mc_id=Portal-Microsoft_Azure_Monitoring&tabs=portal#controlling-costs

Steps to reproduce

1. Deploy ALZ RI to your tenant
2. Deploy a service that is covered by a custom policy for diagnostic setting and explore the AllMetrics setting.
3. Deploy a service that is covered by a built in policy for diagnostic setting and explore the AllMetrics setting.
4. Compare results and confirm inconsistency.

[jtracey93]

Thanks for raising @stalejohnsen.

To answer the ALZ Bicep hardcoding to true question, this is because we pull this value from this repo here: https://github.com/Azure/Enterprise-Scale/blob/72bd2b8756788430b100d01d82d67906471a3d1c/src/resources/Microsoft.Authorization/policySetDefinitions/Deploy-Diagnostics-LogAnalytics.json#L1605-L1623

This means it is also the same in the TF module, as the source of truth is this repo 👍

Looping in @paulgrimley & @Springstone for their thoughts as to whether we investigate and include this work in the current policy-refresh work or whether we do as part of the diagnostic settings migration to more built-in. I propose the latter option.

Thoughts?

[Springstone]

@jtracey93 @stalejohnsen I agree that it would make sense to address this in the overall diagnostic settings migration stream, and to include this in the Q2CY23 policy refresh cycle as this requires quite a bit of validation and testing. Added to the review spreadsheet.

[jtracey93]

@paulgrimley @Springstone can we link this to the ADO WIT using the AB# comment feature please

[paulgrimley]

Just created and captured AB#27275

[Springstone]

Closing this as we've deprecated all our diagnostic settings policies and shifted to the PG owned initiative to do the same. Please review https://aka.ms/alz/whatsnew for details.