New logging code

0 views
Skip to first unread message

Jeppe Nejsum Madsen

unread,
Feb 14, 2010, 8:40:41 AM2/14/10
to lif...@googlegroups.com
Hi,

I've added first shot at the new logging code:

http://github.com/dpp/liftweb/blob/e7ed6c6bc013aea768bfe34a6e4eca22d26407e8/framework/lift-base/lift-common/src/main/scala/net/liftweb/common/Logging.scala

I've tried to keep it as simple as possible, really just a Scala layer
on top of the SLF4J api.

Note that no backend (log4j or logback) configuration is included. This
has to go into lift-util to use runmode etc.

You can have your choice of a nested logger:

object MyObj extends Logging {
logger.info("nested Hello")
}

or direct access:

object MyObj extends Loggable {
info("direct Hello")
}

Thoughts?

Next step, when this has been committed, is to update Lift internals to
use the new code (as part of #310) and deprecate the old logging in util.

/Jeppe

Marius

unread,
Feb 14, 2010, 9:49:44 AM2/14/10
to Lift
Overall it's looking pretty good, but why do we need both Logging and
Loggable ?

Br's,
Marius

On 14 feb., 15:40, Jeppe Nejsum Madsen <je...@ingolfs.dk> wrote:
> Hi,
>
> I've added first shot at the new logging code:
>

> http://github.com/dpp/liftweb/blob/e7ed6c6bc013aea768bfe34a6e4eca22d2...

Jeppe Nejsum Madsen

unread,
Feb 14, 2010, 10:07:26 AM2/14/10
to lif...@googlegroups.com
On Sun, Feb 14, 2010 at 3:49 PM, Marius <marius...@gmail.com> wrote:
> Overall it's looking pretty good, but why do we need both Logging and
> Loggable ?

Not sure if we need both. I prefer Loggable because it minimizes
clutter, but in some case (such as inside a Specification) there might
already be a warn etc in scope, in which case Logging can be used.

/Jeppe

Heiko Seeberger

unread,
Feb 14, 2010, 11:59:32 AM2/14/10
to lif...@googlegroups.com
On 14 February 2010 14:40, Jeppe Nejsum Madsen <je...@ingolfs.dk> wrote:
Hi,

I've tried to keep it as simple as possible, really just a Scala layer
on top of the SLF4J api.

I think that's a very good decision!
 
Note that no backend (log4j or logback) configuration is included. This
has to go into lift-util to use runmode etc.

You can have your choice of a nested logger:

 object MyObj extends Logging {
  logger.info("nested Hello")
 }

or direct access:

 object MyObj extends Loggable {
  info("direct Hello")
 }

Thoughts?

I really like to have Logging and Loggable, but I would call them vice versa and change Logging to Logger. As an example think of Iterable and Iterator. In general an Xable gives you access to an X. Hence we should have Loggable with a method logger that returns a Logger and we should have Logger to mix in all the logging methods. Why change from Logging to Logger? Because we should call it what it is, not what it does.

Cheers,

Heiko

Work: weiglewilczek.com
Blog: heikoseeberger.name
Follow me: twitter.com/hseeberger
OSGi on Scala: scalamodules.org
Lift, the simply functional web framework: liftweb.net

Jeppe Nejsum Madsen

unread,
Feb 14, 2010, 2:10:43 PM2/14/10
to lif...@googlegroups.com
On Sun, Feb 14, 2010 at 5:59 PM, Heiko Seeberger
<heiko.s...@googlemail.com> wrote:
> On 14 February 2010 14:40, Jeppe Nejsum Madsen <je...@ingolfs.dk> wrote:
>>
>> Hi,
>>
>> I've tried to keep it as simple as possible, really just a Scala layer
>> on top of the SLF4J api.
>
> I think that's a very good decision!
>
>>
>> Note that no backend (log4j or logback) configuration is included. This
>> has to go into lift-util to use runmode etc.
>>
>> You can have your choice of a nested logger:
>>
>>  object MyObj extends Logging {
>>   logger.info("nested Hello")
>>  }
>>
>> or direct access:
>>
>>  object MyObj extends Loggable {
>>   info("direct Hello")
>>  }
>>
>> Thoughts?
>
> I really like to have Logging and Loggable, but I would call them vice versa
> and change Logging to Logger. As an example think of Iterable and Iterator.
> In general an Xable gives you access to an X. Hence we should have Loggable
> with a method logger that returns a Logger and we should have Logger to mix
> in all the logging methods. Why change from Logging to Logger? Because we
> should call it what it is, not what it does.

Makes sense, and that was actually close to what I had initially: The
Logger trait was called LiftLogger, but this clashed with the current
LiftLogger.

This name (Logger in current code) probably doesn't matter too much as
it's usually not needed in client code. But AbstractLogger doesn't
sound very nice :-)

