Built-in Attributes break if in 5.x if you have previously been setting them as custom attributes in 4.x

[bschaeffer]

Describe the bug We recently became aware of the EOL policy for the go sdk. Since we're running a version of the SDK that has been unsupported for whatever reason for over 1y4m, we decided it was time to upgrade from 4.13 (?) to 5.4. When we did, all targeting by email was broken. Under the 4.13 SDK, this behavior worked fine. We set email as a custom attribute and our users were able to create flag rules and target variations using that value. When we upgraded to 5.4, this functionality broke. Variations targeting specific email addresses were ignored. Obviously the DSL has changed, but semantically, we are calling the same APIs.

4.13:

import ldclient "gopkg.in/launchdarkly/go-server-sdk.v4"

atts := map[string]interface{}{"email": "foo@bar.com"}
user := ldclient.NewUser("somekey")
user.Custom = &atts

5.4

atts := map[string]interface{}{"email": "foo@bar.com"}
valueMap := ldvalue.CopyArbitraryValueMap(atts)
user := lduser.NewUserBuilder(target.GetKey()).CustomAll(valueMap)

There is basically no reason to expect the 4.13 code to work differently from the 5.4 code, but they do. We have identified the solution, which is to call builder.Email("foo@bar.com"), but that this call is required or that the other will not work is not apparent at all.

To reproduce

• Use the 5.4 SDK
• Set rules for a flag variation targeting a specific email that is different than the default variation
• Set that email as a custom attribute: builder.Custom("email", ldvalue.String("foo@bar.com"))
• Evaluate the flag.
• Observe that the variation is not matched and it falls through to the default for the flag.

Expected behavior

The old behvior continues to work and the flag is matched on email if its sent as a custom attribute.

SDK version 5.4

Language version, developer tools go 1.18

OS/platform mac/debian

[eli-darkly]

I think there may have been something going on with your older code that was not quite what you thought. That is: I don't see how it could have ever worked as you describe.

First, the code sample you gave for 4.x is not valid code: there was no lduser package in the 4.x SDK, and there was never a function called NewTarget. But, that aside— assuming those are just typos based on writing it from memory, and changing custom to Custom— the 4.x SDK should not have ever been able to reference a custom attribute called email when evaluating a flag, any more than the 5.x SDK is. It would not prevent you from putting that value into the Custom map, but the value would be ignored. If you look at the method User.valueOf, which is what the evaluation engine always calls to get a user attribute value by name, you'll see that the hard-coded built-in attribute names always result in returning the dedicated top-level fields; it only gets a value from Custom if it does not match any of those names.

Now, if it did indeed do what you described— if I'm wrong about the logic that's being used, even though I think it's fairly straightforward— then that would have been a bug in the 4.x SDK, and patching the 5.x SDK to behave the same way would be a regression. That's because the LaunchDarkly user data model is not specific to the Go SDK; it's a standard across all of them, and part of that standard is that custom attributes are not supposed to have the same names as built-in ones, and will be ignored if they do. LaunchDarkly— that is, the LaunchDarkly evaluation service that is used by all client-side SDKs— will always ignore such attributes if they are present inside of custom, and so will all of the other server-side SDKs. So it would not be an option for us to retain the incorrect behavior just in order to remain consistent with Go SDK 4.x (if in fact that was the behavior of 4.x, which, again, I'm doubtful of).

[eli-darkly]

Also, more generally: since we follow the semantic versioning standard, we reserve the right to make breaking changes in a major version release such as 5.0.0. It's unfortunate that the "custom attributes can't have the same names as built-in ones" rule was not documented adequately in the Go SDK 4.x docs, but there was also no documentation saying that that should work (and there is a logical reason to think it shouldn't: if two attributes, one built-in and one custom, could have the same name, how would the flag evaluator know which one to use?), so I would disagree that "There is basically no reason to expect the 4.13 code to work differently from the 5.4 code".

Again, I am not convinced that this was in fact a functional change. But if it was, then the change was in the direction of becoming consistent with all other LaunchDarkly products, and consistent cross-platform behavior is essential to us because many of our customers use the same flags on multiple platforms. Also, even if it were possible to treat the Go SDK as independent in that regard, every customer using the Go SDK since the 5.0 release has had the current behavior, in which a flag rule that references email will never use a custom attribute value... so if we changed that, then that would be a breaking change for our entire Go customer base (at least, those using currently supported versions).

[bschaeffer]

but there was also no documentation saying that that should work

Give me a break.

[bschaeffer]

Now, if it did indeed do what you described— if I'm wrong about the logic that's being used, even though I think it's fairly straightforward— then that would have been a bug in the 4.x SDK, and patching the 5.x SDK to behave the same way would be a regression.

That is super duper easy for you to test instead of writing four paragraphs calling me a liar.

[bschaeffer]

FWIW I updated the 4.13 to match exactly how we are building targets pre upgrade. All I am asking for I guess is that this should be called out somewhere by somebody as a breaking change, because its functionality that existed in 4.x that is broken 5.x.

since we follow the semantic versioning standard, we reserve the right to make breaking changes in a major version release such as 5.0.0

^ This is on top of your EOL policy is one of the most customer hostile things I have ever come across from an enterprise software company

[louis-launchdarkly]

Hello @bschaeffer, I understand it must have been frustrating for you when the SDK suddenly stopped working for you after the upgrade.

Thanks to your report, we found out that there was a bug before 4.16 that the SDK drilled down into the Custom attributes when the SDK should not do that. This was fixed in 4.16 but we didn't note that on the changelog. We will update the changelog to address this fix.

As you have suggested, we are also looking into adding the document to call out that a custom attribute that has the same name as the built-in attribute would result in unexpected behavior.

Using the builder.Email method is the correct way to set the "email" for the user.

[louis-launchdarkly]

https://github.com/launchdarkly/go-server-sdk/pull/72

[bschaeffer]

Thanks for the reply! We have updated our implementation and looks like will be rolling out our update ASAP.

[eli-darkly]

@bschaeffer My manager kindly took over this conversation, but I'd like to say for the record that at no time did I call you a liar. I take such an accusation pretty seriously since a large part of my job is to field problem reports from customers, and often investigating those reports involves discrepancies that are hard to explain, as in this case. That doesn't mean anyone isn't being honest; it means that there is some confusing thing that needs a further look. Which we did, and as Louis explained, the answer turned out to be somewhat different than your original guess: that is, this was not something that got "broken in 5.x", it was a change that had happened much earlier, and that's why I couldn't see the behavior you described when I tested with a less old 4.x version.

I am sorry for the confusion, but please understand that responses like mine are not personal in any way, and that I was in fact trying to investigate your report and to clarify a related point that you made about breaking changes.

[bschaeffer]

@eli-darkly Thank you for pinging me here. I want to apologize for taking it personally and calling you out (unwarranted) as a result. I took some other stuff out on this issue and I regret it. Thank you and your team for investigating the issue. I'll do better in the future when opening and discussing these issues.