ApplicationInsights-dotnet
ApplicationInsights-dotnet copied to clipboard
Support caching AAD Tokens
Azure now recommends that callers should cache AAD Tokens.
Caching and management of the lifespan for the AccessToken is considered the responsibility of the caller
https://github.com/Azure/azure-sdk-for-net/blob/d80851c1b8b9621ce0a72ddc1839b2fb3200e350/sdk/core/Azure.Core/src/TokenCredential.cs#LL22C13-L22C13
@TimothyMothra agreed. #2720 #2539
I'm changing it to bug rather than enhancement. Without caching this Application Insights SDK can exhaust token query limit and cause throttling by Azure Metadata Service. Which will impact all operations which rely on Managed Identities. We need to fix it. Without it this SDK cannot be used for production workloads.
@zakimaksyutov Any update on this? We started seeing this issue due to https://github.com/microsoft/ApplicationInsights-dotnet/issues/2539 and the logs in the live metrics are unreadable due to the apps logging every call to the msi endpoint.
I would really like to see this fixed, or get the Azure.Identity guys to implement caching on all TokenCredential
types.
Due to security concerns AAD authentication on AppInsights is getting more and more common.
I just spend a whole day, trying to figure out why running an application locally maxed out my CPU. While it runs fine in Azure.
And the difference was ManagedIdentityCredential
vs AzureCliCredential
.
The fix is not going to come from this repo.
Most of the TokenCredential types do support caching today. https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/identity/Azure.Identity/samples/TokenCache.md#credentials-supporting-token-caching Unfortunately AzureCliCredential is not one of them. Please leave feedback on their github repo.