Contrib Logging change
Phil Hagelberg
I really like the clojure.contrib.logging library, but I find myself
often getting tricked into thinking it works like println:
(info "Doing a thing with a vector: " my-vec)
or
(catch Exception e
(warn "Problem doing a thing with: " my-vec e))
Attached is a patch altering the logging library to behave this way by
accepting var-args and treating the last one as a throwable if applicable.
What do you think? Do other people have the same problem I do?
-Phil
Richard Newman
I just define much the same macros that you do :)
I rarely log a Throwable, so I don't complicate my macros to do that.
I just switch to (log :info (str ...) my-throwable) in that instance.
Timothy Pratley
> Attached is a patch altering the logging library to behave this way by
> accepting var-args and treating the last one as a throwable if applicable.
> What do you think? Do other people have the same problem I do?
Excellent idea.
+1 from me.
Mark Derricutt
--
Pull me down under...
> --
> You received this message because you are subscribed to the Google
> Groups "Clojure" group.
> To post to this group, send email to clo...@googlegroups.com
> Note that posts from new members are moderated - please be patient with your first post.
> To unsubscribe from this group, send email to
> clojure+u...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/clojure?hl=en
>
Chris Dean
> I really like the clojure.contrib.logging library, but I find myself
> often getting tricked into thinking it works like println:
Nice idea.
My $0.02: Checking the the type of the last arg seems too dwim to me,
but it will certainly work. I've been thinking recently that
info/warn/etc should be println or printf like and not take an
exception, while log itself should be the only thing that does take an
exception. That would break lots of code, so it's probably off the
table.
Instead, like others, I have my own macro. This one use printf
semantics. For example:
(flog :debug "Added %s [%s] to the write queue"
(:query subject) (count msgs))
(defmacro flog [level fmt & args]
"Logs a message, using clojure.contrib.logging/log after running the
arguments though format. Should move this to logging.clj"
`(log ~level (format ~@(cons fmt args))))
Cheers,
Chris Dean
Phil Hagelberg
>> I really like the clojure.contrib.logging library, but I find myself
>> often getting tricked into thinking it works like println:
>
> My $0.02: Checking the the type of the last arg seems too dwim to me,
> but it will certainly work. I've been thinking recently that
> info/warn/etc should be println or printf like and not take an
> exception, while log itself should be the only thing that does take an
> exception. That would break lots of code, so it's probably off the
> table.
Yeah, backwards-compatibility is the reason I kept it so it could take
an exception for the last argument.
I agree that it's cleaner to just use the log macro directly if you have
a throwable argument, but unfortunately since this library is part of
contrib it's not really possible to make breaking changes.
-Phil
Alex Taggart
Alex Taggart
Chris Dean
Alex Taggart <alex.t...@gmail.com> writes:
> Ok here's what I threw together: http://gist.github.com/275527
>
> It adds both println-style args as well as a formatting version.
One downside of the new macros is that there is now one more check done
at runtime for every use. There used to be just the impl-enabled? and
now there is a instance? call as well.
Also, I think format requires at least one arg, so (logf :debug) isn't
valid. Probably should change the signature to
(defmacro logf [level fmt-or-throwable & args] ...)
Cheers,
Chris Dean
ataggart
On Jan 12, 12:19 pm, Chris Dean <ctd...@sokitomi.com> wrote:
> One downside of the new macros is that there is now one more check done
> at runtime for every use. There used to be just the impl-enabled? and
> now there is a instance? call as well.
True. One optimization I could make is to cover the most common case
(calling with a string literal as the second arg) by checking for
(instance? String x) outside of the syntax quote. Then again, that
case (and perhaps others) may be obviated by the JVM optimizer.
>
> Also, I think format requires at least one arg, so (logf :debug) isn't
> valid. Probably should change the signature to
>
> (defmacro logf [level fmt-or-throwable & args] ...)
Which is why I added the arglist metadata (shown via doc):
[level fmt & args] [level throwable fmt & fmt-args]
Also, the -or- is somewhat misleading.
Timothy Pratley
1) It wont *break* any code, it may change the log output slightly as
the exception is rendered by str or format instead of by the java
logging formatter.
2) I think this is least surprising.
3) The exception taking java call could be expose as logex.
4) For logs, toString on the exception rarely enough though. I get the
feeling that exceptions in the normal log arg list still need to be
treated specially when converting to a string such that the stack
trace is captured. But I think this special treatment should not be
arity dependent, ie: (log "OMG, foo was " foo ", it should never be!"
exception "What should I do?") should not behave differently from (log
"OMG" exception). I guess there are two options here: Use special
stringification of exceptions, or leave it to the user to use logex or
similar if they want full exception details.
5) I expect the configurable java formatting rules that most people
care about are line breaks and stamping, not the string contents. Note
that the object list taking call is not exposed, and nobody has been
asking for that - it seems quite undesirable to be wrangling java log
formatting like this from inside Clojure. The exception string
contents is important however.
Regards,
Tim.
Phil Hagelberg
> Ok here's what I threw together: http://gist.github.com/275527
>
> It adds both println-style args as well as a formatting version.
>
> I like the idea of the level-specific convenience macros behaving like
> print, but I'd prefer to move the throwable to be an optional first
> arg. I tried to make it so it's a not-quite-breaking change for
> extant code relying on the old two-arg form; worst-case they'll get an
> extra bit of text at the end of the message.
I think that without versioning this library independently of the rest
of contrib, you're stuck having to provide backwards-compatibility.
Have you thought about spinning this out into a separate lib and
implementing the cleaner breaking change there? Now that we have clojars
etc, keeping everything inside contrib is not as important for
distribution purposes as it once was.
-Phil
Chris Dean
> Which is why I added the arglist metadata (shown via doc):
> [level fmt & args] [level throwable fmt & fmt-args]
Ah, good call. I missed that.
Cheers,
Chris Dean
ataggart
On Jan 12, 4:46 pm, Timothy Pratley <timothyprat...@gmail.com> wrote:
> My thought would be to drop exceptions from normal logging calls.
You mean dropping it from the level-specific macros such as error and
fatal? I'm inclined to disagree; being able to access the exception
instance itself is a Good Thing.
It should be a moot issue anyway since I'm just going to leave the
extant macros as they are, and just add logf, logp, and the associated
level-specific versions, e.g., debugf and debugp. Hopefully that
makes everyone happy.
I'm also including the optimization I mentioned earlier regarding
skipping the instance? check for the common case where a string
literal proves there's no throwable param.
New macros: http://gist.github.com/275928
Timothy Pratley
2010/1/13 ataggart <alex.t...@gmail.com>:
> You mean dropping it from the level-specific macros such as error and
> fatal? I'm inclined to disagree; being able to access the exception
> instance itself is a Good Thing.
Yes. You are quite right; preserving the exception is more important.
If I understand correctly, the latest version is used as follows:
(log/spy foo)
(log/error "OMG!!" exception)
(log/debugp exception "OMG!! foo:" foo)
(log/debugp "OMG!! foo:" foo "should be impossible")
(log/debugf "OMG!! foo: %d should be impossible" foo)
(log/debugp "Something expected resulted in" foo)
I'm worried that log and logp having opposite overloading it will trip
users up. But I don't see a good alternative, and it might not be a
valid concern, just an initial reaction.
Writing the calls out like this made me realize that many use cases
for debugp are similar to spy and would be much more informative if
using spy. But perhaps spy has some limitations in that it takes a
single expression, which is very good when you want to leave the
expression in place (+ x (spy y)). It might be convenient to have
automatic spy-like expansion in a println-like format:
(when (> 10 (+ x y))
(log/spyp "Oh dear," (+ x y) "should never happen." x y)
(dosomethingaboutit))
;logs "Oh dear, (+ x y) => 5 should never happen. x => 3 y => 2"
;should spyp return a value though? maybe the name is badly chosen.
And/or it might be nice to have some sort of (log/assertp pred expr
print-expr) form.
(log/assertp (partial < 10) (+ x y)) "Oh dear" x y)
;logs "(+ x y) => 5 Oh dear x => 3 y => 2"
;throws via (assert ...) java.lang.AssertionError: Assert failed:
((partial < 10) (+ x y)) (NO_SOURCE_FILE:0)
Would these be welcome additions, or polluting an already full namespace?
> I'm also including the optimization I mentioned earlier regarding
> skipping the instance? check for the common case where a string
> literal proves there's no throwable param.
Neat trick!
Regards,
Tim.
ataggart
Patch submitted:
http://www.assembla.com/spaces/clojure-contrib/tickets/58
On Jan 13, 5:03 pm, Timothy Pratley <timothyprat...@gmail.com> wrote:
> Hi Alex,
>
> 2010/1/13 ataggart <alex.tagg...@gmail.com>: