Uploaded image for project: 'Grouper'
  1. Grouper
  2. GRP-105

dont delegate error or debug logging anymore so it doesnt hide caller class/method/line#

    XMLWordPrintable

Details

    • Improvement
    • Resolution: Fixed
    • Major
    • 1.4.0
    • 1.3.0
    • API
    • None

    Description

      > > When I look in the logs I see a difference. Are you seeing something
      > > different? If so then I will retract my suggestion...
      > >
      > > When I change my log4j mask to show class, method, and line#:
      > >
      > > log4j.appender.grouper_stderr.layout.ConversionPattern =
      > > %d

      {yyyy/MM/dd HH:mm:ss.SSS}

      [%t] %-5p %C

      {1}

      .%M(%L) - %m%n
      > >
      > > (The C, M, and L), then
      > >
      > > ############## OLD WAY WITH DELEGATE
      > >
      > > when I log with the delegate:
      > >
      > > ErrorLog.error(HibernateSession.class, errorString);
      > >
      > > I see:
      > > 2008/03/27 12:51:13.359 [main] ERROR ErrorLog.error(46) -
      > > [edu.internet2.middleware.grouper.hibernate.HibernateSession] Problem
      > > in HibernateSession: HibernateSession: isNew: true, isReadonly: true,
      > > grouperTransactionType: READONLY_NEW
      > >
      > > This means that in class ErrorLog, in method "error", on line 46, it
      > > was logged. This will be in my logs for every ErrorLog entry. It
      > > says HibernateSession is the class where the error originated, but
      > > where in hibernate session? It is an easter egg hunt ('tis the
      > > season!)
      > >
      > > ############### NEW WAY
      > >
      > > But with LOG.error(errorString) directly in the class that logs, I
      > see:
      > >
      > > 2008/03/27 12:47:37.093 [main] ERROR
      > > HibernateSession.callbackHibernateSession(251) - Problem in
      > > HibernateSession: HibernateSession: isNew: true, isReadonly: true,
      > > grouperTransactionType: READONLY_NEW
      > >
      > > This means in class HibernateSession in method
      > "callbackHibernateSession", line 251, this was logged.
      > >
      > > Also notice that the delegate one (first example), the fully
      > qualified classname is logged, and in mine it isn't. The user should
      > be able to specify that in the log4j mask, and with delegate, they
      > cant.
      > >
      > > I know you might not have had this error mask on before, and you have
      > done just fine troubleshooting problems, but I bet you would have saved
      > time if you wouldn't have had to search in classes to see where things
      > were logged and instantly knew the line number. No matter what your
      > preference is, Grouper should not take this log4j feature out of the
      > hands of users of grouper. They should get to decide for themselves if
      > they want to use it.
      > >
      > > Therefore, in the future (post 1.3), we need to remove the delegate
      > or address in another way (removing is my preference since I don't see
      > it serving any real purpose besides not having to declare a logger in
      > each class... but then you have to pass the class to each call which
      > leads to copy/paste problems... correct me if wrong). Let me know if
      > we need to discuss at next call or if I should open a jira. Will be
      > easy no-risk fix, and I don't mind doing it.
      > >
      > > Incidentally, if we were using the log4j api (which isn't a
      > suggestion), then you can log with delegate:
      > >
      > >
      > http://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/Category.
      > > html
      > >
      > > method: log(java.lang.String callerFQCN, Priority level,
      > > java.lang.Object message, java.lang.Throwable t)
      > >
      > > However, with commons-logging which is more generic, I don't think
      > there is a way to do this.
      > >
      > > With EventLog, I think the delegate does real work, and the line
      > number is less important, so I am not suggesting we change that one.
      > Though we could with a different type of delegate:
      > EventLog.LOG.warn(EventLog.format(s, msg)); But not a huge deal.
      > >
      > > DebugLog should also be changed, though its not used in as many
      > places so will be less controversial...
      > >
      > > Thanks!
      > > Chris

      Attachments

        Activity

          People

            chris.hyzer@at.internet2.edu Chris Hyzer (upenn.edu)
            chris.hyzer@at.internet2.edu Chris Hyzer (upenn.edu)
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: