Add time metrics to AuthenticationResult metadata #2581
Conversation
bgavrilMS
commented
Apr 26, 2021
bgavrilMS
commented
- time in total
- time spent in HTTP
- time spent in token cache callbacks
bgavrilMS
force-pushed
the
bogavril/spike-timein
branch
from
c7840fb
to
3c1c598
Compare
There was a problem hiding this comment.
Some thoughts + small nits below (but most are shared in our offline discussion). Good to see the data results and comparison. BenchmarkDotNet does seem to offer custom columns that could maybe show the duration metrics; iteration/global setup methods can be used to set up the CCA and cache; and WithIterationCount/WithIterationTime can modify how many times the iterations are called - (although that seems like more work and you already have working test).
src/client/Microsoft.Identity.Client/AuthenticationResultMetadata.cs
Outdated
Show resolved
Hide resolved
|
|
||
| /// <summary> | ||
| /// Time (in ms) MSAL spent in reading and writing to the token cache, i.e. in the OnBeforeAccess, OnAfterAccess etc. callbacks. | ||
| /// Does not include internal MSAL logic for searching through the cache once loaded. |
There was a problem hiding this comment.
Should this be included to measure the whole cache operation? Otherwise, only measuring accessing serialization cache, the property name might be misleading then.
| Stopwatch sw = new Stopwatch(); | ||
| sw.Start(); |
There was a problem hiding this comment.
Nit: can be combined into Stopwatch sw = Stopwatch.StartNew();
| @@ -12,6 +12,8 @@ namespace Microsoft.Identity.Client.Http | |||
| { | |||
| internal interface IHttpManager | |||
| { | |||
| long LastRequestTime { get; } | |||
| // execute won't start here because IEnumerable is lazy | ||
| IEnumerable<Task<AuthenticationResult>> tasks = accounts.Select(acc => | ||
| pca.AcquireTokenSilent(TestConstants.s_scope, acc).ExecuteAsync()); | ||
| //return cca.AcquireTokenForClient(new[] { "scope" }) |
There was a problem hiding this comment.
Uncomment and use s_cca here?
| @@ -234,6 +104,8 @@ private void ConfigureCacheSerialization(IPublicClientApplication pca) | |||
| /// </summary> | |||
| internal class ParallelRequestMockHanler : IHttpManager | |||
There was a problem hiding this comment.
ParallelRequestMockHandler
| /// - provides a standard reponse for discovery calls | ||
| /// - responds with valid tokens based on a naming convention (uid = "uid" + rtSecret, upn = "user_" + rtSecret) | ||
| /// </summary> | ||
| internal class ParallelRequestMockHanler : IHttpManager |
There was a problem hiding this comment.
typo: ParallelRequestMockHandler
| [Route("[controller]")] | ||
| public class StaticDictionaryController : ControllerBase | ||
| { | ||
| private readonly ILogger<SingletonController> _logger; |
There was a problem hiding this comment.
private readonly ILogger<StaticDictionaryController> _logger;
