Clojurians Log v2

Q about io.pedestal.log; I believe this line of code: https://github.com/pedestal/pedestal/blob/master/log/src/io/pedestal/log.clj#L247 may be broken. It attempts to use the override-logger as a LoggerSource instance, rather than a function that is passed a string and returns a LoggerSource. Unless I'm missing something, you can't currently use

-Dio.pedestal.log.overrideLogger=my.namespace/create-logger-source

My scenario is to redirect code writing to io.pedestal.log/log to instead invoke our legacy, in-house logging framework.

I believe the overrideLogger system property works when used w/ log/info, log/debug, etc., as they go through the private log-expr function: https://github.com/pedestal/pedestal/blob/master/log/src/io/pedestal/log.clj#L275 ... which does what you'd expect; treat the override-logger as a function that generates LoggerSource instances.

@hlship, yep that looks like a bug to me. I’ll file an issue and address

Fix landed on master. Thanks for reporting!

There may be more to it than that.

I copied master's log.clj to my project and am trying to use it.

But I think there's an issue in actually using it.

Because my defaultLogger function needs to require io.pedestal.log so that it can implement the LoggerSource protocol.

But https://github.com/pedestal/pedestal/blob/621b6896aab6582499e0076bd9c73d5aeb01dae9/log/src/io/pedestal/log.clj#L196 needs the Var in question to resolvable when io.pedestal.log is loading, resulting in a loop.

Perhaps use a delay to break the cycle?

Or move LoggerSource protocol to a separate namespace.

I can do some trickery with alter-var-root! to work around this, I hope.

@hlship here’s one way to address that. I’ll be honest, I’d rather the protocol live in a separate ns but unfortunately that’s not how it was implemented. This method of overriding is also used for tracing and has been a source of confusion as well. I’m going to chat with Paul about it.

Fortunately, override-logger is public, so I can just alter-var-root it in my server startup code, which should catch most of what I need.

Ok. I’ll work on clarifying how this is intended to be done in the docs. It’s come up a number of times.

I could work on a PR that made use of a delay, but it's problematic because override-logger is public.

Then again, it seems like no one could possibly be using it via the sysprop/envvar at all.

FWIW, spoke to Paul and he stated that what I posted is the intended setup, albeit other methods could work as well.

It just feels a bit broken in a way that's all but impossible to fix in a backwards compatible way. Logging ... it'll always bite your ass.

yeah, in retrospect, splitting out the protocol would have been ideal.

Is there a way to move it to a new namespace but alias the protocol (and protocol methods) back into io.pedestal.log?

I don’t know for sure but I doubt it

I'm still struggling with the above approach; I think AOT is getting in the way, and (for the time being) need AOT my server.

Hm could be

I uberjar my sample with :aot :all and it seems to work. Wierd.

I was getting ClassNotFoundException for LoggerSource. Not sure what's up with that.