/Jeppe

Heiko Seeberger

unread,
Feb 15, 2010, 3:17:46 AM2/15/10
to lif...@googlegroups.com
On 14 February 2010 20:10, Jeppe Nejsum Madsen <je...@ingolfs.dk> wrote:

Makes sense, and that was actually close to what I had initially: The
Logger trait was called LiftLogger, but this clashed with the current
LiftLogger.

This name (Logger in current code) probably doesn't matter too much as
it's usually not needed in client code. But AbstractLogger doesn't
sound very nice :-)

Even if (probably) not needed, we should try to name it the best possible way. Changing now causes no pain at all, but later ... you know.

Heiko

Jeppe Nejsum Madsen

unread,
Feb 15, 2010, 3:45:38 AM2/15/10
to lif...@googlegroups.com
On Mon, Feb 15, 2010 at 9:17 AM, Heiko Seeberger
<heiko.s...@googlemail.com> wrote:
> On 14 February 2010 20:10, Jeppe Nejsum Madsen <je...@ingolfs.dk> wrote:
>>
>> Makes sense, and that was actually close to what I had initially: The
>> Logger trait was called LiftLogger, but this clashed with the current
>> LiftLogger.
>>
>> This name (Logger in current code) probably doesn't matter too much as
>> it's usually not needed in client code. But AbstractLogger doesn't
>> sound very nice :-)
>
> Even if (probably) not needed, we should try to name it the best possible
> way. Changing now causes no pain at all, but later ... you know.

Agreed. Suggestions?

/Jeppe

Heiko Seeberger

unread,
Feb 15, 2010, 3:52:42 AM2/15/10
to lif...@googlegroups.com
On 15 February 2010 09:45, Jeppe Nejsum Madsen <je...@ingolfs.dk> wrote:
> Even if (probably) not needed, we should try to name it the best possible
> way. Changing now causes no pain at all, but later ... you know.

Agreed. Suggestions?

I already made mine, just to make sure everyone has a chance to see them:
Like Iterable and Iterator lets call the trait that offers the logging methods (info, warn, etc.) Logger and the trait that gives access to a Logger should be called Loggable.

Heiko

Jeppe Nejsum Madsen

unread,
Feb 15, 2010, 9:16:10 AM2/15/10
to lif...@googlegroups.com
On Mon, Feb 15, 2010 at 9:52 AM, Heiko Seeberger
<heiko.s...@googlemail.com> wrote:
> On 15 February 2010 09:45, Jeppe Nejsum Madsen <je...@ingolfs.dk> wrote:
>>
>> > Even if (probably) not needed, we should try to name it the best
>> > possible
>> > way. Changing now causes no pain at all, but later ... you know.
>>
>> Agreed. Suggestions?
>
> I already made mine, just to make sure everyone has a chance to see them:
> Like Iterable and Iterator lets call the trait that offers the logging
> methods (info, warn, etc.) Logger and the trait that gives access to a
> Logger should be called Loggable.

I know, but before there was an extra (abstract) trait Logger which
was what I asked about :-). But this is a moot point now, it has been
eliminated and naming is now as you proposed above....

For convenience:
http://github.com/dpp/liftweb/blob/add01980aa81875617f38260d710e0558c7ae1b1/framework/lift-base/lift-common/src/main/scala/net/liftweb/common/Logging.scala

One issue remains, which I don't know how to handle (if possible at
all): The current mixins use the dynamic type of an instance to
determine the logger name. This may not always be the preferred way.
E.g:

trait PaymentSystem extends Logger
trait FullfillmentSystem extends Logger

object MyStore extends PaymentSystem with FullfillmentSystem with Logger

Now everything in the subsystems will be logged with the MyStore
logger which is not how I would like to see it. The only solution I've
found so far is to not use the mixins and create private loggers in
the subsystem, where the static type is known. E.g. val logger =
Logger(classOf[PaymentSystem])

But this kind of restricts the usage of the mixins. Any thoughts on
this issue is appreciated....

/Jeppe

