Skip to content
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
cca920b
Add Pomelo.EntityFrameworkCore.MySql component.
bgrainger Dec 1, 2023
a0e6c0d
Add Pomelo.EntityFrameworkCore.MySql tests.
bgrainger Dec 1, 2023
75f1b1f
Merge main into pomelo.
bgrainger Dec 19, 2023
d94b366
Remove incorrect documentation.
bgrainger Dec 19, 2023
e46b6d5
Remove irrelevant TODO comments.
bgrainger Dec 19, 2023
b932eb2
Update XML documentation.
bgrainger Dec 19, 2023
58c86ce
Update XML documentation to current conventions.
bgrainger Dec 19, 2023
d41e043
Add documentation for Pomelo metrics.
bgrainger Dec 19, 2023
5ab2474
Change catch block to the exception Pomelo throws.
bgrainger Dec 19, 2023
5fdfef5
Merge main into pomelo.
bgrainger Jan 4, 2024
4712079
Reuse existing TestDbContext.cs file.
bgrainger Jan 4, 2024
953bb3f
Use configuration schema generator.
bgrainger Jan 4, 2024
e6101a9
Remove "Container" from AddMySql.
bgrainger Jan 4, 2024
2623f93
Allow ServerVersion setting to be optional.
bgrainger Jan 4, 2024
f133452
Remove (now optional) ServerVersion setting from test.
bgrainger Jan 4, 2024
cdb6909
Merge main into pomelo.
bgrainger Jan 24, 2024
59f9952
Fix bad merge.
bgrainger Jan 24, 2024
4d9913d
Remove MySqlConnector logging categories from Pomelo section.
bgrainger Jan 24, 2024
a73e4fe
Delete meaningless comment.
bgrainger Jan 24, 2024
db875a4
Add Microsoft.Extensions.Configuration.Binder.
bgrainger Jan 24, 2024
823722c
Revert "Remove (now optional) ServerVersion setting from test."
bgrainger Jan 24, 2024
bc605cf
Add basic MySQL Aspire.Hosting tests.
bgrainger Jan 24, 2024
817940b
Add Pomelo EFCore integration tests.
bgrainger Jan 24, 2024
ab0419a
Merge main into pomelo.
bgrainger Jan 24, 2024
0964125
Incorporate project change from dff1467f.
bgrainger Jan 24, 2024
079c30e
PR feedback
eerhardt Jan 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Aspire.sln
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "CosmosEndToEnd.ApiService",
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Playground.ServiceDefaults", "playground\Playground.ServiceDefaults\Playground.ServiceDefaults.csproj", "{25208C6F-0A9D-4D60-9EDD-256C9891B1CD}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Aspire.Pomelo.EntityFrameworkCore.MySql", "src\Components\Aspire.Pomelo.EntityFrameworkCore.MySql\Aspire.Pomelo.EntityFrameworkCore.MySql.csproj", "{C565532A-0754-44FE-A0C7-78D5338DDBCA}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Aspire.Pomelo.EntityFrameworkCore.MySql.Tests", "tests\Aspire.Pomelo.EntityFrameworkCore.MySql.Tests\Aspire.Pomelo.EntityFrameworkCore.MySql.Tests.csproj", "{BFAF55A8-A737-4EC1-BBA2-76001A8F16E0}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -539,6 +543,14 @@ Global
{25208C6F-0A9D-4D60-9EDD-256C9891B1CD}.Debug|Any CPU.Build.0 = Debug|Any CPU
{25208C6F-0A9D-4D60-9EDD-256C9891B1CD}.Release|Any CPU.ActiveCfg = Release|Any CPU
{25208C6F-0A9D-4D60-9EDD-256C9891B1CD}.Release|Any CPU.Build.0 = Release|Any CPU
{C565532A-0754-44FE-A0C7-78D5338DDBCA}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{C565532A-0754-44FE-A0C7-78D5338DDBCA}.Debug|Any CPU.Build.0 = Debug|Any CPU
{C565532A-0754-44FE-A0C7-78D5338DDBCA}.Release|Any CPU.ActiveCfg = Release|Any CPU
{C565532A-0754-44FE-A0C7-78D5338DDBCA}.Release|Any CPU.Build.0 = Release|Any CPU
{BFAF55A8-A737-4EC1-BBA2-76001A8F16E0}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{BFAF55A8-A737-4EC1-BBA2-76001A8F16E0}.Debug|Any CPU.Build.0 = Debug|Any CPU
{BFAF55A8-A737-4EC1-BBA2-76001A8F16E0}.Release|Any CPU.ActiveCfg = Release|Any CPU
{BFAF55A8-A737-4EC1-BBA2-76001A8F16E0}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -633,6 +645,8 @@ Global
{51DDD6BC-1D6C-466A-B509-FC49E3BD72E4} = {DBEDDF76-1C33-4943-8CCB-337A7D48AFF5}
{EABB20A8-CDA2-4AFE-A5B1-FB631200CD64} = {DBEDDF76-1C33-4943-8CCB-337A7D48AFF5}
{25208C6F-0A9D-4D60-9EDD-256C9891B1CD} = {D173887B-AF42-4576-B9C1-96B9E9B3D9C0}
{C565532A-0754-44FE-A0C7-78D5338DDBCA} = {27381127-6C45-4B4C-8F18-41FF48DFE4B2}
{BFAF55A8-A737-4EC1-BBA2-76001A8F16E0} = {4981B3A5-4AFD-4191-BF7D-8692D9783D60}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {6DCEDFEC-988E-4CB3-B45B-191EB5086E0C}
Expand Down
2 changes: 2 additions & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,12 @@
<PackageVersion Include="Microsoft.Orleans.Server" Version="8.1.0-nightly.20240111.1" />
<PackageVersion Include="Microsoft.Orleans.Reminders.AzureStorage" Version="8.1.0-nightly.20240111.1" />
<PackageVersion Include="MySqlConnector.DependencyInjection" Version="2.3.1" />
<PackageVersion Include="MySqlConnector.Logging.Microsoft.Extensions.Logging" Version="2.1.0" />
<PackageVersion Include="Npgsql.DependencyInjection" Version="8.0.0" />
<PackageVersion Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="8.0.0" />
<PackageVersion Include="Oracle.EntityFrameworkCore" Version="8.21.121" />
<PackageVersion Include="Polly" Version="8.2.0" />
<PackageVersion Include="Pomelo.EntityFrameworkCore.MySql" Version="8.0.0-beta.2" />
<PackageVersion Include="RabbitMQ.Client" Version="6.7.0" />
<PackageVersion Include="StackExchange.Redis" Version="2.7.4" />
<PackageVersion Include="Swashbuckle.AspNetCore" Version="6.5.0" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>$(NetCurrent)</TargetFramework>
<IsPackable>true</IsPackable>
<PackageTags>$(ComponentEfCorePackageTags) pomelo mysql sql</PackageTags>
<Description>A MySQL provider for Entity Framework Core that integrates with Aspire, including connection pooling, health checks, logging, and telemetry.</Description>
<PackageIconFullPath>$(SharedDir)SQL_256x.png</PackageIconFullPath>
</PropertyGroup>

