Let consumers of MemoryCache access metrics
maryamariyan opened this issue · 43 comments
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
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.
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",
};
}
}
}
}
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
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: |
|
| Milestone: | - |
I like this design better. How would this work with the fact that the cache is an interface though...
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...
- 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)
- 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
}OR, we can use default interface members on .NET Core only...😄.
Make a time machine and stop making everything interfaces : D
+1 😄
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) { ... }
}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.
Do we have provision to check the space consumed by each key in the cache ?
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:
- read requests
- read hits
- cache records
- estimate of the total cache size in memory
- write requests
- trim events
a) nice to have: trim reason as a dimension, e.g. expired or evicted.
b) nice to have: number of trimmed records - remove requests
a) nice to have: remove hits (when an item to remove was found in the cache)
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:
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:
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
@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.
- We changed the extension method to an instance method (not virtual at this time)
- We changed MemoryCacheStatistics to readonly with get+init
- We renamed the properties on MemoryCacheStatistics to reduce redundancy, and then to enhance clarity.
namespace Microsoft.Extensions.Caching.Memory
{
public partial class MemoryCache
{
public MemoryCacheStatistics GetCurrentStatistics() { throw null; }
}
public partial readonly struct MemoryCacheStatistics
{
public long TotalRequests { get; init; }
public long TotalHits { get; init; }
public long CurrentEntryCount { get; init; }
}
}In the approved API proposal we exposed the method on the concrete type
We understand that it may be inconvenient for users that might want to access this via the interface and need it casted. regardless, the GetCurrentStatistics would only work with the right concrete type, so if we wanted to expose this on the interface we'd need to decide on either throwing or returning null for unexpected concrete types.
We thought about using DIMs that would throw exception or return null. The more desirable approach would be to add a new interface called IMemoryCacheHasStats
interface IMemoryCacheHasStats
{
MemoryCacheStatistics GetCurrentStatistics();
}and update MemoryCache to implement IMemoryCacheHasStats as well.
- We should wait until we have evidence that anyone needs them on the interface
- If there is, then we'd prefer a pattern like
IMemoryCache2 : IMemoryCachesuch that we always have a latest version that has all the features, rather than a permutation of all possible implementations. DIMs would be another approach.
namespace Microsoft.Extensions.Caching.Memory
{
// interface IMemoryCacheHasStats
// {
// MemoryCacheStatistics GetCurrentStatistics();
// }
public partial class MemoryCache /* : IMemoryCacheHasStats */
{
public MemoryCacheStatistics GetCurrentStatistics();
}
public partial readonly struct MemoryCacheStatistics
{
public long TotalRequests { get; init; }
public long TotalHits { get; init; }
public long CurrentEntryCount { get; init; }
public long? CurrentSize { get; init; }
}
}We should wait until we have evidence that anyone needs them on the interface
I don't understand this outcome, what is the guidance for people that use IMemoryCache?
what is the guidance for people that use IMemoryCache?
Who are the people that need to access the cache statistics, but don't know the concrete implementation of the cache?
My thinking is that the owners of the cache are the ones who want to report statistics on it. And if you own the cache, you know if you are working with our MemoryCache, or some other implementation.
That doesn't answer the question. When people are using dependency injection to configure the cache globally using AddMemoryCache, how does code in the application get that instance and get statistics? Are we saying that scenario isn't important? That's the main use case.
how does code in the application get that instance and get statistics?
It gets the IMemoryCache from the DI container, and casts it. The application knows if it is using a MemoryCache or not. If the application is using a different cache implementation, it will need to get that cache implementation to implement a statistics API, and it can call that.
I think this is different than Adding/Removing from the cache. I can imagine library code (like ASP.NET) needing to grab a cache, but it doesn't know the concrete implementation, and add to it or remove from it. Yes, in that case abstracting away the implementation is necessary. But I don't see libraries needing to get the stats out of a cache.
But if we are wrong, and this is necessary, we can always add it in the future.
It gets the IMemoryCache from the DI container, and casts it. The application knows if it is using a MemoryCache or not. If the application is using a different cache implementation, it will need to get that cache implementation to implement a statistics API, and it can call that.
I think this is horrible, but lets document it so everyone at least knows what horrible thing we're telling them to do 😄 . https://docs.microsoft.com/en-us/aspnet/core/performance/caching/memory?view=aspnetcore-6.0#use-imemorycache
The usage I'd imagine is a background thread that polls and egresses these statics to via System.Diagnostics.Metrics.
Also cc @IEvangelist for non ASP.NET Core MemoryCache docs.
Hi @davidfowl
Also cc @IEvangelist for non ASP.NET Core
MemoryCachedocs.
I got you: https://docs.microsoft.com/dotnet/core/extensions/caching
The use case is get the IMemoryCache impl from DI, I'm not sure how I feel about the cast approach. Feels a bit of an after thought. Why not have introduce another interface (ducks to avoid small office items being thrown at him)?
Maybe IMemoryCacheWithStatistics, then this interface is implemented by MemoryCache. Then only interested consumers get the stats, instead of using the IMemoryCache I'd use IMemoryCacheWithStats.
public interface IMemoryCacheWithStats : IMemoryCache
{
MemoryCacheStatistics GetCurrentStatistics();
}@IEvangelist It's not natural/easy with ASP.NET DI to register the same instance with two interfaces.
About this feature I would have preferred to see event counters instead.
Or if the API scenario is preferred, just try and cast to the MemoryCache instance to get the stats.
About this feature I would have preferred to see event counters instead. Or if the API scenario is preferred, just try and cast to the
MemoryCacheinstance to get the stats.
I like the event counters idea too.
Event counters don't work well because they need to be per cache instance.
I still would like them even if they show data across all cache instances.
And even if there is an API for each instance.
Event counters don't work well because they need to be per cache instance.
One idea there is to add a string "Name" or "Prefix" property to the MemoryCacheOptions. This would be used to identify/qualify the memory cache instance in the event counter / diagnostics metric.
That's how System.Runtime.Caching took care of it:
Since when can we have "dimensions" in event counters? I thought they were static strings?
Let me rephrase it ;) I do use dynamic counters all day long. My comment is about how a tool like dotnet-counters would be able to display them? You'd have to know the named counters in advance.
I don't understand this outcome, what is the guidance for people that use
IMemoryCache?
This discussion is precisely the reason why we keep saying that interfaces aren't great because it's impossible to easily add over time :-)
If we strongly believe that we need to have feature parity between the interface and the class, we should do IMemoryCacheX where X is an incrementing integer. I don't think an interface per feature is a good idea because (1) it doesn't make DI any easier and (2) it creates permutational complexity.
I think this is horrible, but lets document it so everyone at least knows what horrible thing we're telling them to do 😄 .
I agree on the documentation, but I don't think it's that horrible. The guidance would be "if you own the instance, hard casting is fine, and if you don't try casting it and access the stats if it succeeds". I don't understand how this would be different if we had done IMemoryCacheWithStatistics given that the proposed shape didn't even inherit IMemoryCache.
This discussion is precisely the reason why we keep saying that interfaces aren't great because it's impossible to easily add over time :-)
Don't blame the interface, this is a decision we made 😄
Let me rephrase it ;) I do use dynamic counters all day long. My comment is about how a tool like dotnet-counters would be able to display them? You'd have to know the named counters in advance.
dotnet-counters learned new tricks to complement the new Meter APIs in .NET 6. If you run this code:
using System.Diagnostics.Metrics;
Meter meter = new Meter("Cache");
ObservableGauge<int> hits = meter.CreateObservableGauge("Hits", CalculateHits);
ObservableGauge<int> size = meter.CreateObservableGauge("Size", CalculateSize);
static IEnumerable<Measurement<int>> CalculateHits()
{
yield return new Measurement<int>(24, KeyValuePair.Create<string,object?>("Name", "Red"));
yield return new Measurement<int>(19, KeyValuePair.Create<string, object?>("Name", "Blue"));
yield return new Measurement<int>(1456, KeyValuePair.Create<string, object?>("Name", "Green"));
}
static IEnumerable<Measurement<int>> CalculateSize()
{
yield return new Measurement<int>(54, KeyValuePair.Create<string, object?>("Name", "Red"));
yield return new Measurement<int>(100, KeyValuePair.Create<string, object?>("Name", "Blue"));
yield return new Measurement<int>(395833, KeyValuePair.Create<string, object?>("Name", "Green"));
}
Console.ReadLine();Then you can monitor that with dotnet-counters and see dimensions populated dynamically:
> dotnet-counters monitor -p <pid> --counters Cache
Press p to pause, r to resume, q to quit.
Status: Running
[Cache]
Hits
Name=Blue 19
Name=Green 1,456
Name=Red 24
Size
Name=Blue 100
Name=Green 395,833
Name=Red 54
Since when can we have "dimensions" in event counters?
That limitation is still there for EventCounters, but the new Meter APIs support dimensions: https://docs.microsoft.com/en-us/dotnet/core/diagnostics/metrics-instrumentation#multi-dimensional-metrics.
I had suggested that we start with APIs because it is an initial investment that can let people get unblocked. For example anyone who urgently wants to create an EventCounter or Meter to expose that data can now do so for their own service. .NET could certainly offer published metrics out-of-the-box in the future once we think we've got the right feedback and design for it. I didn't want to encourage @maryamariyan to rush a solution here before we've had a chance to learn what people want.
I think it would be great for MemoryCache instance if we can name them and use it as the dimension. That's what you referred to @eerhardt ?
Yes.
This discussion is precisely the reason why we keep saying that interfaces aren't great because it's impossible to easily add over time :-)
Don't blame the interface, this is a decision we made 😄
I'm not trying to assign blame; just stating the practical limitations of particular API choices :-)
I'm not trying to assign blame; just stating the practical limitations of particular API choices :-)
I think when you make interfaces the enemy designs like this happen... I don't have a problem with the method being on the concrete type. I have a big problem with there not being a nice way to make the interface scenario work without casting to a concrete type.
Here are a couple of examples from a very quick cursory glance:
- https://github.com/dsuryd/dotNetify/blob/master/DotNetifyLib.SignalR/MemoryCacheAdapter.cs - Wrapping an existing IMemoryCache
- https://github.com/VirtoCommerce/vc-storefront/blob/5f2ea0cab77df3805d6a8818c557e6710ad4970c/VirtoCommerce.Storefront/Caching/StorefrontMemoryCache.cs#L12
- https://github.com/SpatialFocus/MethodCache.Fody/blob/master/src/SpatialFocus.MethodCache.Sample/MyMemoryCache.cs
There's no silver bullet here, but we should have expose something like:
interface IMemoryCacheHasStats
{
MemoryCacheStatistics GetCurrentStatistics();
}So other can decorate/expose the same sort of stats if they wanted to.
So other can decorate/expose the same sort of stats if they wanted to.
If the scenario is important then I think Immo provided a suggested path: IMemoryCache2 : IMemoryCache. Presumably when we add another metric in the future we'd introduce IMemoryCache3, IMemoryCache4, and so on so that consumers can determine which iteration of the functionality producers support.
The point of contention appears to be @terrajobst says "We should wait until we have evidence that anyone needs them" and @davidfowl says we should support it now.
I don't consider myself well informed on customer usage for the M.E.C, but fwiw I lean towards Immo's position at the moment. That stems from:
- Not adding it now is a reversible decision, adding it is not reversible
- I'm speculating that it is rare that 3rd party A makes a custom IMemoryCache implementation that is consumed by 3rd party B and that B needs programmatic access to the stats for that cache. If my speculation is correct then I'd rather let A and B agree on the cache statistics interface between themselves rather than Microsoft being the arbiter of a rarely used interface.
I consider myself VERY well informed on customer usage and I think the 90% case for ASP.NET Core application is usage via the interface and DI. Casting is an unpleasant way to consume metrics in those cases.
If the scenario is important then I think Immo provided a suggested path: IMemoryCache2 : IMemoryCache. Presumably when we add another metric in the future we'd introduce IMemoryCache3, IMemoryCache4, and so on so that consumers can determine which iteration of the functionality producers support.
I think this is just a smell and we should avoid this and instead introduce discrete feature interfaces where possible and then hop on the DIM train sometime in the future.
I chatted some with @davidfowl on Friday to better understand his position + investigated the APIs on my own + watched the video. I've changed my opinion, I would be in favor of having an interface now.
I think the initial discussion about how we would handle alternate implementations, while entirely valid, gives an impression that the issue only affects a small fraction of M.E.C usage that has those custom implementations, which in turn lead me to believe that it would be safe to wait and see how big a fraction of users that really was and/or let them make custom interfaces. The stronger argument IMO is what David alluded to at the end, that we've trained users via docs, blogs, samples, etc that the canonical and recommended way to use a MemoryCache (and to use most things from the DI container) is via an interface. Its not that users couldn't cast it if they knew, but rather that it will be unintuitive and distasteful for them when they do that cast.
In the video there was a question "does anyone use the interface?" and the discussion in the room made that sound uncertain. I think we can confidently say people make heavy use of the interface:
- The docs exclusively refer to the interface
- GitHub search shows 39K references to IMemoryCache
- @davidfowl looks at lots of customer code and asserts it is the overwhelmingly common pattern
- I checked the top 5 non-Microsoft blog/article results for "caching in .net core". All of them exclusively used the IMemoryCache interface.
To make it a little more concrete, this seems like a likely pattern a dev would write with this feature. It comes from the docs except instead of updating a cache we would be logging the stats for the cache.
services.AddMemoryCache();
services.AddHostedService<MemoryCacheStatsLogger>();
...
public class MemoryCacheStatsLogger : IHostedService
{
public MemoryCacheStatsLogger(IMemoryCacheWithStats cache, ...)
// or IMemoryCache2, I'm not advocating for a particular interface name here. They key is that it is an interface.
{
// some code to create a Meter or to set up a timer that will call back to the cache.GetStats() function.
}
}So AddMemoryCache would also register the singleton instance as IMemoryCacheWithStats?
RegisterTransient<IMemoryCacheWithStats>(sp => sp.GetService<IMemoryCache>() as IMemoryCacheWithStats);
I think that would be logical yes.
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; }
}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)
- 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; }
// }