-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Add initial dependency injection support for LaunchDarkly provider #49
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
base: main
Are you sure you want to change the base?
Changes from all commits
cc52ab2
1292120
d9ec58b
86b9c04
8fbfab0
92515b1
830ad98
c958f44
1f6beb8
267bde5
741f6d2
63f26df
c803a2d
5e6b828
ccbd1d6
54a5753
85a36b1
a010d52
2ba3481
3721562
b6ee8ed
2e8fb99
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 |
|---|---|---|
| @@ -1,32 +1,40 @@ | ||
| | ||
| Microsoft Visual Studio Solution File, Format Version 12.00 | ||
| # Visual Studio Version 16 | ||
| VisualStudioVersion = 16.0.30114.105 | ||
| # Visual Studio Version 17 | ||
| VisualStudioVersion = 17.14.36127.28 d17.14 | ||
| MinimumVisualStudioVersion = 10.0.40219.1 | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "LaunchDarkly.OpenFeature.ServerProvider", "src\LaunchDarkly.OpenFeature.ServerProvider\LaunchDarkly.OpenFeature.ServerProvider.csproj", "{B61EC563-2D25-47C8-86A4-0C3A8C625109}" | ||
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "LaunchDarkly.OpenFeature.ServerProvider.Tests", "test\LaunchDarkly.OpenFeature.ServerProvider.Tests\LaunchDarkly.OpenFeature.ServerProvider.Tests.csproj", "{A1B1DB34-6B22-4AA4-9974-6EE67145BDB9}" | ||
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection", "src\LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection\LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection.csproj", "{DB523227-21AE-4B92-A263-05050CEF6C8D}" | ||
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection.Tests", "test\LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection.Tests\LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection.Tests.csproj", "{F2C8E3A6-12D4-4B8F-9A7E-3F5C8D6B4E2A}" | ||
| EndProject | ||
| Global | ||
| GlobalSection(SolutionConfigurationPlatforms) = preSolution | ||
| Debug|Any CPU = Debug|Any CPU | ||
| Release|Any CPU = Release|Any CPU | ||
| EndGlobalSection | ||
| GlobalSection(SolutionProperties) = preSolution | ||
| HideSolutionNode = FALSE | ||
| EndGlobalSection | ||
| GlobalSection(ProjectConfigurationPlatforms) = postSolution | ||
| {B61EC563-2D25-47C8-86A4-0C3A8C625109}.Debug|Any CPU.ActiveCfg = Debug|Any CPU | ||
| {B61EC563-2D25-47C8-86A4-0C3A8C625109}.Debug|Any CPU.Build.0 = Debug|Any CPU | ||
| {B61EC563-2D25-47C8-86A4-0C3A8C625109}.Release|Any CPU.ActiveCfg = Release|Any CPU | ||
| {B61EC563-2D25-47C8-86A4-0C3A8C625109}.Release|Any CPU.Build.0 = Release|Any CPU | ||
| {E8CE160B-0A65-480F-AA3F-028AD9F17F6E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU | ||
| {E8CE160B-0A65-480F-AA3F-028AD9F17F6E}.Debug|Any CPU.Build.0 = Debug|Any CPU | ||
| {E8CE160B-0A65-480F-AA3F-028AD9F17F6E}.Release|Any CPU.ActiveCfg = Release|Any CPU | ||
| {E8CE160B-0A65-480F-AA3F-028AD9F17F6E}.Release|Any CPU.Build.0 = Release|Any CPU | ||
| {A1B1DB34-6B22-4AA4-9974-6EE67145BDB9}.Debug|Any CPU.ActiveCfg = Debug|Any CPU | ||
| {A1B1DB34-6B22-4AA4-9974-6EE67145BDB9}.Debug|Any CPU.Build.0 = Debug|Any CPU | ||
| {A1B1DB34-6B22-4AA4-9974-6EE67145BDB9}.Release|Any CPU.ActiveCfg = Release|Any CPU | ||
| {A1B1DB34-6B22-4AA4-9974-6EE67145BDB9}.Release|Any CPU.Build.0 = Release|Any CPU | ||
| {DB523227-21AE-4B92-A263-05050CEF6C8D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU | ||
| {DB523227-21AE-4B92-A263-05050CEF6C8D}.Debug|Any CPU.Build.0 = Debug|Any CPU | ||
| {DB523227-21AE-4B92-A263-05050CEF6C8D}.Release|Any CPU.ActiveCfg = Release|Any CPU | ||
| {DB523227-21AE-4B92-A263-05050CEF6C8D}.Release|Any CPU.Build.0 = Release|Any CPU | ||
| {F2C8E3A6-12D4-4B8F-9A7E-3F5C8D6B4E2A}.Debug|Any CPU.ActiveCfg = Debug|Any CPU | ||
| {F2C8E3A6-12D4-4B8F-9A7E-3F5C8D6B4E2A}.Debug|Any CPU.Build.0 = Debug|Any CPU | ||
| {F2C8E3A6-12D4-4B8F-9A7E-3F5C8D6B4E2A}.Release|Any CPU.ActiveCfg = Release|Any CPU | ||
| {F2C8E3A6-12D4-4B8F-9A7E-3F5C8D6B4E2A}.Release|Any CPU.Build.0 = Release|Any CPU | ||
| EndGlobalSection | ||
| GlobalSection(SolutionProperties) = preSolution | ||
| HideSolutionNode = FALSE | ||
| EndGlobalSection | ||
| EndGlobal |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <!--x-release-please-start-version--> | ||
| <Version>2.1.1</Version> | ||
| <!--x-release-please-end--> | ||
| <!-- The BUILDFRAMEWORKS variable allows us to override the target frameworks with a | ||
| single framework that we are testing; this allows us to test with older SDK | ||
| versions that would error out if they saw any newer target frameworks listed | ||
| here, even if we weren't running those. --> | ||
| <BuildFrameworks Condition="'$(BUILDFRAMEWORKS)' == ''">net8.0</BuildFrameworks> | ||
| <TargetFrameworks>$(BUILDFRAMEWORKS)</TargetFrameworks> | ||
|
|
||
| <DebugType>portable</DebugType> | ||
| <AssemblyName>LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection</AssemblyName> | ||
| <OutputType>Library</OutputType> | ||
| <PackageId>LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection</PackageId> | ||
| <RootNamespace>LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection</RootNamespace> | ||
| <LangVersion>7.3</LangVersion> | ||
| <Description> | ||
| Dependency injection support for the LaunchDarkly OpenFeature .NET Server Provider. | ||
| Enables seamless integration of LaunchDarkly feature flagging with .NET applications via OpenFeature’s DI patterns. | ||
| </Description> | ||
| <Authors>LaunchDarkly</Authors> | ||
| <Owners>LaunchDarkly</Owners> | ||
| <Company>LaunchDarkly</Company> | ||
| <Authors>LaunchDarkly</Authors> | ||
| <Owners>LaunchDarkly</Owners> | ||
| <Copyright>Copyright 2022 LaunchDarkly</Copyright> | ||
| <PackageLicenseExpression>Apache-2.0</PackageLicenseExpression> | ||
| <PackageProjectUrl>https://github.com/launchdarkly/openfeature-dotnet-server</PackageProjectUrl> | ||
| <RepositoryUrl>https://github.com/launchdarkly/openfeature-dotnet-server</RepositoryUrl> | ||
| <RepositoryBranch>main</RepositoryBranch> | ||
| <IncludeSymbols>true</IncludeSymbols> | ||
| <SymbolPackageFormat>snupkg</SymbolPackageFormat> | ||
|
|
||
| <!-- fail if XML comments are missing or invalid --> | ||
| <WarningsAsErrors>1570,1571,1572,1573,1574,1580,1581,1584,1591,1710,1711,1712</WarningsAsErrors> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="OpenFeature.DependencyInjection" Version="[2.2.0, 3.0.0)" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\LaunchDarkly.OpenFeature.ServerProvider\LaunchDarkly.OpenFeature.ServerProvider.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <DocumentationFile>bin\$(Configuration)\$(TargetFramework)\LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection.xml</DocumentationFile> | ||
| </PropertyGroup> | ||
| </Project> |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,165 @@ | ||||||||||||||||||||||||||
| using LaunchDarkly.Sdk.Server; | ||||||||||||||||||||||||||
| using Microsoft.Extensions.DependencyInjection; | ||||||||||||||||||||||||||
| using Microsoft.Extensions.DependencyInjection.Extensions; | ||||||||||||||||||||||||||
| using Microsoft.Extensions.Options; | ||||||||||||||||||||||||||
| using OpenFeature; | ||||||||||||||||||||||||||
| using OpenFeature.DependencyInjection; | ||||||||||||||||||||||||||
| using System; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| namespace LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||
| /// Provides extension methods for configuring the <see cref="OpenFeatureBuilder"/> to use LaunchDarkly as a <see cref="FeatureProvider"/>. | ||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||
| public static partial class LaunchDarklyOpenFeatureBuilderExtensions | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||
| /// Configures the <see cref="OpenFeatureBuilder"/> to use LaunchDarkly as the default provider | ||||||||||||||||||||||||||
| /// using the specified <see cref="Configuration"/> instance. | ||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||
| /// <param name="builder">The <see cref="OpenFeatureBuilder"/> instance to configure.</param> | ||||||||||||||||||||||||||
| /// <param name="configuration">A pre-built LaunchDarkly <see cref="Configuration"/>.</param> | ||||||||||||||||||||||||||
| /// <returns>The updated <see cref="OpenFeatureBuilder"/> instance.</returns> | ||||||||||||||||||||||||||
| /// <exception cref="ArgumentNullException"> | ||||||||||||||||||||||||||
| /// Thrown when the <paramref name="configuration"/> argument is <c>null</c>. | ||||||||||||||||||||||||||
| /// </exception> | ||||||||||||||||||||||||||
| public static OpenFeatureBuilder UseLaunchDarkly(this OpenFeatureBuilder builder, Configuration configuration) | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| if (configuration == null) | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| throw new ArgumentNullException(nameof(configuration), "Configuration cannot be null."); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return RegisterLaunchDarklyProvider( | ||||||||||||||||||||||||||
| builder, | ||||||||||||||||||||||||||
| () => configuration, | ||||||||||||||||||||||||||
| sp => sp.GetRequiredService<Configuration>() | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||
| /// Configures the <see cref="OpenFeatureBuilder"/> to use LaunchDarkly as a domain-scoped provider | ||||||||||||||||||||||||||
| /// using the specified <see cref="Configuration"/> instance. | ||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||
| /// <param name="builder">The <see cref="OpenFeatureBuilder"/> instance to configure.</param> | ||||||||||||||||||||||||||
| /// <param name="domain">A domain identifier (e.g., tenant or environment).</param> | ||||||||||||||||||||||||||
| /// <param name="configuration">A pre-built LaunchDarkly <see cref="Configuration"/> specific to the domain.</param> | ||||||||||||||||||||||||||
| /// <returns>The updated <see cref="OpenFeatureBuilder"/> instance.</returns> | ||||||||||||||||||||||||||
| /// <exception cref="ArgumentNullException"> | ||||||||||||||||||||||||||
| /// Thrown when the <paramref name="configuration"/> argument is <c>null</c>. | ||||||||||||||||||||||||||
| /// </exception> | ||||||||||||||||||||||||||
| public static OpenFeatureBuilder UseLaunchDarkly(this OpenFeatureBuilder builder, string domain, Configuration configuration) | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| if (configuration == null) | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| throw new ArgumentNullException(nameof(configuration), "Configuration cannot be null."); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return RegisterLaunchDarklyProvider( | ||||||||||||||||||||||||||
| builder, | ||||||||||||||||||||||||||
| domain, | ||||||||||||||||||||||||||
| () => configuration, | ||||||||||||||||||||||||||
| (sp, key) => sp.GetRequiredKeyedService<Configuration>(key) | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||
| /// Configures the <see cref="OpenFeatureBuilder"/> to use LaunchDarkly as the default provider | ||||||||||||||||||||||||||
| /// using the specified SDK key and optional configuration delegate. | ||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||
| /// <param name="builder">The <see cref="OpenFeatureBuilder"/> instance to configure.</param> | ||||||||||||||||||||||||||
| /// <param name="sdkKey">The SDK key used to initialize the LaunchDarkly configuration.</param> | ||||||||||||||||||||||||||
| /// <param name="configure">An optional delegate to customize the <see cref="ConfigurationBuilder"/>.</param> | ||||||||||||||||||||||||||
| /// <returns>The updated <see cref="OpenFeatureBuilder"/> instance.</returns> | ||||||||||||||||||||||||||
| public static OpenFeatureBuilder UseLaunchDarkly(this OpenFeatureBuilder builder, string sdkKey, Action<ConfigurationBuilder> configure = null) | ||||||||||||||||||||||||||
| => RegisterLaunchDarklyProvider( | ||||||||||||||||||||||||||
| builder, | ||||||||||||||||||||||||||
| () => CreateConfiguration(sdkKey, configure), | ||||||||||||||||||||||||||
| sp => sp.GetRequiredService<Configuration>()); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||
| /// Configures the <see cref="OpenFeatureBuilder"/> to use LaunchDarkly as a domain-scoped provider | ||||||||||||||||||||||||||
| /// using the specified SDK key and optional configuration delegate. | ||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||
| /// <param name="builder">The <see cref="OpenFeatureBuilder"/> instance to configure.</param> | ||||||||||||||||||||||||||
| /// <param name="domain">A domain identifier (e.g., tenant or environment).</param> | ||||||||||||||||||||||||||
| /// <param name="sdkKey">The SDK key used to initialize the LaunchDarkly configuration.</param> | ||||||||||||||||||||||||||
| /// <param name="configure">An optional delegate to customize the <see cref="ConfigurationBuilder"/>.</param> | ||||||||||||||||||||||||||
| /// <returns>The updated <see cref="OpenFeatureBuilder"/> instance.</returns> | ||||||||||||||||||||||||||
| public static OpenFeatureBuilder UseLaunchDarkly(this OpenFeatureBuilder builder, string domain, string sdkKey, Action<ConfigurationBuilder> configure = null) | ||||||||||||||||||||||||||
| => RegisterLaunchDarklyProvider( | ||||||||||||||||||||||||||
| builder, | ||||||||||||||||||||||||||
| domain, | ||||||||||||||||||||||||||
| () => CreateConfiguration(sdkKey, configure), | ||||||||||||||||||||||||||
| (sp, key) => sp.GetRequiredKeyedService<Configuration>(key)); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||
| /// Registers LaunchDarkly as the default feature provider using the given configuration factory and resolution logic. | ||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||
| /// <param name="builder">The <see cref="OpenFeatureBuilder"/> instance to configure.</param> | ||||||||||||||||||||||||||
| /// <param name="createConfiguration">A delegate that returns a <see cref="Configuration"/> instance.</param> | ||||||||||||||||||||||||||
| /// <param name="resolveConfiguration">A delegate that resolves the <see cref="Configuration"/> from the service provider.</param> | ||||||||||||||||||||||||||
| /// <returns>The updated <see cref="OpenFeatureBuilder"/> instance.</returns> | ||||||||||||||||||||||||||
| private static OpenFeatureBuilder RegisterLaunchDarklyProvider( | ||||||||||||||||||||||||||
| OpenFeatureBuilder builder, | ||||||||||||||||||||||||||
| Func<Configuration> createConfiguration, | ||||||||||||||||||||||||||
| Func<IServiceProvider, Configuration> resolveConfiguration) | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| // Perform early configuration validation to ensure the provider is correctly constructed. | ||||||||||||||||||||||||||
| // This ensures any misconfiguration is caught during application startup rather than at runtime. | ||||||||||||||||||||||||||
| var config = createConfiguration(); | ||||||||||||||||||||||||||
| builder.Services.TryAddSingleton(config); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return builder.AddProvider(serviceProvider => new Provider(resolveConfiguration(serviceProvider))); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||
| /// Registers LaunchDarkly as a domain-scoped feature provider using the given configuration factory and resolution logic. | ||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||
| /// <param name="builder">The <see cref="OpenFeatureBuilder"/> instance to configure.</param> | ||||||||||||||||||||||||||
| /// <param name="domain">A domain identifier (e.g., tenant or environment).</param> | ||||||||||||||||||||||||||
| /// <param name="createConfiguration">A delegate that returns a domain-specific <see cref="Configuration"/> instance.</param> | ||||||||||||||||||||||||||
| /// <param name="resolveConfiguration">A delegate that resolves the domain-scoped <see cref="Configuration"/> from the service provider.</param> | ||||||||||||||||||||||||||
| /// <returns>The updated <see cref="OpenFeatureBuilder"/> instance.</returns> | ||||||||||||||||||||||||||
| private static OpenFeatureBuilder RegisterLaunchDarklyProvider( | ||||||||||||||||||||||||||
|
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. I don't follow the benefit of adding the config to the ServiceCollection. If we remove that, this could be simplified. Can you share the intent behind storing it?
Suggested change
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. I'm not sure why the "Apply suggestion" messed up, it should be a replacement for the whole method, not just added in. I mainly wanted to include it as a discussion point about storing the config. |
||||||||||||||||||||||||||
| OpenFeatureBuilder builder, | ||||||||||||||||||||||||||
| string domain, | ||||||||||||||||||||||||||
| Func<Configuration> createConfiguration, | ||||||||||||||||||||||||||
| Func<IServiceProvider, object, Configuration> resolveConfiguration) | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| // Perform early validation of the configuration to ensure it is valid before registration. | ||||||||||||||||||||||||||
| // This approach is consistent with the default (non-domain) registration path and helps fail fast on misconfiguration. | ||||||||||||||||||||||||||
| var config = createConfiguration(); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Register the domain-scoped configuration as a keyed singleton. | ||||||||||||||||||||||||||
| builder.Services.TryAddKeyedSingleton(domain, (_, key) => config); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Register the default configuration provider, which resolves the appropriate domain-scoped configuration | ||||||||||||||||||||||||||
| // using the default name selection policy defined in PolicyNameOptions. | ||||||||||||||||||||||||||
| // This enables resolving Configuration via serviceProvider.GetRequiredService<Configuration>() | ||||||||||||||||||||||||||
| // when no specific domain key is explicitly provided. | ||||||||||||||||||||||||||
| builder.Services.TryAddSingleton(provider => | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| var policy = provider.GetRequiredService<IOptions<PolicyNameOptions>>().Value; | ||||||||||||||||||||||||||
| var name = policy.DefaultNameSelector(provider); | ||||||||||||||||||||||||||
| return provider.GetRequiredKeyedService<Configuration>(name); | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Register the domain-scoped provider instance. | ||||||||||||||||||||||||||
| return builder.AddProvider(domain, (serviceProvider, key) => new Provider(resolveConfiguration(serviceProvider, key))); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||
| /// Creates a new <see cref="Configuration"/> using the specified SDK key and optional configuration delegate. | ||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||
| /// <param name="sdkKey">The SDK key used to initialize the configuration.</param> | ||||||||||||||||||||||||||
| /// <param name="configure">An optional delegate to customize the <see cref="ConfigurationBuilder"/>.</param> | ||||||||||||||||||||||||||
| /// <returns>A fully constructed <see cref="Configuration"/> instance.</returns> | ||||||||||||||||||||||||||
| private static Configuration CreateConfiguration(string sdkKey, Action<ConfigurationBuilder> configure = null) | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| var configBuilder = Configuration.Builder(sdkKey); | ||||||||||||||||||||||||||
| configure?.Invoke(configBuilder); | ||||||||||||||||||||||||||
| return configBuilder.Build(); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <!-- The BUILDFRAMEWORKS variable allows us to override the target frameworks with a | ||
| single framework that we are testing; this allows us to test with older SDK | ||
| versions that would error out if they saw any newer target frameworks listed | ||
| here, even if we weren't running those. | ||
|
|
||
| Tests need to run against a specific platform implementation. netstandard2.0 is | ||
| an API, and not a platform, so it is not included in this list. | ||
| Additional information: https://xunit.net/docs/why-no-netstandard | ||
|
|
||
| --> | ||
| <BuildFrameworks Condition="'$(BUILDFRAMEWORKS)' == ''">net8.0</BuildFrameworks> | ||
| <TargetFrameworks>$(BUILDFRAMEWORKS)</TargetFrameworks> | ||
|
|
||
| <GenerateMSBuildEditorConfigFile>false</GenerateMSBuildEditorConfigFile> | ||
| <IsPackable>false</IsPackable> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" /> | ||
| <PackageReference Include="xunit" Version="2.4.1" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3"> | ||
| <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> | ||
| <PrivateAssets>all</PrivateAssets> | ||
| </PackageReference> | ||
| <PackageReference Include="Microsoft.TestPlatform.ObjectModel" Version="16.6.1" Condition="$(TargetFramework.StartsWith('net4')) AND '$(OS)' == 'Unix'" /> | ||
| <PackageReference Include="coverlet.collector" Version="3.1.0"> | ||
| <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> | ||
| <PrivateAssets>all</PrivateAssets> | ||
| </PackageReference> | ||
| <PackageReference Include="Microsoft.Extensions.Logging" Version="8.0.1" /> | ||
| <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="8.0.1" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\..\src\LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection\LaunchDarkly.OpenFeature.ServerProvider.DependencyInjection.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> |
Uh oh!
There was an error while loading. Please reload this page.
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.
AddProviderregisters a transient factory method. I don't believe we should be resolving transient instances ofProviderhere. Please correct me if I am wrong, but my understanding is that LaunchDarkly strongly recommends instances ofLdClientto be singletons (for a given sdk-key/environment).Providerencapsulates the construction of theLdClientwithin its own constructor and thus, its lifetime should follow the same guideline. I believe a more correct approach would be to registerProvideras a singleton and resolve it using DI as part of the factory method. e.g.I'm assuming OpenFeature uses a transient factory method so that they don't dictate lifetime to provider implementations. Instead, each provider can be registered per their own guidance.
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.
You’re absolutely right that
LdClientis recommended to be used as a singleton. However, in this case, the factory method is registered as transient because its lifecycle is ultimately governed by the client that consumes it.In OpenFeature, the initialization happens here, and the
_featureApiis registered as a singleton. Consumers don’t interact with providers directly - they useIFeatureClient, which goes through this API (singleton). Given that flow, the effective usage aligns with the singleton recommendation.Registering the provider as transient also leaves flexibility: consumers can create and manage their own provider instance if they have specific scenarios requiring different lifetimes. This avoids OpenFeature itself enforcing a single pattern and instead allows SDK consumers to decide. In the standard usage (via
OpenFeatureandIFeatureClient), it will still behave as a singleton, which follows LaunchDarkly’s guidance.That said, if the LaunchDarkly team concludes that
Providermust always be a singleton for correctness, we can adapt by layering a singleton registration on top of the transient one. For example:Here
AddProviderwires up the necessary OpenFeature services internally (viaTryAdd), and the explicitAddSingletonoverrides the provider’s registration.My recommendation is to keep it as-is (registered as transient), for the reasons I outlined above. That said, this is an excellent highlight, and I fully agree it’s an important consideration that we should call out explicitly in the documentation.
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.
@dgioulakis what do you think?
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.
Hey @arttonoyan, sorry for the delay. I was out for a bit, and then came down with covid and was out some more.
I see what you're saying, although I'm not a maintainer of this repository. It would be best to get someone from LD to review. I think your second
AddSingletonwill not overwrite as you intend unless you specify the service type e.g.builder.Services.AddSingleton<FeatureProvider>(serviceProvider => new Provider(...)).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 LaunchDarkly Client should be a singleton but I think the approach to let OpenFeature dictate the provider lifetime here is okay. Especially since it in effect becomes a singleton.