HodorNV / ALOps

ALOps
55 stars 24 forks source link

[FEATURE REQUEST] Support setting `applicationInsightsConnectionString` config #740

Open nres0001 opened 3 months ago

nres0001 commented 3 months ago

Is your feature request related to a problem? Please describe.

The applicationInsightsKey property in the app.json file has been deprecated since runtime version 7.2. Using the applicationinsightskey property on the ALOpsAppCompiler@2 task results in the following warning:

warning AL0667: 'applicationInsightsKey' is being deprecated in the versions: '7.2' or greater. This property is being deprecated in favor of 'applicationInsightsConnectionString'. This warning will become an error in a future release.

In addition, AppSourceCop throws a warning if applicationInsightsConnectionString is not set:

warning AS0092: The app.json file must specify an Azure Application Insights resource with the property 'applicationInsightsConnectionString' for monitoring operations related to this extension. See https://learn.microsoft.com/dynamics365/business-central/dev-itpro/administration/telemetry-overview for additional information.

Describe the solution you'd like

The ALOpsAppCompiler(@3?) task should have an applicationinsightsconnectionstring property to set the corresponding app.json property.

Describe alternatives you've considered

One option is to check in the property on the app.json in the source code repository. However, this will result in developer telemetry being sent to the Application Insights instance being used for production if the app.json is not modified after checkout by developers. This alternative is too error prone to consider implementing.

Another option is to modify the app.json file in the pipeline prior to running the ALOpsAppCompiler@2 task. However, all other properties of the app.json file that we've needed to modify at build time have properties in the compile task to set them, so we haven't implemented this yet.

Additional context

This Microsoft docs page indicates the deadline to switch to connection strings for data ingestion is 31 March 2025:

https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-application-insights-for-extensions

Transition to using connection strings for data ingestion in Azure Application Insights by 31 March 2025. On 31 March 2025, technical support for instrumentation key–based global ingestion in the Azure Application Insights feature of Azure Monitor will end. After that date, your Azure Application Insights resources will continue to receive data, but Microsoft no longer provide updates or customer support for instrumentation key–based global ingestion.

nres0001 commented 3 months ago

For now I've implemented a workaround to add or modify the applicationInsightsConnectionString property on the app.json using an Azure DevOps pipeline PowerShell task. This task will read from the pipeline variable appInsightsConnectionString for the connection string.

- task: PowerShell@2
  displayName: 'Set Application Insights Connection String'
  inputs:
    targetType: 'inline'
    script: |
      $AppJsonPath = '$(Build.SourcesDirectory)/path/to/app.json'
      $AppJson = Get-Content -Path $AppJsonPath | ConvertFrom-Json

      if (Get-Member -InputObject $AppJson -Name applicationInsightsConnectionString -MemberType NoteProperty) {
          $AppJson.applicationInsightsConnectionString = $env:APPINSIGHTSCONNECTIONSTRING
      }
      else {
          Add-Member -InputObject $AppJson -NotePropertyName applicationInsightsConnectionString -NotePropertyValue $env:APPINSIGHTSCONNECTIONSTRING
      }

      $AppJson | ConvertTo-Json | Set-Content -Path $AppJsonPath
Arthurvdv commented 3 months ago

We use the applicationinsightskey parameter on the ALOpsAppCompiler@2, which also supports an applicationInsightsConnectionString as input.

  - task: ALOpsAppCompiler@2
    inputs:
      applicationinsightskey: "InstrumentationKey=c2027aab-fak3-4108-bdb9-e0a44f01b6e6;IngestionEndpoint=https://westeurope-0.in.applicationinsights.azure.com/;LiveEndpoint=https://westeurope.livediagnostics.monitor.azure.com/"

This will then create an applicationInsightsConnectionString property in the app.json. Probably works also on the ALOpsAppCompiler@1, but I didn't test this, so I'm not 100% sure.

nres0001 commented 3 months ago

Just tested ALOpsAppCompiler@2 and it has the behavior you've described. Not what I would have expected given the documentation, but at least I won't have to use the workaround. Thanks for your help!

waldo1001 commented 2 months ago

Fair point. I'll update documentation. We just wanted to keep it simple (basically make sure you could use both).