Db.addLogFunc

123 views
Skip to first unread message

jon

unread,
Aug 6, 2009, 1:01:48 PM8/6/09
to Lift
Hi,

I have the following in boot:
DB.addLogFunc((query, len) => Log.info("The query: "+query+" took
"+len+" milliseconds"))

I was expecting the query parameter to be sql, but it's actually some
sort of guid

"INFO - The query: 6839c016-0122-f09a-9c96-0000003844e8 took 5
milliseconds"

Any ideas?
I'm running with derby.

Thanks,

Jon



marius d.

unread,
Aug 6, 2009, 1:03:54 PM8/6/09
to Lift
Yeah we're aware of that. That is based on toString application which
is JDBC driver dependent. I think Derek started some work on this to
correct this behavior. Derek ?


Br's,
Marius

Derek Chen-Becker

unread,
Aug 6, 2009, 2:58:25 PM8/6/09
to lif...@googlegroups.com
Well, I started looking at it and determined that the only way for us to truly log the queries would be to essentially make our own wrappers over Statement and PreparedStatement. There are projects (log4jdbc, notably) that already do this, and in a transparent manner. I'm not sure that adding a whole bunch of SQL logging directly to Lift is better than leveraging some existing libraries to do it.

Derek

marius d.

unread,
Aug 6, 2009, 3:19:03 PM8/6/09
to Lift
Probably building our own wrappers would be more lightweight then 3-rd
party. Jus' guessing

Br's,
Marius

On Aug 6, 9:58 pm, Derek Chen-Becker <dchenbec...@gmail.com> wrote:
> Well, I started looking at it and determined that the only way for us to
> truly log the queries would be to essentially make our own wrappers over
> Statement and PreparedStatement. There are projects (log4jdbc, notably) that
> already do this, and in a transparent manner. I'm not sure that adding a
> whole bunch of SQL logging directly to Lift is better than leveraging some
> existing libraries to do it.
>
> Derek
>

Derek Chen-Becker

unread,
Aug 6, 2009, 4:48:00 PM8/6/09
to lif...@googlegroups.com
If there's a consensus that we want our own JDBC wrappers I'll go ahead and write them.

Derek

jon

unread,
Aug 10, 2009, 6:40:23 PM8/10/09
to Lift
btw, the query param happens to include the actual sql under h2.

Derek Chen-Becker

unread,
Aug 11, 2009, 12:18:02 AM8/11/09
to lif...@googlegroups.com
Right. The current impl just does a toString on the vendor's PreparedStatement. Some Vendors print GUIDs, some print the SQL, but there really aren't any requirements one way or the other.

Derek

marius d.

unread,
Aug 11, 2009, 3:33:55 AM8/11/09
to Lift
Please do so. If you need any help for some reason (time availability
etc.) please let me know. As a note probably the wrappers should be
only only when there is at least one log function registered.

Br's,
Marius

On Aug 6, 11:48 pm, Derek Chen-Becker <dchenbec...@gmail.com> wrote:
> If there's a consensus that we want our own JDBC wrappers I'll go ahead and
> write them.
>
> Derek
>

Derek Chen-Becker

unread,
Aug 11, 2009, 1:08:09 PM8/11/09
to lif...@googlegroups.com
Will do.

Derek Chen-Becker

unread,
Aug 15, 2009, 10:02:03 AM8/15/09
to lif...@googlegroups.com
OK, a preliminary version of log wrappers is checked in on wip-dcb-sql-log-wrappers. I'll merge it on Tuesday if no one sees any problems with it.

Derek

Bill Venners

unread,
Aug 15, 2009, 5:22:13 PM8/15/09
to lif...@googlegroups.com
Hi Derek,

I noticed you checked in a change to Lift that updated the Maven ScalaTest ref. However the ref is to version 0.9.4, whereas the current version is 0.9.5. 

Thanks.

Bill
----
Bill Venners
Artima, Inc.

Derek Chen-Becker

unread,
Aug 15, 2009, 8:07:18 PM8/15/09
to lif...@googlegroups.com
I only fixed the groupId because maven was giving me a warning that it had changed. I can bump the versions to 0.9.5 this week and if that builds fine I'll check it in.

marius d.

unread,
Aug 16, 2009, 10:09:01 AM8/16/09
to Lift
Derek,

I was looking to your code, GOOD stuff ! One thing though.The
LoggedStatement.toString would return a List of log entries and not a
query but the user's Log function expects a query and the time spent
on that query so there is a little miss match between when user's
function expects and what we're feeding to it. Also you are creating
strings like:

"Exec query \"%s\" : rs = %s".format(sql, rs)"

...which may be perfect. But I wonder if user's won't expect just the
query/update and nothing else and they will do their own logging and
formatting.


Br's,
Marius

On Aug 16, 3:07 am, Derek Chen-Becker <dchenbec...@gmail.com> wrote:
> I only fixed the groupId because maven was giving me a warning that it had
> changed. I can bump the versions to 0.9.5 this week and if that builds fine
> I'll check it in.
>
> On Sat, Aug 15, 2009 at 3:22 PM, Bill Venners <b...@artima.com> wrote:
> > Hi Derek,
>
> > I noticed you checked in a change to Lift that updated the Maven ScalaTest
> > ref. However the ref is to version 0.9.4, whereas the current version is
> > 0.9.5.
>
> > Thanks.
>
> > Bill----
> > Bill Venners
> > Artima, Inc.
> >http://www.artima.com
>
> > On Aug 15, 2009, at 7:02 AM, Derek Chen-Becker <dchenbec...@gmail.com>
> > wrote:
>
> > OK, a preliminary version of log wrappers is checked in on
> > wip-dcb-sql-log-wrappers. I'll merge it on Tuesday if no one sees any
> > problems with it.
>
> > Derek
>
> > On Tue, Aug 11, 2009 at 11:08 AM, Derek Chen-Becker <<dchenbec...@gmail.com>
> > dchenbec...@gmail.com> wrote:
>
> >> Will do.
>

Derek Chen-Becker

unread,
Aug 16, 2009, 10:18:46 AM8/16/09
to lif...@googlegroups.com
Yeah, I was actually thinking about that same angle last night. Perhaps I could provide some control var on DB to enable detailed or basic logging? I think that it would be very useful in some cases, for example, to know that underlying code is setting max rows or max fetch size, but I agree that most people will probably just want basic statements logged. Also, with a prepared statement someone *could* execute multiple queries in one go, so I can't get rid of the list of stuff. Another approach would be to give the logFuncs List to the Logging classes and then they could decide whether to log (and time) each execution individually or all at once (if someone sets batch mode, for example). What do you think?

Derek

Derek Chen-Becker

unread,
Aug 16, 2009, 10:22:35 AM8/16/09
to lif...@googlegroups.com
Also, I just pushed a bump to 0.9.5 for scalatest to 1.0_maint. It'll make it into 1.0.1.

Derek

marius d.

unread,
Aug 16, 2009, 10:31:37 AM8/16/09
to Lift


On Aug 16, 5:18 pm, Derek Chen-Becker <dchenbec...@gmail.com> wrote:
> Yeah, I was actually thinking about that same angle last night. Perhaps I
> could provide some control var on DB to enable detailed or basic logging?

Yeah ... or maybe for the logFunc to not pass just a query string but
an object (abstracted by a trait) and let the user extract from there
whatever is needed. Just the plain query and/or other stuff. Yes this
would mean a breaking change but I think a good one.

> I
> think that it would be very useful in some cases, for example, to know that
> underlying code is setting max rows or max fetch size, but I agree that most
> people will probably just want basic statements logged. Also, with a
> prepared statement someone *could* execute multiple queries in one go, so I
> can't get rid of the list of stuff.

Totally agree, but please see my notes above.

> Another approach would be to give the
> logFuncs List to the Logging classes and then they could decide whether to
> log (and time) each execution individually or all at once (if someone sets
> batch mode, for example). What do you think?

Based on the above I'd probably opt in for a trait rather then the
List.

Perhaps :

trait LogStatement {

def getStatement: (String, Long) // returns the statement and the
tiem spent

def getDetailedInfo: List[String] // /returns your list

}


... or you may be thinking of a better abstraction.

And logFuncs would become:

logFuncs: List[(LogStatement) => Any] = Nil


Br's,
Marius

>
> Derek

Derek Chen-Becker

unread,
Aug 16, 2009, 10:38:18 AM8/16/09
to lif...@googlegroups.com
1.1-SNAPSHOT has been bumped to 0.9.5 as well.

Derek Chen-Becker

unread,
Aug 16, 2009, 11:56:52 AM8/16/09
to lif...@googlegroups.com
I really like the idea of changing the logFunc type if everyone is OK with that. To further refine it, the trait could define three defs:

def getStatements : List[(String,Long)]
def getMetaCommands : List[(String,Long)]
def getAll : List[(String,Long)]

getAll would return the union of the first two, in proper sequential order. I could further refine this by making use of case classes instead of (String,Long):

sealed abstract class DBLogEntry
case class LoggedStatement(statement : String, duration : Long) extends DBLogEntry
case class LoggedMetaCommand(statement : String, duration : Long) extends DBLogEntry

I don't know enough detail on JDBC to know whether or not meta commands require round-trips to the DB or if the spec requires that they're all local, but it seems like we should just time everything.

Thoughts?

marius d.

unread,
Aug 16, 2009, 12:20:26 PM8/16/09
to Lift


On Aug 16, 6:56 pm, Derek Chen-Becker <dchenbec...@gmail.com> wrote:
> I really like the idea of changing the logFunc type if everyone is OK with
> that. To further refine it, the trait could define three defs:
>
> def getStatements : List[(String,Long)]
> def getMetaCommands : List[(String,Long)]
> def getAll : List[(String,Long)]
>
> getAll would return the union of the first two, in proper sequential order.
> I could further refine this by making use of case classes instead of
> (String,Long):
>
> sealed abstract class DBLogEntry
> case class LoggedStatement(statement : String, duration : Long) extends
> DBLogEntry
> case class LoggedMetaCommand(statement : String, duration : Long) extends
> DBLogEntry
>
> I don't know enough detail on JDBC to know whether or not meta commands
> require round-trips to the DB or if the spec requires that they're all
> local, but it seems like we should just time everything.
>
> Thoughts?

I like it ... and I'd go for concrete cases classes(LoggedStatement,
LoggedMetaCommand ...) above as opposed to tupples. So at least you
got my vote :)

