-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add Dependency Injection and Hosting support for OpenFeature #310
base: main
Are you sure you want to change the base?
Changes from 97 commits
963b54c
a803429
79ee277
9f3c40e
6786bd8
f373eb0
1f86298
f1039a2
5905cc6
6c3c15e
bd20d40
441aa4f
fa04665
fcea8a6
fc8dcab
0a996aa
76e1816
84dd0e0
572d67b
d7e5fbb
f60b5d8
8b59e53
c644df6
f4f108b
43bcde9
47e87c2
3650de2
0027489
5638163
5de7ceb
2e99210
880df92
9258706
19664b0
9c21943
a23c5ea
7906a3b
1850159
63ead66
70708ec
d615b04
ad95765
ce23cdc
7716f4a
12d48c2
1f0468e
d480da4
cc5f5bc
90d172c
2072e45
c503b30
9a5a343
be43fcc
84f8e18
94f4d66
756a7f7
36b5f8c
c9339ec
980d0be
d4a5e3d
8387def
5077a1c
95e8ddc
f157496
49991ec
5bc2c47
9ea9b01
4066b89
367518b
70c269f
b20bc13
c251f56
8b2ab4b
3800447
e111714
cb63b1c
0e53494
dc765e6
4f80593
0a76193
ac98ddd
4af39ce
0fa6dc3
926c0d4
9b8d9f2
aa488ae
1f8e9e4
3aaa7df
ffef1ac
1acb4fa
96889dd
54c145e
55582d7
8ebdad4
3089129
fe5e072
10ea9f1
3436db5
5738f3c
0ce727c
e8350d7
eaf0319
7916d14
af78c5a
4fc100e
d536515
916f60a
387a88b
4ac7f20
17a66ca
c424b2a
1de9096
ca44115
3d33caa
dfd66d1
d169b0b
ec6a81b
a853b52
6849899
73bb930
749b18d
f6e072d
b6a8e2d
ce56e84
f1b8a9b
90481c0
8252e47
9704d2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
namespace OpenFeature; | ||
|
||
/// <summary> | ||
/// Defines the contract for managing the lifecycle of a feature api. | ||
/// </summary> | ||
public interface IFeatureLifecycleManager | ||
{ | ||
/// <summary> | ||
/// Ensures that the feature provider is properly initialized and ready to be used. | ||
/// This method should handle all necessary checks, configuration, and setup required to prepare the feature provider. | ||
/// </summary> | ||
/// <param name="cancellationToken">Propagates notification that operations should be canceled.</param> | ||
/// <returns>A Task representing the asynchronous operation of initializing the feature provider.</returns> | ||
/// <exception cref="InvalidOperationException">Thrown when the feature provider is not registered or is in an invalid state.</exception> | ||
ValueTask EnsureInitializedAsync(CancellationToken cancellationToken = default); | ||
|
||
/// <summary> | ||
/// Gracefully shuts down the feature api, ensuring all resources are properly disposed of and any persistent state is saved. | ||
/// This method should handle all necessary cleanup and shutdown operations for the feature provider. | ||
/// </summary> | ||
/// <param name="cancellationToken">Propagates notification that operations should be canceled.</param> | ||
/// <returns>A Task representing the asynchronous operation of shutting down the feature provider.</returns> | ||
ValueTask ShutdownAsync(CancellationToken cancellationToken = default); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
using Microsoft.Extensions.DependencyInjection; | ||
using Microsoft.Extensions.Logging; | ||
|
||
namespace OpenFeature.Internal; | ||
|
||
internal sealed partial class FeatureLifecycleManager : IFeatureLifecycleManager | ||
{ | ||
private readonly Api _featureApi; | ||
private readonly IServiceProvider _serviceProvider; | ||
private readonly ILogger<FeatureLifecycleManager> _logger; | ||
|
||
public FeatureLifecycleManager(Api featureApi, IServiceProvider serviceProvider, ILogger<FeatureLifecycleManager> logger) | ||
{ | ||
_featureApi = featureApi; | ||
_serviceProvider = serviceProvider; | ||
_logger = logger; | ||
} | ||
|
||
/// <inheritdoc /> | ||
public async ValueTask EnsureInitializedAsync(CancellationToken cancellationToken = default) | ||
{ | ||
this.LogStartingInitializationOfFeatureProvider(); | ||
var featureProvider = _serviceProvider.GetService<FeatureProvider>(); | ||
if (featureProvider == null) | ||
{ | ||
throw new InvalidOperationException("Feature provider is not registered in the service collection."); | ||
} | ||
await _featureApi.SetProviderAsync(featureProvider).ConfigureAwait(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might want an optional value for domain, or some other method for working with domain-scoped providers. |
||
} | ||
|
||
/// <inheritdoc /> | ||
public async ValueTask ShutdownAsync(CancellationToken cancellationToken = default) | ||
{ | ||
this.LogShuttingDownFeatureProvider(); | ||
await _featureApi.ShutdownAsync().ConfigureAwait(false); | ||
} | ||
|
||
[LoggerMessage(200, LogLevel.Information, "Starting initialization of the feature provider")] | ||
partial void LogStartingInitializationOfFeatureProvider(); | ||
|
||
[LoggerMessage(200, LogLevel.Information, "Shutting down the feature provider")] | ||
partial void LogShuttingDownFeatureProvider(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
// @formatter:off | ||
// ReSharper disable All | ||
#if NETCOREAPP3_0_OR_GREATER | ||
// https://github.com/dotnet/runtime/issues/96197 | ||
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Runtime.CompilerServices.CallerArgumentExpressionAttribute))] | ||
#else | ||
#pragma warning disable | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
namespace System.Runtime.CompilerServices; | ||
|
||
[AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = false)] | ||
internal sealed class CallerArgumentExpressionAttribute : Attribute | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please consider using PolySharp. It is smart and customizable source generator which will detect what exact feature need to be poly-filled and generate the code for that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion! I’ll definitely take a closer look and see how it can fit into our current approach. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @arttonoyan / @wwalendz-relativity, thanks for bringing this to our attention. I need someone else to confirm if bringing an extra library to use these polyfils is okay. @toddbaert or @beeme1mr ? |
||
{ | ||
public CallerArgumentExpressionAttribute(string parameterName) | ||
{ | ||
ParameterName = parameterName; | ||
} | ||
|
||
public string ParameterName { get; } | ||
} | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
using System.Diagnostics; | ||
using System.Runtime.CompilerServices; | ||
|
||
namespace OpenFeature; | ||
|
||
[DebuggerStepThrough] | ||
internal static class Guard | ||
{ | ||
public static void ThrowIfNull(object? argument, [CallerArgumentExpression(nameof(argument))] string? paramName = null) | ||
{ | ||
if (argument is null) | ||
throw new ArgumentNullException(paramName); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// @formatter:off | ||
// ReSharper disable All | ||
#if NET5_0_OR_GREATER | ||
// https://github.com/dotnet/runtime/issues/96197 | ||
[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Runtime.CompilerServices.IsExternalInit))] | ||
#else | ||
#pragma warning disable | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.ComponentModel; | ||
|
||
namespace System.Runtime.CompilerServices; | ||
|
||
/// <summary> | ||
/// Reserved to be used by the compiler for tracking metadata. | ||
/// This class should not be used by developers in source code. | ||
/// </summary> | ||
[EditorBrowsable(EditorBrowsableState.Never)] | ||
static class IsExternalInit { } | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFrameworks>netstandard2.0;net6.0;net8.0;net462</TargetFrameworks> | ||
<ImplicitUsings>enable</ImplicitUsings> | ||
<Nullable>enable</Nullable> | ||
<RootNamespace>OpenFeature</RootNamespace> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\OpenFeature\OpenFeature.csproj" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo"> | ||
<_Parameter1>$(AssemblyName).Tests</_Parameter1> | ||
</AssemblyAttribute> | ||
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo"> | ||
<_Parameter1>DynamicProxyGenAssembly2</_Parameter1> | ||
</AssemblyAttribute> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<Folder Include="MultiTarget\" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
using Microsoft.Extensions.DependencyInjection; | ||
|
||
namespace OpenFeature; | ||
|
||
/// <summary> | ||
/// Describes a <see cref="OpenFeatureBuilder"/> backed by an <see cref="IServiceCollection"/>. | ||
/// </summary> | ||
/// <param name="services">The services being configured.</param> | ||
public class OpenFeatureBuilder(IServiceCollection services) | ||
{ | ||
/// <summary> The services being configured. </summary> | ||
public IServiceCollection Services { get; } = services; | ||
|
||
/// <summary> | ||
/// Indicates whether the evaluation context has been configured. | ||
/// This property is used to determine if specific configurations or services | ||
/// should be initialized based on the presence of an evaluation context. | ||
/// </summary> | ||
public bool IsContextConfigured { get; internal set; } | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,65 @@ | ||||||
using Microsoft.Extensions.DependencyInjection.Extensions; | ||||||
using OpenFeature.Model; | ||||||
|
||||||
namespace OpenFeature; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
/// <summary> | ||||||
/// Contains extension methods for the <see cref="OpenFeatureBuilder"/> class. | ||||||
/// </summary> | ||||||
public static partial class OpenFeatureBuilderExtensions | ||||||
{ | ||||||
/// <summary> | ||||||
/// This method is used to add a new context to the service collection. | ||||||
/// </summary> | ||||||
/// <param name="builder">The <see cref="OpenFeatureBuilder"/> instance.</param> | ||||||
/// <param name="configure">the desired configuration</param> | ||||||
/// <returns>The <see cref="OpenFeatureBuilder"/> instance.</returns> | ||||||
public static OpenFeatureBuilder AddContext( | ||||||
this OpenFeatureBuilder builder, | ||||||
Action<EvaluationContextBuilder> configure) | ||||||
{ | ||||||
Guard.ThrowIfNull(builder); | ||||||
Guard.ThrowIfNull(configure); | ||||||
|
||||||
return builder.AddContext((b, _) => configure(b)); | ||||||
} | ||||||
|
||||||
/// <summary> | ||||||
/// This method is used to add a new context to the service collection. | ||||||
/// </summary> | ||||||
/// <param name="builder">The <see cref="OpenFeatureBuilder"/> instance.</param> | ||||||
/// <param name="configure">the desired configuration</param> | ||||||
/// <returns>The <see cref="OpenFeatureBuilder"/> instance.</returns> | ||||||
public static OpenFeatureBuilder AddContext( | ||||||
this OpenFeatureBuilder builder, | ||||||
Action<EvaluationContextBuilder, IServiceProvider> configure) | ||||||
{ | ||||||
Guard.ThrowIfNull(builder); | ||||||
Guard.ThrowIfNull(configure); | ||||||
|
||||||
builder.IsContextConfigured = true; | ||||||
builder.Services.TryAddTransient(provider => | ||||||
{ | ||||||
var contextBuilder = EvaluationContext.Builder(); | ||||||
configure(contextBuilder, provider); | ||||||
return contextBuilder.Build(); | ||||||
}); | ||||||
|
||||||
return builder; | ||||||
} | ||||||
|
||||||
/// <summary> | ||||||
/// Adds a feature provider to the service collection. | ||||||
/// </summary> | ||||||
/// <typeparam name="T">The type of the feature provider, which must inherit from <see cref="FeatureProvider"/>.</typeparam> | ||||||
/// <param name="builder">The <see cref="OpenFeatureBuilder"/> instance.</param> | ||||||
/// <param name="providerFactory">A factory method to create the feature provider, using the service provider.</param> | ||||||
/// <returns>The <see cref="OpenFeatureBuilder"/> instance.</returns> | ||||||
public static OpenFeatureBuilder AddProvider<T>(this OpenFeatureBuilder builder, Func<IServiceProvider, T> providerFactory) | ||||||
where T : FeatureProvider | ||||||
{ | ||||||
Guard.ThrowIfNull(builder); | ||||||
builder.Services.TryAddSingleton<FeatureProvider>(providerFactory); | ||||||
return builder; | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,53 @@ | ||||||
using Microsoft.Extensions.DependencyInjection; | ||||||
using Microsoft.Extensions.DependencyInjection.Extensions; | ||||||
using OpenFeature.Internal; | ||||||
using OpenFeature.Model; | ||||||
|
||||||
namespace OpenFeature; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
/// <summary> | ||||||
/// Contains extension methods for the <see cref="IServiceCollection"/> class. | ||||||
/// </summary> | ||||||
public static class OpenFeatureServiceCollectionExtensions | ||||||
{ | ||||||
/// <summary> | ||||||
/// This method is used to add OpenFeature to the service collection. | ||||||
/// OpenFeature will be registered as a singleton. | ||||||
/// </summary> | ||||||
/// <param name="services"><see cref="IServiceCollection"/></param> | ||||||
/// <param name="configure">the desired configuration</param> | ||||||
/// <returns>the current <see cref="IServiceCollection"/> instance</returns> | ||||||
public static IServiceCollection AddOpenFeature(this IServiceCollection services, Action<OpenFeatureBuilder> configure) | ||||||
{ | ||||||
Guard.ThrowIfNull(services); | ||||||
Guard.ThrowIfNull(configure); | ||||||
|
||||||
services.TryAddSingleton(Api.Instance); | ||||||
services.TryAddSingleton<IFeatureLifecycleManager, FeatureLifecycleManager>(); | ||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of having branching in all places where we would like to consume There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don’t register the EvaluationContext as a singleton because the client can operate without any context or with null. Under the hood, it checks and sets the Empty context as needed. The purpose of IsContextConfigured is to avoid redundant service resolution and nullability checks. If no context is registered, we bypass fetching the EvaluationContext entirely, preventing unnecessary GetService or GetRequiredService calls. Also, when the context is registered, I register it as transient. The reason for this is to control its lifecycle within the scope of the class where it’s used—in this case, IFeatureClient. Will be confusing to register the context as transient in one case and Empty as singleton in another. However, this approach ensures proper lifecycle management based on the context's availability and usage. Additionally, the IsContextConfigured check is only performed once during the service provider building process. In scenarios where context resolution isn’t required, the IFeatureClient can still function without a context, but engineers can always explicitly use ExecutionContext.Empty if needed |
||||||
var builder = new OpenFeatureBuilder(services); | ||||||
configure(builder); | ||||||
|
||||||
if (builder.IsContextConfigured) | ||||||
{ | ||||||
services.TryAddScoped<IFeatureClient>(static provider => | ||||||
{ | ||||||
var api = provider.GetRequiredService<Api>(); | ||||||
var client = api.GetClient(); | ||||||
var context = provider.GetRequiredService<EvaluationContext>(); | ||||||
client.SetContext(context); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Context can be added at both the global and client level. Global context is most useful for static values (for example, a value representing the timezone of the server or the cloud provider it's running on, or the application version). We might need multiple contexts setting methods for the different scopes: global, client, and perhaps transaction level once we implement the transaction propagation feature. As it is, I'm not sure it's obvious to a user which one would be used when they do |
||||||
return client; | ||||||
}); | ||||||
} | ||||||
else | ||||||
{ | ||||||
services.TryAddScoped<IFeatureClient>(static provider => | ||||||
{ | ||||||
var api = provider.GetRequiredService<Api>(); | ||||||
return api.GetClient(); | ||||||
}); | ||||||
} | ||||||
|
||||||
return services; | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason for not resolving
FeatureProvider
directly in the ctor beside having custom exception for that situation? In effect it not allow call-site validation to detect thatFeatureProvider
is missing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main reason for not resolving FeatureProvider directly in the constructor is that some implementations of FeatureProvider are not fully controlled by us, and they may execute significant logic during construction. For example, certain providers like LaunchDarkly perform extensive initialization within the constructor, which can take time and potentially fail (as seen in LaunchDarkly's Provider Implementation and LdClient Constructor).
By not resolving FeatureProvider in the constructor, we can safely perform this logic asynchronously in a start method, which avoids delays or failures during object instantiation. Additionally, I use GetService to log meaningful messages in case of any issues.
In short, this approach allows us to handle potential failures asynchronously during startup and ensures proper logging in case of errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally providers are doing such initialization in their
initialize()
method, but we can't control them all, and I see your point. Generally I'd prefer constructor resolution as well but I can understand the advantages you describe.