Decouple a bit from onelog, add tools.logging and timbre implementations

[nberger]

I wanted to use the library on a project that is using taoensso/timbre but I couldn't, because of conflicting dependencies versions but also because it was not enought to supply alternative #{:error :debug :info} functions: For example, trace was being called directly, there was no way to pass a custom implementation.

[pjlegato]

Wow, thanks. That looks like a lot of good work. I have a request before I merge it...

The main goal I have for this project is extreme and utter simplicity. I really want the default use case to be very, very simple, where an absolute minimum of required code can be employed to get something useful going immediately. Flexibility and configurability are also fine, but they can't alter the primary mission, which is to be dead simple.

Could you change this a bit so that the default use case (with onelog) is identical codewise to before? What I mean by this is that users shouldn't have to add any extra code beyond what is there now, for example :exclusions [com.taoensso/timbre onelog]] or :logger-impl (make-onelog-logger), just to make it work in a default OneLog mode. Additional code can be used to activate Timbre mode, but the library has to provide reasonable defaults without any particular user specification of what logging backend they want to use.

I do appreciate your efforts and the time you've obviously spent on this. Thanks.

[nberger]

Glad to hear that you liked it :). And thanks for your thoughtful answer.

One question about leaving the default case identical to before... is it really about the code, or also about dependencies? I'm asking this because I changed the dependency with onelog to have :scope "provided". This way it won't be installed as a transitive dependency in a project using this library, so a project that wants to use ring.middleware.logger with onelog will have to explicitly declare the onelog dependency.

This is very important to me, because that's what motivated this change: onelog was pulling other dependencies that were in conflict with other dependencies in my project.

If you are ok with leaving :scope "provided", I think I can do what you asked: to use onelog as logger when used as (logger/wrap-with-logger my-ring-app). I'll need to do a bit of additional refactoring to extract a new ns that will be the ns to require to use the logger with timbre or tools.logging without pulling onelog. That refactor doesn't look like complex, but just wanted to point that out.

Please let me know what do you think about the dependencies, I think that's what's most important.

[pjlegato]

Can we set it up so that onelog is included by default, but you can just :exclude it when you want to use some other logging backend?

[nberger]

Why do you want onelog to be the option included by default? tools.logging already does a nice work detecting which library is available among slf4j, Apache commons-logging, log4j, and java.util.logging. I understand that onelog chooses log4j to have the advantage of better default configuration, but at the cost of less flexibility and forcing the dependency on log4j. That's an issue for many projects.

I'm not sure about "market share" of onelog, tools.logging and timbre, but it seems tools.logging is the most popular, and it's also the one that brings the fewest transitive dependencies, so it's the less opinionated and has the best chance to not generate conflicts.

@danielcompton said in clojurians today that:

I wish it didn’t use onelog, and just used clojure.tools.logging if it used c.t.logging then that would fit the 90% use case, although not timbre users

Apart from that, I think we would need to undertake more refactoring to extract some namespaces (or perhaps a core library + smaller satellite libraries) to make it possible to use onelog by default but use timbre/tools.logging when needed without bringing onelog dependency at the same time.

What do you think about leaving 0.5.x with onelog by default, as it is, and making the switch in 0.6.x. Existing users will not be affected: they don't have the need to move to 0.6.x.

[pjlegato]

Well, I want onelog to be the default because I wrote it, and so I know it works exactly the way I want it to work. :) I don't want a more flexibile logging library, I want something that is super-simple and just works. That's what onelog and r.m.logger are, and I don't plan to change that.

That said, I'm happy to make them more extensible if it can be done in ways that do not alter the core premise and design.

Tools.logging doesn't support colors, and automatically inserts spaces into your sequence of logged items. (For example, if you do (log/info "dividing" x "by" y), you get "dividing 1 by 2".) There were some other minor things that I don't remember now. In any case, I'm not a fanatic -- if others prefer that style, that's fine, they can use that. There is no one-size-fits-all solution. That's why I wrote my own.

[danielcompton]

The issue with depending on onelog is that it forces the whole app to use log4j as well. As ring.middleware.logger is not the primary logging component of my app, but just an added extra, I don't really want it to be determining which logging framework I use. However I totally understand your perspective of just wanting something that "just works".

