Skip to content
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

[API Proposal]: Make the ResourceUtilization.Snapshot property public #5389

Closed
evgenyfedorov2 opened this issue Aug 28, 2024 · 1 comment · Fixed by #5392
Closed

[API Proposal]: Make the ResourceUtilization.Snapshot property public #5389

evgenyfedorov2 opened this issue Aug 28, 2024 · 1 comment · Fixed by #5392
Labels
api-approved API was approved in API review, it can be implemented api-suggestion Early API idea and discussion, it is NOT ready for implementation

Comments

@evgenyfedorov2
Copy link
Contributor

evgenyfedorov2 commented Aug 28, 2024

Background and motivation

The Snapshot property of Microsoft.Extensions.Diagnostics.ResourceMonitoring.ResourceUtilization struct is internal, but I suggest making it public, so that any implementation of the public interface IResourceUtilizationPublisher would be able to access it. This is also required by for a 1P library and partners.

API Proposal

namespace Microsoft.Extensions.Diagnostics.ResourceMonitoring;

/// <summary>
/// Captures resource usage at a given point in time.
/// </summary>
[SuppressMessage("Performance", "CA1815:Override equals and operator equals on value types", Justification = "Comparing instances is not an expected scenario")]
public readonly struct ResourceUtilization
{
    public double CpuUsedPercentage { get; }
    public double MemoryUsedPercentage { get; }
    public ulong MemoryUsedInBytes { get; }
    public SystemResources SystemResources { get; }

-   internal Snapshot Snapshot { get; } = default;
+   public Snapshot Snapshot { get; } = default;

API Usage

// a custom implementation of IResourceUtilizationPublisher:
internal class MyCounterPublisher : IResourceUtilizationPublisher
{
    ...
    public ValueTask PublishAsync(ResourceUtilization utilization, CancellationToken cancellationToken)
    {
        ...
        utilization.Snapshot // do something with it.
   }
}

Alternative Designs

No response

Risks

No response

@evgenyfedorov2 evgenyfedorov2 added untriaged api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Aug 28, 2024
@RussKie
Copy link
Member

RussKie commented Aug 29, 2024

Sounds reasonable to me. Required to unblock 1P implementations. Relates to #5385.
Approved.

@RussKie RussKie added api-approved API was approved in API review, it can be implemented and removed untriaged labels Aug 29, 2024
@RussKie RussKie closed this as completed Aug 29, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented api-suggestion Early API idea and discussion, it is NOT ready for implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants