Merging pmaytak/perf #636
Merged
Merging pmaytak/perf #636
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sts with a previously acquired token.
…tServices. Minor TestRunner refactor.
…ve operations. Added loop counter to test runner.
* Perf project. * Add TestController. * Added Perf.Client project that creates and HttpClient and sends requests with a previously acquired token. * Updated to not using the token array. Small logging fixes. * Mostly updated to use AcquireTokenSilent instead of Tokens array. * Added a duration check to the inner loop. * Disable JWT Handler debug messages. Add Accept header. * Added empty action to weather controller. * serialize msal cache + add timers * Added three controllers with different dependencies to IntegrationTestServices. Minor TestRunner refactor. * Update Kesterl Url. * add msal logs * updates to perf testing w/app services * use distributed cache * fix constants in test * fix controller action * remove memory cache updates & fix test * try to fix tests * adding in timers around msal cache lookup * update testrunner part 5 * one more * Jmprieur/perf with scalable Public Client Application cache (#614) * Adding a scalable token cache that writes each entry in a different file * Moving the PCA creation in the loop to avoid the cache to grow (MSAL perf bug) * Adding comments Co-authored-by: jennyf19 <jeferrie@microsoft.com> * Fixing a couple of issues: - the AddMemoryCache override that takes a delegate does not accept null (fix done by Jenny lost in translation) - In the test runner allows to reduce the number of users Simplifying the matrix: - the UI test does not need to be on several frameworks (as it's not what is being tested) - the Integration test either (as the web API is only netcoreapp3.1) * Fixing the build Co-authored-by: pmaytak <34331512+pmaytak@users.noreply.github.com> Co-authored-by: Jean-Marc Prieur <jmprieur@microsoft.com>
* changes to test runner * fix user start value + add app insights logging * Update MicrosoftIdentityAppCallingWebApiAuthenticationBuilder.cs Co-authored-by: Jean-Marc Prieur <jmprieur@microsoft.com>
…ource with the new files. Added counters to App Insights telemetry.
* changes to test runner * fix user start value + add app insights logging * In memory db for test runner, and resilience in the test run itself, ensuring the runner continues to run until canceled by esc, or ctrl + c or ctrl + x * remove copy * update runner to not stop * Fix issue updating cache after refresh from AAD. Also update title to reflect summary of status * Added persistence of cache after each loop and loading of cache on start-up. This minimizes disk operations and enables restarting the console with-out having to fetch tokens again if those are still valid * Enable persistence if token was not obtained from cache * Testing Parallel.For and adding locks around file persistence. Cache and logging both are not threadsafe atm and will need more work. * Parrallel.For and Some persistence. Including a small pause between loops. * limiting persistence * removing parralel * Minor clean-up. Removing parrallel'ism. Requires more work and refactorings to make safe. Still not great code but better than previous iterations. * Update output display for test runner Co-authored-by: Jennyf19 <jeferrie@microsoft.com> Co-authored-by: Henrik Metzger <henrikm@microsoft.com>
|
will wait to review when integration tests are fixed. |
|
@jennyf19 Integration test is fixed. I moved perf related code from IntegrationTestService to new project PerformanceTestService. |
jennyf19
reviewed
Oct 1, 2020
tests/IntegrationTests/IntegrationTestService/IntegrationTestService.csproj
Show resolved
Hide resolved
jennyf19
reviewed
Oct 1, 2020
jennyf19
reviewed
Oct 1, 2020
jennyf19
reviewed
Oct 1, 2020
tests/Microsoft.Identity.Web.Test.Integration/Microsoft.Identity.Web.Test.Integration.csproj
Show resolved
Hide resolved
jennyf19
reviewed
Oct 1, 2020
tests/PerformanceTests/Microsoft.Identity.Web.Perf.Benchmark/TokenAcquisitionTests.cs
Show resolved
Hide resolved
jennyf19
reviewed
Oct 1, 2020
tests/PerformanceTests/Microsoft.Identity.Web.Perf.Client/appsettings.json
Show resolved
Hide resolved
jennyf19
reviewed
Oct 1, 2020
tests/PerformanceTests/Microsoft.Identity.Web.Perf.Client/appsettings.json
Show resolved
Hide resolved
jennyf19
reviewed
Oct 1, 2020
tests/PerformanceTests/Microsoft.Identity.Web.Perf.Client/ScalableTokenCacheHelper.cs
Show resolved
Hide resolved
jennyf19
reviewed
Oct 1, 2020
tests/PerformanceTests/Microsoft.Identity.Web.Perf.Client/TestRunner.cs
Outdated
Show resolved
Hide resolved
|
there were some changes made to the msalInMemoryTokenCacheProvider & one other class in my previous branch, but i'll go back and add those in after you merge. In reply to: 702424303 [](ancestors = 702424303) |
jmprieur
reviewed
Oct 2, 2020
| @@ -0,0 +1,40 @@ | |||
| // Copyright (c) Microsoft Corporation. All rights reserved. | |||
There was a problem hiding this comment.
I'll verify if we need it.
tests/PerformanceTests/Microsoft.Identity.Web.Perf.Client/appsettings.json
Show resolved
Hide resolved
| [ApiController] | ||
| [Authorize] | ||
| [Route("SecurePage")] | ||
| public class WeatherForecastController : ControllerBase |
There was a problem hiding this comment.
We probably want to rename the file and the class tio SecurePageController ?
There was a problem hiding this comment.
Yea, I'll refactor the naming.
| /// <summary> | ||
| /// Adds benchmarking counters on top of <see cref="MsalDistributedTokenCacheAdapter"/>. | ||
| /// </summary> | ||
| public class BenchmarkMsalDistributedTokenCacheAdapter : MsalDistributedTokenCacheAdapter |
There was a problem hiding this comment.
Yes, it's used in place of normal distributed cache class.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merges only test related changes. Merge perf client and benchmark project. Includes Henrik's test runner changes.
Note: Will fix integration tests after merge.
#88