Derek Chen-Becker

unread,
Aug 28, 2009, 12:28:08 PM8/28/09
to lif...@googlegroups.com
Hi all,
    I've made a change to the Mapper logging functionality in wip-dcb-sql-log-wrappers. The DB.addLogFunc method has changed to:

addLogFunc( f: (DBLog,Long) => Any)

where DBLog is a new trait (below) and the Long corresponds to the *total* duration of a given DB execution method. DBLog is defined as:

trait DBLog {
  ... private stuff up here ...
  /** Return a list of all of the DBStatementEntry instances in the log buffer */
  def statementEntries : List[DBStatementEntry] = ...

  /** Return a list of all of the DBMetaEntry instances in the log buffer */
  def metaEntries : List[DBMetaEntry] = ...

  /** Return all log buffer entries */
  def allEntries : List[DBLogEntry] = ...
}

and we have some new event class/traits:

trait DBLogEntry {
  def statement : String
  def duration : Long
}
object DBLogEntry {
  def unapply(obj : Any) = obj match {
    case entry : DBLogEntry => Some(entry.statement,entry.duration)
    case _ => None
  }
}
case class DBStatementEntry(statement : String, duration : Long) extends DBLogEntry
case class DBMetaEntry(statement : String, duration : Long) extends DBLogEntry

