pedestal 2020-01-10 | Slack Archive

hlship01:01:57

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

because you get a runtime exception trying to apply the LoggerSource protocol to a Var. Am I missing something?

hlship01:01:32

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

hlship01:01:43

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.

ddeaguiar14:01:01

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

ddeaguiar15:01:35

Fix landed on master. Thanks for reporting!

hlship18:01:34

There may be more to it than that.

hlship18:01:50

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

hlship18:01:26

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

hlship18:01:11

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

hlship18:01:27

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.

hlship18:01:10

Perhaps use a delay to break the cycle?

hlship18:01:36

Or move LoggerSource protocol to a separate namespace.

hlship18:01:31

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

ddeaguiar18:01:15

@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.

hlship18:01:03

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.

ddeaguiar19:01:41

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

hlship19:01:04

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

hlship19:01:30

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

ddeaguiar19:01:07

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

hlship19:01:08

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.

ddeaguiar19:01:37

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

hlship19:01:05

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

ddeaguiar19:01:03

I don’t know for sure but I doubt it

hlship19:01:10

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.

ddeaguiar19:01:45

Hm could be

ddeaguiar19:01:08

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

hlship19:01:03

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

2020-01-10

Channels