Skip to content

Conversation

@amadeuszl
Copy link
Contributor

@amadeuszl amadeuszl commented Aug 26, 2025

Fix #6496

Microsoft Reviewers: Open in CodeFlow

@amadeuszl amadeuszl marked this pull request as ready for review November 6, 2025 12:15
@amadeuszl amadeuszl requested a review from a team as a code owner November 6, 2025 12:15
Copilot AI review requested due to automatic review settings November 6, 2025 12:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors resource monitoring to extract quota calculation logic into dedicated ResourceQuotaProvider implementations and adds Kubernetes support for resource monitoring. The changes separate concerns by moving resource limit/request determination out of snapshot providers into reusable quota providers.

Key changes:

  • Introduces ResourceQuota model and ResourceQuotaProvider abstract base class for platform-agnostic quota retrieval
  • Creates WindowsContainerResourceQuotaProvider and LinuxResourceQuotaProvider implementations for existing platforms
  • Adds new Microsoft.Extensions.Diagnostics.ResourceMonitoring.Kubernetes library with Kubernetes-specific quota provider that reads from environment variables

Reviewed Changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
ResourceQuota.cs New model class defining CPU and memory quota properties (max and baseline)
ResourceQuotaProvider.cs New abstract base class for quota provider implementations
WindowsContainerResourceQuotaProvider.cs Extracts Windows quota logic from WindowsContainerSnapshotProvider
WindowsContainerSnapshotProvider.cs Refactored to use ResourceQuotaProvider dependency
LinuxResourceQuotaProvider.cs Extracts Linux quota logic from LinuxUtilizationProvider
LinuxUtilizationProvider.cs Refactored to use ResourceQuotaProvider dependency
ILinuxUtilizationParser.cs Adds GetMinMemoryInBytes() method for reading memory requests
LinuxUtilizationParserCgroupV1.cs Stub implementation returning 0 (no request support in v1)
LinuxUtilizationParserCgroupV2.cs Implements GetMinMemoryInBytes() reading memory.min/memory.low files
KubernetesMetadata.cs Reads Kubernetes resource limits/requests from environment variables
KubernetesResourceQuotaProvider.cs Kubernetes-specific quota provider converting millicores to cores
KubernetesResourceQuotaServiceCollectionExtensions.cs DI registration for Kubernetes resource monitoring
ResourceMonitoringServiceCollectionExtensions.cs Registers appropriate ResourceQuotaProvider for each platform
Test files Updates all tests to create ResourceQuotaProvider instances; adds comprehensive Kubernetes test coverage

Comment on lines +9 to +10
/// Maximum values define the upper limits of resource usage, while baseline values specify
/// the minimum assured resource allocations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does it translate into K8s request and limits?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

max are limits, and baselines are requests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@evgenyfedorov2 evgenyfedorov2 Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think it makes sense to reflect that in XML documentation here?

</PropertyGroup>

<PropertyGroup>
<Stage>normal</Stage>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be dev stage initially. After one release (= one month) we can change it to normal. And in this case we don't have to bother with EnablePackageValidation because it is not enabled for dev packages:

<PackageValidationBaselineVersion Condition="'$(Stage)' == 'normal' and '$(PackageValidationBaselineVersion)' == ''">9.10.0</PackageValidationBaselineVersion>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me verify that. I would prefer to release it already in December as normal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[API Proposal]: Kubernetes specialized Resource Monitoring

2 participants