Statements are SQL statements, prepared or immediate, and meta entries correspond to things like getMaxRows, getGeneratedKeys, etc. As you can see, each individual statement records its own duration as well, so you can get fine-grained detail on all activity. An example log function in Boot could look like:

    DB.addLogFunc {
      case (query, time) => {
        Log.info("All queries took " + time + "ms: ")
        query.allEntries.foreach({ case DBLogEntry(stmt, duration) => Log.info(stmt + " took " + duration + "ms")})
        Log.info("End queries")
      }
    }

And we get output like:

INFO - All queries took 17ms:
INFO - Exec update "INSERT INTO users (lastname,locale,password_pw,password_slt,validated,uniqueid,timezone,firstname,email,superuser,textarea) VALUES ("C","en_US","GzwLqDpmJ6TrECg06bGKvOAQxyc=","1JTAWGSSYLJHXASO",1,"DU0G0RT3IFOA0NHSY5QQQTX42BOIHDGI","US/Mountain","D","d...@c.com",0,"")" : updated 1 rows took 9ms
INFO - Get generated keys : rs = oracle.jdbc.driver.OracleReturnResultSet@23f9e6e5 took 2ms
INFO - Closed Statement took 0ms
INFO - End queries

Note that this code does introduce a breaking change if you're already using log functions, since the addLogFunc signature has changed. If I can get a review on the code before I merge to master I would appreciate it.

Derek

marius d.

