Details

    • Type: Wish Wish
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.1.0
    • Component/s: None
    • Labels:
      None

      Description

      Pig is logging quite a lot into System.out or System.err. Using a embedded pig in a production environment requires a logging abstraction like log4j, commons logging, slf4j or something like that.
      I would be happy to work on a patch if we decide what would be the best choice. Hadoop uses log4j.
      Thanks.
      Stefan

      1. PIG-83-v01.patch
        60 kB
        Benjamin Francisoud
      2. log4j.properties
        1.0 kB
        Benjamin Francisoud
      3. PIG-83-v02.patch
        99 kB
        Benjamin Francisoud
      4. logging.properties
        1 kB
        Benjamin Francisoud
      5. PIG-83-v03.patch
        76 kB
        Benjamin Francisoud

        Issue Links

          Activity

          Hide
          Alan Gates added a comment -

          Applied the latest patch using logging commons (PIG-83-v03.patch), revision 627115. Thanks Benjamin.

          Show
          Alan Gates added a comment - Applied the latest patch using logging commons ( PIG-83 -v03.patch), revision 627115. Thanks Benjamin.
          Hide
          Alan Gates added a comment -

          While we don't quite have unanimity, it appears a majority of users and developers prefer the common logging abstraction over java.util.Logging. Unless I hear violent objections, I'll work on integrating the commons patch some time next week.

          Show
          Alan Gates added a comment - While we don't quite have unanimity, it appears a majority of users and developers prefer the common logging abstraction over java.util.Logging. Unless I hear violent objections, I'll work on integrating the commons patch some time next week.
          Hide
          Stefan Groschupf added a comment -

          Great! Thanks!

          Show
          Stefan Groschupf added a comment - Great! Thanks!
          Hide
          Benjamin Francisoud added a comment -

          Well anything is better than System.out.println()

          Here's a patch to use commons logging everywhere, the previous one became obsolete because of PIG-32.
          This one does the same as patch v01 but take it even further by removing PigLogger class and use Commons Logging instead.
          There is no dependency to log4j at the import level (it only uses commons logging interfaces)

          Show
          Benjamin Francisoud added a comment - Well anything is better than System.out.println() Here's a patch to use commons logging everywhere, the previous one became obsolete because of PIG-32 . This one does the same as patch v01 but take it even further by removing PigLogger class and use Commons Logging instead. There is no dependency to log4j at the import level (it only uses commons logging interfaces)
          Hide
          Stefan Groschupf added a comment -

          Hi,
          sorry to say but from my experience within the last year java.util logging is a pain for serious production systems.
          We also should learn from the hadoop folks. They had a very long discussion about this topic and did use java.util logging in the first step but finally moved to log4j.
          Pig might should not do the same mistake again.

          Show
          Stefan Groschupf added a comment - Hi, sorry to say but from my experience within the last year java.util logging is a pain for serious production systems. We also should learn from the hadoop folks. They had a very long discussion about this topic and did use java.util logging in the first step but finally moved to log4j. Pig might should not do the same mistake again.
          Hide
          Benjamin Francisoud added a comment -

          sample java logging conf file

          Show
          Benjamin Francisoud added a comment - sample java logging conf file
          Hide
          Benjamin Francisoud added a comment -

          patch to use java.util.logging

          Modifications

          • Replaced System.out.println with logger.info
          • Replaced System.err.println with logger.warning (since there is no error level in java logging)
          • Deleted PigLogger (which was using log4j) and replace every call by java.util.logging
          • Replaced PigLogger.fatal with logger.severe (which is the highest log level in java logging)
          • Replaced PigLogger.error with logger.warning
          • The only method I could find in the api to log a full stacktrace was logger.log(Level.XXX, e.getMessage(), e); (java logging api sucks in my opinion)
          • removed options in the command line tool to set log level since java logging api specify that it(s the user who is in charge of specifying a logging.properties file (see Configuration below)

          Special cases
          Exception in 2 classes where the system.out are for command line help or usage printing:

          • Left Main.usage()
          • GruntParser#printHelp()

          Configuration
          From java logging documentation:

          • JavaTM Logging Overview - chapt 1.8 Configuration File
            " The logging configuration can be initialized using a logging configuration file that will be read at startup. This logging configuration file is in standard java.util.Properties format."
            " The default logging configuration that ships with the JRE is only a default, and can be overridden by ISVs, system admins, and end users."
          • LogManager Api
            " If "java.util.logging.config.class" property is not set, then the "java.util.logging.config.file" system property can be used to specify a properties file (in java.util.Properties format). The initial logging configuration will be read from this file.
            If neither of these properties is defined then, as described above, the LogManager will read its initial configuration from a properties file "lib/logging.properties" in the JRE directory. "

          To use and configure logging level, output etc...
          You need to either:

          • change "lib/logging.properties" in the JRE directory
          • set a -Djava.util.logging.config.file=/pig/trunk/logging.properties

          I will provide an example configuration file (a modify version of a jpox conf)

          Show
          Benjamin Francisoud added a comment - patch to use java.util.logging Modifications Replaced System.out.println with logger.info Replaced System.err.println with logger.warning (since there is no error level in java logging) Deleted PigLogger (which was using log4j) and replace every call by java.util.logging Replaced PigLogger.fatal with logger.severe (which is the highest log level in java logging) Replaced PigLogger.error with logger.warning The only method I could find in the api to log a full stacktrace was logger.log(Level.XXX, e.getMessage(), e); (java logging api sucks in my opinion) removed options in the command line tool to set log level since java logging api specify that it(s the user who is in charge of specifying a logging.properties file (see Configuration below) Special cases Exception in 2 classes where the system.out are for command line help or usage printing: Left Main.usage() GruntParser#printHelp() Configuration From java logging documentation: JavaTM Logging Overview - chapt 1.8 Configuration File " The logging configuration can be initialized using a logging configuration file that will be read at startup. This logging configuration file is in standard java.util.Properties format." " The default logging configuration that ships with the JRE is only a default, and can be overridden by ISVs, system admins, and end users." LogManager Api " If "java.util.logging.config.class" property is not set, then the "java.util.logging.config.file" system property can be used to specify a properties file (in java.util.Properties format). The initial logging configuration will be read from this file. If neither of these properties is defined then, as described above, the LogManager will read its initial configuration from a properties file "lib/logging.properties" in the JRE directory. " To use and configure logging level, output etc... You need to either: change "lib/logging.properties" in the JRE directory set a -Djava.util.logging.config.file=/pig/trunk/logging.properties I will provide an example configuration file (a modify version of a jpox conf )
          Hide
          Pi Song added a comment - - edited

          As a user, I vote for commons-logging/log4j. Different systems have different requirements. You should keep it generic. By adding dependencies on external logging framework which is used by almost everyone, I don't see that's an issue. It's a very common case that when someone is trying to turn on logging for an app, the first thing he'll do (before going for the app's user manual) is looking for log4j.xml!!!

          Show
          Pi Song added a comment - - edited As a user, I vote for commons-logging/log4j. Different systems have different requirements. You should keep it generic. By adding dependencies on external logging framework which is used by almost everyone, I don't see that's an issue. It's a very common case that when someone is trying to turn on logging for an app, the first thing he'll do (before going for the app's user manual) is looking for log4j.xml!!!
          Hide
          Johannes Zillmann added a comment -

          I (as a user) would be not so enthusiastic about java.util logging.
          Its much more harder to configure (put properties in the jre home or specify an extra system property) and if you embed pig into your application a logging abstraction like commons-logging avoids the maintenance and configuration of 2 logger implementaion.

          So with commons-logging the user can use it logging framework of choice!

          Show
          Johannes Zillmann added a comment - I (as a user) would be not so enthusiastic about java.util logging. Its much more harder to configure (put properties in the jre home or specify an extra system property) and if you embed pig into your application a logging abstraction like commons-logging avoids the maintenance and configuration of 2 logger implementaion. So with commons-logging the user can use it logging framework of choice!
          Hide
          Benjamin Reed added a comment -

          I agree with Sam. I'd rather use the standard APIs rather than add new ones. (I realize that we get commons from hadoop, but you many not get that if you used a different backend.)

          Show
          Benjamin Reed added a comment - I agree with Sam. I'd rather use the standard APIs rather than add new ones. (I realize that we get commons from hadoop, but you many not get that if you used a different backend.)
          Hide
          Benjamin Francisoud added a comment -

          A sample log4j.properties you can add to your src folder to test the patch.

          Show
          Benjamin Francisoud added a comment - A sample log4j.properties you can add to your src folder to test the patch.
          Hide
          Benjamin Francisoud added a comment -

          This patch remove all System.out.println and System.err.println from java files and replace them with commons logging interface.

          Except in 2 classes where the system.out are for command line help or usage printing:

          • Main#usage()
          • GruntParser#printHelp()

          The rule I applied to make this patch:

          • System.out.println is transformed to logger.info
          • System.err.println is transformed to logger.error

          I made some exception when it was obvious that some System.out where actually debug logs. for instance : PigContext#doHod()

          While doing this I improved some error loggin to log the full stacktrace (not only the message) (see PIG-80)
          In TimestampedTuple, FuncCond, Grunt, GruntParser, PigScriptParser:
          Usually it's just transforming from

          System.err.println(e.getMessage())

          to

          logger.error(e)

          Can you review my patch please, if you don't like this one I can make an other one with java native logging just vote for it on the mailing list [1]

          thanks

          [1] http://www.mail-archive.com/pig-dev%40incubator.apache.org/msg00611.html

          Show
          Benjamin Francisoud added a comment - This patch remove all System.out.println and System.err.println from java files and replace them with commons logging interface. Except in 2 classes where the system.out are for command line help or usage printing: Main#usage() GruntParser#printHelp() The rule I applied to make this patch: System.out.println is transformed to logger.info System.err.println is transformed to logger.error I made some exception when it was obvious that some System.out where actually debug logs. for instance : PigContext#doHod() While doing this I improved some error loggin to log the full stacktrace (not only the message) (see PIG-80 ) In TimestampedTuple, FuncCond, Grunt, GruntParser, PigScriptParser: Usually it's just transforming from System .err.println(e.getMessage()) to logger.error(e) Can you review my patch please, if you don't like this one I can make an other one with java native logging just vote for it on the mailing list [1] thanks [1] http://www.mail-archive.com/pig-dev%40incubator.apache.org/msg00611.html
          Hide
          Benjamin Francisoud added a comment -

          All commons logging and log4j classes are already included in hadoop15.jar
          So java.util.Logging of org.apache.commons.logging.Log are already in the classpath...

          ps: I don't like too much dependencies either

          Show
          Benjamin Francisoud added a comment - All commons logging and log4j classes are already included in hadoop15.jar So java.util.Logging of org.apache.commons.logging.Log are already in the classpath... ps: I don't like too much dependencies either
          Hide
          Sam Pullara added a comment -

          Please use java.util.Logging rather than any more dependencies.

          Show
          Sam Pullara added a comment - Please use java.util.Logging rather than any more dependencies.
          Hide
          Benjamin Francisoud added a comment -

          I just started a thread on this subject on the dev mailing list [1] and created a wiki page [2] to hold the result of the discussion.

          Can you have a look and answer in the mail thread, I think mailing list are better for general discussion.
          The jira will then keep track of the patches version, implementation details etc...

          I'm working on a patch atm, might be done later today or tomorrow.

          [1] http://www.mail-archive.com/pig-dev%40incubator.apache.org/msg00611.html
          [2] http://wiki.apache.org/pig/LoggingPolicy

          Show
          Benjamin Francisoud added a comment - I just started a thread on this subject on the dev mailing list [1] and created a wiki page [2] to hold the result of the discussion. Can you have a look and answer in the mail thread, I think mailing list are better for general discussion. The jira will then keep track of the patches version, implementation details etc... I'm working on a patch atm, might be done later today or tomorrow. [1] http://www.mail-archive.com/pig-dev%40incubator.apache.org/msg00611.html [2] http://wiki.apache.org/pig/LoggingPolicy

            People

            • Assignee:
              Benjamin Francisoud
              Reporter:
              Stefan Groschupf
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development