Returning cache status ("hit", "miss", "stale" etc) in context

[craigharvi3]

The problem We (Sounds) are in the process of migrating to using EMF logs where we want to capture all logs/metrics for a single request in a single JSON object.

In order to do this, we need to capture cache events for every request (and be confident that the event that was captured was indeed for that request).

We created https://github.com/bbc/http-transport-emf-stats to do this for us. It uses the event emitter that's exported from http-transport-cache and sets up listeners for the cache event's that are emitted (you can see that happening in here https://github.com/bbc/http-transport-emf-stats/blob/master/src/stats.js#L30).

The problem from what I've been seeing during testing is that when we have requests going to multiple RMS endpoints (like we have in Radioplayer), because they are listening to events on the same event name (in our case, cache.rms.hit for example), I don't think we can guarantee that the listener that fires is for the actual request that triggered it - so when we try to build up a stats object to return to our RMS client, the stats may not be accurate.

I hope all that makes sense.

Suggested solution Because we need to match up the status of the cache (hit, miss etc) with our request , it would be good if the cache plugin could set a variable in the context that describes it's "status" (I can't think of a better word for it, but it would basically tell us if the request hit, missed, errored, or returned stale etc).

We could then make use of this variable in our stats plugin (https://github.com/bbc/http-transport-emf-stats/blob/master/src/stats.js#L145) and then return the stats to our RMS client.

Alternatives considered So far, this is the only solution I can think of that guarantees the cache "status" that is returned is from the same request although if there's a better alternative, that would be fantastic.

Additional context Here's an absolutely terrible overview of the libraries/plugins we're using in RMS client:

Any questions/suggestions/feedback please let me know 👍

[niklasR]

I think that sounds sensible. Adding to the context shouldn't have any negative impact that I can think of. The alternative might be to add it to the response object, if served from the cache? That way a cache "miss" would only be very implicit.

@nspragg any opinions?

[craigharvi3]

@niklasR Cool. I'm just finishing up a PR for the functionality that I know will work with the stats plugin. Once that's ready to go, i'll send it your way and you can have a look 👍

[craigharvi3]

@niklasR PR created here https://github.com/bbc/http-transport-cache/pull/35. Feedback very much welcome and appreciated.

[simonespa]

Hi @craigharvi3 thanks for looking into that. I agree with the solution, the "context" object is a cheaper way to communicate between plugins. This way you read a value from memory in a synch fashion rather than asynchronously listening to events in a message-passing approach.

Said that, we should do an integration test between the two plugins to make sure that when the "emf-stats" one kicks in, the value has been set. The reason why I say this is because the plugins are called in a certain order throughout the call stack. Everything defined before calling next is called in order, everything after that is called in reverse order. If there is an exception, the rest of the plugin will be called only if the exception is thrown.

[niklasR]

Closed in #36