dapr / dotnet-sdk

Dapr SDK for .NET
Apache License 2.0
1.11k stars 331 forks source link

Prioritize IConfiguration for environment variable retrieval during DI registrations #1336

Open WhitWaldo opened 4 weeks ago

WhitWaldo commented 4 weeks ago

In today's .NET Dapr Client SDK, environment variables like DAPR_API_KEY, DAPR_HTTP_ENDPOINT and DAPR_GRPC_ENDPOINT are sourced directly as named, which can lead to unexpected registration conflicts to modern .NET developers. I suggest that the DI registration extensions for the DaprClient itself, but also the Workflow and Jobs clients be updated to prioritize retrieving these values directly from an injected IConfiguration for consistency and to align with ASP.NET Core's centralized configuration management approach detailed here.

For example, when registering each of these environment variables with a prefix, the current approach would recognize the prefixed variables leading to the client ignoring the specified and registered keys in favor or the defaults:

var builder = WebApplication.CreateBuilder(args);
builder.Configuration.AddEnvironmentVariables("test_"); //Retrieves all the envvars that start with "test_" and removes the prefix when sourced from IConfiguration
builder.Services.AddDaprClient(); //Would ignore the IConfiguration and default to empty strings as it wouldn't find the expected envvars

var app = builder.Build();
app.Run();

Currently, the DaprClient DI registration bypasses IConfiguration leading to direct envvar retrieval or default empty strings as a fallback defying expectations.

This change should have a minimal impact as it wouldn't require signature changes and would simply augment the implementation already in use to source first from IConfiguration, where available, and then to an environment variable and finally an empty string.

I'll provide a PR within the next week with a proposal to illustrate what I'm suggesting and would welcome any feedback.

WhitWaldo commented 4 weeks ago

/assign