-
Notifications
You must be signed in to change notification settings - Fork 25k
Add metrics unit testing #30068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add metrics unit testing #30068
Conversation
|
|
||
| ## Test metrics in ASP.NET Core apps | ||
|
|
||
| It's possible to test metrics in ASP.NET Core apps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| It's possible to test metrics in ASP.NET Core apps. | |
| It's possible to test metrics in ASP.NET Core apps with the `MetricCollector<T>`. |
?
The MetricCollector is the heart here for testing, so I'd like to see that type given in a prominent place.
| The proceeding test: | ||
|
|
||
| * Bootstraps a web app in memory with <xref:Microsoft.AspNetCore.Mvc.Testing.WebApplicationFactory%601>. `Program` in the factory's generic argument specifies the web app. | ||
| * Collects metrics values with <xref:Microsoft.Extensions.Telemetry.Testing.Metering.MetricCollector<T>>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a valid link but is breaking the build for some reason. @tdykstra ?
you could use
[`MetricCollector<T>`](/dotnet/api/microsoft.extensions.telemetry.testing.metering.metriccollector-1.-ctor?view=dotnet-plat-ext-8.0#microsoft-extensions-telemetry-testing-metering-metriccollector-1-ctor(system-diagnostics-metrics-instrument((-0))-system-timeprovider))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't a valid link -- it needed %601 instead of <T>
| // Assert | ||
| Assert.Equal("Hello World!", await response.Content.ReadAsStringAsync()); | ||
|
|
||
| await collector.WaitForMeasurementsAsync(minCount: 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the code under test does not produce the intended metric, is this just going to hang indefinitely until some higher level test timeout occurs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added WaitAsync - #30091
| * Requires a package reference to `Microsoft.Extensions.Telemetry.Testing`. | ||
| * The `MetricCollector` is created using the web app's `IMeterFactory`. This allows the collector to only report metrics values recorded by test. | ||
| * Includes the meter name, `Microsoft.AspNetCore.Hosting`, and counter name, `http.server.request.duration` to collect. | ||
| * Makes a HTTP request to the app web. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
app web -> web app?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addresses dotnet/aspnetcore#33387
Internal previews