farmer icon indicating copy to clipboard operation
farmer copied to clipboard

Question about default values in WebApp for Application Insights

Open MartinWa opened this issue 2 years ago • 4 comments

In Builders.WebApp the default values are:

"APPINSIGHTS_PROFILERFEATURE_VERSION", "1.0.0"
"APPINSIGHTS_SNAPSHOTFEATURE_VERSION", "1.0.0"
"ApplicationInsightsAgent_EXTENSION_VERSION", "~2"
"DiagnosticServices_EXTENSION_VERSION", "~3"
"InstrumentationEngine_EXTENSION_VERSION", "~1"
"SnapshotDebugger_EXTENSION_VERSION", "~1"
"XDT_MicrosoftApplicationInsights_BaseExtensions", "~1"
"XDT_MicrosoftApplicationInsights_Mode", "recommended"

Some of these settings have a performance impact according to the documentation

Looking at Terraform Web App module they are disabled:

APPINSIGHTS_PROFILERFEATURE_VERSION             = "1.0.0"
APPINSIGHTS_SNAPSHOTFEATURE_VERSION             = "1.0.0"
DiagnosticServices_EXTENSION_VERSION            = "~3"
InstrumentationEngine_EXTENSION_VERSION         = "disabled"
SnapshotDebugger_EXTENSION_VERSION              = "disabled"
XDT_MicrosoftApplicationInsights_BaseExtensions = "disabled"
XDT_MicrosoftApplicationInsights_Java           = "1"
XDT_MicrosoftApplicationInsights_Mode           = "recommended"
XDT_MicrosoftApplicationInsights_NodeJS         = "1"
XDT_MicrosoftApplicationInsights_PreemptSdk     = "disabled"

What is the reason for the default being enabled in Farmer?

MartinWa avatar Aug 26 '21 14:08 MartinWa

It was reversed engineered from what the portal creates when you create an App Insights instance and link to an App Service.

I have no clue as to what each of them do, so if you can just explain what the differences are, we could look at either switching the output or parameterising it.

isaacabraham avatar Aug 26 '21 19:08 isaacabraham

I am no expert but I think that the current default values should change. According to the documentation I would say that they should be:

  • InstrumentationEngine_EXTENSION_VERSION = "disabled"

Documentation says: "This setting has performance implications and impacts cold start/startup time."

  • XDT_MicrosoftApplicationInsights_BaseExtensions = "disabled"

Documentation says: "Performance warning: application cold start up time will be affected"

  • SnapshotDebugger_EXTENSION_VERSION = "disabled"

Does not seem to have performance implications but should be a conscious choice to enable: https://docs.microsoft.com/en-us/azure/azure-monitor/app/snapshot-debugger

  • As for DiagnosticServices_EXTENSION_VERSION it seems like it is recommended to have it turned on. This would then mean that the following should be default:
"APPINSIGHTS_PROFILERFEATURE_VERSION", "1.0.0"
"DiagnosticServices_EXTENSION_VERSION", "~3"

This also matches with the Terraform settings above. I would prefer having this as defaults, I think it is safer for production. They could perhaps also be added as parameters (but that is not as urgent).

Just a side note: According to this documentation all settings that ends with _EXTENSION_VERSION will not follow a swap between slots. This can lead to confusion if setting some of the above values and then swapping. Perhaps mention this in the documentation.

MartinWa avatar Aug 26 '21 20:08 MartinWa

@MartinWa thanks for this. My only consideration is what to do for existing Farmer users of the webapp { } builder, because this will make changes to existing templates with no code changes - something I'm always a little wary of doing. In this case though it seems like the changes are relatively minor.

Where's the documentation on the first two properties (the ones that impact cold startup time)?

isaacabraham avatar Aug 27 '21 06:08 isaacabraham

The documentation for the two first properties is at https://docs.microsoft.com/en-us/azure/azure-monitor/app/azure-web-apps?tabs=net#automate-monitoring

I have done some more digging: "ApplicationInsightsAgent_EXTENSION_VERSION", "~2" will turn on the extension for Application Insights. (as perhaps would be obvious by the name). As we are using the SDK to send telemetry it will apparently not be used:

If both agent-based monitoring and manual SDK-based instrumentation is detected, in .NET only the manual instrumentation settings will be honored, while in Java only the agent-based instrumentation will be emitting the telemetry. This is to prevent duplicate data from being sent

so it should perhaps be kept default so it covers both cases.

Also note that the APPINSIGHTS_INSTRUMENTATIONKEY has an more advanced version APPLICATIONINSIGHTS_CONNECTION_STRING that will take over if set.

MartinWa avatar Aug 27 '21 09:08 MartinWa