[apollo-datasource-rest] Feature request: expose cache status to callers

[nwalters512]

trafficstars

Hey folks 👋 We have an existing subclass of RESTDataSource that logs a variety of metrics for each call to fetch. We're trying to instrument our data sources to better understand how caching/memoization is used in production. However, RESTDataSource doesn't make it easy to figure out this information; the best we could do was manually querying the cache and memoizedResults to try to infer what's happening. However, in the end, we ended up forking RESTDataSource/HTTPCache to make cache status information first-class data in the return values from get/post/etc. We defined a new type, FetchResult that wraps the original response with cache metadata:

export interface FetchResult<TResult> {
  context: {
    cacheHit: boolean;
    memoized: boolean;
  };
  response: Promise<TResult>;
}

We then updated the get/post/etc. to return a FetchResult:

  protected async get<TResult = any>(
    path: string,
    params?: URLSearchParamsInit,
    init?: RequestInit
  ): Promise<FetchResult<TResult>> {
    return this.fetch<TResult>(
      Object.assign({ method: 'GET', path, params }, init)
    );
  }

Finally, we changed RESTDataSource#fetch and HTTPCache#fetch to return objects with that same context property. With this, we could update our subclass of RESTDataSource to automatically report whether particular requests were served by the cache or were memoized.

Here's our implementation in a Gist: https://gist.github.com/nwalters512/472b5fb7d4cc7d32c4cecaa69b21baf5. The important bits:

• definition of FetchResult
• updated get/etc.
• returning cacheHit: false from HTTPCache#fetch
• returning cacheHit: true from HTTPCache#fetch
• returning cacheHit for revalidated cache data
• consuming cache status from HTTPCache
• returning memoized: true for memoized requests

While this works, it's less than ideal to have to fork RESTDataSource and HTTPCache, since that introduces additional maintenance burden on our team. Ideally, this could be provided by the apollo-datasource-rest package itself. Does Apollo have any interest in adding this functionality? It doesn't necessarily need to use the same FetchResult interface we invented, but we'd appreciate anything that would give us more insight into how the cache is used.

[glasser]

This is a good feature suggestion and we're partway there now:

• There's a fetch method that returns more than just the parsed body (the methods like get are convenience wrappers that fill in the method and pluck off parsedBody
• Its return type (DataSourceFetchResult) has a requestDeduplication field that gives information similar to the memoized concept described here
• It also has an httpCache field which for now just has a cacheWritePromise (which is mostly intended for error handling and making tests deterministic but does tell you whether or not you wrote to the HTTP cache), which is a good place to put more stuff like cacheHit

We're not going to have time to implement the rest of this as part of the development spike we're doing right now, but if somebody else wanted to add more fields to httpCache it would be a great PR for us to review. I think you'd want to be able to learn if it was a cache hit, a cache miss but we wrote the value to the cache, a cache almost-hit where it was revalidated with a 304 response, etc. Would also be interesting to return the TTL if it's writing to the cache. This should be a backwards-compatible change.

[stevengssns]

Not sure if I need to put the PR in Draft status or not, but in any case I hope to get some feedback before adding the unit tests etc.