Skip to content

Commit

Permalink
Fix database issues (#630) (#637)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
eerhardt authored Nov 1, 2023
1 parent 88b0197 commit f990b7b
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ public static class AspireSqlServerEFCoreSqlClientExtensions
/// <param name="builder">The <see cref="IHostApplicationBuilder" /> to read config from and add services to.</param>
/// <param name="connectionName">A name used to retrieve the connection string from the ConnectionStrings configuration section.</param>
/// <param name="configureSettings">An optional delegate that can be used for customizing options. It's invoked after the settings are read from the configuration.</param>
/// <param name="configureDbContextOptions">An optional delegate to configure the <see cref="DbContextOptions"/> for the context.</param>
/// <remarks>Reads the configuration from "Aspire:Microsoft:EntityFrameworkCore:SqlServer:{typeof(TContext).Name}" config section, or "Aspire:Microsoft:EntityFrameworkCore:SqlServer" if former does not exist.</remarks>
/// <exception cref="ArgumentNullException">Thrown if mandatory <paramref name="builder"/> is null.</exception>
/// <exception cref="InvalidOperationException">Thrown when mandatory <see cref="MicrosoftEntityFrameworkCoreSqlServerSettings.ConnectionString"/> is not provided.</exception>
public static void AddSqlServerDbContext<[DynamicallyAccessedMembers(RequiredByEF)] TContext>(
this IHostApplicationBuilder builder,
string connectionName,
Action<MicrosoftEntityFrameworkCoreSqlServerSettings>? configureSettings = null) where TContext : DbContext
Action<MicrosoftEntityFrameworkCoreSqlServerSettings>? configureSettings = null,
Action<DbContextOptionsBuilder>? configureDbContextOptions = null) where TContext : DbContext
{
ArgumentNullException.ThrowIfNull(builder);

Expand Down Expand Up @@ -115,6 +117,8 @@ void ConfigureDbContext(DbContextOptionsBuilder dbContextOptionsBuilder)
builder.CommandTimeout(settings.Timeout);
}
});

configureDbContextOptions?.Invoke(dbContextOptionsBuilder);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public static partial class AspireEFPostgreSqlExtensions
/// <param name="builder">The <see cref="IHostApplicationBuilder" /> to read config from and add services to.</param>
/// <param name="connectionName">A name used to retrieve the connection string from the ConnectionStrings configuration section.</param>
/// <param name="configureSettings">An optional delegate that can be used for customizing options. It's invoked after the settings are read from the configuration.</param>
/// <param name="configureDbContextOptions">An optional delegate to configure the <see cref="DbContextOptions"/> for the context.</param>
/// <remarks>
/// <para>
/// Reads the configuration from "Aspire:Npgsql:EntityFrameworkCore:PostgreSQL:{typeof(TContext).Name}" config section, or "Aspire:Npgsql:EntityFrameworkCore:PostgreSQL" if former does not exist.
Expand All @@ -44,7 +45,8 @@ public static partial class AspireEFPostgreSqlExtensions
public static void AddNpgsqlDbContext<[DynamicallyAccessedMembers(RequiredByEF)] TContext>(
this IHostApplicationBuilder builder,
string connectionName,
Action<NpgsqlEntityFrameworkCorePostgreSQLSettings>? configureSettings = null) where TContext : DbContext
Action<NpgsqlEntityFrameworkCorePostgreSQLSettings>? configureSettings = null,
Action<DbContextOptionsBuilder>? configureDbContextOptions = null) where TContext : DbContext
{
ArgumentNullException.ThrowIfNull(builder);

Expand Down Expand Up @@ -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);
});
}

Expand All @@ -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);
}
}
}
1 change: 0 additions & 1 deletion src/Components/Aspire.Npgsql/Aspire.Npgsql.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
<PackageReference Include="Npgsql.DependencyInjection" />
<PackageReference Include="Npgsql.OpenTelemetry" />
<PackageReference Include="OpenTelemetry.Extensions.Hosting" />
<PackageReference Include="OpenTelemetry.Instrumentation.EventCounters" />
</ItemGroup>

</Project>
11 changes: 5 additions & 6 deletions src/Components/Aspire.Npgsql/AspirePostgreSqlNpgsqlExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
}
Expand Down
40 changes: 20 additions & 20 deletions src/Components/Telemetry.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<string, string?>("ConnectionStrings:sqlconnection", ConnectionString),
new KeyValuePair<string, string?>("Aspire:Microsoft:EntityFrameworkCore:SqlServer:MaxRetryCount", "304"),
new KeyValuePair<string, string?>("Aspire:Microsoft:EntityFrameworkCore:SqlServer:Timeout", "608")
]);

builder.AddSqlServerDbContext<TestDbContext>("sqlconnection", configureDbContextOptions: optionsBuilder =>
{
optionsBuilder.UseSqlServer(sqlBuilder =>
{
sqlBuilder.MinBatchSize(123);
});
});

var host = builder.Build();
var context = host.Services.GetRequiredService<TestDbContext>();

#pragma warning disable EF1001 // Internal EF Core API usage.

var extension = context.Options.FindExtension<SqlServerOptionsExtension>();
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<SqlServerRetryingExecutionStrategy>(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.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<string, string?>("ConnectionStrings:npgsql", ConnectionString),
new KeyValuePair<string, string?>("Aspire:Npgsql:EntityFrameworkCore:PostgreSQL:MaxRetryCount", "304")
]);

builder.AddNpgsqlDbContext<TestDbContext>("npgsql", configureDbContextOptions: optionsBuilder =>
{
optionsBuilder.UseNpgsql(npgsqlBuilder =>
{
npgsqlBuilder.CommandTimeout(123);
});
});

var host = builder.Build();
var context = host.Services.GetRequiredService<TestDbContext>();

#pragma warning disable EF1001 // Internal EF Core API usage.

var extension = context.Options.FindExtension<NpgsqlOptionsExtension>();
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<NpgsqlRetryingExecutionStrategy>(executionStrategy);
Assert.Equal(304, retryStrategy.MaxRetryCount);

#pragma warning restore EF1001 // Internal EF Core API usage.
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ public class TestDbContext : DbContext
{
public TestDbContext(DbContextOptions<TestDbContext> options) : base(options)
{
Options = options;
}

public DbContextOptions<TestDbContext> Options { get; }

public DbSet<CatalogBrand> CatalogBrands => Set<CatalogBrand>();

public class CatalogBrand
Expand Down

0 comments on commit f990b7b

Please sign in to comment.