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

Remove the logging magic #118

Open
BasieP opened this issue Aug 15, 2022 · 4 comments
Open

Remove the logging magic #118

BasieP opened this issue Aug 15, 2022 · 4 comments

Comments

@BasieP
Copy link

BasieP commented Aug 15, 2022

hi,

request here..
I cannot disable logging right now.. and I really don't want my application to log all the stuff the websocket client is logging.
Libraries are not accually suppost to log stuff..

There are accually two reasons for that.

  1. the application that uses the library loses control over the content of the logfile
  2. you might want to do stuff (other then logging) when a specific thing occurs, and right now there is no way to do that.

Why not remove the magic called 'LibLog' and replace it by simple delegates?
I can imagine two different approaches.

  1. create a couple of delegates (trace, debug, info, warn, error) and emit the 'debug' stuff trough the 'debug' delegate, etc.
  2. create functional delegates that log specific events. ('connected' for example).

Ofcourse is the first option the quickest, but most ugly. It solves problem 1 (flooding of logfile) but not problem 2, since you can't distinct between different types of errors.

So I would like to see functional delegates (or events) on which my application can subscribe (please not with reactive), and where my application can define actions, based upon the functional meaning.

And ofcourse, a lot of logging can simply be removed, because it looks a lot like it's only there for debug purposes..
like this:

    if (IsStarted)
            {
                Logger.Debug(L("Client already started, ignoring.."));
                return;
            }
@zeroed-tech
Copy link

I agree with the above. If anyone is using Serilog and needs a workaround for this in the mean time, this will filter out all logging events from WebsocketClient:
Log.Logger = new LoggerConfiguration().Filter.ByExcluding(Matching.FromSource<WebsocketClient>())

@Marfusios
Copy link
Owner

Hey @BasieP ,

definitely a valid point.
It is already planned and initially handled by #50, but it is a breaking change, so I haven't yet managed to finish it.

I think we don't need to reimplement the wheel, abstraction Microsoft.Extensions.Logging from MS seems very sufficient and already supported by most of the OS logging libraries.

You can disable logging, usually by lowering the log level, Serilog example in appsettings.json:

{
  "Serilog": {
    "MinimumLevel": {
      "Default": "Debug",
      "Override": {
        "Websocket.Client": "Error"
      }
    }
  }
}

@SrBrahma
Copy link

@Marfusios I think no logging is better than migrating the logging.

@jbriggs22
Copy link

@Marfusios are there any plans to get that PR to use Microsoft.Extensions.Logging in new version? or is there a more active fork of this project?

I would also like an option with no logging or MS logging instead, would try it out if there is a preview version available on NuGet.

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

No branches or pull requests

5 participants