Indrajit Raychaudhuri

unread,
Feb 15, 2010, 3:20:34 PM2/15/10
to Lift

On Feb 15, 1:52 pm, Heiko Seeberger <heiko.seeber...@googlemail.com>
wrote:


> On 15 February 2010 09:45, Jeppe Nejsum Madsen <je...@ingolfs.dk> wrote:
>
> > > Even if (probably) not needed, we should try to name it the best possible
> > > way. Changing now causes no pain at all, but later ... you know.
>
> > Agreed. Suggestions?
>
> I already made mine, just to make sure everyone has a chance to see them:

> Like *Iterable* and *Iterator* lets call the trait that offers the logging
> methods (info, warn, etc.) *Logger* and the trait that gives access to a *
> Logger* should be called *Loggable*.

+1

Indrajit Raychaudhuri

unread,
Feb 15, 2010, 3:57:48 PM2/15/10
to lif...@googlegroups.com

The restriction might be worthwhile in this case for the sake of
predictability. Having class/object specific logger is to be able to
filter in/out the logs (via configuration) at deployment time. It should
not be too sensitive to minor rearrangement of mixins in the code.

Would it be overcomplicated to make the Logger trait typed?


>
> /Jeppe
>

Jeppe Nejsum Madsen

unread,
Feb 15, 2010, 4:10:28 PM2/15/10
to lif...@googlegroups.com
On Mon, Feb 15, 2010 at 9:57 PM, Indrajit Raychaudhuri
<indr...@gmail.com> wrote:

>> http://github.com/dpp/liftweb/blob/add01980aa81875617f38260d710e0558c7ae1b1/framework/lift-base/lift-common/src/main/scala/net/liftweb/common/Logging.scala
>>
>> One issue remains, which I don't know how to handle (if possible at
>> all): The current mixins use the dynamic type of an instance to
>> determine the logger name. This may not always be the preferred way.
>> E.g:
>>
>> trait PaymentSystem extends Logger
>> trait FullfillmentSystem extends Logger
>>
>> object MyStore extends PaymentSystem with FullfillmentSystem with Logger
>>
>> Now everything in the subsystems will be logged with the MyStore
>> logger which is not how I would like to see it. The only solution I've
>> found so far is to not use the mixins and create private loggers in
>> the subsystem, where the static type is known. E.g. val logger =
>> Logger(classOf[PaymentSystem])
>>
>> But this kind of restricts the usage of the mixins. Any thoughts on
>> this issue is appreciated....
>
> The restriction might be worthwhile in this case for the sake of
> predictability. Having class/object specific logger is to be able to filter
> in/out the logs (via configuration) at deployment time. It should not be too
> sensitive to minor rearrangement of mixins in the code.
>
> Would it be overcomplicated to make the Logger trait typed?

Not sure I follow? Logger is already a trait...Or did you mean something else?

Having thought a little more about this, it seems like both solutions
(mixin and private loggers) has their uses:

Mixins as convenience for application services that should not be
subclassed and private loggers for reusable components .

/Jeppe

Naftoli Gugenheim

unread,
Feb 15, 2010, 6:49:52 PM2/15/10
to liftweb
I don't think it's theoretically possible to have the same trait "instance" recognize which superclass that it was mixed in to is doing the logging. Because if A mixes in T, and B extends A and also mixes in T, T is not really mixed in twice. For the same reason using a type parameter would not help because any given class can only have one type "value."
The closest thing I can think of is using the stack trace to find the method calling the logging method, and figuring out which trait it comes from. In other words, not a solution.
If it's important, you may want the logging method to take an implicit parameter, say of type
case class LoggerName(name: String)
Then your trait can provide a default implicit, but I'm guessing if a subclass has it's own (say private) implicit val, it would be used.
To determine if this is true, try:
case class N(s: String)
trait T {
  implicit val x = N("t")
  def m(implicit p: N) = println(p)
}
class A extends T {
  private implicit val y = N("a")
  m
}
class B extends A {
  private implicit val z = N("b")
  m
}
new T {}
new A
new B


2010/2/15 Jeppe Nejsum Madsen <je...@ingolfs.dk>

/Jeppe

--
You received this message because you are subscribed to the Google Groups "Lift" group.
To post to this group, send email to lif...@googlegroups.com.
To unsubscribe from this group, send email to liftweb+u...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/liftweb?hl=en.


Reply all
Reply to author
Forward
0 new messages