Cache hit instrumentation

[danielhartnell]

Thanks for creating this gem. We've found it quite useful.

ActiveSupport::Notifications has been okay for tracking cache hits (subscribing to /cache_read/). I would find it useful to have access to attributes like operation_name or the field name for our observability.

Do you have any suggestions for achieving this?

My first thought was to try to include plaintext strings in the cache key which I could extract from the cache key in the ActiveSupport::Notifications payload. If I recall correctly, object_cache_key worked but I wasn't sure if I was going to break something by using it for this purpose. It would also be nice not to have to explicitly define it on each field we want to cache.

Do you think the GraphQL gem's tracing might be suitable here?

I see how I can implement one of the Trace methods like execute_query within my own app:

https://graphql-ruby.org/api-doc/2.4.8/GraphQL/Tracing/Trace

And I see how I could add something which would identify if there was a cache hit or miss here:

https://github.com/DmitryTsepelev/graphql-ruby-fragment_cache/blob/master/lib/graphql/fragment_cache/fragment.rb

I'm not sure how I would connect those two though. Is there some sort of query metadata that I could access? It would be nice to find a generic solution and I'd be happy to open a PR with any changes here if we find something you like.

[DmitryTsepelev]

Hey hey!

I haven't tried it out, but looks like it should be easy enough to patch https://github.com/DmitryTsepelev/graphql-ruby-fragment_cache/blob/master/lib/graphql/fragment_cache/cacher.rb#L57. It has an access both to the query and the cache key: at that moment you can remember the operation name (from query) using the same key and access it later.

I'd be happy if we had some kind of hooks in the gem.

WDYT?

[danielhartnell]

Hey @DmitryTsepelev

Thanks so much! So, I took a look at persist and batched_persist. Let me know if you think I overlooked something but I didn't see a way to capture a cache hit or miss for each fragment.

I was able to collect that that by having access to fragments and cache_keys_to_values in Fragment#read_multi. I created an PR here:

#125

After running specs, I ended up having to rescue from errors when query was not available. What I came up with seems to work well and specs pass now but I'm not sure if it's a little hacky.

Let me know if you don't like this approach and I can change it up. I just might need a little more help spotting anything I overlooked.

Upon review of my PR, it just occurred to me that I'm overlooking read calls when the cache store does not respond to read_multi. If we continue with this implementation I can try to fix that.

[danielhartnell]

I think we can consider this fixed by #125