Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

FlagTracker does not pick up flag changes in flagdata.json #224

Closed
seangatewood opened this issue Feb 17, 2021 · 5 comments
Closed

FlagTracker does not pick up flag changes in flagdata.json #224

seangatewood opened this issue Feb 17, 2021 · 5 comments

Comments

@seangatewood
Copy link

Describe the bug
When reading flags from a file, any listeners registered using addFlagValueChangeListener are not triggered when flags in the file change, unless you also change the flags' version property. We believe this is not ideal if the flag is in the flags map, and is not possible if the flag is in the flagValues map.

To reproduce
I made a quick test:

  @Test
  void test_flagTracker_calls_listener_when_reading_from_file() {
    String filepath = System.getProperty("user.home") + "/flagdata.json";
    LDClient ldClient = getLdClient(filepath);
    FlagTracker tracker = ldClient.getFlagTracker();
    LDUser ldUser = new LDUser("key");
    String flagKey = "my-boolean-flag-key";

    AtomicBoolean listenerWasCalled = new AtomicBoolean(false);

    tracker.addFlagValueChangeListener(flagKey, ldUser, changeEvent -> {
      listenerWasCalled.set(true);
    });
    
    boolean initialValue = ldClient.boolVariation(flagKey, ldUser, false);
    setFeatureInJsonFile(flagKey, !initialValue, filepath);
    await().atMost(15, TimeUnit.SECONDS).until(() -> ldClient.boolVariation(flagKey, ldUser, false) != initialValue);

    assertThat(listenerWasCalled.get()).isTrue(); // fails
  }

  private LDClient getLdClient(String filepath) {
    LDConfig config = new LDConfig.Builder()
        .dataSource(FileData.dataSource().filePaths(filepath).autoUpdate(true))
        .events(Components.noEvents())
        .build();
    return new LDClient("nonnull sdk key", config);
  }

Only the last assertion fails, with the following in ~/flagdata.json:

{
  "flags": {
    "flag-key-1": {
      "key": "flag-key-1",
      "on": true,
      "variations": [
        "a",
        "b"
      ],
      "version": 1
    }
  },
  "segments": {
    "segment-key-1": {
      "key": "segment-key-1",
      "includes": [
        "user-key-1"
      ],
      "version": 1
    }
  },
  "flagValues": {
    "my-string-flag-key": "value-1",
    "my-boolean-flag-key": true,
    "my-integer-flag-key": 3
  }
}

If you set a breakpoint here, you can see there is a difference between the flag in oldData vs in fullDataSetToMap(allData), even though the versions are the same.

oldData

image

fullDataSetToMap(allData)

image

So there are no changed items sent into sendChangeEvents:
image

Expected behavior
It would be nice if we could detect the change there in computeChangedItemsForFullDataSet, so the Tracker can pick up flag changes in the json file.

Logs
(I just see this from the client)

[Test worker] INFO com.launchdarkly.sdk.server.LDClient - Waiting up to 5000 milliseconds for LaunchDarkly client to start...

SDK version
We are using version 5.2.2

Language version, developer tools
Java 8

OS/platform
MacOS 10.15.7

Additional context
We are using the "Reading flags from a file" feature to provide our developers with an isolated environment for testing/prototyping purposes. If this can't be fixed in the SDK, we may have to set up our own polling to detect changes in the file to enable our developers to test features that update their state with change listeners.

@eli-darkly
Copy link
Contributor

You're right that the mechanism FileData is currently using to inject flag data does not play well with FlagTracker, because it's not providing version numbers. I don't think we would want to address this by making FlagTracker diff the entire flag configuration to detect changes even if the version number hasn't changed— flag configurations can be very large. But it might not be too hard to make FileData generate version numbers; I'll look into it.

@eli-darkly
Copy link
Contributor

eli-darkly commented Feb 17, 2021

The fix I'm thinking of would cause the SDK to signal a flag change for every flag whenever the data file was updated (since the file data source would set all of the flag versions to 1 on the first load, 2 on the second load, etc.). So if you were using addFlagChangeListener (not addFlagValueChangeListener), you would get false positives— which are already inherent in the LaunchDarkly update model, since there are lots of ways minor details of a flag configuration could change that wouldn't actually be significant. But using addFlagValueChangeListener as you're doing would correctly only notify you of changes that really affect the value.

@seangatewood
Copy link
Author

Thank you for the quick response! Your solution does sound better to me, and would probably be more localized within FileData as well. 🙂

LaunchDarklyCI pushed a commit that referenced this issue Feb 19, 2021
…eta-2

further fix to packaging test for beta versions
@eli-darkly
Copy link
Contributor

We've released version 5.2.3, which should fix this.

@seangatewood
Copy link
Author

Verified on my end. 🎉 Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants