apollographql / apollographql/datasource-rest

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

Open
#41 2 comments 1 reaction 0 assignees View on GitHub
PRs welcome
Dominant language
TypeScript
Stars
48
Forks
21
PR merge metrics
No merged PRs in 30d

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:

```ts
export interface FetchResult {
context: {
cacheHit: boolean;
memoized: boolean;
};
response: Promise;
}
```

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

```ts
protected async get(
path: string,
params?: URLSearchParamsInit,
init?: RequestInit
): Promise> {
return this.fetch(
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`](https://gist.github.com/nwalters512/472b5fb7d4cc7d32c4cecaa69b21baf5#file-restdatasource-ts-L50-L56)
* [updated `get`/etc.](https://gist.github.com/nwalters512/472b5fb7d4cc7d32c4cecaa69b21baf5#file-restdatasource-ts-L161-L209)
* [returning `cacheHit: false` from `HTTPCache#fetch`](https://gist.github.com/nwalters512/472b5fb7d4cc7d32c4cecaa69b21baf5#file-httpcache-ts-L104-L109)
* [returning `cacheHit: true` from `HTTPCache#fetch`](https://gist.github.com/nwalters512/472b5fb7d4cc7d32c4cecaa69b21baf5#file-httpcache-ts-L129-L134)
* [returning `cacheHit` for revalidated cache data](https://gist.github.com/nwalters512/472b5fb7d4cc7d32c4cecaa69b21baf5#file-httpcache-ts-L160-L168)
* [consuming cache status from `HTTPCache`](https://gist.github.com/nwalters512/472b5fb7d4cc7d32c4cecaa69b21baf5#file-restdatasource-ts-L268-L274)
* [returning `memoized: true` for memoized requests](https://gist.github.com/nwalters512/472b5fb7d4cc7d32c4cecaa69b21baf5#file-restdatasource-ts-L281-L288)

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.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reading RESTDataSource#get, post, and fetch alongside HTTPCache#fetch, using the linked Gist to compare the proposed cacheHit and memoized metadata paths. Determine how cache hits, revalidated data, misses, and memoized requests should be represented in the public return values; done means callers can observe those statuses without forking either class.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, backend, observability
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.