<ItemGroup>
<Compile Include="..\Common\ConfigurationSchemaAttributes.cs" Link="ConfigurationSchemaAttributes.cs" />
<Compile Include="..\Common\HealthChecksExtensions.cs" Link="HealthChecksExtensions.cs" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Configuration.Binder" />
<PackageReference Include="Microsoft.Extensions.Diagnostics.HealthChecks.EntityFrameworkCore" />
<PackageReference Include="MySqlConnector.Logging.Microsoft.Extensions.Logging" />
<PackageReference Include="Pomelo.EntityFrameworkCore.MySql" />
<PackageReference Include="OpenTelemetry.Extensions.Hosting" />
<PackageReference Include="OpenTelemetry.Instrumentation.EventCounters" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using Aspire;
using Aspire.Pomelo.EntityFrameworkCore.MySql;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using MySqlConnector.Logging;
using OpenTelemetry.Metrics;

namespace Microsoft.Extensions.Hosting;

/// <summary>
/// Provides extension methods for registering a MySQL database context in an Aspire application.
/// </summary>
public static partial class AspireEFMySqlExtensions
{
private const string DefaultConfigSectionName = "Aspire:Pomelo:EntityFrameworkCore:MySql";
private const DynamicallyAccessedMemberTypes RequiredByEF = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.PublicProperties;

/// <summary>
/// Registers the given <see cref="DbContext" /> as a service in the services provided by the <paramref name="builder"/>.
/// Enables db context pooling, corresponding health check, logging and telemetry.
/// </summary>
/// <typeparam name="TContext">The <see cref="DbContext" /> that needs to be registered.</typeparam>
/// <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:Pomelo:EntityFrameworkCore:MySql:{typeof(TContext).Name}" config section, or "Aspire:Pomelo:EntityFrameworkCore:MySql" if former does not exist.
/// </para>
/// <para>
/// The <see cref="DbContext.OnConfiguring" /> method can then be overridden to configure <see cref="DbContext" /> options.
/// </para>
/// </remarks>
/// <exception cref="ArgumentNullException">Thrown if mandatory <paramref name="builder"/> is null.</exception>
/// <exception cref="InvalidOperationException">Thrown when mandatory <see cref="PomeloEntityFrameworkCoreMySqlSettings.ConnectionString"/> is not provided.</exception>
public static void AddMySqlDbContext<[DynamicallyAccessedMembers(RequiredByEF)] TContext>(
this IHostApplicationBuilder builder,
string connectionName,
Action<PomeloEntityFrameworkCoreMySqlSettings>? configureSettings = null,
Action<DbContextOptionsBuilder>? configureDbContextOptions = null) where TContext : DbContext
{
ArgumentNullException.ThrowIfNull(builder);

PomeloEntityFrameworkCoreMySqlSettings settings = new();
var typeSpecificSectionName = $"{DefaultConfigSectionName}:{typeof(TContext).Name}";
var typeSpecificConfigurationSection = builder.Configuration.GetSection(typeSpecificSectionName);
if (typeSpecificConfigurationSection.Exists()) // https://github.com/dotnet/runtime/issues/91380
{
typeSpecificConfigurationSection.Bind(settings);
}
else
{
builder.Configuration.GetSection(DefaultConfigSectionName).Bind(settings);
}

if (builder.Configuration.GetConnectionString(connectionName) is string connectionString)
{
settings.ConnectionString = connectionString;
}

configureSettings?.Invoke(settings);

if (settings.DbContextPooling)
{
builder.Services.AddDbContextPool<TContext>(ConfigureDbContext);
}
else
{
builder.Services.AddDbContext<TContext>(ConfigureDbContext);
}

if (settings.HealthChecks)
{
// calling MapHealthChecks is the responsibility of the app, not Component
builder.TryAddHealthCheck(
name: typeof(TContext).Name,
static hcBuilder => hcBuilder.AddDbContextCheck<TContext>());
}

if (settings.Tracing)
{
builder.Services.AddOpenTelemetry()
.WithTracing(tracerProviderBuilder =>
{
// add tracing from the underlying MySqlConnector ADO.NET library
tracerProviderBuilder.AddSource("MySqlConnector");
});
}

if (settings.Metrics)
{
builder.Services.AddOpenTelemetry()
.WithMetrics(meterProviderBuilder =>
{
// Currently EF provides only Event Counters:
// https://learn.microsoft.com/ef/core/logging-events-diagnostics/event-counters?tabs=windows#counters-and-their-meaning
meterProviderBuilder.AddEventCountersInstrumentation(eventCountersInstrumentationOptions =>
{
// The magic strings come from:
// https://github.com/dotnet/efcore/blob/a1cd4f45aa18314bc91d2b9ea1f71a3b7d5bf636/src/EFCore/Infrastructure/EntityFrameworkEventSource.cs#L45
eventCountersInstrumentationOptions.AddEventSources("Microsoft.EntityFrameworkCore");
});

// add metrics from the underlying MySqlConnector ADO.NET library
meterProviderBuilder.AddMeter("MySqlConnector");
});
}

void ConfigureDbContext(IServiceProvider serviceProvider, DbContextOptionsBuilder dbContextOptionsBuilder)
{
// use the legacy method of setting the ILoggerFactory because Pomelo EF Core doesn't use MySqlDataSource
if (serviceProvider.GetService<ILoggerFactory>() is { } loggerFactory)
{
MySqlConnectorLogManager.Provider = new MicrosoftExtensionsLoggingLoggerProvider(loggerFactory);
}

var connectionString = settings.ConnectionString ?? string.Empty;

ServerVersion serverVersion;
if (settings.ServerVersion is null)
{
if (string.IsNullOrEmpty(connectionString))
{
ThrowForMissingConnectionString();
}
serverVersion = ServerVersion.AutoDetect(connectionString);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Playing with this change locally, I'm a bit concerned about this now. I'm trying to run the integration tests (which aren't run in CI yet - we are working on that). If the Pomelo test is run by itself, it fails because just trying to get the DbContext out of DI is trying to establish a connection. But the MySql docker container isn't up yet (it takes a while to get up) and this line is throwing because it can't make a connection.

I wonder if we should retry this up to the MaxRetryCount. It isn't a great experience to fail getting the DbContext because it happens as part of ASP.NET calling into the user's code. So there's no chance for them to retry.

Thoughts? Maybe this shouldn't block the initial check-in, since you can work around the issue by setting ServerVersion explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should retry this up to the MaxRetryCount.

And then still fail with a fatal error if it can't make a connection? (As opposed to making up a value like "8.0.0" or "5.7.28" or something that might not actually be safe if the user is running MariaDB, depending on how much this changes Pomelo's behavior.)

That definitely seems like an improvement over the status quo to me. (Unless of course 100% of the time it still fails but now just takes longer.)

In my original PR (before 2623f93), this was a mandatory option that had to be supplied by the user. That was less user-friendly on one hand, but maybe more predictable?

Very random thought (and may be going completely down the wrong path): is there some "magic" we could do by detecting if there's a ContainerImageAnnotation? E.g., set an appropriate default ServerVersion by faking a version string based on whether the user is using mysql:latest or mariadb:11.0? (That's not actually customisable in AddMySqlContainer right now though maybe it should be.) That only works for the local Docker scenario, but maybe that's OK because for production deployment we can assume that the "real" database is up and serving requests?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss the options in a new issue explicitly for this. Can you open it? or do you want me to?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
else
{
serverVersion = ServerVersion.Parse(settings.ServerVersion);
}

var builder = dbContextOptionsBuilder.UseMySql(connectionString, serverVersion, builder =>
{
// delay validating the ConnectionString until the DbContext is configured. This ensures an exception doesn't happen until a Logger is established.
if (string.IsNullOrEmpty(connectionString))
{
ThrowForMissingConnectionString();
}

// Resiliency:
// 1. Connection resiliency automatically retries failed database commands: https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/wiki/Configuration-Options#enableretryonfailure
if (settings.MaxRetryCount > 0)
{
builder.EnableRetryOnFailure(settings.MaxRetryCount);
}
});

configureDbContextOptions?.Invoke(dbContextOptionsBuilder);

void ThrowForMissingConnectionString()
{
throw new InvalidOperationException($"ConnectionString is missing. It should be provided in 'ConnectionStrings:{connectionName}' or under the 'ConnectionString' key in '{DefaultConfigSectionName}' or '{typeSpecificSectionName}' configuration section.");
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Aspire;
using Aspire.Pomelo.EntityFrameworkCore.MySql;

[assembly: ConfigurationSchema("Aspire:Pomelo:EntityFrameworkCore:MySql", typeof(PomeloEntityFrameworkCoreMySqlSettings))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also add all the logging categories that are in Telemetry.md. That way they light up in the appsettings.json file intellisense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I shouldn't have duplicated the MySqlConnector logging categories into the Pomelo section in Telemetry.md. (I assume they will ultimately get pulled in through being a transitive dependency?) I'll remove them from Telemetry.md.

Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
{
"properties": {
"Aspire": {
"type": "object",
"properties": {
"Pomelo": {
"type": "object",
"properties": {
"EntityFrameworkCore": {
"type": "object",
"properties": {
"MySql": {
"type": "object",
"properties": {
"ConnectionString": {
"type": "string",
"description": "Gets or sets the connection string of the MySQL database to connect to."
},
"DbContextPooling": {
"type": "boolean",
"description": "Gets or sets a boolean value that indicates whether the DbContext will be pooled or explicitly created every time it's requested.",
"default": true
},
"HealthChecks": {
"type": "boolean",
"description": "Gets or sets a boolean value that indicates whether the database health check is enabled or not.",
"default": true
},
"MaxRetryCount": {
"type": "integer",
"description": "Gets or sets the maximum number of retry attempts."
},
"Metrics": {
"type": "boolean",
"description": "Gets or sets a boolean value that indicates whether the OpenTelemetry metrics are enabled or not.",
"default": true
},
"ServerVersion": {
"type": "string",
"description": "Gets or sets the server version of the MySQL database to connect to."
},
"Tracing": {
"type": "boolean",
"description": "Gets or sets a boolean value that indicates whether the OpenTelemetry tracing is enabled or not.",
"default": true
}
},
"description": "Provides the client configuration settings for connecting to a MySQL database using EntityFrameworkCore."
}
}
}
}
}
}
}
},
"type": "object"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Aspire.Pomelo.EntityFrameworkCore.MySql;

/// <summary>
/// Provides the client configuration settings for connecting to a MySQL database using EntityFrameworkCore.
/// </summary>
public sealed class PomeloEntityFrameworkCoreMySqlSettings
{
/// <summary>
/// Gets or sets the connection string of the MySQL database to connect to.
/// </summary>
public string? ConnectionString { get; set; }

/// <summary>
/// Gets or sets the server version of the MySQL database to connect to.
/// </summary>
public string? ServerVersion { get; set; }

/// <summary>
/// Gets or sets a boolean value that indicates whether the DbContext will be pooled or explicitly created every time it's requested.
/// </summary>
/// <value>
/// The default value is <see langword="true"/>.
/// </value>
/// <remarks>Should be set to false in multi-tenant scenarios.</remarks>
public bool DbContextPooling { get; set; } = true;

/// <summary>
/// Gets or sets the maximum number of retry attempts.
/// </summary>
/// <value>
/// The default is 6.
/// Set it to 0 to disable the retry mechanism.
/// </value>
public int MaxRetryCount { get; set; } = 6;

/// <summary>
/// Gets or sets a boolean value that indicates whether the database health check is enabled or not.
/// </summary>
/// <value>
/// The default value is <see langword="true"/>.
/// </value>
public bool HealthChecks { get; set; } = true;

/// <summary>
/// Gets or sets a boolean value that indicates whether the OpenTelemetry tracing is enabled or not.
/// </summary>
/// <value>
/// The default value is <see langword="true"/>.
/// </value>
public bool Tracing { get; set; } = true;

/// <summary>
/// Gets or sets a boolean value that indicates whether the OpenTelemetry metrics are enabled or not.
/// </summary>
/// <value>
/// The default value is <see langword="true"/>.
/// </value>
public bool Metrics { get; set; } = true;
}
Loading