Add cloud symweb support #4848
Add cloud symweb support #4848
Conversation
Add back -mi (add symweb symbol server), -nocache and -interface options to setsymbolserver command. Add back --internal-server support to dotnet-symbol. Clean up the HttpSymbolStore.cs - fix HttpResponse not being properly disposed. Removed the separate authenticated client instance/var. Fix dotnet-symbol for some heap dumps. Some didn't have coreclr.dll's debug directory contents. Catch error and continue downloading files.
src/Microsoft.Diagnostics.DebugServices.Implementation/SymbolService.cs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| private readonly List<string> InputFilePaths = new(); | ||
| private readonly List<string> CacheDirectories = new(); | ||
| private readonly List<ServerInfo> SymbolServers = new(); | ||
| private TokenCredential TokenCredential = new DefaultAzureCredential(includeInteractiveCredentials: true); |
There was a problem hiding this comment.
why is symbol interactive but not dotnet/dump?
There was a problem hiding this comment.
dotnet-dump has the option to be interactive with setsymbolserver --interactive. Since we do have customers that script dotnet-dump, I made it opt-in. With dotnet-symbol, I assume (maybe incorrectly) that it will always be interactive.
There was a problem hiding this comment.
Should I make dotnet-dump interactive by default and the script users disable it?
There was a problem hiding this comment.
hmm On linux I expect a good share to be headless and that's going to be the main analyze customer base for this. Thsi sounds good.
There was a problem hiding this comment.
You are ok with leaving this alone: dotnet-symbol is interactive by default and dotnet-dump isn't.
| { | ||
| try | ||
| { | ||
| AccessToken accessToken = await TokenCredential.GetTokenAsync(new TokenRequestContext(["api://af9e1c69-e5e9-4331-8cc5-cdf93d57bafa/.default"]), token).ConfigureAwait(false); |
There was a problem hiding this comment.
Am I right in thinking this gets called on every symbol server request? If so, it can be unnecessarily expensive. Tokens are typically valid for several hours (the expiration timestamp is part of the AccessToken). It's wise to cache the token in memory and only call the underlying TokenCredential if the cached value is close to expiration (within ~5 minutes).
Some implementations (e.g. the Managed Identity token endpoint for Azure VMs and Azure App Service) -- but not all -- have their own token caches which makes subsequent calls cheap, but they still incur a network call (albeit to a localhost service).
There was a problem hiding this comment.
This might have been my fault - I thought there was caching in Azure.Identity like MSAL with an application does (thought it was since it's backed by the library). The docs at https://learn.microsoft.com/en-us/dotnet/api/azure.identity.defaultazurecredential.gettokenasync?view=azure-dotnet specify:
Acquired tokens are cached by the credential instance. Token lifetime and refreshing is handled automatically. Where possible, reuse credential instances to optimize cache effectiveness.
There was a problem hiding this comment.
This is why Lee in CLRMD cached the token until it was 2 minutes before the expiration. Should I add similar logic?
There was a problem hiding this comment.
The docs say it's cached - If that's not the case I am surprised (I know CLI creds are). I am not sure if browser creds are. DateTime has its own issues with time skew, but looking at the implementation of Azure.Identity this might be important given the cascading nature of creds and the fact some of them start a process.
There was a problem hiding this comment.
So are we ok? Is there something you want me to do to the new token caching code?
There was a problem hiding this comment.
And looks like MI token caching isn't supported. I am not too concerned with caches and IAM revocations since the tokens are so short lived for other types. This library isn't something I expect to be used much in the MI context though so it's ok.
|
So everything is reviewed and approved? |
Add back -mi (add symweb symbol server), -nocache and -interface options to setsymbolserver command.
Add back --internal-server support to dotnet-symbol.
Clean up the HttpSymbolStore.cs - fix HttpResponse not being properly disposed. Removed the separate authenticated client instance/var.
Fix dotnet-symbol for some heap dumps. Some didn't have coreclr.dll's debug directory contents. Catch error and continue downloading files.