launchdarkly / node-server-sdk

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

variation call doesn't return consistent result for flag presence #238

Closed cuzox closed 2 years ago

cuzox commented 2 years ago

Describe the bug We have a flag with a contains rule in the email attribute. When client.variation is called the first time with key, and email, it properly evaluates whether the user should belong to that flag, and returns true if it matches. Later on the same variation call is made with the same key but with without email, and it returns false for the same flag for which true was returned when both key and email were provided.

To reproduce Call client.variation with user.key and user.email to evaluate a flag that should resolve to true, true will be the response. Call client.variation with the same user.key as before but without user.email, false will be the response.

Expected behavior client.variation to identify the same user if less attributes were provided but the same key was sent.

SDK version launchdarkly-node-server-sdk: 6.2.3

Language version, developer tools Node v16.13.2

OS/platform WSL2 | Ubuntu 18.04.6

eli-darkly commented 2 years ago

Short answer: no, that's not how any of the LaunchDarkly SDKs work. It's not a bug, it's the defined behavior.

The SDKs do not store user attributes in the way you may be thinking. In your example, when you provided a user with key and email first, the SDK used those properties for that particular variation call— and, possibly, sent them to LD in analytics event data— but it did not, and cannot, remember them for use in a later variation call that does not provide the email. Each call to variation operates only on 1. the parameters you gave it just now and 2. the current flag configuration. That is universally true of all of the LaunchDarkly server-side SDKs.

eli-darkly commented 2 years ago

The user data that LaunchDarkly accumulates on the service side and displays on the dashboard is a somewhat different story. There, the attributes do persist from previous SDK activity, and they have a behavior (currently— this may not always be the case) of merging together attributes from different calls: so, if you provided only email at one time but only name at another time, you might see the user listed on the dashboard with both email and name.

But that is retrospective analytics data that is accumulated in a database on the service side; those stored properties are not available to the SDKs. And that is deliberate, because the overhead of trying to make the SDKs work that way would be massive, and also because it would make flag evaluation behavior non-deterministic (that is, you would not be able to know for sure that if you called variation with such-and-such parameters, and that if the flag configuration had not changed, that you would consistently get the same answer).

cuzox commented 2 years ago

If the the first call creates the user in LD if it doesn't exist, allows for the flag evaluation logic to run and then return whether the provided flag was assigned to the user, I would expect the second call see that a user with the key already exists (since it knows not to create it again) and just return whether it has the passed in flag.

I get that that's just not how it works and that there's plenty technical limitations on this, I just thinkg that it's a bit counterintuitive.

Regardless, thank you for the insight!

eli-darkly commented 2 years ago

I would expect the second call see that a user with the key already exists (since it knows not to create it again) and just return whether it has the passed in flag

It's not exactly that the SDK "knows not to create it again." The SDK doesn't actually "create" users; it sends analytics data to LaunchDarkly, which includes user attributes. It does have some deduplication logic to avoid sending user data for the same user key every single time it does an evaluation, but that is more of a simple key-caching thing— like, if it has seen the same key within the last few minutes, it will not re-send the data. But that also means that in your example of setting different user attributes at different times for the same key, there is no guarantee that the SDK will actually send both sets of attributes to LD; if you did the one with just a key first, and then you did one with both key and email soon afterward, the latter might not ever be included in the analytics data.

But the other reason your idea doesn't really make sense in the LD model is that you used the phrase "whether it has the passed-in flag". LD does not have a notion of a user having a flag; that is not a property of a user (and, a flag is not necessarily a boolean anyway, it could have many possible values, so it's not just "have" vs. "not have"). Instead, there is just the answer to the question "what do you get if you evaluate this flag for such-and-such user properties." The evaluation logic is rerun each time, based only on the input parameters and the flag configuration— that's what I meant about the behavior being deterministic. If it also depended on some (possibly different) parameters that you had used in a previous variation call, then an individual evaluation would be non-deterministic, and that's highly undesirable for a feature flag system. You don't want a piece of code in one part of your application to change the flag results for some other part of your application just by having done an evaluation. Each one is responsible for its own parameters.

Also, if you think about how many flags and users can exist in an LD environment, and the fact that flags can be updated, you can see why it would be very impractical for the SDK to try to remember "whether [this user] has the passed-in flag." There is not just one flag, there could be thousands, and a web application can be servicing requests for thousands of users at any given time. So, if we did not rerun the evaluation logic each time, we would have to be storing potentially millions of combinations of previous variation results in memory... and, any one of those could become out of date at any moment if the SDK received a flag update.

For all of those reasons, a "user" in the SDKs does not mean a persistent entity that lives in the SDK. It is a container for parameters to an evaluation, period.

cuzox commented 2 years ago

The evaluation logic is rerun each time, based only on the input parameters and the flag configuration— that's what I meant about the behavior being deterministic

If the flags are evaluated every time from the given attributes, how can it be deterministic when you have percentage rollout configured?

eli-darkly commented 2 years ago

If the flags are evaluated every time from the given attributes, how can it be deterministic when you have percentage rollout configured?

Rollouts don't use a random number generator; they compute a one-way hash value from the user key (or other attributes, depending on the flag configuration), and then if you have for instance a 30%-70% division in your rollout, that corresponds to a cutoff of (30% * the maximum hash value). The distribution of hash values is fairly even and unpredictable for any given large set of inputs, so the effect is that the users are assigned pseudo-randomly— but consistently, since the inputs are only the user properties and the flag configuration.

eli-darkly commented 2 years ago

Another thing to keep in mind, on this general subject of predictable evaluations and the SDKs not retaining user state, is that there is not just one SDK instance evaluating flags. A webapp or service will likely have many instances of the app running, each with its own SDK client— possibly including different services written in different languages— and, if there is any client-side usage of LD (e.g. in mobile apps or front-end JS code), flag evaluations for those are done by the LD service endpoints rather than by the client-side SDK. So if we did something like using a random-number generator for rollouts, or having SDKs remember user properties they had previously gotten from the app code, there would be no way to keep that state consistent across all of the different places where a particular flag might be getting evaluated for a particular user.

cuzox commented 2 years ago

Awesome, I got it! Thank you for taking the time to break things down. I understand the reasoning behind the implementation and I can see how the current approach is a good one. After interacting with the SDK as intended things are working as expected. Again, appreciate your thoroughness 🙏🏽