- Notifications
You must be signed in to change notification settings - Fork845
Add memory usage metric#6586
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Add memory usage metric#6586
Uh oh!
There was an error while loading.Please reload this page.
Conversation
6989b03 tod671e88Compare This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
9779fd3 todd6b70fCompare This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
ac948f7 to120a012Compare This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
2b4b3ed to66a792dCompare66a792d toe06592fCompare This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
e06592f to244d332Compare244d332 to412064aCompare This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
...rosoft.Extensions.Diagnostics.ResourceMonitoring/Windows/WindowsContainerSnapshotProvider.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
.../Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring.Tests/Linux/AcceptanceTest.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
.../Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring.Tests/Linux/AcceptanceTest.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
.../Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring.Tests/Linux/AcceptanceTest.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
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.
Pull Request Overview
Adds a newcontainer.memory.usage metric (bytes) as an ObservableUpDownCounter across Windows and Linux, updates logging, and extends tests to validate the new instrument.
- Introduce
ContainerMemoryUsageconstant and register it in both Windows and Linux providers. - Refactor snapshot providers to expose container memory usage and separate percentage calculations, updating log messages.
- Extend unit and acceptance tests to subscribe to and assert on the new memory‐usage metric.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/.../Windows/WindowsContainerSnapshotProviderTests.cs | Added tests for combined container/process memory metrics and standalone usage test |
| test/.../Linux/LinuxUtilizationProviderTests.cs | CapturedContainerMemoryUsage in measurements, updated expected sample/retry counts |
| test/.../Linux/AcceptanceTest.cs | Subscribed to and asserted on new container memory usage gauge |
| test/.../ResourceUtilizationHealthChecks/ResourceHealthCheckExtensionsTests.cs | AddedGetCurrentProcessMemoryUsage setup for health‐check tests |
| src/Shared/Instruments/ResourceUtilizationInstruments.cs | Addedpublic const string ContainerMemoryUsage constant with XML docs |
| src/Libraries/.../Windows/WindowsSnapshotProvider.cs | Refactored memory methods, added_processMemoryLocker, new up/down counter, log rename |
| src/Libraries/.../Windows/Log.cs | AddedContainerMemoryUsageData andProcessMemoryPercentageData messages |
| src/Libraries/.../Linux/Log.cs | RenamedMemoryUsageData toMemoryPercentageData, added newMemoryUsageData |
| src/Libraries/.../Linux/LinuxUtilizationProvider.cs | Refactored memory methods, added up/down counter for usage, updated retry helper |
Comments suppressed due to low confidence (2)
src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/WindowsContainerSnapshotProvider.cs:234
- Logging while holding the _memoryLocker can lead to lock contention or unexpected delays. It’s safer to capture the values inside the lock and invoke the logger after releasing the lock to minimize the duration of the critical section.
_logger.ProcessMemoryPercentageData(processMemoryUsage, _memoryLimit, _processMemoryPercentage);src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/Log.cs:69
- The
totalMemoryparameter represents a byte count but is typed asdouble. For consistency withprocessMemoryUsageand to avoid implicit conversions, consider changingtotalMemorytoulong.
double memoryPercentage);...rosoft.Extensions.Diagnostics.ResourceMonitoring/Windows/WindowsContainerSnapshotProvider.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...rosoft.Extensions.Diagnostics.ResourceMonitoring/Windows/WindowsContainerSnapshotProvider.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
fb806a6 intodotnet:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Fixes#6587
Microsoft Reviewers:Open in CodeFlow
Adding
container.memory.usagemetric withObservableUpDownCounterinstrument type, measured in bytes.