[pjlegato]

I'm not sure that the "simplicity" goal is compatible with "not loading onelog at all".. Onelog itself is supposed to be an abstraction layer over all logging-related stuff. Maybe the thing to do is make Onelog more configurable so you can get the backend you want?

[nberger]

Well, I want onelog to be the default because I wrote it, and so I know it works exactly the way I want it to work. :) I don't want a more flexibile logging library, I want something that is super-simple and just works. That's what onelog and r.m.logger are, and I don't plan to change that.

That's fine. It's just that it doesn't work that way for many users, including me, because as Daniel said I don't want to use log4j. I tried to make it the default and at the same time be able to easily switch to one of the other options while not loading the onelog code (which in turn activates log4j, etc.) and I couldn't do it in the limited amount of time I put. It can be done, for sure. One way would be to replicate the "logger-factory" strategy of tools.logging, but then why are not just using tools.logging?

Another option would be to relayout the code into smaller namespaces and load just what's needed. I don't have time to do that now, I prefer to just use [com.nberger/ring.middleware.logger "0.6.0-SNAPSHOT" :exclusions [onelog]] which comes from https://github.com/nberger/ring.middleware.logger/tree/0.6.0

That said, I'm happy to make them more extensible if it can be done in ways that do not alter the core premise and design.

Tools.logging doesn't support colors, and automatically inserts spaces into your sequence of logged items. (For example, if you do (log/info "dividing" x "by" y), you get "dividing 1 by 2".) There were some other minor things that I don't remember now. In any case, I'm not a fanatic -- if others prefer that style, that's fine, they can use that. There is no one-size-fits-all solution. That's why I wrote my own.

Isn't this issue already solved by r.m.logger by always logging a single string instead of many strings? I'm using it with timbre and I see the colorized output (and I love it! :P), so I assume it works on tools.logging too, not only on onelog.

[nberger]

One more thing: If we switch the default to use tools.logging, we can still make the code change to use onelog as small as:

(require '[ring.middleware.logger.onelog :as logger])
(logger/wrap-with-logger app)

As opposed to the current code:

(require '[ring.middleware.logger :as logger])
(logger/wrap-with-logger app)

Which is a minor change for a bump to a 0.6.x version IMHO

[pjlegato]

As I said already, I'm not going to change the default for r.m.logger away from onelog or alter the core design substantially without a very compelling reason to do so. That many people would prefer something else does not count as a compelling reason.

Part of the design of the onelog / r.m.logger system is that all logging-related stuff is abstracted away by the onelog library. Log4j and its friends were deemed overengineered and too complex to work with directly. Adding additional backends to onelog could happen. Adding additional backends to r.m.logger itself won't happen; the entire point of onelog is to keep all of that complexity (how logging happens) completely seperate from the code that is doing the logging.

I understand that it doesn't work for many users, and I'm fine with that. I have no desire to preach to anyone on how they ought to do logging. Do whatever works for you. This project is here for those whose needs align with it. r.m.logger is a very specific solution that fits a specific problem exactly. It's not ever going to be to be all things to all people, and I'm not going down the path of adding features to satisfy every constituency that exists, because that results in incoherent clutter and a "big ball of mud" architecture. For those who want something simple that just works without configuration, this is it. If you have more complex needs, such as wanting to switch the backend library, this is probably not it.

Again, I am happy to accept pull requests that do not fundamentally change the core design tenets of the project, among which are:

• All logging operations are abstracted through Onelog.
• Dead simple -- there should be no configuration necessary to get something working with reasonable defaults.

If you want to add different backend options, those should be in Onelog, not r.m.logger. r.m.logger will stay abstracted away from any specific logging libraries, by Onelog.

Another option is to fork the code and make a derived project that uses tools.logging rather than Onelog. I don't mind. That's the entire point of open source -- when someone else's code doesn't work for you, you can take the code and modify it to suit your needs.

[nberger]

Sure, that's one of the good things of open source. I like it more when forks can be avoided, but that's not always the case.

Thanks for the project, I'm giving it a great use!