FileDataSource no longer emits flag changes on file updates

[fhawkes]

Describe the bug
The FileDataSource no longer correctly tracks streaming (realtime) changes to flags when updating the json file on disk.

To reproduce
Create a LD client utilizing the FileDataSource with autoUpdate turned on and pointing to one json file on disk that looks similar to this

{
    "flagValues": {
        "coolFlag": false
    }
}

While creating the client subscribe to flag changes like so

this.client.on('update', (flagChange) => {
	// Expecting the flag change to be emitted, it is not
    logger.trace(`LaunchDarkly flag changed ${flagChange.key}.`);
});

Expected behavior
When changing the value of an existing flag in the json file, I expect events to be emitted by the LD client informing us about flag changes.

Logs
If applicable, add any log output related to your problem.

SDK version
@launchdarkly/node-server-sdk 9.0.2

Language version, developer tools
Node v18.17.1

OS/platform
macOS 13.5.2

Additional context
I did some digging and it appears that in the most recent version of this package we hardcode the flag version to always be 1 here

js-core/packages/shared/sdk-server/src/data_sources/FileDataSource.ts

Line 24 in 388c287

version: 1,

This prevents the DataSourceUpdates code here from ever detecting a change with the flag because is specifically looks at the version which will always be equal.

js-core/packages/shared/sdk-server/src/data_sources/DataSourceUpdates.ts

Line 162 in 388c287

if (newValue && oldValue && newValue.version <= oldValue.version) {

In previous versions of the sdk that were no typescript the version flag was not included. We could pull the version in from the the existing flag in the feature store if it exists, but I didn't see a clean way to fetch that value at a glance.

[kinyoklion]

Hello @fhawkes,

Thank you for pointing this out. We will look into it.

As a work around the more defined flag format, with flags: should work if you include and increment the version. I know this is not ideal.

{
  "flags": {
    "coolFlag": {
      "key": "coolFlag",
      "on": true,
      "fallthrough": {
        "variation": 0
      },
      "variations": [
        false,
        true
      ],
      "version": 0
    }
  }
}

Thank you,
Ryan

Filed internally as 224098

[fhawkes]

@kinyoklion thanks for your response, If I have time I'll submit a PR for it. The biggest hurdle I was facing is that the types being passed around aren't consistent with the objects being used.

For instance the FileDataSource is constructed with a FeatureStore, but it's actually an instance of DataSourceUpdates getting passed in.

That means the actual unit test for FileDataSource aren't quite right, but they'd need to change drastically in-order to use the DataSourceUpdates instead of passing in a FeatureStore

[kinyoklion]

Yeah, this is an oversight in the updates moving it to typescript. Conceptually an update processor, the component this acts as, is a write only mechanism. It isn't intended to have access to what is already in the store.

We've approached this a few different ways among the various SDKs. I think we have typically decoupled the version from anything real. Making a map of the flags and version markers for each. Then just incrementing that version whenever a change is detected.

[fhawkes]

@kinyoklion Yeah this is exactly what I have in a branch locally, I just can't get through the unit tests if I actually inject DataSourceUpdates into the FileDataSource instead of the FeatureStore directly.

It quickly turned into a head scratcher because of the callbacks.

Just to be clear though, we use the FileDataSource in our CI test to simulate feature flag changes. Right now it's super simple just to update the flag value in the old sdk with the expectation that the version doesn't matter.

It would be a nontrivial refactor on our end to move to the more descriptive flag file definition.

Sadly this blocks us from upgrading to the new sdks :/

[kinyoklion]

@fhawkes This has been released in 9.0.4. Closing this issue. Let us know if you encounter other problems.