dapr / dotnet-sdk

Dapr SDK for .NET
Apache License 2.0
1.12k stars 340 forks source link

AddDaprClient incompatible with keyed services #1235

Closed onionhammer closed 9 months ago

onionhammer commented 9 months ago

Expected Behavior

When adding dapr client to service builder, this should not error

https://weblogs.asp.net/ricardoperes/net-8-dependency-injection-changes-keyed-services

// Add dapr client
builder.Services.AddDaprClient(client =>
{
    client.UseJsonSerializationOptions(JsonExtensions.JsonOptions);
});

Actual Behavior

When adding dapr to your serviceBuilder, if there are keyed services present, adding dapr fils with exception:

Unhandled exception. System.InvalidOperationException: This service descriptor is keyed. Your service provider may not support keyed services.

Steps to Reproduce the Problem

Add a dapr client to an application with keyed services defined, i.e. with Orleans:

builder.Host.UseOrleans(siloBuilder =>
{
    siloBuilder.AddActivityPropagation();
});
artyom-p commented 9 months ago

Same here, related to this issue.

image

Sen-Gupta commented 9 months ago

builder.AddServiceDefaults(); builder.Services.AddDaprClient();

System.InvalidOperationException HResult=0x80131509 Message=This service descriptor is keyed. Your service provider may not support keyed services. Source=Microsoft.Extensions.DependencyInjection.Abstractions StackTrace: at Microsoft.Extensions.DependencyInjection.ServiceDescriptor.ThrowKeyedDescriptor() at Microsoft.Extensions.DependencyInjection.ServiceDescriptor.get_ImplementationType() at Microsoft.Extensions.DependencyInjection.DaprServiceCollectionExtensions.<>c.b__0_0(ServiceDescriptor s) at System.Linq.Enumerable.Any[TSource](IEnumerable1 source, Func2 predicate) at Microsoft.Extensions.DependencyInjection.DaprServiceCollectionExtensions.AddDaprClient(IServiceCollection services, Action`1 configure) at Insights.Worker.Manager.Program.Main(String[] args) in D:\Trade\InsightsOnDaprAspire\src\Workers\Manager\Program.cs:line 15

onionhammer commented 9 months ago

For the contributors, here's a PR that addresses this in Microsoft.Identity.Web, for reference:

https://github.com/AzureAD/microsoft-identity-web/pull/2676

rliberoff commented 9 months ago

Guys!

We really need this for .NET Aspire. Do you have any update when We can expect to have this fixed?

Thank you!

Sen-Gupta commented 9 months ago

I'm not sure it this works for you, I placed AddDapr above all calls in service registration and it works.

Additionally, I downgraded Assemblies to 8.0.0 for DependencyInjection.

Sent from Outlook for Androidhttps://aka.ms/AAb9ysg


From: Rodrigo Liberoff @.> Sent: Thursday, February 15, 2024 10:18:19 PM To: dapr/dotnet-sdk @.> Cc: Sen Gupta @.>; Comment @.> Subject: Re: [dapr/dotnet-sdk] AddDaprClient incompatible with keyed services (Issue #1235)

Guys!

We really need this for .NET Aspire. Do you have any update when We can expecto have this fixed?

Thank you!

— Reply to this email directly, view it on GitHubhttps://github.com/dapr/dotnet-sdk/issues/1235#issuecomment-1946564317, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AB5R3ZXYARV5LAKXVPTAQWDYTY35HAVCNFSM6AAAAABDG6UO3OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBWGU3DIMZRG4. You are receiving this because you commented.Message ID: @.***>

rliberoff commented 9 months ago

Thank you @Sen-Gupta.

The issue is not with AddDapr but with AddDaprClient.

This happens with the newest .NET Aspire Preview 3.

Sen-Gupta commented 9 months ago

Apologies, I meant that only.

I attached the file, I would be able to share more details tomorrow.

It is midnight in my time zone.

Regards Sen

Sent from Outlook for Androidhttps://aka.ms/AAb9ysg


From: Rodrigo Liberoff @.> Sent: Thursday, February 15, 2024 10:34:01 PM To: dapr/dotnet-sdk @.> Cc: Sen Gupta @.>; Mention @.> Subject: Re: [dapr/dotnet-sdk] AddDaprClient incompatible with keyed services (Issue #1235)

Thank you @Sen-Guptahttps://github.com/Sen-Gupta.

The issue is not with AddDapr but with AddDaprClient.

This happens with the newest .NET Aspire Preview 3.

— Reply to this email directly, view it on GitHubhttps://github.com/dapr/dotnet-sdk/issues/1235#issuecomment-1946600423, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AB5R3ZT3BRSDEVHPRWEF6A3YTY5YDAVCNFSM6AAAAABDG6UO3OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBWGYYDANBSGM. You are receiving this because you were mentioned.Message ID: @.***>

using Insights.BuildingBlocks.EventBus; using Insights.BuildingBlocks.EventBus.Abstractions;

using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Diagnostics.HealthChecks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.ResponseCompression; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Logging;

using OpenTelemetry.Logs; using OpenTelemetry.Metrics; using OpenTelemetry.Trace;

using Serilog; using Serilog.Exceptions;

namespace Microsoft.Extensions.Hosting;

public static class Extensions { public static IHostApplicationBuilder AddServiceDefaults(this IHostApplicationBuilder builder) {

    builder.AddDapr();

if DEBUG

    builder.ConfigureOpenTelemetry();

endif

    builder.AddDefaultHealthChecks();

    builder.Services.AddServiceDiscovery();

    builder.Services.ConfigureHttpClientDefaults(http =>
    {
        // Turn on resilience by default
        http.AddStandardResilienceHandler();

        // Turn on service discovery by default
        http.UseServiceDiscovery();
    });

    return builder;
}

/// <summary>
/// Configure Serilog to write to the console and OpenTelemetry for Aspire structured logs.
/// </summary>
/// <remarks>
/// ⚠ This method MUST be called before the <see cref="OpenTelemetryLoggingExtensions.AddOpenTelemetry(ILoggingBuilder)"/> method to still send structured logs via OpenTelemetry. ⚠
/// </remarks>
internal static IHostApplicationBuilder ConfigureSerilog(this IHostApplicationBuilder builder)
{
    // Removes the built-in logging providers
    builder.Logging.ClearProviders();

    var seqServerUrl = builder.Configuration["SeqServerUrl"]!;

    var appName = builder.Configuration["AppMeta:AppName"]!;

    var loggerConfiguration = new LoggerConfiguration()
        .MinimumLevel.Debug()
        .ReadFrom.Configuration(builder.Configuration)
        .Enrich.FromLogContext()
        .Enrich.FromLogContext()
        .Enrich.WithMachineName()
        .Enrich.WithExceptionDetails();

    if (!string.IsNullOrEmpty(appName))
    {
        loggerConfiguration.Enrich.WithProperty("ApplicationName", appName);
    }
    if (!string.IsNullOrEmpty(seqServerUrl))
    {
        loggerConfiguration.WriteTo.Seq(seqServerUrl);
    }
    loggerConfiguration.WriteTo.Debug();
    loggerConfiguration.WriteTo.Console();

    // Including the writeToProviders=true parameter allows the OpenTelemetry logger to still be written to
    builder.Services.AddSerilog((_, config) => 
    {
        config = loggerConfiguration;
        // Configure Serilog as desired here for every project (or use IConfiguration for configuration variations between projects)
    }, writeToProviders: true);

    return builder;
}

public static IHostApplicationBuilder ConfigureOpenTelemetry(this IHostApplicationBuilder builder)
{
    builder.Logging.AddOpenTelemetry(logging =>
    {
        logging.IncludeFormattedMessage = true;
        logging.IncludeScopes = true;
    });

    builder.Services.AddOpenTelemetry()
        .WithMetrics(metrics =>
        {
            metrics.AddRuntimeInstrumentation()
                   .AddBuiltInMeters();
        })
        .WithTracing(tracing =>
        {
            if (builder.Environment.IsDevelopment())
            {
                // We want to view all traces in development
                tracing.SetSampler(new AlwaysOnSampler());
            }

            tracing.AddAspNetCoreInstrumentation()
                   .AddGrpcClientInstrumentation()
                   .AddHttpClientInstrumentation();
        });

    builder.AddOpenTelemetryExporters();

    return builder;
}

private static IHostApplicationBuilder AddOpenTelemetryExporters(this IHostApplicationBuilder builder)
{
    var useOtlpExporter = !string.IsNullOrWhiteSpace(builder.Configuration["OTEL_EXPORTER_OTLP_ENDPOINT"]);

    if (useOtlpExporter)
    {
        builder.Services.Configure<OpenTelemetryLoggerOptions>(logging => logging.AddOtlpExporter());
        builder.Services.ConfigureOpenTelemetryMeterProvider(metrics => metrics.AddOtlpExporter());
        builder.Services.ConfigureOpenTelemetryTracerProvider(tracing => tracing.AddOtlpExporter());
    }

    // Uncomment the following lines to enable the Prometheus exporter (requires the OpenTelemetry.Exporter.Prometheus.AspNetCore package)
    // builder.Services.AddOpenTelemetry()
    //    .WithMetrics(metrics => metrics.AddPrometheusExporter());

    // Uncomment the following lines to enable the Azure Monitor exporter (requires the Azure.Monitor.OpenTelemetry.Exporter package)
    // builder.Services.AddOpenTelemetry()
    //    .UseAzureMonitor();

    return builder;
}

public static IHostApplicationBuilder AddDefaultHealthChecks(this IHostApplicationBuilder builder)
{
    builder.Services.AddHealthChecks()
        // Add a default liveness check to ensure app is responsive
        .AddCheck("self", () => HealthCheckResult.Healthy(), ["live"]);

    return builder;
}

public static WebApplication MapDefaultEndpoints(this WebApplication app)
{
    // Uncomment the following line to enable the Prometheus endpoint (requires the OpenTelemetry.Exporter.Prometheus.AspNetCore package)
    // app.MapPrometheusScrapingEndpoint();

    // All health checks must pass for app to be considered ready to accept traffic after starting
    app.MapHealthChecks("/health");

    // Only health checks tagged with the "live" tag must pass for app to be considered alive
    app.MapHealthChecks("/alive", new HealthCheckOptions
    {
        Predicate = r => r.Tags.Contains("live")
    });

    return app;
}

private static MeterProviderBuilder AddBuiltInMeters(this MeterProviderBuilder meterProviderBuilder) =>
    meterProviderBuilder.AddMeter(
        "Microsoft.AspNetCore.Hosting",
        "Microsoft.AspNetCore.Server.Kestrel",
        "System.Net.Http");

public static void AddDapr(this IHostApplicationBuilder builder)
{
    builder.Services.AddDaprClient();
    builder.Services.AddSingleton<IEventBus, DaprEventBus>();
    builder.Services.AddSingleton<IHttpContextAccessor, HttpContextAccessor>();

}

public static void AddCustomSerilog(this WebApplicationBuilder builder)
{
    var environmentName = builder.Configuration["AppMeta:Env"]!;
    var seqServerUrl = builder.Configuration["SeqServerUrl"]!;
    var appName = builder.Configuration["AppMeta:AppName"]!;
    Log.Logger = new LoggerConfiguration()
   .MinimumLevel.Debug()
   .ReadFrom.Configuration(builder.Configuration)
   .WriteTo.Console()
   .WriteTo.Seq(seqServerUrl)
   .Enrich.WithProperty("ApplicationName", appName)
   .Enrich.WithProperty("Env", environmentName)
   .Enrich.FromLogContext()
   .Enrich.WithMachineName()
   .Enrich.WithExceptionDetails()
   .CreateLogger();
    builder.Host.UseSerilog();
}

}

rliberoff commented 9 months ago

From further investigation, we found out that the offending dependency is with library Microsoft.Extensions.Http.Resilience when updated from version 8.1.0 to 8.2.0. Hope it helps.

Sen-Gupta commented 9 months ago

@rliberoff

Double Confirmation!

That's correct. I did upgrade Microsoft.Extensions.Http.Resilience to 8.2.0.