Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double logging of SQL when using EF and Npgsql in DI #2821

Open
roji opened this issue Jul 29, 2023 · 1 comment
Open

Double logging of SQL when using EF and Npgsql in DI #2821

roji opened this issue Jul 29, 2023 · 1 comment
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@roji
Copy link
Member

roji commented Jul 29, 2023

Since we have NpgsqlDataSource, the "best practice" way of setting up EFCore.PG in an ASP.NET application is as follows:

builder.Services.AddNpgsqlDataSource(@"Host=localhost;Username=test;Password=test");
builder.Services.AddDbContextPool<BlogContext>(o => o.UseNpgsql());

In other words, the NpgsqlDataSource is added separately of EF (that's the lower-level ADO.NET layer), and then EFCore.PG automatically finds that in DI (that's UseNpgsql()).

This works really well, allows clear layer separation and configurability of each layer. Unfortunately, both layers automatically look for an ILoggerFactory in DI and log through it. And since both components log SQLs at Info level, the user gets to see their SQL twice. This can be worked around by setting the default log-level for Npgsql.Command to Warn, preferring the EF logs (which generally provide more information).

Long-term, the solution is likely related to #2542: we already want to allow users to do NpgsqlDataSource configuration via the EF UseNpgsql API, so that EF-level configs can also take care of ADO-level configs. For example, since NodaTime support at the EFCore.PG layer relies on NodaTime support at the Npgsql layer, doing UseNodaTime() in EFCore.PG's UseNpgsql() would implicitly turn on NodaTime on the NpgsqlDataSource as well (today users need to do UseNodaTime() twice).

If we implement this, then the EFCore.PG UseNpgsql() could use an API to tell NpgsqlDataSourceBuilder to not log SQL, since that's already taken care of by EF.

/cc @ajcvickers @adamsitnik @davidfowl

Full source for minimal appbuilder.Services.AddNpgsqlDataSource(@"Host=localhost;Username=test;Password=test");
builder.Services.AddDbContextPool(o => o.UseNpgsql());

var app = builder.Build();

app.MapGet("/", async (BlogContext context) => "Hello World!: " + await context.Blogs.CountAsync());

app.Run();

public class BlogContext : DbContext
{
public BlogContext(DbContextOptions options)
: base(options)
{
}

public DbSet<Blog> Blogs { get; set; }

}

public class Blog
{
public int Id { get; set; }
public string? Name { get; set; }
}

</details>
@roji roji added the enhancement New feature or request label Jul 29, 2023
@roji roji added this to the 8.0.0 milestone Jul 29, 2023
@roji roji self-assigned this Jul 29, 2023
@adamsitnik
Copy link
Contributor

This can be worked around by setting the default log-level for Npgsql.Command to War

Is it possible to do that with NpgsqlDataSourceBuilder or any other existing API?

So far the best thing I found is:

builder.Configuration.AddInMemoryCollection(new KeyValuePair<string, string>[1]
{
    new KeyValuePair<string, string>("Logging:LogLevel:Npgsql.Command", "Warning")
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants