Unleash / unleash-client-node

Unleash client SDK for Node.js
https://docs.getunleash.io
Apache License 2.0
210 stars 71 forks source link

The future of `instanceId`, semver and deprecation timelines #645

Closed nagyv closed 2 weeks ago

nagyv commented 1 month ago

Describe the feature request

I'd like to start a discussion about the future of the instanceId parameter, and if Unleash decides to deprecate it what alternatives and timelines will it offer?

Background

The accidental removal of the user-provided instanceId was a breaking change for users who use Unleash with GitLab.

Accidents happen, and I don't want to blame anyone here. You are doing an awesome job in many other ways. Thank you for your hard work on Unleash!

Solution suggestions

Instead of suggesting a solution, I'd like to ask the rationale behind changing the current behavior and the future plans Unleash might have with instanceId.

sighphyre commented 1 month ago

Hey @nagyv,

The changes should have been rolled back in all the affected SDK and the Proxy as well. There should be equivalent releases on latest for each SDK that restores the instanceId. Huge apologies for that.

Accidents happen, and I don't want to blame anyone here. That's very kind of you, you're welcome to blame me though, haha. This was mostly my plan and I was definitely overeager here to be completely honestly with you, I hadn't realized this was going to cause problems with the Gitlab integration, that's on me.

I can give you our high level plan going forward - which is not to break compatibility with Gitlab without a lot of advance warning and communication. Likely we won't break compatibility at all but I can't make promises about a distant future we haven't planned for yet.

To give you the nitty gritty details so you can follow my thought processes (and hopefully feel a little less worried that we're going to set fire to your setup!), Unleash has no formal and ongoing relationship with Gitlab. Gitlab uses our SDKs to support their feature flag implementation which is Unleash-like but not Unleash (insofar as we're aware). We love that, we think it's awesome and we want to support that but the lack of ongoing communication means sometimes ideas develop in different ways and the integration rots.

I'd like to ask the rationale behind changing the current behavior

The reason we want to change the instanceId is because the way this handled today in the SDKs is blocking some things that we'd like to do in the Unleash side around how we can surface insights into SDKs behaviors in the wild. Effectively allowing a public override here means we're never confident that SDKs are actually unique or if there's a thousand instances out in the wild with a hardcoded value. It makes doing things like drawing graphs of unique, connected instances through a daisy chained Edge setup really hard. So this is more about getting a unique instance id than it is about taking away the existing one.

future plans Unleash might have with instanceId

We'd still like to get a truly unique instance id for each our SDKs, that being said, we're not planning on changing the behavior or accessibility of the instanceId field in the SDKs themselves. Not right now at least. instanceId will remain untouched going forward for the foreseeable future - we can get what we need without changing that field. So the plan is effectively this:

Gosh I hope that answers your question, but I'm going to leave this issue open for a bit to give folks a chance to poke us with more questions if they need

Again, huge apologies for the poorly planned breaking change

nagyv commented 1 month ago

Thanks @sighphyre for the detailed answer and candidness. I appreciate it.

On our end, we learned that customers sometimes require 12+ months to migrate when we deprecate something. We are not against breaking changes, but would like to be aware of them in due time. We try to follow your releases the best we can.

Last year I spoke with part of the Unleash leadership, so I think we can connect whenever needed.

Thank you once again for your great work!

naderghanbari commented 1 month ago

As a user of Gitlab Feature Flags, Unleash Node Client, Unleash Java SDK, and Unleash Proxy, I stumbled upon this incompatibility the very first day version 6 of the node client was released.

I understand the changes have been reverted, and I'm not suggesting a general workaround either.

But allow me to report that the following is working fine in our node-client version 6.0.0 (after the removal of instanceId from the config).

import {UnleashConfig} from "unleash-client"

const config: UnleashConfig = 
{
  url: "https://gitlab.com/api/v4/feature_flags/unleash/YOUR_GITLAB_PROJECT_ID",
  appName: "YOUR_ENV_NAME",
  customHeaders: {
    "UNLEASH-INSTANCEID": "YOUR_INSTANCE_ID_PER_GITLAB_INSTURCTIONS"
  },
}

Unleash Proxy (versions that use node-client versions that have instanceId dropped), however, can not benefit from this workaround. If Unleash Proxy provided a way to provide custom headers, then the same would work I suppose.

In any case, I guess what I'm saying is that custom headers can be considered as a middle ground, but it would neverthelss be a breaking change.