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

MySqlConnector.Logging.Microsoft.Extensions.Logging is not prefixing the category name like other loggers #1080

Closed
KirillBorunov opened this issue Nov 13, 2021 · 5 comments

Comments

@KirillBorunov
Copy link

Software versions
MySqlConnector.Logging.Microsoft.Extensions.Logging version: 2.0.0

Describe the bug
The mentioned package is not prefixing the log categories.
So I see categories like "ConnectionPool" instead of "MySqlConnector.ConnectionPool".
Because of this, collisions are possible and I cannot filter all the MySqlConnector logs with single filter, need to know each possible category.

Here is the missing prefix:

public IMySqlConnectorLogger CreateLogger(string name) => new MicrosoftExtensionsLoggingLogger(m_loggerFactory.CreateLogger(name));

The other logging packages DO prefix:

public IMySqlConnectorLogger CreateLogger(string name) => new Log4netLogger(LogManager.GetLogger(s_loggerAssembly, "MySqlConnector." + name));

public SerilogLogger(string name) => m_logger = Serilog.Log.ForContext("SourceContext", "MySqlConnector." + name);

public IMySqlConnectorLogger CreateLogger(string name) => new NLogLogger(LogManager.GetLogger("MySqlConnector." + name));

Exception
Not applicable.

Code sample

LoggerFactory = Microsoft.Extensions.Logging.LoggerFactory.Create(cfg => 
    cfg.AddConsole()
    .SetMinimumLevel(LogLevel.Debug)
    .AddFilter("ConnectionPool", LogLevel.Information) //How can I filter all MySqlConnector logs? How I can know all posible categories?
);

MySqlConnectorLogManager.Provider = new MicrosoftExtensionsLoggingLoggerProvider(LoggerFactory);

Expected behavior
I expect a prefix "MySqlConnector." just like on the other loggers.

Additional context
None

@bgrainger
Copy link
Member

I think you're right, but it's surprising no one's pointed this out before. (And now it would be a breaking behavioural change... so it would have been nice to know a week ago, before shipping 2.0. 😀)

For now, it's fairly simple to work around with your own code:

sealed class MySqlConnectorPrefixLoggerFactory : ILoggerFactory
{
	readonly ILoggerFactory _factory;
	
	public MySqlConnectorPrefixLoggerFactory(ILoggerFactory factory) => _factory = factory;
	
	public void AddProvider(ILoggerProvider provider) => _factory.AddProvider(provider);

	public ILogger CreateLogger(string categoryName) => _factory.CreateLogger("MySqlConnector." + categoryName);

	public void Dispose() => _factory.Dispose();
}

// ... other code

MySqlConnectorLogManager.Provider = new MicrosoftExtensionsLoggingLoggerProvider(new MySqlConnectorPrefixLoggerFactory(loggerFactory));

(Does such a category-name-prefixing type already exist somewhere? It seems like a useful type but I couldn't find one after a brief search.)

I'll have to think about how best to implement this change (to avoid upgrade surprises from 1.0.0 -> 2.0.0 -> fixed version).

@KirillBorunov
Copy link
Author

KirillBorunov commented Nov 14, 2021

For now, it's fairly simple to work around with your own code:

That's exactly what I did. It works OK.

I'll have to think about how best to implement this change (to avoid upgrade surprises from 1.0.0 -> 2.0.0 -> fixed version).

What if you allow to at least specify a OPTIONAL prefix on the constructor?
Maybe on all the logger packages. Maybe someone wants to use a category "DB" instead of "MySqlConnector" to log all database related messages the MySqlConnector ones + custom ones.
That way you will not fix it directly but add a feature that will allow users to do what they want with categories/prefixes.

new MicrosoftExtensionsLoggingLoggerProvider(LoggerFactory, "DB"); //constructor should ensure there is a dot

@bgrainger
Copy link
Member

I was thinking an opt-out, e.g., MicrosoftExtensionsLoggingLoggerProvider(ILoggerFactory loggerFactory, bool omitMySqlConnectorPrefix = false);

This would give better logging by default, but allow users who were relying on the old category names to opt out.

@KirillBorunov
Copy link
Author

Your way seems easier to implement, and solves the original problem, I think its OK.

@bgrainger
Copy link
Member

Fixed in 2.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants