harness / ff-dotnet-server-sdk Goto Github PK

I'd like to get the changes from #9 incorporated into a nuget package that I can pull in via automation build. If we could get that bugfix released, that'd be great, thanks!

https://github.com/harness/ff-dotnet-server-sdk/blob/main/client/api/Config.cs#L58

public int WriteTimeout { get => WriteTimeout;  }

should be

public int WriteTimeout { get => writeTimeout;  }

Hello,
I'm looking for some additional guidance on the usage of this library in a modern ASP.NET app. Specifically, I was looking to wrap this in a generic Service that I register with Dotnet's DI. Is a Singleton the only pattern we can use with this library? Do you have any examples of registering it with DI? Or do you recommend using the Singleton built into the library?

If we create a new CfClient to reset everything, the long-running event processor associated with the Disruptor created here keeps running. It won't get any new events, but it can't be cleaned up by GC. Ideally the reference to the Disruptor would be kept so when MetricsProcessor.Stop is called, it can call the Disruptor Halt method.

The Client is logging what appears to be recoverable "errors" into our logging system which then triggers alerts in our downstream application monitoring systems. There is no easy way for us to filter out this noise since the logs do not identify themselves as originating from this Client.

This library is misusing Serilog and logging in general. Specific violations include:

1. Logging should be optional in 3rd party libraries. The consumers of your library should have the ability to disable logging completely OR use a logging library other than Serilog, such as the ubiquitous Microsoft.Extensions.Logging.Abstractions library (of which Serilog has an implementation).
2. Every logged message should have a SourceContext property used to identify the source of the log. Logging libraries have built-in support for filtering based on the value of the SourceContext property.
3. Do not use string interpolation when logging messages. Instead, give the logger a message template and template arguments and let the logger format the message -- if necessary.
4. Use the logger's built-in support for exceptions. When logging an exception, the logger has an overload which takes the exception as the first argument. The logger can then format and destructure the exception if configured to do so.
5. Avoid logging expected or recoverable errors as Error. Log them as Warning or Information instead. Reserve Error logs for truly erroneous behavior.

HarnessConnector.MetricHttpClient and ApiHttpClient set the timeout using TimeSpan.FromSeconds.

The value used is config.ConnectionTimeout.

The default value of the backing connectionTimeout field is 10,000. The comments nearby say:

/** timeout in milliseconds to connect to CF Server */

Add this to gitignore

The currentStream is null in polling mode, causing this to throw a null ref.

If an invalid API key is provided, this sdk will retry forever using that bad API key instead of throwing an exception. This makes it difficult to use in our APIs since calls will just hang if bad config is provided.

If it was possible to expose the auth error and stop the timer that would be great. Maybe a new config value for max number of retries as well.

We're not exactly sure when this started happening, but it seems pretty recent. When a target is added or removed from a target group, the change is received by the SDK but ends up ignored, due to the segment retrieved from Harness having a version of zero, instead of the version indicated by the streaming message. Screenshots below illustrate what I'm seeing: