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]: Expose ISnapshotProvider interface and Snapshot struct #5385

Closed
evgenyfedorov2 opened this issue Aug 27, 2024 · 3 comments · Fixed by #5392
Closed

[API Proposal]: Expose ISnapshotProvider interface and Snapshot struct #5385

evgenyfedorov2 opened this issue Aug 27, 2024 · 3 comments · 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 work in progress 🚧

Comments

@evgenyfedorov2
Copy link
Contributor

Background and motivation

The Microsoft.Extensions.Diagnostics.ResourceMonitoring.ISnapshotProvider interface is implemented three times in this repo:

  1. https://github.com/dotnet/extensions/blob/main/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/WindowsContainerSnapshotProvider.cs
  2. https://github.com/dotnet/extensions/blob/main/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Windows/WindowsSnapshotProvider.cs
  3. https://github.com/dotnet/extensions/blob/main/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/Linux/LinuxUtilizationProvider.cs

which confirms the interface is useful. In addition to that, in one of Microsoft internal repos we would like to implement this interface as well. Therefore, I propose to make the ISnapshotProvider interface public, so that everyone would benefit from that. Additionally, it will require making the Snapshot struct public because the GetSnapshost() method of the ISnapshotProvider interface returns a Snapshot.

API Proposal

namespace Microsoft.Extensions.Diagnostics.ResourceMonitoring;

/// <summary>
/// An interface to be implemented by a provider that represents an underlying system and gets resources data about it.
/// </summary>

-internal interface ISnapshotProvider
+public interface ISnapshotProvider
{
    /// <summary>
    /// Gets the static values of CPU and memory limitations defined by the system.
    /// </summary>
    SystemResources Resources { get; }

    /// <summary>
    /// Get a snapshot of the resource utilization of the system.
    /// </summary>
    /// <returns>An appropriate sample.</returns>
#pragma warning disable S4049 // Properties should be preferred
    Snapshot GetSnapshot();
}
namespace Microsoft.Extensions.Diagnostics.ResourceMonitoring;

/// <summary>
/// A snapshot of CPU and memory usage taken periodically over time.
/// </summary>

-internal readonly struct Snapshot
+public readonly struct Snapshot
{
    /// <summary>
    /// Gets the total CPU time that has elapsed since startup.
    /// </summary>
    public TimeSpan TotalTimeSinceStart { get; }

    /// <summary>
    /// Gets the amount of kernel time that has elapsed since startup.
    /// </summary>
    public TimeSpan KernelTimeSinceStart { get; }

    /// <summary>
    /// Gets the amount of user time that has elapsed since startup.
    /// </summary>
    public TimeSpan UserTimeSinceStart { get; }

    /// <summary>
    /// Gets the memory usage within the system in bytes.
    /// </summary>
    public ulong MemoryUsageInBytes { get; }

    /// <summary>
    /// Initializes a new instance of the <see cref="Snapshot"/> struct.
    /// </summary>
    /// <param name="totalTimeSinceStart">The time at which the snapshot was taken.</param>
    /// <param name="kernelTimeSinceStart">The amount of kernel time that has elapsed since startup.</param>
    /// <param name="userTimeSinceStart">The amount of user time that has elapsed since startup.</param>
    /// <param name="memoryUsageInBytes">The memory usage within the system in bytes.</param>
    public Snapshot(
        TimeSpan totalTimeSinceStart,
        TimeSpan kernelTimeSinceStart,
        TimeSpan userTimeSinceStart,
        ulong memoryUsageInBytes)
    {
        _ = Throw.IfLessThan(memoryUsageInBytes, 0);
        _ = Throw.IfLessThan(kernelTimeSinceStart.Ticks, 0);
        _ = Throw.IfLessThan(userTimeSinceStart.Ticks, 0);

        TotalTimeSinceStart = totalTimeSinceStart;
        KernelTimeSinceStart = kernelTimeSinceStart;
        UserTimeSinceStart = userTimeSinceStart;
        MemoryUsageInBytes = memoryUsageInBytes;
    }

    /// <summary>
    /// Initializes a new instance of the <see cref="Snapshot"/> struct.
    /// </summary>
    /// <param name="timeProvider">The time provider.</param>
    /// <param name="kernelTimeSinceStart">The amount of kernel time that has elapsed since startup.</param>
    /// <param name="userTimeSinceStart">The amount of user time that has elapsed since startup.</param>
    /// <param name="memoryUsageInBytes">The memory usage within the system in bytes.</param>
    /// <remarks>This is a internal constructor to be used in unit tests only.</remarks>
    internal Snapshot(
        TimeProvider timeProvider,
        TimeSpan kernelTimeSinceStart,
        TimeSpan userTimeSinceStart,
        ulong memoryUsageInBytes)
    {
        _ = Throw.IfLessThan(memoryUsageInBytes, 0);
        _ = Throw.IfLessThan(kernelTimeSinceStart.Ticks, 0);
        _ = Throw.IfLessThan(userTimeSinceStart.Ticks, 0);

        TotalTimeSinceStart = TimeSpan.FromTicks(timeProvider.GetUtcNow().Ticks);
        KernelTimeSinceStart = kernelTimeSinceStart;
        UserTimeSinceStart = userTimeSinceStart;
        MemoryUsageInBytes = memoryUsageInBytes;
    }
}

API Usage

Possible to have a custom implementation of ISnapshotProvider which then can be manually registered in Dependency Injection

internal sealed class MyCustomSnapshotProvider : ISnapshotProvider
{
  ...
}

services.AddSingleton<ISnapshotProvider, MyCustomSnapshotProvider>();

and injected into the ResourceMonitorService' constructor here https://github.com/dotnet/extensions/blob/main/src/Libraries/Microsoft.Extensions.Diagnostics.ResourceMonitoring/ResourceMonitorService.cs#L55

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 27, 2024
@RussKie
Copy link
Member

RussKie commented Aug 28, 2024

This is required to support 1P partners. Looks good to me.

@joperezr @geeknoid do you have any concerns or objections?

@RussKie RussKie added waiting-on-team 👋 api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews and removed untriaged labels Aug 28, 2024
@geeknoid
Copy link
Member

This looks good. We had debated making this public in the first place, and decided to wait until some use materialized.

@RussKie RussKie added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Aug 29, 2024
@RussKie
Copy link
Member

RussKie commented Aug 29, 2024

Marking as "approved". @evgenyfedorov2 feel free to make necessary changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 work in progress 🚧
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@RussKie @geeknoid @evgenyfedorov2 and others