Improve Redis instrumentation

[mwear]

The Redis instrumentation, which is a port of the dd-trace version, instruments redis by patching the call and call_pipeline methods. We might be missing some calls by not also patching call_loop and call_pipelined. We should look into adding coverage for the additional methods.

See: #176 (review)

[fbogsany]

Is this required for the RC milestone @mwear?

[mwear]

It's not. I believe these were suggestions that I captured based on the Shopify version of the Redis instrumentation. If we are not missing anything by not instrumenting the methods discussed above, we can consider closing this.

[fbogsany]

I'm going to keep this open, but remove the milestone.

[fhwang]

I can give this a try 🙋

[fbogsany]

Thanks @fhwang ! I've assigned it to you.

[fhwang]

@mwear Regarding call_loop: This is primarily used for pubsub subscribers. Is it settled that we want to treat these messages as a new span in the same trace of the publisher? And if so, is there guidance in the spec about how to propagate context over Redis messages?

[mwear]

I opened this issue to capture some feedback from @fbogsany based on Shopify's internal instrumentation, so I will defer to him on specifics. I think making sure that we are generating spans for all Redis operations would be a good first step. I don't know that there is any guidance on context propagation over Redis, but it's an interesting idea. I took a quick look at JS Redis instrumentation, and it doesn't handle it.

[fbogsany]

Shopify doesn't use Redis pubsub, so I can't offer any insights there. Perhaps we can just instrument call_pipelined for now?

[fhwang]

What's the general principle regarding what level of method calls we're hooking into? call_pipelined is actually called by call_pipeline, so if we instrument both we'll be getting a lot of double spans.

I'm not sure either of these methods count as the public interface of this library -- they aren't documented, they aren't directly tested. Should we simply instrument every call to every public method on the main Redis class that results in an immediate call to the redis-server?