-
Notifications
You must be signed in to change notification settings - Fork 24
Description
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: falsefromHTTPCache#fetch - returning
cacheHit: truefromHTTPCache#fetch - returning
cacheHitfor revalidated cache data - consuming cache status from
HTTPCache - returning
memoized: truefor 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.