harness / ff-dotnet-server-sdk

.net Server SDK for integrating with Harness Feature Flag service.
https://harness.io/
Apache License 2.0
5 stars 10 forks source link

Package Version possible regression issue. Version 1.1.9 works, Version 1.7.1 does not. #139

Open Michael-ICS opened 1 month ago

Michael-ICS commented 1 month ago

My team has created an abstraction / wrapper helper class called FeatureFlagsService to help us with multiple projects using our harness instance. This simplifies our development process so that we can pass in a "base" key value with sub-feature being specified.

An example would look like: "MyApp_EnableNewLoginFunction" By setting the base "MyApp" in appsettings.json all we have to do is specify the second part "EnableNewLoginFunction" and call our wrapper / helper method as an injected class within our project structure.

Full code for "IFeatureFlagsService" and "FeatureFlagsService" at bottom of this post.

[ApiController]
public class FeatureFlagsController : BaseApiController
{
    private readonly IFeatureFlagsService _flagService;
    private readonly ILogger<FeatureFlagsController> _logger;

    public FeatureFlagsController(
        IFeatureFlagsService flagsService,
        IControllerDependencyBundle commonDependencies,
        ILogger<FeatureFlagsController> logger) : base(commonDependencies)
    {
        _flagService = flagsService;
        _logger = logger;
    }

...

}

From here we can then reference our _flagService instance injected by the DI container service.

What we are running into is when I attempted to update from 1.1.9 to 1.7.1 we are running into an issue where this pattern no longer works. I have attempted to dive in and look at the code provided in the repo and I have not yet seen where the issue is coming from. Hopefully, you may be able to identify the issue and why version 1.1.9 works and 1.7.1 does not.

Thank you for the help and review on this issue.

Setup and repo steps:

  1. Create a .NET 8 Console Application
  2. Import ff-dotnet-server-sdk version 1.1.9 nuget package
  3. Replace the Program.cs contents with the following.
  4. Configure Environment Variables and Harness instance with a flag set boolean as true when enabled.
  5. Run the code. (This will succeed and return "True")
  6. Update the ff-dotnet-server-sdk version to 1.7.1
  7. Do a clean rebuild of the project.
  8. Run the code. (This will fail with a warning returning "False")
False
warn: io.harness.cfsdk.client.api.InnerClient[0]
      SDKCODE(eval:6001): SDK Not initialized - Failed to evaluate Boolean variation for default, flag harnessappdemodarkmode and the default variation False is being returned

Example Code below

using io.harness.cfsdk.client.dto;
using io.harness.cfsdk.client.api;

namespace Harness;

internal class Program
{
    public static String apiKey = Environment.GetEnvironmentVariable("FF_API_KEY");
    public static String flagName = Environment.GetEnvironmentVariable("FF_FLAG_NAME") is string v && v.Length > 0 ? v : "harnessappdemodarkmode";

    static void Main(string[] args)
    {
        // If I use <PackageReference Include="ff-dotnet-server-sdk" Version="1.7.1" /> this call will fail.
        // If I use <PackageReference Include="ff-dotnet-server-sdk" Version="1.1.9" /> this call will work.
        var _featureFlagsService = new FeatureFlagsService(new HarnessSettings() { ApiKey = apiKey });
        Console.WriteLine(_featureFlagsService.GetFeatureFlagExact(flagName));
    }
}

public class HarnessSettings
{
    public string ApiKey { get; set; }
    public string KeyNameStart { get; set; }
}

public interface IFeatureFlagsService
{
    bool GetFeatureFlagByConvention(string featureFlagName, bool defaultValue = false, bool global = false);
    bool GetFeatureFlagExact(string featureFlagName, bool defaultValue = false);
}

public class FeatureFlagsService : IFeatureFlagsService
{
    private readonly ICfClient _cfClient;
    private readonly HarnessSettings _settings;

    public FeatureFlagsService(ICfClient cfClient, HarnessSettings settings)
    {
        _cfClient = cfClient;
        _settings = settings;
    }

    public FeatureFlagsService(HarnessSettings settings)
    {
        _settings = settings;

        var harnessConfig = Config.Builder()
            .SetAnalyticsEnabled(true)
            .Build();

        CfClient.Instance.Initialize(settings.ApiKey, harnessConfig).Wait();

        _cfClient = CfClient.Instance;
    }

    public bool GetFeatureFlagByConvention(string featureFlagName, bool defaultValue = false, bool global = false)
    {
        var fullFlagName = global ? $"global_{featureFlagName}" : $"{_settings.KeyNameStart}_{featureFlagName}";
        return GetFeatureFlagExact(fullFlagName, defaultValue);
    }

    public virtual bool GetFeatureFlagExact(string featureFlagName, bool defaultValue = false)
    {
        return _cfClient.boolVariation(featureFlagName, Target.builder()
            .Name("default")
            .Identifier("default")
            .build(), defaultValue);
    }
}
Michael-ICS commented 1 month ago

I did some further research and found that the issue that I am having happened from 1.3.0 to 1.4.0. Where version 1.3.0 works with my specified code sample and version 1.4.0 does not and then returning the warning and error message.

erdirowlands commented 1 month ago

Hi @Michael-ICS,

I've been able to reproduce this issue and can confirm there is a regression when using Wait on the Task returned by CfClient.Instance.Initialize

For example:

CfClient.Instance.Initialize(settings.ApiKey, harnessConfig).Wait();

A bit of background on synchronous init in the SDK: in version 1.4.0, we deprecated the old synchronous initialization methods and introduced a new method called WaitForInitialization with an optional timeout argument. You can find more details in the 1.4.0 release notes.

It's highly likely that the issue with using Wait on the Task was always present. It didn’t actually wait for the SDK to finish initializing, but it appeared to work because the SDK initialized slightly slower in < 1.4.0. With the improvements made to the SDK, initialization happens faster, which can result in the SDK not being fully initialized by the time a variation is evaluated, leading to the default variation being returned (and the error log you are seeing).

Whether or not the issue existed before, it appeared to work in versions prior to 1.4.0, so this is indeed a regression.

The quickest way to resolve this is by updating your wrapper to use WaitForInitialzation which will block until the SDK has fully loaded all config from Harness. I also recommend to use the timeout for added safety:

CfClient.Instance.Initialize(apiKey, Config.Builder().LoggerFactory(loggerFactory).Build()); // No need to call Wait as synchronous init is handled by WaitForInitialzation
_cfClient = CfClient.Instance;
var isInit = _cfClient.WaitForInitialization(30000); 
if (!isInit) Console.WriteLine("Failed to initialize the SDK within 30 seconds");

Alternatively, a better approach and the one I recommend that avoids dealing with the Task would be to use an instance of CfClient directly, instead of the singleton returned by CfClient.Instance. You could do:

_cfClient = new CfClient(apiKey, Config.Builder().LoggerFactory(loggerFactory).Build());
var isInit = _cfClient.WaitForInitialization(30000); 
if (!isInit) Console.WriteLine("Failed to initialize the SDK within 30 seconds");

The key here is using WaitForInitialization to ensure the SDK is fully initialized before making any evaluations.

Again I understand that this is a regression and I apologise. Please let me know if the above works for you. Once you do, I will speak with the team about what we want to do with the Task based Initialize method.

Michael-ICS commented 1 month ago

No worries, thank you for the help and direction on this. I'll make those updates to our library.