Pass loggerFactory into MySqlConnection

[trejjam]

Hi, I am not completely sure this is how it should be implemented - speaking about passing ILoggerFactory. But I would love to have a native way to attach the logger pipeline into mysqlConnector.

I would be glad to receive any guidance on how to make this or similar things accepted

[roji]

@trejjam this is the wrong approach; it looks like it creates a MySqlDataSource instance for each and every instance of MySqlRelationalConnection; MySqlDataSource typically represents a connection pool and should be shared.

In general, an EF provider (e.g. the Pomelo provider) should not be concerned with configuring the lower-level ADO.NET provider (in this case the MySqlConnector). MySqlConnector itself should accept the ILoggingFactory - I suggest raising this on that repo. The way to do this is to have the Pomelo provider accept an already-configured data source, and to have that used where a database connection is needed.

@lauxjpn I really hope to get to building DbDataSource support into EF itself for 9.0; I've added DbDataSource specifically to EFCore.PG in the meantime, so you could copy that, but ideally this would all be managed in the core.

[trejjam]

I was not aware of MySqlRelationalConnection lifetime. I expected that it would be close to singleton. Accepting MySqlDataSource or using AddMySqlDataSource would be a preferred way for me too.

https://github.com/mysql-net/MySqlConnector/releases/tag/2.3.0
Add new MySqlConnector.DependencyInjection package: mysql-net/MySqlConnector#1271.
MySqlDataSource and MySqlConnection can be registered with dependency injection by using builder.Services.AddMySqlDataSource(connectionString).
This also configures logging automatically.

[bgrainger]

The way to do this is to have the Pomelo provider accept an already-configured data source, and to have that used where a database connection is needed.

One potential issue with this is that it looks like Pomelo sometimes (or always? I haven't fully followed the code path) adds options to the connection string:

Pomelo.EntityFrameworkCore.MySql/src/EFCore.MySql/Storage/Internal/MySqlRelationalConnection.cs

Line 75 in 0209341

protected virtual MySqlConnectionStringBuilder AddConnectionStringOptions(MySqlConnectionStringBuilder builder)

This wouldn't be possible when using a MySqlDataSource directly as the connection string for the data source is fixed. Perhaps the Pomelo code would need to detect unsupported options and throw an exception with instructions on how to fix the problem, rather than implicitly coercing them?

[trejjam]

It could be done if AddMySqlDataSource is using Options pattern:

new ServiceDescriptor(
typeof(MySqlDataSource),
serviceProvider =>
{
	var dataSourceBuilder = new MySqlDataSourceBuilder(
                serviceProvider.GetService<IOptions<MySqlConnectionStringBuilder>>().Value.ConnectionString
        ).UseLoggerFactory(serviceProvider.GetService<ILoggerFactory>());
	dataSourceBuilderAction?.Invoke(dataSourceBuilder);
	return dataSourceBuilder.Build();
},
dataSourceLifetime));

EF then can do:

serviceCollection.AddOptions<MySqlConnectionStringBuilder>()
  .Configure<IOptions<PomeloEfCoreOptions>>((pomeloOptions, mysqlOptions) => mysqlOptions.TreatTinyAsBoolean = pomeloOptions.TreatTinyAsBoolean);

serviceCollection.AddSingleton<IValidateOptions<MySqlConnectionStringBuilder>, EfCoreMysqlValidation>();

public class EfCoreMysqlValidation : IValidateOptions<MySqlDataSourceOptions>
{
    public EfCoreMysqlValidation (IOptions<PomeloEfCoreOptions> pomeloEfCoreOptions) {
        _pomeloEfCoreOptions = pomeloEfCoreOptions;
    }

    public ValidateOptionsResult Validate(string? name, MySqlConnectionStringBuilder options)
    {
        // validate options.TreatTinyAsBoolean == _pomeloEfCoreOptions.Value.TreatTinyAsBoolean
    }
}

[trejjam]

By that EF can set and also verify configuration (so a user does not shoot himself in the leg) and heavy lifting is done in the MysqlConnector library

[lauxjpn]

In general, an EF provider (e.g. the Pomelo provider) should not be concerned with configuring the lower-level ADO.NET provider (in this case the MySqlConnector). MySqlConnector itself should accept the ILoggingFactory - I suggest raising this on that repo. The way to do this is to have the Pomelo provider accept an already-configured data source, and to have that used where a database connection is needed.

@trejjam I agree with @roji.

---

I've added DbDataSource specifically to EFCore.PG in the meantime, so you could copy that, but ideally this would all be managed in the core.

@roji Thanks, I'll take a look at it.

[trejjam]

Closing in favor of #1817