Adding ioredis support.

[guilhermef]

Ioredis is the top recommended client for Redis, and It's the only one that supports Sentinel and cluster.
Right now a few tests are broken, I'll change the PR as soon as it passes.
I'll just leave this here to get some help from the community on a few issues.

[guilhermef]

It now adds support to ioredis and has the same integration tests as the other Redis module.

[pedrosnk]

👍 This PR is really useful for my projects

[martinkuba]

@guilhermef thanks for doing this, nice work. It looks like it is more or less a drop in for the redis module. We should be able to include it in one of our upcoming releases.

[guilhermef]

thanks @martinkuba .

[philidem]

Is there a newrelic recommendation on how to utilize this change while we're waiting on an official merge? I could replace newrelic/lib/instrumentations.js and also copy ioredis.js to newrelic/lib/instrumentation/ioredis.js after running npm install but not sure if there is a more preferred approach.

[martinkuba]

@philidem yes, that's what you would have to do for now. Another option would be to fork and install from Github.

[philidem]

This is the code that I used to manually instrument ioredis:

const newrelic = require('newrelic');

const Redis = require('ioredis');

let orig_sendCommand = Redis.prototype.sendCommand;

let nextId = 0;

Redis.prototype.sendCommand = function(command, stream) {
    let promise = orig_sendCommand.call(this, command, stream);
    if (promise) {
        promise.finally(newrelic.createTracer('redis ' + command.name, () => {
            // nothing to do
        }));
    }

    return promise;
};

It seems to be working but if you see anything wrong with this approach then please let me know.

[martinkuba]

I would take this wrapping out - we probably do not want to represent these as callback segments (the bind() call is meant specifically for callbacks). Also, wrapping 'then' here will not work well with chained promises, as far as how they would be displayed in traces. Better place to handle this would be by instrumenting Bluebird, which we are planning to work on.

[martinkuba]

@guilhermef See my comment about wrapping 'then'. In addition, ioredis allows using callbacks instead of promises, which would need more work. We think we can address all of this by instrumenting Bluebird better. If you take out wrapping 'then', we will accept the PR.

[guilhermef]

Sure, I'll change the PR this weekend.
Em 31/03/2016 20:58, "Martin Kuba" notifications@github.com escreveu:

@guilhermef https://github.com/guilhermef See my comment about wrapping
'then'. In addition, ioredis allows using callbacks instead of promises,
which would need more work. We think we can address all of this by
instrumenting Bluebird better. If you take out wrapping 'then', we will
accept the PR.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#212 (comment)

[teddychan]

is there any plan to merge it and release?
as we are using ioredis heavily and need to be supported.

[pungoyal]

👍 we need this support as well

[pwnsrma]

This is really useful @guilhermef 👍

[lykkin]

This was merged out of band in v1.26.2, thanks so much!

[wong2]

What should I do to use this support?

[guilhermef]

@wong2 node-newrelic supports ioredis out-of-the-box: https://github.com/newrelic/node-newrelic/blob/master/lib/instrumentation/ioredis.js

[wong2]

@guilhermef yeah I find it out, it's because I'm using async/await

[comtaler]

@wong2 I am using async/await for ioredis as well. is there a way to make it work?

[NatalieWolfe]

@wong2 @guilhermef The New Relic Node Agent supports async/await on Node 8 since 2.3.0. If you're having problems with async/await support in the Node Agent, I recommend checking our support website and maybe opening a post on our forums where our support engineers can help you.