Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.1.0
    • Fix Version/s: 0.2.0
    • Component/s: cli
    • Labels:
      None

      Description

      I believe there are two issues here:

      1) services are not logging anything
      2) logging output does not seem to be setup correctly

      for 1) we should add info level (at least) messages to the service implementations to indicate progress
      for 2) we should include a default log4j.properties (or xml) in the shaded jar which has sane defaults (see attachment for an example I whipped up and am using myself)

      1. log4j.properties
        0.7 kB
        Patrick Hunt
      2. WHIRR-106.patch
        26 kB
        Tom White
      3. WHIRR-106.patch
        26 kB
        Tom White
      4. WHIRR-106.patch
        26 kB
        Tom White

        Activity

        Patrick Hunt created issue -
        Hide
        Patrick Hunt added a comment -

        attaching a sample log4j.properties which outputs info level messages to the console, debug to a whirr_cli.log file in the cwd.

        Show
        Patrick Hunt added a comment - attaching a sample log4j.properties which outputs info level messages to the console, debug to a whirr_cli.log file in the cwd.
        Patrick Hunt made changes -
        Field Original Value New Value
        Attachment log4j.properties [ 12455842 ]
        Hide
        Tom White added a comment -
        • Added logging to each service. This is at info level, and is useful for a fairly high-level view of what Whirr is doing.
        • The CLI now logs info level to the console and everything else (for org.apache.whirr and jclouds.compute categories) to a file called whirr.log. The jclouds.compute category is nice because it logs things like what the runscript output is, which saves SSHing to the instances.
        • Added an appender to the test log4j.xml files for the whirr logging output.
        • Also fixed the test log4j.xml files to have a root-level appender (log4j was complaining about this before).
        • Changed the poms so that log4j is used for tests and for the CLI, but otherwise is not a dependency (slf4j is used as the logging facade). In principle this means that clients using Whirr as a library could use any logging supplier.

        Patrick mentioned offline that it might be nice to have an interface between Whirr and jclouds which does a lot of the service logging for you (i.e. the writer of a Whirr service). I agree, but think this is bigger in scope and probably belongs in another JIRA.

        Show
        Tom White added a comment - Added logging to each service. This is at info level, and is useful for a fairly high-level view of what Whirr is doing. The CLI now logs info level to the console and everything else (for org.apache.whirr and jclouds.compute categories) to a file called whirr.log. The jclouds.compute category is nice because it logs things like what the runscript output is, which saves SSHing to the instances. Added an appender to the test log4j.xml files for the whirr logging output. Also fixed the test log4j.xml files to have a root-level appender (log4j was complaining about this before). Changed the poms so that log4j is used for tests and for the CLI, but otherwise is not a dependency (slf4j is used as the logging facade). In principle this means that clients using Whirr as a library could use any logging supplier. Patrick mentioned offline that it might be nice to have an interface between Whirr and jclouds which does a lot of the service logging for you (i.e. the writer of a Whirr service). I agree, but think this is bigger in scope and probably belongs in another JIRA.
        Tom White made changes -
        Attachment WHIRR-106.patch [ 12455953 ]
        Tom White made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Assignee Tom White [ tomwhite ]
        Hide
        Adrian Cole (Inactive) added a comment -

        for logging, it is pretty easy to interface into the jclouds logging system.

        using context().utils().loggerFactory() will give you a logger instance based on the scope you specify.

        Show
        Adrian Cole (Inactive) added a comment - for logging, it is pretty easy to interface into the jclouds logging system. using context().utils().loggerFactory() will give you a logger instance based on the scope you specify.
        Hide
        Tom White added a comment -

        Thanks Adrian. For Whirr I think it makes sense to use a logging facade like slf4j, rather than tightly binding to jclouds logging system.

        Show
        Tom White added a comment - Thanks Adrian. For Whirr I think it makes sense to use a logging facade like slf4j, rather than tightly binding to jclouds logging system.
        Hide
        Adrian Cole (Inactive) added a comment -

        ahh sure. I misunderstood. well, if you end up choosing slf4j, jclouds can make an adapter to use it.

        Show
        Adrian Cole (Inactive) added a comment - ahh sure. I misunderstood. well, if you end up choosing slf4j, jclouds can make an adapter to use it.
        Hide
        Tom White added a comment -

        Here's a minor update that logs the Hadoop cluster web UI URL.

        if you end up choosing slf4j, jclouds can make an adapter to use it.

        Good to hear. Also, have you considered using slf4j in jclouds?

        Show
        Tom White added a comment - Here's a minor update that logs the Hadoop cluster web UI URL. if you end up choosing slf4j, jclouds can make an adapter to use it. Good to hear. Also, have you considered using slf4j in jclouds?
        Tom White made changes -
        Attachment WHIRR-106.patch [ 12456074 ]
        Hide
        Tom White added a comment -

        Synced with trunk.

        Show
        Tom White added a comment - Synced with trunk.
        Tom White made changes -
        Attachment WHIRR-106.patch [ 12456082 ]
        Hide
        Adrian Cole (Inactive) added a comment -

        wrt slj4j in jclouds, I've considered it. Personally, I'm not a big fan of using the classloader as a DI tool. I do like the recent shift to null loggers, which we did over a year ago. However, there's really never been a log-related feature we've needed and are lacking.

        More importantly, slf4j isn't defacto: rather it is one of several popular log apis. While light, it still represents a dependency those who just use log4j, java logging (or run in google appengine, android, etc) wouldn't care to bring in. Log veneers are extremely short and easy to maintain code. It is an easy burden to accept on behalf of the users.

        That said, we will make a slf4j adapter, so that those who use it can have that supported in jclouds. That makes total sense and I've logged this below:

        http://code.google.com/p/jclouds/issues/detail?id=374

        Show
        Adrian Cole (Inactive) added a comment - wrt slj4j in jclouds, I've considered it. Personally, I'm not a big fan of using the classloader as a DI tool. I do like the recent shift to null loggers, which we did over a year ago. However, there's really never been a log-related feature we've needed and are lacking. More importantly, slf4j isn't defacto: rather it is one of several popular log apis. While light, it still represents a dependency those who just use log4j, java logging (or run in google appengine, android, etc) wouldn't care to bring in. Log veneers are extremely short and easy to maintain code. It is an easy burden to accept on behalf of the users. That said, we will make a slf4j adapter, so that those who use it can have that supported in jclouds. That makes total sense and I've logged this below: http://code.google.com/p/jclouds/issues/detail?id=374
        Hide
        Patrick Hunt added a comment -

        +1, lgtm, tests pass and output from cli is more informative.

        Show
        Patrick Hunt added a comment - +1, lgtm, tests pass and output from cli is more informative.
        Hide
        Tom White added a comment -

        Thanks for the review, Patrick. I've just committed this.

        Show
        Tom White added a comment - Thanks for the review, Patrick. I've just committed this.
        Tom White made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        1d 6h 35m 1 Tom White 30/Sep/10 00:09
        Patch Available Patch Available Resolved Resolved
        4d 20h 20m 1 Tom White 04/Oct/10 20:30

          People

          • Assignee:
            Tom White
            Reporter:
            Patrick Hunt
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development