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

Let consumers of MemoryCache access metrics #50406

Closed
2 tasks
Tracked by #64015
maryamariyan opened this issue Mar 30, 2021 · 43 comments · Fixed by #66479
Closed
2 tasks
Tracked by #64015

Let consumers of MemoryCache access metrics #50406

maryamariyan opened this issue Mar 30, 2021 · 43 comments · Fixed by #66479
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Extensions-Caching feature-request
Milestone

Comments

@maryamariyan
Copy link
Member

maryamariyan commented Mar 30, 2021

Background and Motivation

We want to add an API to expose memory cache statistics to allow consumers of caching to write their own EventSource/EventCounters and their own metrics.

Usage sample

Check out this gist: https://gist.github.com/maryamariyan/487816f724993c88a39fa29fe397aede

Approved API

Video

The feature owners have agreed that they're willing to split the reference assembly to try out a Default Interface Method.

We also changed the statistics type to a class (partially from future expansion concerns).

    public partial class MemoryCache
    {
        public MemoryCacheStatistics? GetCurrentStatistics() { throw null; }
    }

    public partial interface IMemoryCache
    {
#if NET_7_OR_GREATER
        public MemoryCacheStatistics? GetCurrentStatistics() => null;
#endif
    }

    public partial class MemoryCacheStatistics
    {
        public MemoryCacheStatistics() {}
        
        public long TotalMisses { get; init; } 
        public long TotalHits { get; init; }
        public long CurrentEntryCount { get; init; }
        public long? CurrentEstimatedSize { get; init; }
    }
Original Description ## API suggestion
    public static partial class MemoryCacheExtensions
    {
        public static Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics? GetCurrentStatistics(
            this Microsoft.Extensions.Caching.Memory.MemoryCache memoryCache) { throw null; }
    }

    public partial struct MemoryCacheStatistics
    {
        public long CacheRequests { get; set; } 
        public long CacheHits { get; set; }
        public long CacheCount { get; set; }
        // public long EstimatedSize { get; set; } // consider adding later
    }

In contrast, System.Runtime.Caching already presented the following counters:

- Cache Total Entries / shows the total number of entries.
- Cache Total Hits / shows the number of times something was retrieved from the cache.
- Cache Total Misses / shows the number of times something was attempted to be retrieved from the cache but did not exist.
- Cache Total Hit Ratio / shows a ratio of the number of items found in the cache against the number of accesses.
- Cache Total Turnover Rate / is the number of items added to or removed from the cache per second.

Usage Examples

  • Using metrics name Microsoft.Extensions.Caching.Memory:

We would have metrics built-in using the proposed API. But developers would also be able to add their own event counters, using the proposed API.

As a developer, I may end up using GetCurrentStatistics in a callback passed down to a metrics API or an event counter.

We want MemoryCacheStatistics to help developer custom build their own metrics such as:

cacheRequestsPerSecond = (cacheRequestsAtTime2 - cacheRequestsAtTime1) / seconds_between_calls

or CacheHitRatio as CacheHits/CacheRequests.


image

Link to prototype here, does not use the API but shows how we could add built-in metrics.


Sample usage/screenshot for event counter:

    [EventSource(Name = "Microsoft-Extensions-Caching-Memory")]
    internal sealed class CachingEventSource : EventSource
    {
/// ...
        protected override void OnEventCommand(EventCommandEventArgs command)
        {
            if (command.Command == EventCommand.Enable)
            {
                if (_cacheHitsCounter == null)
                {
                    _cacheHitsCounter = new PollingCounter("cache-hits", this, () =>
                        _memoryCache.GetCurrentStatistics().CacheHits)
                    {
                        DisplayName = "Cache hits",
                    };
                }
            }
        }
    }
image

Scope for this issue is:

  • Add the proposed API
  • Present code snippets that show how to use this API to add either event counters or System.Diagnostics.Metrics

Alternative Designs

Presented here, we'd be implementing EventSource ourselves in the assemblies.

Risks

The design assumes the metrics are cheap enough that we could track them all the time. If that is not the case then we'd want some APIs that turn the tracking on or subscribes.

TBA.

cc: @noahfalk

@maryamariyan maryamariyan added api-suggestion Early API idea and discussion, it is NOT ready for implementation feature-request area-Extensions-Caching labels Mar 30, 2021
@ghost
Copy link

ghost commented Mar 30, 2021

Tagging subscribers to this area: @eerhardt, @maryamariyan, @michaelgsharp
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

TBA.

API suggestion

GetCurrentStatistics would be able to expose event counter information such as cache-hits, cache-misses, etc.

class MemoryCache
{
    MemoryCacheStatistics GetCurrentStatistics() { ... }
    ...
}
struct MemoryCacheStatistics
{
    long ObjectsStored { get; }
    long CacheRequests { get; }
    long CacheHits { get; }
    long CacheCount { get; }
    long EstimatedSize { get; }
    ... // any other counters we also need
}

TODO:

  • To come up with the exact set of counters, we could defer back at what metrics existed in the past.

Usage Examples

TBA.

Alternative Designs

Presented here, we'd be implementing EventSource ourselves in the assemblies.

Risks

The design assumes the counters are cheap enough that we could track them all the time. If that is not the case then we'd want some APIs that turn the tracking on or subscribes.

TBA.

cc: @noahfalk

Author: maryamariyan
Assignees: -
Labels:

api-suggestion, area-Extensions-Caching, feature request

Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Mar 30, 2021
@davidfowl
Copy link
Member

I like this design better. How would this work with the fact that the cache is an interface though...

@noahfalk
Copy link
Member

I would retitle this as 'let consumers of MemoryCache access metrics'. We don't need to presume they will create an EventSource/EventCounters (and I am guessing they won't)

How would this work with the fact that the cache is an interface though...

Make a time machine and stop making everything interfaces : D
Alternately...

  1. Default interface method (I feel like there was some reason we were reticent to use DIM in BCL APIs but I don't recall what it was. Maybe I am remembering wrong)
  2. COM QI style
IMemoryCache cache = ...
if(cache is IMemoryCacheStatistics stats)
{
    MemoryCacheStatisticsSnapshot snapshot = stats.GetCurrentStatistics();
}
else
{
    // not supported on older builds, user has to deal with it or upgrade to a newer version
}

@davidfowl
Copy link
Member

OR, we can use default interface members on .NET Core only...😄.

@stephentoub
Copy link
Member

Make a time machine and stop making everything interfaces : D

+1 😄

@maryamariyan maryamariyan changed the title Allow consumers of caching to write their own EventSource/EventCounters Let consumers of MemoryCache access metrics Mar 30, 2021
@maryamariyan
Copy link
Member Author

How would this work with the fact that the cache is an interface though...

Updated to:

    public class MemoryCacheExtensions
    {
        public static MemoryCacheStatistics GetCurrentStatistics(this IMemoryCache) { ... }
    }

@cemheren
Copy link

I'm not familiar with the potential implementation, but we could consider hinting or enforcing collection interval for these metrics. For example if the stat for ObjectsStored is really expensive to calculate or only updated every 30 seconds there is no point the consumer fetching the statistics more frequently right? In fact they may incur unexpected perf hits doing so. This proposal may hide potential complexity of getting these stats. This is a non-issue of all the stats are cheap to calculate or constantly kept up to date.

@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Apr 4, 2021
@maryamariyan maryamariyan added this to the Future milestone Apr 4, 2021
@Naidile-P-N
Copy link

Do we have provision to check the space consumed by each key in the cache ?

@nadyalobalzo
Copy link

Our team uses memory cache for various scenarios. One of the scenarios is to use several layers of caching to support request routing. Memory cache is used to capture the most up to date routing information. We need to be able to understand the size of the cache in terms of object count vs its usefulness. We also need to be able to monitor memory usage. Having a directional estimate for memory usage is better than not having any estimate and thus relying on available memory or object count to project what the cache might be taking up. We emit some of these metrics by hand currently.

Here is the list of helpful metrics in the order of priority as we see it:

Number of:

  1. read requests
  2. read hits
  3. cache records
  4. estimate of the total cache size in memory
  5. write requests
  6. trim events
    a) nice to have: trim reason as a dimension, e.g. expired or evicted.
    b) nice to have: number of trimmed records
  7. remove requests
    a) nice to have: remove hits (when an item to remove was found in the cache)

@maryamariyan
Copy link
Member Author

maryamariyan commented Feb 13, 2022

Met with @sebastienros to review this API. Here's some new observations:

Observation 1: API for IDistributedCache

Looking at ref/Microsoft.Extensions.Caching.Abstractions.cs and ref/Microsoft.Extensions.Caching.Memory.cs and the APIs exposed, it doesn't look like IMemoryCache is available through IDistributedCache. But looking at the implementation of IDistributedCache in MemoryCacheDistributed:

public class MemoryDistributedCache : IDistributedCache
{
private readonly IMemoryCache _memCache;

It looks like IDistributedCache uses an IMemoryCache but it is not exposed. So for developer scenarios using IDistributedCache and wanting access to event counter information, we'd probably need another extension method taking IDistributedCache:

public static class MemoryCacheExtensions
{
+    public static MemoryCacheStatistics GetCurrentStatistics(this IDistributedCache) { ... }
    public static MemoryCacheStatistics GetCurrentStatistics(this IMemoryCache) { ... }
}

To inspect a sample usage, I found this API AddDistributedMemoryCache, tracing it into this test usage:

[Fact]
public void AddDistributedMemoryCache_DoesntConflictWithMemoryCache()
{
// Arrange
var services = new ServiceCollection();
services.AddDistributedMemoryCache();
services.AddMemoryCache();
var key = "key";
var memoryValue = "123";
var distributedValue = new byte[] { 1, 2, 3 };
// Act
var serviceProvider = services.BuildServiceProvider();
var distributedCache = serviceProvider.GetService<IDistributedCache>();
var memoryCache = serviceProvider.GetService<IMemoryCache>();
memoryCache.Set(key, memoryValue);
distributedCache.Set(key, distributedValue);
// Assert
Assert.Equal(memoryValue, memoryCache.Get(key));
Assert.Equal(distributedValue, distributedCache.Get(key));
}

It doesn't seem like there would be a way to access memory cache event counter information for the IDistributedCache.

One concern with exposing this API on the interface is we have no guarantee that (a) IDistributedCache is implemented with a memory cache or not. It might make more sense to expose this API for MemoryCache instead where we are guaranteed to be computing these caching metrics for.

public static class MemoryCacheExtensions
{
+    public static MemoryCacheStatistics GetCurrentStatistics(this MemoryCache memoryCache) { ... }
}

Observation 2: In what ways would the API be used?

What would the developer do using the new APIs? For cased where the developer uses the new APIs to create an event counter with them, then they would need to create a timer to extract statistics and show as event counters, rather than querying them using an event pipe. To facilitate with this use case maybe it's worth also adding the event counters, and not just the proposed API.

TODO:

  • Experiment how we could use this new API to produce .NET metrics presented here
  • Experiment how it would look like for developer to use this new API to add event counters with.

Observation 3: Compare approaches

Using event counters is a way for consumers to access the metrics which is the alternative approach presented in #36560).

Since there might be multiple instances of the memory cache, using the event counters approach might mean we would have to aggregate statistics (such as cache hit, cache miss) for all memory caches being used. But with the proposed APIs, the memory cache statistics could be retrieved per memory cache.

/cc @davidfowl

@maryamariyan
Copy link
Member Author

maryamariyan commented Feb 17, 2022

@nadyalobalzo I wanted to learn more about your use cases.

  • For these statistics, do you end up needing to react to the metrics information in proc or out of process?
  • How do you currently monitor the metric information?

UPDATE:

Reached out to them offline and learned that they use multiple memory cache instances in their apps so it would be more useful for them to use metrics APIs instead which allows for using dimensions.

@maryamariyan maryamariyan added the api-ready-for-review API is ready for review, it is NOT ready for implementation label Feb 24, 2022
@maryamariyan maryamariyan added blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 15, 2022
@sebastienros
Copy link
Member

So AddMemoryCache would also register the singleton instance as IMemoryCacheWithStats?

RegisterTransient<IMemoryCacheWithStats>(sp => sp.GetService<IMemoryCache>() as IMemoryCacheWithStats);

@davidfowl
Copy link
Member

I think that would be logical yes.

@bartonjs
Copy link
Member

bartonjs commented Mar 15, 2022

Video

The feature owners have agreed that they're willing to split the reference assembly to try out a Default Interface Method.

We also changed the statistics type to a class (partially from future expansion concerns).

    public static partial class MemoryCache
    {
        public MemoryCacheStatistics? GetCurrentStatistics() { throw null; }
    }

    public partial interface IMemoryCache
    {
#if NET_7_OR_GREATER
        public MemoryCacheStatistics? GetCurrentStatistics() => null;
#endif
    }

    public partial class MemoryCacheStatistics
    {
        public MemoryCacheStatistics() {}
        
        public long TotalMisses { get; init; } 
        public long TotalHits { get; init; }
        public long CurrentEntryCount { get; init; }
        public long? CurrentEstimatedSize { get; init; }
    }

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 15, 2022
@maryamariyan
Copy link
Member Author

maryamariyan commented Mar 21, 2022

Making an API revision request to support opt-in capability.

    public partial class MemoryCacheOptions : Microsoft.Extensions.Options.IOptions<Microsoft.Extensions.Caching.Memory.MemoryCacheOptions>
    {
+        public bool TrackStatistics { get { throw null; } set { } }
    }

For more information refer to #66479 (comment)

@maryamariyan maryamariyan added blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-approved API was approved in API review, it can be implemented labels Mar 21, 2022
@terrajobst
Copy link
Member

terrajobst commented Mar 22, 2022

Video

  • Looks good as proposed
namespace Microsoft.Extensions.Caching.Memory;

public partial class MemoryCacheOptions : IOptions<MemoryCacheOptions>
{
    public bool TrackStatistics { get; set; }
}

// Already approved API:
//
// public static partial class MemoryCache
// {
//     public MemoryCacheStatistics? GetCurrentStatistics();
// }
// 
// public partial interface IMemoryCache
// {
// #if NET_7_OR_GREATER
//      public MemoryCacheStatistics? GetCurrentStatistics() => null;
// #endif
// }
// 
// public partial class MemoryCacheStatistics
// {
//     public MemoryCacheStatistics() {}
//     
//     public long TotalMisses { get; init; } 
//     public long TotalHits { get; init; }
//     public long CurrentEntryCount { get; init; }
//     public long? CurrentEstimatedSize { get; init; }
// }

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 22, 2022
@maryamariyan maryamariyan added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-approved API was approved in API review, it can be implemented labels Mar 29, 2022
@maryamariyan maryamariyan added api-approved API was approved in API review, it can be implemented and removed blocking Marks issues that we want to fast track in order to unblock other important work api-ready-for-review API is ready for review, it is NOT ready for implementation labels Apr 8, 2022
@ghost ghost locked as resolved and limited conversation to collaborators May 8, 2022
@maryamariyan maryamariyan modified the milestones: 7.0.0, 8.0.0 Jun 16, 2022
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 area-Extensions-Caching feature-request
Projects
None yet
Development

Successfully merging a pull request may close this issue.