launchdarkly / node-server-sdk

LaunchDarkly Server-side SDK for Node
Other
79 stars 65 forks source link

Not able to fetch variation from redis persistent store #169

Closed vgupta closed 4 years ago

vgupta commented 4 years ago

I have two microservices. One microservice is connecting with launchdarly and storing all flags in redis using redis persistance store. Example

let sdk_key = "sdk_key";

const redisOpts = { url: 'redis://localhost:6379' }; const store = LaunchDarkly.RedisFeatureStore(redisOpts, 30, 'my-key-prefix');

const config = { featureStore: store }; var ldClient = LaunchDarkly.init(sdk_key, config);

await ldClient.waitForInitialization(); ldClient.variation("testflag", { "key": "keyvalue" }, false, function (err, showFeature) { if (showFeature) { console.log("show"); // application code to show the feature } else { console.log("hide"); // the code to run if the feature is off } }); I verified redis database and database has testflag and corresponding value. I have another microservice that is trying to fetch variation from redis persistance store but i am not getting same value of 'showFeature' as above example.

let sdk_key = "sdk_key";

const redisOpts = { url: 'redis://localhost:6379' }; const store = LaunchDarkly.RedisFeatureStore(redisOpts, null, 'my-key-prefix');

const config = { featureStore: store, useLdd:true }; var ldClient = LaunchDarkly.init(sdk_key, config);

await ldClient.waitForInitialization(); ldClient.variation("testflag", { "key": "keyvalue" }, false, function (err, showFeature) { if (showFeature) { console.log("show"); // application code to show the feature } else { console.log("hide"); // the code to run if the feature is off } });

can you check and fix this.

eli-darkly commented 4 years ago

We might need more information than just the code sample (for instance, we don't know anything about how this feature flag is configured, so we may want to look at your data)... and this really seems like a support request rather than a specific issue with the SDK code. Could you please go to https://support.launchdarkly.com/ and submit your request there?

vgupta commented 4 years ago

It's not support request. It is a bug. When i am passing redis client which is already connected with redis rather than redis url, it's working.

const LaunchDarkly = require('launchdarkly-node-server-sdk');

const redis = require('redis');
async function initLD() {
    let sdk_key = "sdkkey";

    var clientConfig = {
        host: "127.0.0.1",
        port: 6379
    };

    var client = await redisClient(clientConfig);
    const redisOpts = {
        client: client       
    };
    const store = LaunchDarkly.RedisFeatureStore(redisOpts, 30, 'my-key-prefix');

    const config = {
        featureStore: store,
        useLdd:true
    };
    ldClient = LaunchDarkly.init(sdk_key, config);

    await ldClient.waitForInitialization();
    ldClient.variation("testflag", { "key": "keyvalue" }, false,
        function (err, showFeature) {
            if (showFeature) {
                console.log("show");
                // application code to show the feature
            } else {
                console.log("hide");
                // the code to run if the feature is off
            }
        });
}

async function redisClient(redisOpts) {
    var client = null;
    return new Promise((resolve) => {
        client = redis.createClient(redisOpts)
        let connected = false;
        client.on('error', err => {
            // Note that we *must* have an error listener or else any connection error will trigger an
            // uncaught exception.
            console.log('Redis error - ' + err);
        });
        client.on('reconnecting', info => {
            console.log('Attempting to reconnect to Redis (attempt #' + info.attempt +
                ', delay: ' + info.delay + 'ms)');
        });
        client.on('connect', () => {
            connected = true;
            resolve(client);
        });
        client.on('end', () => {
            connected = false;
        });

    })

}
initLD();
eli-darkly commented 4 years ago

To be clear, I'm not saying that there is not a bug here. I'm saying that unless the problem is pretty clearly localized to the SDK code in a way that can be reproduced from just the issue description, we normally try to handle the problem through the support channel, because then they can do things like ask for your actual SDK key so we can try to reproduce it with the exact same data that you have; we can't do that on a Github issue. That's why the issue template that comes up when you select "Bug" is phrased in the way that it is. Especially when trying to diagnose issues that involve multiple components running in different environments, it can sometimes take a while to find out what's going on and we may need more information.

So, if you do want to try to sort it out here, I do need to ask for clarification on a couple of things.

  1. "When i am passing redis client which is already connected with redis rather than redis url, it's working." - Do you mean that you made that change in both of the services, or just one (and if so, which one)?

  2. Have you tried testing with a somewhat longer-running service, instead of one that just evaluates a single flag and immediately quits?

These two questions are related, because I'm trying to see why service 1 (the one that connects to LaunchDarkly) would be working while service 2 (the one that has useLdd:true) is not. They both have the same Redis integration. However, there is also an in-memory cache, so it's possible that Redis is actually failing in both services, so service 1 is not actually storing the flag in Redis... but service 1 is still able to use the flag, because it has just received it from LaunchDarkly so it is cached. In that case, I would expect that if you changed service 1 to sit around for at least 30 seconds (the cache TTL) and then evaluate the flag again, you would see it fail as it would try to read the flag from Redis. If it does not fail then something else must be going on.

  1. You didn't mention any log output. We mention logging specifically in the issue template because it's often very helpful in diagnosing problems. By default, the SDK logs everything except low-level debug messages to standard output; is that output available in your microservice environment?

  2. What version of the SDK are you using?

vgupta commented 4 years ago

Answer 1) No, In Service1, i am passing redis client like below. This is connecting with launchdarly server and syncing all flags to redis. This is working file.

    var sdk_key = "";// sdk key
    var clientConfig = {
        host: "127.0.0.1",
        port: 6379
    };

    var client = await redisClient(clientConfig);
    const redisOpts = {
        client: client       
    };
    const store = LaunchDarkly.RedisFeatureStore(redisOpts, 30, 'my-key-prefix');

    const config = {
        featureStore: store
    };
    ldClient = LaunchDarkly.init(sdk_key, config);
    await ldClient.waitForInitialization();```

  When i am trying to get variation from ldClient, i am getting correct value. I testing using changing rules etc... on launchdarly account

  Service2, I am passing  redisOpts rather than redis client like below
var sdk_key = "";// sdk key

const redisOpts = {
    host: "127.0.0.1",
    port: 6379      
};
const store = LaunchDarkly.RedisFeatureStore(redisOpts, 30, 'my-key-prefix');

const config = {
    featureStore: store,
   userLDD:true
};
ldClient = LaunchDarkly.init(sdk_key, config);
await ldClient.waitForInitialization();
   when i am fetching variation of any feature using ldClient, i am getting below message 

    Unknown feature flag "' + key + '"; returning default value.

  I tried to ad debug points in node js launchdarly sdk code and found that 
doGet function of redis_feature_store.js has been called while redis client is not connected with redis  yet. when doGet call finished, i am getting breakpoint inside of 

client.on("connect", () => { if (!initialConnect) { logger.warn("Reconnected to Redis"); } initialConnect = false; connected = true; }



I think, redis client should be connected before. 

So I removed url and password from redisOpts paramter and passed connected redis client  like service with useLD true.

Now i am getting correct variation from redis store.

3) Yes. Look service 2 message when i am fetching variation of any flag.
  Unknown feature flag "' + key + '"; returning default value.

4) I am using version 5.10.0.

It;'s very easy to reproduce.

1) Create a a file that will use existing connected redis client with RedisFeatureStore.
    useLDD = false

2) Create a file that will use redisOpts (url, password) with RedisFeatureStore.
  useLDD = true
3) Create a file that will use use existing connected redis with RedisFeatureStore.
  useLDD = true
Run all these files  to fetch variation on some time interval.

File 1 and file 3  will print response with correct variation value as expected
File 2 is always giving response Unknown feature flag "' + key + '"; returning default value.

Sorry, i cann't further reply. It's upto you, either want to fix or not.

Thanks
eli-darkly commented 4 years ago

Thanks for the additional information. Your theory about it not being connected yet is something I will look into, although it is contrary to what the Redis library documentation says (operations are supposed to be queued until the connection is established) and we haven't seen such behavior before.

Unfortunately, I haven't been able to reproduce the problem. This code works perfectly for me, as long as the database has already been populated by another process that's not using LDD mode:

var sdk_key = "";

const redisOpts = {
    host: "127.0.0.1",
    port: 6379      
};
const store = LaunchDarkly.RedisFeatureStore(redisOpts, 30, 'my-key-prefix');

const config = {
    featureStore: store,
    useLdd:true
};

ldClient = LaunchDarkly.init(sdk_key, config);

ldClient.waitForInitialization().then(() = {
    client.variation('flag-key', {key: 'user-key'}, null, function(err, value) {
        console.log('flag value: ' + value);
    });
});
setTimeout(() => {}, 2000); // just so the async code has time to run

The only significant change I made from your example code was that you had misspelled useLdd as userLDD. But I assume that wasn't the code you actually ran, since then it would have ignored the misspelled option and behaved exactly like your other service that doesn't use LDD mode.

eli-darkly commented 4 years ago

Since we have still been unable to reproduce this using the code shown above, and no other customers have reported a similar issue, and the original reporter has said they do not intend to reply any further, I'm closing this issue.