Log file naming

2010-06-09
2013-05-20
  • I'm using log4cpp in a task running in more instances.

    I saw that there are trouble in rolling files as each instance of my task
    writes on the same file, but each log4cpp instance thinks to be the only one
    file writer, so that the file size check is not safe.

    I think is better to allow each process to write on its own log file,
    so I'm starting to modify log4cpp in order to allow my task to change the log file name in some way,
    and I'd love to contribute the patch to the project.

    I'm ready to discuss the implementation here
    (I need to derive a class from RollingFileAdapter, but that class is hardcoded in PropertyConfiguratorimpl,
    so I'm going to modify in some way PropertyConfigurator and  PropertyConfiguratorimpl)
    so please let me know if somebody is interested in it

    Thanks
    Tom

     
  • darkangel
    darkangel
    2010-06-09

    Tom, I not really understand what you are trying to do.
    If you had one process that launch children processes and each of its should log into same file that not RollFileAppender's job. You do need one log file per process. Or you need some IPC and source/sink appenders that would listen all childs, merge all messages and write to one file. 
    Please describe you problem and what you try to achieve.
    PS. I'm not English speaking person so be carefully :)  

     
  • I need to run a task in this way:
    for i in "1 2 3 4 5 … "; do mytask $i; done

    used in this way, RollFileAppender has problem,
    so I need to decide the name of the log file in each mytask instance.

    My idea is to write in the configure file the name of a new appender,
    and modify log4cpp so that will be possible to pass a pointer to my new appender.

    Below there is the patch I'm working to,
    I'm now trying to compile it in a Solaris 64 bit enviroment.

    diff -ur log4cpp-1.0/include/log4cpp/PropertyConfigurator.hh orig/log4cpp-1.0/include/log4cpp/PropertyConfigurator.hh
    -- log4cpp-1.0/include/log4cpp/PropertyConfigurator.hh Wed Jun  9 17:21:09 2010
    +++ orig/log4cpp-1.0/include/log4cpp/PropertyConfigurator.hh    Sat May 10 00:16:25 2003
    @@ -43,12 +43,9 @@
           
            @since 0.3.2
         **/
    -       class Appender;
         class LOG4CPP_EXPORT PropertyConfigurator {
             public:
    -        static void configure(const std::string& initFileName,
    -                                                         Appender * externalAppender = NULL,
    -                                                         const char * externalAppenderName = "") throw (ConfigureFailure);
    +        static void configure(const std::string& initFileName) throw (ConfigureFailure);
         };
    }
    diff -ur log4cpp-1.0/src/PropertyConfigurator.cpp orig/log4cpp-1.0/src/PropertyConfigurator.cpp
    -- log4cpp-1.0/src/PropertyConfigurator.cpp    Wed Jun  9 17:21:20 2010
    +++ orig/log4cpp-1.0/src/PropertyConfigurator.cpp       Sat Oct 26 20:30:55 2002
    @@ -11,10 +11,8 @@

    namespace log4cpp {

    -    void PropertyConfigurator::configure(const std::string& initFileName,
    -                                                                                Appender * externalAppender,
    -                                                                                const char * externalAppenderName) throw (ConfigureFailure) {
    -        PropertyConfiguratorImpl configurator(externalAppender, externalAppenderName);
    +    void PropertyConfigurator::configure(const std::string& initFileName) throw (ConfigureFailure) {
    +        PropertyConfiguratorImpl configurator;
            
             configurator.doConfigure(initFileName);
         }

    diff -ur log4cpp-1.0/src/PropertyConfiguratorImpl.cpp orig/log4cpp-1.0/src/PropertyConfiguratorImpl.cpp
    -- log4cpp-1.0/src/PropertyConfiguratorImpl.cpp        Wed Jun  9 17:02:19 2010
    +++ orig/log4cpp-1.0/src/PropertyConfiguratorImpl.cpp   Fri May 23 00:45:48 2003
    @@ -56,8 +56,7 @@

    namespace log4cpp {

    -    PropertyConfiguratorImpl::PropertyConfiguratorImpl(Appender * externalAppender, const char * externalAppenderName):
    -               m_externalAppender(externalAppender), m_externalAppenderName(externalAppenderName) {
    +    PropertyConfiguratorImpl::PropertyConfiguratorImpl() {
         }

         PropertyConfiguratorImpl::~PropertyConfiguratorImpl() {
    @@ -253,9 +252,6 @@
                 appender = new NTEventLogAppender(appenderName, source);
             }
    #endif // WIN32
    -        else if ( ( m_externalAppender !=NULL ) && (appenderType == m_externalAppenderName)) {
    -                       appender = m_externalAppender;
    -        }
             else {
                 throw ConfigureFailure(std::string("Appender '") + appenderName +
                                        "' has unknown type '" + appenderType + "'");
    diff -ur log4cpp-1.0/src/PropertyConfiguratorImpl.hh orig/log4cpp-1.0/src/PropertyConfiguratorImpl.hh
    -- log4cpp-1.0/src/PropertyConfiguratorImpl.hh Wed Jun  9 17:20:48 2010
    +++ orig/log4cpp-1.0/src/PropertyConfiguratorImpl.hh    Sat Oct 26 20:30:55 2002
    @@ -26,7 +26,7 @@
             public:
             typedef std::map<std::string, Appender*> AppenderMap;

    -        PropertyConfiguratorImpl(Appender * externalAppender=NULL, const char * externalAppenderName="");
    +        PropertyConfiguratorImpl();
             virtual ~PropertyConfiguratorImpl();
             virtual void doConfigure(const std::string& initFileName)
                 throw (ConfigureFailure);
    @@ -78,9 +78,6 @@

             Properties _properties;
             AppenderMap _allAppenders;
    -              
    -               Appender * m_externalAppender;
    -               std::string m_externalAppenderName;
         };
    }

     
  • darkangel
    darkangel
    2010-06-09

    Okay. I see. But can't see why you need to patch log4cpp. You already decide to have for each task its own config file. If you have one conf per instance with different appender name in each why do you need patch?

     
  • Unlikely I can't use a different conf per instance,
    because I dont know how many tasks will run,
    I wrote 12 3 4 5 …. where "…" could be some hundred,
    and I can't prepare hundreds conf file

    it is much easier for me to create for each process a different log file,
    where file name is related to the time of process start and the PID,
    like YYMMDDHHMMSS_PID.log

    To have it, I need to derive a my own class from RollingFile (I need the function it has)
    and pass an instance of my class in some way to the library.

    This is what the code I sent should do,
    it is very conservative against the library,
    please feel free to comment it

    Tom

     
  • darkangel
    darkangel
    2010-06-09

    Ok. From my point of view you choose is a wrong way to resolve you problem. The better way would modify property configurator to understand some,say, modificators to appenders names. Than you give as generic way to resolve such kind of problems in non-intrusive generic fashion.

    log4cpp.appender.A3=RollingFileAppender
    log4cpp.appender.A3.fileName=my-task-%d%p.log
    log4cpp.appender.A3.maxFileSize=80
    log4cpp.appender.A3.maxBackupIndex=1

    What do you think?

     
  • darkangel
    darkangel
    2010-06-09

    Sorry for the first sentence. You way not wrong. It solves you problem. But I don't want to inject to library some specific path to resolve such problems. I think my proposal solve you and others problems without need to modify they and yours applications code, just config files. And I think that better :)

     
  • Ok, I' going to implement your solution.

    I'll implement
      %d = HHMMDD
      %t  = HHMMSS
      %p = pid

    It seems the best place to put this new filename parser is the FileAppender class,
    do you agree?

     
  • darkangel
    darkangel
    2010-06-11

    I think we should implement free standing function

    std::string transform_filename(const std::string& filename);

    FileAppender seams to good place for transformation injection. Please implement %% escaping handling to allow % in file names if somebody decide to use it - hope we not break some configurations…

     
  • Hi,
    I have trouble with the CVS access
    (cvs : connect to log4cpp.cvs.sourceforge.net(216.34.181.109):2401 failed: Connection timed out), 
    so I can't do a patch against the latest version, I'll send a patch against the latest tar.

    I have also included this patch, maybe it is already in the log4cpp cvs:

    http://bugs.gentoo.org/attachment.cgi?id=149763