unread,
Aug 28, 2009, 1:20:13 PM8/28/09
to Lift
EXCELLENT WORK DEREK !
> On Sat, Aug 15, 2009 at 8:02 AM, Derek Chen-Becker <dchenbec...@gmail.com>wrote:
>
> > OK, a preliminary version of log wrappers is checked in on
> > wip-dcb-sql-log-wrappers. I'll merge it on Tuesday if no one sees any
> > problems with it.
>
> > Derek
>
> > On Tue, Aug 11, 2009 at 11:08 AM, Derek Chen-Becker <dchenbec...@gmail.com
> > > wrote:
>
> >> Will do.

David

unread,
Sep 23, 2009, 4:39:03 PM9/23/09
to Lift
Hello, I'm trying to use the new logging information as described
here. In my Boot I have:

import net.liftweb.mapper.{DB, DBLogEntry}
snip...

DB.addLogFunc {
case (query, time) => {
Log.info("All queries took " + time + "ms: ")
query.allEntries.foreach({ case DBLogEntry(stmt, duration) =>
Log.info(stmt + " took " + duration + "ms")})
Log.info("End queries")
}
}

I'm using slf4j and logback, my root level is set to trace. I see
trace output from lift, like:
16:32:02.208 [5991680@qtp-10429832-0] TRACE lift.trace:76 - Released
connection ConnectionIdentifier(lift) on thread Thread
[5991680@qtp-10429832-0,5,main]

But I don't see any output from the DB log functions. Is there
anything else I need to do in order to enable logging? I'm using
scala 2.7.5 and lift 1.1-SNAPSHOT.

Thanks, Dave


On Aug 28, 1:20 pm, "marius d." <marius.dan...@gmail.com> wrote:
> EXCELLENT WORK DEREK !
>
> On Aug 28, 7:28 pm, Derek Chen-Becker <dchenbec...@gmail.com> wrote:
>
> > Hi all,
> >     I've made a change to the Mapper logging functionality in
> > wip-dcb-sql-log-wrappers. The DB.addLogFuncmethod has changed to:
> > log functions, since theaddLogFuncsignature has changed. If I can get a

Derek Chen-Becker

unread,
Sep 28, 2009, 1:36:56 PM9/28/09
to lif...@googlegroups.com
I tested it locally on my own code and it works. Just for giggles, does anything print on the console if you change those lines to println?

Derek

David

unread,
Sep 28, 2009, 1:43:24 PM9/28/09
to Lift
Nope, changed to println():

DB.addLogFunc {
case (query, time) => {
println("All queries took " + time + "ms: ")
query.allEntries.foreach({ case DBLogEntry(stmt, duration) =>
println(stmt + " took " + duration + "ms")})
println("End queries")
}
}

Nothing on console.

Thanks, David

On Sep 28, 1:36 pm, Derek Chen-Becker <dchenbec...@gmail.com> wrote:
> I tested it locally on my own code and it works. Just for giggles, does
> anything print on the console if you change those lines to println?
>
> Derek
>

David Pollak

unread,
Sep 28, 2009, 1:47:57 PM9/28/09
to lif...@googlegroups.com
Could this be a JDK 1.5 vs. 1.6 issue?
--
Lift, the simply functional web framework http://liftweb.net
Beginning Scala http://www.apress.com/book/view/1430219890
Follow me: http://twitter.com/dpp
Surf the harmonics

Derek Chen-Becker

unread,
Sep 28, 2009, 2:18:57 PM9/28/09
to lif...@googlegroups.com
I can't rule it out, but because the new logging implements methods on java.sql.Statement that don't exist in Java 5, I would expect some classloading issues or exceptions.

David

unread,
Sep 28, 2009, 3:47:46 PM9/28/09
to Lift
Whoops, I take it back, not sure what happened earlier, but I am now
seeing the timing and queries when I use println. Maybe I did a mvn
clean. Sorry about that.

-Dave

On Sep 28, 1:47 pm, David Pollak <feeder.of.the.be...@gmail.com>
wrote:
> Beginning Scalahttp://www.apress.com/book/view/1430219890

Derek Chen-Becker

unread,
Oct 2, 2009, 3:00:14 PM10/2/09
to lif...@googlegroups.com
The refactored logging (supporting both Java 5 and 6) has been checked into master. I built it on my machine with both JDK1.5 and JDK1.6 JVMs and everything built.

Derek
Reply all
Reply to author
Forward
0 new messages