prevayler
  1. prevayler
  2. PRV-23

Add a logging (/monitor) interface

    Details

    • Type: New Feature New Feature
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.3
    • Component/s: Code
    • Labels:
      None
    • Number of attachments :
      3

      Description

      Remove all instances of System.out.print(ln) from Prevayler's main codebase, and implement the logging interface, as described here: http://paulhammant.com/blog//000241.html

        Activity

        Hide
        Klaus Wuestefeld added a comment -

        I will be travelling in the next couple of days but give me some time after that and I will try an implementation.

        I think there is a design that solves all the issues we agree on:

        • Explicit methods
        • Generic logger interface
        • Named Loggers
          etc.

        None of them are conflicting, so it is just a matter of doing it.

        Show
        Klaus Wuestefeld added a comment - I will be travelling in the next couple of days but give me some time after that and I will try an implementation. I think there is a design that solves all the issues we agree on: Explicit methods Generic logger interface Named Loggers etc. None of them are conflicting, so it is just a matter of doing it.
        Hide
        Klaus Wuestefeld added a comment -

        "OK. Lets try separate monitors for separate components."

        "You've said this a few times"

        I havenīt. Carlos was the one who convinced me.

        "but haven't actually mentioned why you think this will be of any benefit. What is the benefit?"

        Well, we can do different things with different events. That doesn't mean we can't also channel all events, by default, to a common logger interface (that can have an implementation creating named Log4JLoggers, for example).

        See you, Klaus.

        Show
        Klaus Wuestefeld added a comment - "OK. Lets try separate monitors for separate components." "You've said this a few times" I havenīt. Carlos was the one who convinced me. "but haven't actually mentioned why you think this will be of any benefit. What is the benefit?" Well, we can do different things with different events. That doesn't mean we can't also channel all events, by default, to a common logger interface (that can have an implementation creating named Log4JLoggers, for example). See you, Klaus.
        Hide
        Klaus Wuestefeld added a comment -

        "I'd like to mock the Monitor interface and make assertions about what's happening."

        I don't like white-box testing. It sort of violates encapsulation. I should be able to change the order in which I read the journal files, for optimization, for example, and the tests should not give me grief about it.

        I have seen major XP Gurus do it - under the excuse of writing finer grained tests - and I didn't like what I saw.

        Show
        Klaus Wuestefeld added a comment - "I'd like to mock the Monitor interface and make assertions about what's happening." I don't like white-box testing. It sort of violates encapsulation. I should be able to change the order in which I read the journal files, for optimization, for example, and the tests should not give me grief about it. I have seen major XP Gurus do it - under the excuse of writing finer grained tests - and I didn't like what I saw.
        Hide
        Jacob Kjome added a comment -

        Here's a little refactor of the Monitor stuff taking into account Justin's comments here:
        http://jira.codehaus.org/browse/PRV-23#action_19862

        I've added the abstract LoggingMonitor which the other implementations (other than the NullMonitor) extend. I've modified all methods to take a Class as the first parameter so we can obtain the logger name being to be used (and modified PersistentJournal and SimpleInputStream to account for this change). I also modified SimpleInputStream to log to the info level and modified the MainXStream class to use the Log4jMonitor so I could quickly test the Log4jMonitor implementation.

        To test this out, grab a fresh copy of CVS, extract the zip file over the root module ("prevayler"), and everything should be put into place. It includes a log4j.xml configured to log to the info level for org.prevayler. Use the Ant build to run the XStream bank demo such as...

        ant run.demo.bank.xstream

        Now look in prevayler/target/logs/main.log to see the logging and prove it is working.

        Note that I left the notify() methods there. I don't care if we use notify() or go with explicit methods. I just wanted to get this out there for comment.

        Jake

        Show
        Jacob Kjome added a comment - Here's a little refactor of the Monitor stuff taking into account Justin's comments here: http://jira.codehaus.org/browse/PRV-23#action_19862 I've added the abstract LoggingMonitor which the other implementations (other than the NullMonitor) extend. I've modified all methods to take a Class as the first parameter so we can obtain the logger name being to be used (and modified PersistentJournal and SimpleInputStream to account for this change). I also modified SimpleInputStream to log to the info level and modified the MainXStream class to use the Log4jMonitor so I could quickly test the Log4jMonitor implementation. To test this out, grab a fresh copy of CVS, extract the zip file over the root module ("prevayler"), and everything should be put into place. It includes a log4j.xml configured to log to the info level for org.prevayler. Use the Ant build to run the XStream bank demo such as... ant run.demo.bank.xstream Now look in prevayler/target/logs/main.log to see the logging and prove it is working. Note that I left the notify() methods there. I don't care if we use notify() or go with explicit methods. I just wanted to get this out there for comment. Jake
        Hide
        Jacob Kjome added a comment -

        This is implemented. However, we may want to think about dumping the monitors in favor of using the SLF4J interfaces when they get released as 1.0. It would be much more flexible and would only be a few K dependency for the NOP version. Users can drop other statically bound logging implementations of SLF4J as needed into their classpath.

        Jake

        Show
        Jacob Kjome added a comment - This is implemented. However, we may want to think about dumping the monitors in favor of using the SLF4J interfaces when they get released as 1.0. It would be much more flexible and would only be a few K dependency for the NOP version. Users can drop other statically bound logging implementations of SLF4J as needed into their classpath. Jake

          People

          • Assignee:
            Carlos Villela
            Reporter:
            Carlos Villela
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: