launchdarkly / js-client-sdk

LaunchDarkly Client-side SDK for Browser JavaScript
Other
112 stars 65 forks source link

`bootstrap: 'localStorage'` evaluates all feature flags for a new user #193

Closed gligoran closed 4 years ago

gligoran commented 4 years ago

Describe the bug

Initializing a LDClient with a new user and using localStorage all the feature flags are evaluated that are set up for that env and project.

This is problematic because a feature flag might be evaluated on page that has nothing to do with it (i.e. feature flag for the BUT button color is evaluated on terms and conditions page). In addition this messes up insights and experimentation data statistics as feature flags are being evaluated for users that haven't encountered that feature.

To reproduce

I'm initializing my client like this:

import * as LDClient from 'launchdarkly-js-client-sdk';

const user = {
    anonymous: true,
    key: "someGeneratedKey",
    allAttributesPrivate: true,
};

const ldClient = LDClient.initialize(
    LAUNCHDARKLY_CLIENT_SIDE_ID,
    user,
    {
        bootstrap: 'localStorage',
        logger: { // for log purposes below
            debug: console.debug.bind(null, 'LaunchDarkly::debug::'),
            info: console.info.bind(null, 'LaunchDarkly::info::'),
            warn: console.warn.bind(null, 'LaunchDarkly::warn::'),
            error: console.error.bind(null, 'LaunchDarkly::error::'),
        },
    }
);

If I then completely empty local storage and run this code, all of my feature flags will be evaluated (can be seen in the debugger). If I then re-run this code without clearing the cache using the same exact user data, the only flags that will be evaluated are the ones I request via client.variation('feature-name'). If the page doesn't request any feature flags, no flags are evaluated.

Expected behavior

I would expect that only flags that are requested are evaluated and stored in localStorage. Something akin to a read-through cache.

Logs

ldclient-common.es.js:1 LaunchDarkly::debug:: enqueueing "identify" event
ldclient-common.es.js:1 LaunchDarkly::debug:: polling for feature flags at https://app.launchdarkly.com/sdk/evalx/xxxxx/users/xxxxx
ldclient-common.es.js:1 LaunchDarkly::debug:: enqueueing "feature" event
ldclient-common.es.js:1 LaunchDarkly::debug:: enqueueing "feature" event
ldclient-common.es.js:1 LaunchDarkly::debug:: enqueueing "feature" event
ldclient-common.es.js:1 LaunchDarkly::debug:: enqueueing "feature" event
ldclient-common.es.js:1 LaunchDarkly::debug:: enqueueing "feature" event
ldclient-common.es.js:1 LaunchDarkly::debug:: enqueueing "feature" event
ldclient-common.es.js:1 LaunchDarkly::debug:: enqueueing "feature" event
ldclient-common.es.js:1 LaunchDarkly::info:: LaunchDarkly client initialized
httpRequest.js:56 XHR finished loading: OPTIONS "https://app.launchdarkly.com/sdk/goals/xxxxx".
httpRequest.js:56 XHR finished loading: OPTIONS "https://app.launchdarkly.com/sdk/evalx/xxxxx/users/xxxxx".
XHR finished loading: GET "https://app.launchdarkly.com/sdk/goals/xxxxx".
XHR finished loading: GET "https://app.launchdarkly.com/sdk/evalx/xxxxx/users/xxxxx".
ldclient-common.es.js:1 LaunchDarkly::debug:: sending 6 events
httpRequest.js:56 XHR finished loading: OPTIONS "https://events.launchdarkly.com/events/bulk/xxxxx".
XHR finished loading: POST "https://events.launchdarkly.com/events/bulk/xxxxx".

SDK version

launchdarkly-js-client-sdk@2.15.2

Language version, developer tools

JavaScript code in ES6 transpiled with ParcelJS 1.12.4 to ES5.

OS/platform Chrome (Version 78.0.3904.108 (Official Build) (64-bit)) on MacOS Catalina.

eli-darkly commented 4 years ago

Hi. I think this is a misunderstanding, and isn't really related to the localstorage option at all - it is the way flags are always delivered from LD to the JS SDK, it's just that with the local storage option the SDK would sometimes not need to get the flags from LD.

Briefly: the entire set of flags is always evaluated by the LD service and sent to the client together; when you call variation, it is not querying flags on demand from the service (which would be slow), it is just returning a value that it previously received. This bulk evaluation should not affect your dashboard statistics; those are driven by analytics events that are posted to events.launchdarkly.com based on the flags that you actually requested.

Please see this documentation page for more on this.

If I've misunderstood and haven't addressed your concern, could you please clarify whether you are actually seeing a problem related to this on the LD dashboard? You didn't mention one, you just said you expected there would be a problem because you saw an HTTP request.

gligoran commented 4 years ago

Hi. I've done some additional testing and it still seems to me that turning on bootstrap: 'localStorage' produces different results when look at experiments and insights tabs of a feature flag.

Here's the setup I've have for this test:

The steps in the test:

  1. I run my app in a brand new incognito mode. This is to ensure clean localStorage and a unique key for the user.
  2. I open up Page A which initializes LaunchDarkly JS Client. Note: Page B is never visited in this experiment.

In the first run I have bootstrap: 'localStorage' and in the second run I remove it.

Here are the resulting charts from the dashboard (only look at the spikes after the right-most vertical dashed line, as that marks the time of reset for experiments).

Insights: image image

Experiments: image image

You can see that Feature B was evaluated on Page A, where it's not requested when bootstrap: 'localStorage' is used. It also counts that evaluation towards the unique visitors in the experimentation section. That doesn't happen when boostrapping is not enabled.

My expectations are that Feature B would not have been evaluated in this experiment at all.

jennifro commented 4 years ago

Hi there. I received and responded to the support request you created for this issue. Because this does seem to be a support request, please follow up with your notes in that request.