Tapestry 5
  1. Tapestry 5
  2. TAP5-119

Tapestry should not use the page/component Logger for internal logging of class transformation and event dispatch logic

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 5.0.15
    • Fix Version/s: 5.0.16
    • Component/s: None
    • Labels:
      None

      Description

      While debugging with Tapestry5, I noticed one not-so-nice issue. But first, a bit of background.

      I usually configure my logging framework (log4j) to log all statements for my application at debug level during development. E.g. all log records logged to "com.mycompany.myapp" logging category get printed at DEBUG level. All other categories are usually at INFO level. This allows me to focus on the log statements that I have written and ignore all the verbose DEBUG logs of 3rd party libraries.

      Back to Tapestry5. I have configured by log4j as usual (my stuff at DEBUG level, everything else at INFO level). Although I expect to see only the log output produced by MY CODE, I also see some debug output produced by Tapestry internals. Seems that Tapestry class enhancement code logs to the same log category as the class name being enhanced. And the log statements are quite big (documenting the changes being made to the original classes), resulting in lots of unwanted noise in my logs.

      Example:
      I have a page: com.mycompany.myapp.pages.MyPage
      T5 class enhancement code logs its bytecode manipulation operations to log category "com.mycompany.myapp.pages.MyPage", at DEBUG level.

      This is not very nice, as I'm totally uninterested in seeing all this verbose T5-internal logging.

      I understand that it might be beneficial to have that class name as part of the logging category (or maybe not), but at the very least, I would recommend to add a T5 prefix to all those categories. The log category in above example would then become something like "org.apache.tapestry.classenhance.com.mycompany.myapp.pages.MyPage".

      For reference:
      http://thread.gmane.org/gmane.comp.java.tapestry.user/65644

        Activity

        Hide
        Howard M. Lewis Ship added a comment -

        Agreed ... I got a little ways into coding and realized a suffix would be a disaster.

        Show
        Howard M. Lewis Ship added a comment - Agreed ... I got a little ways into coding and realized a suffix would be a disaster.
        Hide
        Fernando Padilla added a comment -

        no please add a PREFIX!

        that's what most loggers use for configuration!!

        Show
        Fernando Padilla added a comment - no please add a PREFIX! that's what most loggers use for configuration!!
        Hide
        Howard M. Lewis Ship added a comment -

        OK, I'll add a ".transformer" suffix to the class name for the class transformation output, and a ".events" suffix to the class name for the event dispatch / method invocation logging.

        Show
        Howard M. Lewis Ship added a comment - OK, I'll add a ".transformer" suffix to the class name for the class transformation output, and a ".events" suffix to the class name for the event dispatch / method invocation logging.
        Hide
        Fernando Padilla added a comment -

        If someone wants, you can even make this configurable.. you can have the ComponentClassTransformerImpl take symbols in its constructor, to let people define what the prefix should be; I can submit a patch for that version if people want it.

        Show
        Fernando Padilla added a comment - If someone wants, you can even make this configurable.. you can have the ComponentClassTransformerImpl take symbols in its constructor, to let people define what the prefix should be; I can submit a patch for that version if people want it.
        Hide
        Fernando Padilla added a comment -

        And it's a one line change:

        — tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassTransformerImpl.java (revision 708604)
        +++ tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassTransformerImpl.java (working copy)
        @@ -131,7 +131,7 @@

        String classname = ctClass.getName();

        • Logger logger = loggerSource.getLogger(classname);
          + Logger logger = loggerSource.getLogger("org.apache.tapestry.enhanced."+classname);

        // If the parent class is in a controlled package, it will already have been loaded and
        // transformed (that is driven by the ComponentInstantiatorSource).

        Show
        Fernando Padilla added a comment - And it's a one line change: — tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassTransformerImpl.java (revision 708604) +++ tapestry-core/src/main/java/org/apache/tapestry5/internal/services/ComponentClassTransformerImpl.java (working copy) @@ -131,7 +131,7 @@ String classname = ctClass.getName(); Logger logger = loggerSource.getLogger(classname); + Logger logger = loggerSource.getLogger("org.apache.tapestry.enhanced."+classname); // If the parent class is in a controlled package, it will already have been loaded and // transformed (that is driven by the ComponentInstantiatorSource).
        Hide
        Fernando Padilla added a comment -

        I am also in the same boat!

        Please, do not let Tapestry hijack our package names for its logging. Those are two totally different things.

        Debugging our application does not mean we have to debug tapestry!

        So I agree, either a different root category ( like stated above, or "tapestry.enhancer.[CLASS]" ), or TRACE level logging!

        Show
        Fernando Padilla added a comment - I am also in the same boat! Please, do not let Tapestry hijack our package names for its logging. Those are two totally different things. Debugging our application does not mean we have to debug tapestry! So I agree, either a different root category ( like stated above, or "tapestry.enhancer. [CLASS] " ), or TRACE level logging!
        Hide
        Andy Blower added a comment -

        An alternative solution is to lower the logging level from DEBUG to TRACE for this output. I'm also missing having a useful DEBUG log level for pages so some solution is very desirable.

        Show
        Andy Blower added a comment - An alternative solution is to lower the logging level from DEBUG to TRACE for this output. I'm also missing having a useful DEBUG log level for pages so some solution is very desirable.

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            Neeme Praks
          • Votes:
            12 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development