feat: Deprecate AddHostedFeatureLifecycle method

[kylejuliandev]

This PR

• Copy all Dependency Injection files over to the Hosting package
• Add [Obsolete()] attribute to AddHostedFeatureLifecycle()
• Ensure that the Hosted Service is injected when you make a call to AddOpenFeature
• Add new test project with the same unit tests from the Dependency Injection tests

This is technically a breaking change because anyone using both packages in their project, which everyone will be, they will run into issues around resolving the correct extension methods.

Related Issues

Fixes #472

Notes

Follow-up Tasks

How to test

[askpt]

@toddbaert This PR is to move the deprecated method into the Hosting library. Since these DIs are still experimental, do you consider this as breaking change?
I am happy just to do a minor version bump instead of major.

[arttonoyan]

@kylejuliandev I suggest moving all files currently in the Hosting package (under the Hosting namespace) into the DependencyInjection package (also under Hosting namespace).

Why this makes sense

1. Better organization: These files are all about hosting-related setup and services. Since dependency injection is how hosting is configured in .NET, keeping them together makes it easier to understand.

2. Easier to use: When users set up OpenFeature with ASP.NET Core, they'll usually work in the DependencyInjection package. Moving everything into that package means they don't have to reference a second package - and everything shows up in one place when using IntelliSense or documentation

---

Also, in the Hosting package, we currently have FeatureLifecycleStateOptions, which were configured using AddHostedFeatureLifecycle. Since we're deprecating that, we should remember to update the docs to show that the hosted service can now be configured using the standard Microsoft configuration pattern.

For example:

services.ConfigureOptions<FeatureLifecycleStateOptions>(options =>
{
    // Optional: configure StartState, StopState, etc.
});

All of these options are optional, but this makes it easier and more consistent with typical .NET configuration.

[askpt]

@kylejuliandev I suggest moving all files currently in the Hosting package (under the Hosting namespace) into the DependencyInjection package (also under Hosting namespace).

I am more inclined to move all DI into the Hosting namespace. We have Hosting specific stuff and DI in the same package. We rarely reference the DI package, and it usually comes included in the Hosting one.

The dependency tree generally looks like:

YourApp
 └─ OpenFeature.Hosting
      └─ OpenFeature.DependencyInjection

We should keep only the DI unless we provide only DI extensions. Since we also offer application configuration, I vote to keep the Hosting and merge the DI into that one.

We might need to make changes to the configuration file since I would like to keep the same namespaces we are using now.

---

Also, in the Hosting package, we currently have FeatureLifecycleStateOptions, which were configured using AddHostedFeatureLifecycle. Since we're deprecating that, we should remember to update the docs to show that the hosted service can now be configured using the standard Microsoft configuration pattern.

For example:

services.ConfigureOptions<FeatureLifecycleStateOptions>(options =>
{
    // Optional: configure StartState, StopState, etc.
});

All of these options are optional, but this makes it easier and more consistent with typical .NET configuration.

Agreed!

[codecov]

Codecov Report

❌ Patch coverage is 92.28296% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.72%. Comparing base (e03aeba) to head (a933931).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...sting/Providers/Memory/FeatureBuilderExtensions.cs	75.00%	8 Missing and 3 partials ⚠️
...eature.Hosting/Internal/FeatureLifecycleManager.cs	86.11%	4 Missing and 1 partial ⚠️
...g/MultiTarget/CallerArgumentExpressionAttribute.cs	0.00%	5 Missing ⚠️
...penFeature.Hosting/OpenFeatureBuilderExtensions.cs	97.98%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #531      +/-   ##
==========================================
- Coverage   88.48%   87.72%   -0.77%     
==========================================
  Files          50       63      +13     
  Lines        2102     2452     +350     
  Branches      245      273      +28     
==========================================
+ Hits         1860     2151     +291     
- Misses        191      247      +56     
- Partials       51       54       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[askpt]

@kylejuliandev the code coverage went quite down. You did a copy-paste of the current code without changes?

[kylejuliandev]

@kylejuliandev the code coverage went quite down. You did a copy-paste of the current code without changes?

Yep, pretty much a copy and paste and update of namespaces and usings. Looking at the code coverage from main: We never produced a report for OpenFeature.Hosting and the OpenFeature.DependencyInjection coverage was only about 60% - https://app.codecov.io/gh/open-feature/dotnet-sdk/tree/main/src

I was planning on adding some additional unit tests to improve the coverage - if that's ok?

[askpt]

@kylejuliandev the code coverage went quite down. You did a copy-paste of the current code without changes?

Yep, pretty much a copy and paste and update of namespaces and usings. Looking at the code coverage from main: We never produced a report for OpenFeature.Hosting and the OpenFeature.DependencyInjection coverage was only about 60% - https://app.codecov.io/gh/open-feature/dotnet-sdk/tree/main/src

I was planning on adding some additional unit tests to improve the coverage - if that's ok?

Go for it! 👍

[WeihanLi]

Should we prefer the Guard.ThrowIfNull(key) method here?

[beeme1mr]

As a follow up, we'll need to update the readme with the updated config. I believe @askpt updated the OTel demo to use DI, so we should help with with the migration once this has been released.

I'd like like to wait for @arttonoyan approval since he was the original author of the DI feature. Thanks!

[askpt]

As a follow up, we'll need to update the readme with the updated config. I believe @askpt updated the OTel demo to use DI, so we should help with with the migration once this has been released.

I'd like like to wait for @arttonoyan approval since he was the original author of the DI feature. Thanks!

Thanks for pointing out the main README.md update @beeme1mr! We should make sure to update to the latest version that doesn't include the AddHosted method.

When the next version is released, I will update the Otel-demo repository 👍

[kylejuliandev]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively refactors the dependency injection capabilities into the OpenFeature.Hosting package and deprecates the AddHostedFeatureLifecycle method. The new DI structure, centered around OpenFeatureBuilder, is well-designed and the extensive test suite is a valuable addition. I've identified a couple of areas for improvement to enhance the API's robustness by adding validation and providing safer default configurations.

[gemini-code-assist]

The XML documentation for this method states that it throws an exception if domain is null or empty. However, the implementation does not validate that the domain string is not empty or whitespace. This could lead to registering a provider with an invalid key. It's best to add this validation to align with the documented contract.

        Guard.ThrowIfNull(builder);

        if (string.IsNullOrWhiteSpace(domain))
        {
            throw new ArgumentException("Domain cannot be null or whitespace.", nameof(domain));
        }

        builder.DomainBoundProviderRegistrationCount++;

[gemini-code-assist]

The DefaultNameSelector property is initialized with null!, which can lead to a NullReferenceException at runtime if it's accessed without being configured. To prevent this, it would be safer to initialize it with a default lambda that returns null.

    public Func<IServiceProvider, string?> DefaultNameSelector { get; set; } = _ => null;