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

calling AddNlog() multi time, will add another Nlog provider #254

Closed
ciiko opened this issue Nov 8, 2018 · 7 comments
Closed

calling AddNlog() multi time, will add another Nlog provider #254

ciiko opened this issue Nov 8, 2018 · 7 comments
Milestone

Comments

@ciiko
Copy link

ciiko commented Nov 8, 2018

on NLog.Extensions.Logging, ( 1.3.0)

  • What is the current result?
    ILoggerFactory Factory = new Microsoft.Extensions.Logging.LoggerFactory();
    Factory .AddNlog();
    Factory .AddNlog();

calling AddNlog() will add another NLog provider, resulting in same line being outputted in log file

  • What is the expected result?
    calling AddNlog() multi time will not add another NLog provider

  • Did you checked the Internal log?
    no

  • Please post full exception details (message, stacktrace, inner exceptions)
    same line being outputted in log file, in direct proportion to how many time i have call AddNlog()

  • Are there any work arrounds? yes/no
    yes

  • Is there a version in which it did worked?
    No

  • Can you help us by writing an unit test?
    yes

@snakefoot
Copy link
Contributor

Pull Requests are always welcome

@304NotModified
Copy link
Member

Not sure if we can detect this. Someone looked at this?

@304NotModified
Copy link
Member

We can only do this by using our own factory (planned for v2)

Currently we we call AddProvider on ILoggerFactory, and it has no hasProvider or something like that:

namespace Microsoft.Extensions.Logging
{
  /// <summary>
  /// Represents a type used to configure the logging system and create instances of <see cref="T:Microsoft.Extensions.Logging.ILogger" /> from
  /// the registered <see cref="T:Microsoft.Extensions.Logging.ILoggerProvider" />s.
  /// </summary>
  public interface ILoggerFactory : IDisposable
  {
    /// <summary>
    /// Creates a new <see cref="T:Microsoft.Extensions.Logging.ILogger" /> instance.
    /// </summary>
    /// <param name="categoryName">The category name for messages produced by the logger.</param>
    /// <returns>The <see cref="T:Microsoft.Extensions.Logging.ILogger" />.</returns>
    ILogger CreateLogger(string categoryName);

    /// <summary>
    /// Adds an <see cref="T:Microsoft.Extensions.Logging.ILoggerProvider" /> to the logging system.
    /// </summary>
    /// <param name="provider">The <see cref="T:Microsoft.Extensions.Logging.ILoggerProvider" />.</param>
    void AddProvider(ILoggerProvider provider);
  }
}

@304NotModified 304NotModified added this to the 2.0 milestone Mar 14, 2019
@snakefoot
Copy link
Contributor

Not sure replacing the default LoggerFactory is always a good idea. See also NLog/NLog.Web#235

But it will resolve this issue. And there is already an "empty" LoggerFactory available right now.

@snakefoot
Copy link
Contributor

snakefoot commented Apr 27, 2019

Maybe the simple workaround is adding a new NLogProviderOptions called ClearExistingProviders. When calling AddNLog then it will remove all other providers (including previous NLog-providers), before adding itself. See also NLog/NLog.Web#192

Maybe also change NLogLoggerFactory so when calling AddNLog() using NLog-provider, then one will replace the existing NLog-provider when ClearExistingProviders=true

@304NotModified 304NotModified self-assigned this Apr 27, 2019
@304NotModified
Copy link
Member

Maybe the simple workaround is adding a new NLogProviderOptions called ClearExistingProviders. When calling AddNLog then it will remove all other providers (including previous NLog-providers), before adding itself. See also NLog/NLog.Web#192

Sounds good to me :)

Maybe also change NLogLoggerFactory so when calling AddNLog() using NLog-provider, then one will replace the existing NLog-provider when ClearExistingProviders=true

AFAIK the NLogLoggerFactory is not used, but this is also fine to me

@snakefoot
Copy link
Contributor

Since ILoggerFactory-interface is now obsolete, and AddNLog for ILoggerBuilder has been fixed in #325 to avoid double registration. Then I believe this issue has been resolved.

@304NotModified 304NotModified modified the milestones: 2.0, 1.6 Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants