From a617a544a4cb935c651ecd11e15365d33754d474 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Wed, 1 Nov 2023 17:19:57 -0500 Subject: [PATCH] Fix database issues (#630) * Allow for configuring DbContextOptions in EF Components Fix #438 * Enable Npgsql Meter instead of using EventCounters Need to disable the prepared_ratio instrument until #629 is fixed. Fix #442 * Update Telemetry doc for updated Metrics --- ...spireSqlServerEFCoreSqlClientExtensions.cs | 6 ++- .../AspireEFPostgreSqlExtensions.cs | 20 ++++---- .../Aspire.Npgsql/Aspire.Npgsql.csproj | 1 - .../AspirePostgreSqlNpgsqlExtensions.cs | 11 ++--- src/Components/Telemetry.md | 40 ++++++++-------- ...SqlServerEFCoreSqlClientExtensionsTests.cs | 48 +++++++++++++++++++ .../AspireEFPostgreSqlExtensionsTests.cs | 45 +++++++++++++++++ .../TestDbContext.cs | 3 ++ 8 files changed, 138 insertions(+), 36 deletions(-) diff --git a/src/Components/Aspire.Microsoft.EntityFrameworkCore.SqlServer/AspireSqlServerEFCoreSqlClientExtensions.cs b/src/Components/Aspire.Microsoft.EntityFrameworkCore.SqlServer/AspireSqlServerEFCoreSqlClientExtensions.cs index b8be09dbd2..265725f4a6 100644 --- a/src/Components/Aspire.Microsoft.EntityFrameworkCore.SqlServer/AspireSqlServerEFCoreSqlClientExtensions.cs +++ b/src/Components/Aspire.Microsoft.EntityFrameworkCore.SqlServer/AspireSqlServerEFCoreSqlClientExtensions.cs @@ -27,13 +27,15 @@ public static class AspireSqlServerEFCoreSqlClientExtensions /// The to read config from and add services to. /// A name used to retrieve the connection string from the ConnectionStrings configuration section. /// An optional delegate that can be used for customizing options. It's invoked after the settings are read from the configuration. + /// An optional delegate to configure the for the context. /// Reads the configuration from "Aspire:Microsoft:EntityFrameworkCore:SqlServer:{typeof(TContext).Name}" config section, or "Aspire:Microsoft:EntityFrameworkCore:SqlServer" if former does not exist. /// Thrown if mandatory is null. /// Thrown when mandatory is not provided. public static void AddSqlServerDbContext<[DynamicallyAccessedMembers(RequiredByEF)] TContext>( this IHostApplicationBuilder builder, string connectionName, - Action? configureSettings = null) where TContext : DbContext + Action? configureSettings = null, + Action? configureDbContextOptions = null) where TContext : DbContext { ArgumentNullException.ThrowIfNull(builder); @@ -115,6 +117,8 @@ void ConfigureDbContext(DbContextOptionsBuilder dbContextOptionsBuilder) builder.CommandTimeout(settings.Timeout); } }); + + configureDbContextOptions?.Invoke(dbContextOptionsBuilder); } } } diff --git a/src/Components/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL/AspireEFPostgreSqlExtensions.cs b/src/Components/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL/AspireEFPostgreSqlExtensions.cs index 8a665fc685..455c015423 100644 --- a/src/Components/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL/AspireEFPostgreSqlExtensions.cs +++ b/src/Components/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL/AspireEFPostgreSqlExtensions.cs @@ -24,6 +24,7 @@ public static partial class AspireEFPostgreSqlExtensions /// The to read config from and add services to. /// A name used to retrieve the connection string from the ConnectionStrings configuration section. /// An optional delegate that can be used for customizing options. It's invoked after the settings are read from the configuration. + /// An optional delegate to configure the for the context. /// /// /// Reads the configuration from "Aspire:Npgsql:EntityFrameworkCore:PostgreSQL:{typeof(TContext).Name}" config section, or "Aspire:Npgsql:EntityFrameworkCore:PostgreSQL" if former does not exist. @@ -44,7 +45,8 @@ public static partial class AspireEFPostgreSqlExtensions public static void AddNpgsqlDbContext<[DynamicallyAccessedMembers(RequiredByEF)] TContext>( this IHostApplicationBuilder builder, string connectionName, - Action? configureSettings = null) where TContext : DbContext + Action? configureSettings = null, + Action? configureDbContextOptions = null) where TContext : DbContext { ArgumentNullException.ThrowIfNull(builder); @@ -111,20 +113,20 @@ public static partial class AspireEFPostgreSqlExtensions builder.Services.AddOpenTelemetry() .WithMetrics(meterProviderBuilder => { - // Currently both EF and Npgsql provide only Event Counters: - // https://www.npgsql.org/doc/diagnostics/metrics.html?q=metrics + // Currently EF provides only Event Counters: // https://learn.microsoft.com/en-us/ef/core/logging-events-diagnostics/event-counters?tabs=windows#counters-and-their-meaning meterProviderBuilder.AddEventCountersInstrumentation(eventCountersInstrumentationOptions => { // The magic strings come from: - // https://github.com/npgsql/npgsql/blob/b3282aa6124184162b66dd4ab828041f872bc602/src/Npgsql/NpgsqlEventSource.cs#L14 // https://github.com/dotnet/efcore/blob/a1cd4f45aa18314bc91d2b9ea1f71a3b7d5bf636/src/EFCore/Infrastructure/EntityFrameworkEventSource.cs#L45 - eventCountersInstrumentationOptions.AddEventSources("Npgsql", "Microsoft.EntityFrameworkCore"); - // not adding Npgsql.Sql here, as it's used only for Command Start&Stop events + eventCountersInstrumentationOptions.AddEventSources("Microsoft.EntityFrameworkCore"); }); - // Very recently Npgsql implemented the Metrics support: https://github.com/npgsql/npgsql/pull/5158 - // Currently it's not available at nuget.org, we need to wait. + // https://github.com/npgsql/npgsql/blob/4c9921de2dfb48fb5a488787fc7422add3553f50/src/Npgsql/MetricsReporter.cs#L48 + meterProviderBuilder.AddMeter("Npgsql"); + + // disable "prepared_ratio" until https://github.com/dotnet/aspire/issues/629 is fixed. + meterProviderBuilder.AddView(instrumentName: "db.client.commands.prepared_ratio", MetricStreamConfiguration.Drop); }); } @@ -148,6 +150,8 @@ void ConfigureDbContext(DbContextOptionsBuilder dbContextOptionsBuilder) // https://www.npgsql.org/doc/connection-string-parameters.html#timeouts-and-keepalive // There is nothing for us to set here. }); + + configureDbContextOptions?.Invoke(dbContextOptionsBuilder); } } } diff --git a/src/Components/Aspire.Npgsql/Aspire.Npgsql.csproj b/src/Components/Aspire.Npgsql/Aspire.Npgsql.csproj index 399acc8b3f..9bdb3682db 100644 --- a/src/Components/Aspire.Npgsql/Aspire.Npgsql.csproj +++ b/src/Components/Aspire.Npgsql/Aspire.Npgsql.csproj @@ -16,7 +16,6 @@ - diff --git a/src/Components/Aspire.Npgsql/AspirePostgreSqlNpgsqlExtensions.cs b/src/Components/Aspire.Npgsql/AspirePostgreSqlNpgsqlExtensions.cs index add4fb834c..5b11e3c7b9 100644 --- a/src/Components/Aspire.Npgsql/AspirePostgreSqlNpgsqlExtensions.cs +++ b/src/Components/Aspire.Npgsql/AspirePostgreSqlNpgsqlExtensions.cs @@ -100,12 +100,11 @@ private static void AddNpgsqlDataSource(IHostApplicationBuilder builder, string builder.Services.AddOpenTelemetry() .WithMetrics(meterProviderBuilder => { - // https://www.npgsql.org/doc/diagnostics/metrics.html?q=metrics - meterProviderBuilder.AddEventCountersInstrumentation(eventCountersInstrumentationOptions => - { - // https://github.com/npgsql/npgsql/blob/b3282aa6124184162b66dd4ab828041f872bc602/src/Npgsql/NpgsqlEventSource.cs#L14 - eventCountersInstrumentationOptions.AddEventSources("Npgsql"); - }); + // https://github.com/npgsql/npgsql/blob/4c9921de2dfb48fb5a488787fc7422add3553f50/src/Npgsql/MetricsReporter.cs#L48 + meterProviderBuilder.AddMeter("Npgsql"); + + // disable "prepared_ratio" until https://github.com/dotnet/aspire/issues/629 is fixed. + meterProviderBuilder.AddView(instrumentName: "db.client.commands.prepared_ratio", MetricStreamConfiguration.Drop); }); } } diff --git a/src/Components/Telemetry.md b/src/Components/Telemetry.md index 7c479fc9de..d3bd8a8219 100644 --- a/src/Components/Telemetry.md +++ b/src/Components/Telemetry.md @@ -110,16 +110,16 @@ Aspire.Npgsql: - "Npgsql" - Metric names: - "Npgsql": - - "ec_Npgsql_bytes_written_per_second" - - "ec_Npgsql_bytes_read_per_second" - - "ec_Npgsql_commands_per_second" - - "ec_Npgsql_total_commands" - - "ec_Npgsql_current_commands" - - "ec_Npgsql_failed_commands" - - "ec_Npgsql_prepared_commands_ratio" - - "ec_Npgsql_connection_pools" - - "ec_Npgsql_multiplexing_average_commands_per_batch" - - "ec_Npgsql_multiplexing_average_write_time_per_batch" + - "db.client.commands.bytes_read" + - "db.client.commands.bytes_written" + - "db.client.commands.duration" + - "db.client.commands.executing" + - "db.client.commands.failed" + - "db.client.connections.create_time" + - "db.client.connections.max" + - "db.client.connections.pending_requests" + - "db.client.connections.timeouts" + - "db.client.connections.usage" Aspire.Npgsql.EntityFrameworkCore.PostgreSQL: - Log categories: @@ -148,16 +148,16 @@ Aspire.Npgsql.EntityFrameworkCore.PostgreSQL: - "ec_Microsoft_EntityFramew_total_optimistic_concurrency_failures" - "ec_Microsoft_EntityF_optimistic_concurrency_failures_per_second" - "Npgsql": - - "ec_Npgsql_bytes_written_per_second" - - "ec_Npgsql_bytes_read_per_second" - - "ec_Npgsql_commands_per_second" - - "ec_Npgsql_total_commands" - - "ec_Npgsql_current_commands" - - "ec_Npgsql_failed_commands" - - "ec_Npgsql_prepared_commands_ratio" - - "ec_Npgsql_connection_pools" - - "ec_Npgsql_multiplexing_average_commands_per_batch" - - "ec_Npgsql_multiplexing_average_write_time_per_batch" + - "db.client.commands.bytes_read" + - "db.client.commands.bytes_written" + - "db.client.commands.duration" + - "db.client.commands.executing" + - "db.client.commands.failed" + - "db.client.connections.create_time" + - "db.client.connections.max" + - "db.client.connections.pending_requests" + - "db.client.connections.timeouts" + - "db.client.connections.usage" Aspire.RabbitMQ.Client: - Log categories: diff --git a/tests/Aspire.Microsoft.EntityFrameworkCore.SqlServer.Tests/AspireSqlServerEFCoreSqlClientExtensionsTests.cs b/tests/Aspire.Microsoft.EntityFrameworkCore.SqlServer.Tests/AspireSqlServerEFCoreSqlClientExtensionsTests.cs index 6278996a31..0ad4c81624 100644 --- a/tests/Aspire.Microsoft.EntityFrameworkCore.SqlServer.Tests/AspireSqlServerEFCoreSqlClientExtensionsTests.cs +++ b/tests/Aspire.Microsoft.EntityFrameworkCore.SqlServer.Tests/AspireSqlServerEFCoreSqlClientExtensionsTests.cs @@ -3,6 +3,9 @@ using Aspire.Components.Common.Tests; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure.Internal; +using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal; +using Microsoft.EntityFrameworkCore.Storage; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -68,4 +71,49 @@ public void ConnectionNameWinsOverConfigSection() // the connection string from config should not be used since it was found in ConnectionStrings Assert.DoesNotContain("unused", actualConnectionString); } + + [Fact] + public void CanConfigureDbContextOptions() + { + var builder = Host.CreateEmptyApplicationBuilder(null); + builder.Configuration.AddInMemoryCollection([ + new KeyValuePair("ConnectionStrings:sqlconnection", ConnectionString), + new KeyValuePair("Aspire:Microsoft:EntityFrameworkCore:SqlServer:MaxRetryCount", "304"), + new KeyValuePair("Aspire:Microsoft:EntityFrameworkCore:SqlServer:Timeout", "608") + ]); + + builder.AddSqlServerDbContext("sqlconnection", configureDbContextOptions: optionsBuilder => + { + optionsBuilder.UseSqlServer(sqlBuilder => + { + sqlBuilder.MinBatchSize(123); + }); + }); + + var host = builder.Build(); + var context = host.Services.GetRequiredService(); + +#pragma warning disable EF1001 // Internal EF Core API usage. + + var extension = context.Options.FindExtension(); + Assert.NotNull(extension); + + // ensure the min batch size was respected + Assert.Equal(123, extension.MinBatchSize); + + // ensure the connection string from config was respected + var actualConnectionString = context.Database.GetDbConnection().ConnectionString; + Assert.Equal(ConnectionString, actualConnectionString); + + // ensure the max retry count from config was respected + Assert.NotNull(extension.ExecutionStrategyFactory); + var executionStrategy = extension.ExecutionStrategyFactory(new ExecutionStrategyDependencies(new CurrentDbContext(context), context.Options, null!)); + var retryStrategy = Assert.IsType(executionStrategy); + Assert.Equal(304, retryStrategy.MaxRetryCount); + + // ensure the command timeout from config was respected + Assert.Equal(608, extension.CommandTimeout); + +#pragma warning restore EF1001 // Internal EF Core API usage. + } } diff --git a/tests/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/AspireEFPostgreSqlExtensionsTests.cs b/tests/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/AspireEFPostgreSqlExtensionsTests.cs index 32899cb1a8..b3b15b233c 100644 --- a/tests/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/AspireEFPostgreSqlExtensionsTests.cs +++ b/tests/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/AspireEFPostgreSqlExtensionsTests.cs @@ -3,9 +3,13 @@ using Aspire.Components.Common.Tests; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure.Internal; +using Microsoft.EntityFrameworkCore.Storage; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using Npgsql.EntityFrameworkCore.PostgreSQL; +using Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure.Internal; using Xunit; namespace Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests; @@ -68,4 +72,45 @@ public void ConnectionNameWinsOverConfigSection() // the connection string from config should not be used since it was found in ConnectionStrings Assert.DoesNotContain("unused", actualConnectionString); } + + [Fact] + public void CanConfigureDbContextOptions() + { + var builder = Host.CreateEmptyApplicationBuilder(null); + builder.Configuration.AddInMemoryCollection([ + new KeyValuePair("ConnectionStrings:npgsql", ConnectionString), + new KeyValuePair("Aspire:Npgsql:EntityFrameworkCore:PostgreSQL:MaxRetryCount", "304") + ]); + + builder.AddNpgsqlDbContext("npgsql", configureDbContextOptions: optionsBuilder => + { + optionsBuilder.UseNpgsql(npgsqlBuilder => + { + npgsqlBuilder.CommandTimeout(123); + }); + }); + + var host = builder.Build(); + var context = host.Services.GetRequiredService(); + +#pragma warning disable EF1001 // Internal EF Core API usage. + + var extension = context.Options.FindExtension(); + Assert.NotNull(extension); + + // ensure the command timeout was respected + Assert.Equal(123, extension.CommandTimeout); + + // ensure the connection string from config was respected + var actualConnectionString = context.Database.GetDbConnection().ConnectionString; + Assert.Equal(ConnectionString, actualConnectionString); + + // ensure the max retry count from config was respected + Assert.NotNull(extension.ExecutionStrategyFactory); + var executionStrategy = extension.ExecutionStrategyFactory(new ExecutionStrategyDependencies(new CurrentDbContext(context), context.Options, null!)); + var retryStrategy = Assert.IsType(executionStrategy); + Assert.Equal(304, retryStrategy.MaxRetryCount); + +#pragma warning restore EF1001 // Internal EF Core API usage. + } } diff --git a/tests/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/TestDbContext.cs b/tests/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/TestDbContext.cs index c72340ff52..6769a77db5 100644 --- a/tests/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/TestDbContext.cs +++ b/tests/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/TestDbContext.cs @@ -9,8 +9,11 @@ public class TestDbContext : DbContext { public TestDbContext(DbContextOptions options) : base(options) { + Options = options; } + public DbContextOptions Options { get; } + public DbSet CatalogBrands => Set(); public class CatalogBrand