Re: New RollingFileAppender semantics
On Sep 23, 2011, at 2:07 AM, Dominik Psenner wrote: I've so far found this: http://logging.apache.org/log4j/companions/extras/apidocs/org/apache/log4j/r olling/package-tree.html Despite the fact that it is there in extras, you're suggesting that it was abandoned? Why? The log4j 1.3 development branch was abandoned since it was became hopelessly binary incompatible with log4j 1.2. We could have renumbered it as log4j 2.0, but we wanted to save 2.0 for a log4j that addressed concurrency issues and not have a 2.0 and then a 3.0 in relatively short order. Selected parts of log4j 1.3 such as the o.a.l.rolling.RollingFileAppender were backported to be compatible with log4j 1.2. The extras companion was the original home but more of them have been migrating from extras into log4j itself. So o.a.l.rolling.RollingFIleAppender is not abandoned, but its original home, the log4j 1.3 development branch is abandoned.
RE: New RollingFileAppender semantics
Take a look at org.apache.log4j.rolling.RollingFileAppender which has pluggable rolling and file naming strategies. http://logging.apache.org/log4j/1.2/apidocs/index.html For a little ancient history, log4j 1.2 has two classes named RollingFileAppender, the original in org.apache.log4j and a next generation (aka 2005 or so) implementation from the abandoned log4j 1.3 effort in org.apache.log4j.rolling. The o.a.l.r.RFA still has its problems, but is an improvement over o.a.l.RFA. I've so far found this: http://logging.apache.org/log4j/companions/extras/apidocs/org/apache/log4j/r olling/package-tree.html Despite the fact that it is there in extras, you're suggesting that it was abandoned? Why?
Re: New RollingFileAppender semantics
Take a look at org.apache.log4j.rolling.RollingFileAppender which has pluggable rolling and file naming strategies. http://logging.apache.org/log4j/1.2/apidocs/index.html For a little ancient history, log4j 1.2 has two classes named RollingFileAppender, the original in org.apache.log4j and a next generation (aka 2005 or so) implementation from the abandoned log4j 1.3 effort in org.apache.log4j.rolling. The o.a.l.r.RFA still has its problems, but is an improvement over o.a.l.RFA. On Sep 20, 2011, at 9:45 AM, Stefan Bodewig wrote: On 2011-09-20, Dominik Psenner wrote: I believe that I have to explain this further. My idea is to split the rolling into two parts: * rolling condition * file naming strategy +1 Stefan
RE: New RollingFileAppender semantics
-Original Message- From: Roy Chastain [mailto:r...@roychastain.org] Sent: Wednesday, September 21, 2011 12:14 AM To: Log4NET Dev Subject: RE: New RollingFileAppender semantics Should I try to write down the API? It should be doable in a couple of hours and right now it looks like I have some free time tomorrow around 20°°-22°°. If so, is there somewhere a sandbox that I could use? I am not sure what you are asking about a sand box etc, but anything that you wish to do to further explain your ideas, is welcome. A sandbox is typically a branch or repository that is used to play around with new code. I'll probably have to share that code through bitbucket as I don't have the privileges to commit to the repository. Or can I get sufficient privileges to commit to a branch in the ASF repository? Roy, didn't you say you already did some work there? Would you share it so that we don't duplicate effort?
RE: New RollingFileAppender semantics
A sandbox is typically a branch or repository that is used to play around with new code. I'll probably have to share that code through bitbucket as I don't have the privileges to commit to the repository. Or can I get sufficient privileges to commit to a branch in the ASF repository? Roy, didn't you say you already did some work there? Would you share it so that we don't duplicate effort? My work was initially changes to the existing code and the more I changed the more I realized that it was digging a deeper hole. That was when I realized more of the subtleties and ambiguities in the existing code. I am not sure that there is anything to share. It is bits and pieces that are not coordinated because I was trying to work within the framework of the current implementation. My current plan is to dummy up a new RFA (configuration points) and write the XML doc for the configuration points and pass the result of that out for review and comments. We must be explicit about the semantics of the new configuration. We have already observed cases where I have not correctly interrupted the meaning of Dominik was suggesting. Once we agree on the configuration and semantics of that configuration, things can go better. I am also thinking about not deriving from FileAppender as Curt mentioned in the emails about the new RFA for log4j. I will have to think about that more after we finalize the new design and I see the new FileAppender with the Mutex locking etc. If we want to support Mutex locking in RFA(NG), then it might be best to continue to derive from FileAppender and wrap all the rolling in the Mutex. If we do not want to support that type of locking, then not getting involved with FileAppender may make the code cleaner, but it will increase the amount of code. -- Roy Chastain
Re: New RollingFileAppender semantics
Thanks for the information. This means that today, specifically in the evening around 20°°, I'm going to try my best to come up with a RollingFileAppenderNG implementation that derives from FileAppender. Therefore the first layout of the RFA-NG will be much like the current RFA. We can still iterate over writereview cycles as often as it takes. Here it comes: https://bitbucket.org/NachbarsLumpi/log4net-patches/src/197c2b931afe/RFA-NG It's not finished at all, but it should make it obvious where this road leads us to. This patch applies safely to trunk@1173276. Feedbacks are welcome! -- Dominik Psenner ## OpenPGP Key Signature # # Key ID: B469318C # # Fingerprint: 558641995F7EC2D251354C3A49C7E3D1B469318C # ##
Re: New RollingFileAppender semantics
On 2011-09-19, Dominik Psenner wrote: file value=bla.log/ datePattern value=MMddHHmm / rollFileConfiguration rollFileCondition size=5MB / rollFileLimitation maxcount=5 / /rollFileConfiguration Grouping the properties that affect the rolling strategy and separating them from the others makes sense to me. It may be even a nice to implement it like that. This opens ways to something like this: rollFileConfiguration rollFileAndCondition rollFileCondition size=5MB / rollFileCondition when=daily / /rollFileAndCondition /rollFileConfiguration You realize you are going down the road of boolean combinators here, aren't you? Do we roll the file if one of the conditions is met, or all of them? But I don't know if that really fits into the log4net is a clone of log4j philosophy. :-) No problem here as we are not touching the architecture only one of the appenders - and a lot of variance is allowed here. I realize you haven't been serious. Stefan
RE: New RollingFileAppender semantics
Grouping the properties that affect the rolling strategy and separating them from the others makes sense to me. It may be even a nice to implement it like that. This opens ways to something like this: rollFileConfiguration rollFileAndCondition rollFileCondition size=5MB / rollFileCondition when=daily / /rollFileAndCondition /rollFileConfiguration You realize you are going down the road of boolean combinators here, aren't you? Do we roll the file if one of the conditions is met, or all of them? Depends on the conjunction used (And vs Or). In my example, the roll file should be done daily and only if the file is not bigger than 5MB. I.e. the algorithms are easy: # and conjuntion foreach condition in conditions: if not condition.isMet(file): return false return true # or conjuction foreach condition in conditions: if condition.isMet(file): return true return false But I don't know if that really fits into the log4net is a clone of log4j philosophy. :-) No problem here as we are not touching the architecture only one of the appenders - and a lot of variance is allowed here. I realize you haven't been serious. Not too serious, but most ideas start off being hilarious. *laughing* Anyway, for now it's just an idea. Please discuss. :-)
RE: New RollingFileAppender semantics
What you are suggesting is completely different from the current
semantics.
Until this email, I understood your example to be equivalent to a
composite roll in the current implementation. In a composite roll, two
different types of rolling take place. One on the date/time boundary
and one on the size boundary. Resulting in
File.datetime1.1
File.datetime1.2
File.datetime2.1
File.datetime2.2
I think that preserving this type of roll is important. Currently we
have Date, Size and Composite rolling. If I have understood what you
are suggesting, you are suggesting the addition of a 4th roll type
called ConditionRollingXX that would have the semantic of AND or OR of
the two (hopefully only two) conditions. XX would be the AND or OR for
the type of condition. If there are more than two conditions, what are
they?
Maybe something like Date, Size, ApplicationUptime, RPCTrigger, .. ?
Did I understand correctly and is this needed?
I believe that I have to explain this further. My idea is to split the
rolling into two parts:
* rolling condition
* file naming strategy
This should ideally deprecate the different rolling strategies and their
hideous behaviour by replacing it with a clear API and a configuration
semantic. This here:
rollFileConfiguration
rollFileCondition when=daily /
rollFileNamingStrategy value=Date /
/rollFileConfiguration
or this here (which is semantically the same, and would be just another
implementation of the condition interface):
rollFileConfiguration
rollFileCondition month=* day=* hour=24 minute=* /
rollFileNamingStrategy value=Date /
/rollFileConfiguration
Is much clearer than the old-fashioned thing:
appender name=RollingLogFileAppender
type=log4net.Appender.RollingFileAppender
file value=logfile /
appendToFile value=true /
rollingStyle value=Date /
datePattern value=MMdd /
layout type=log4net.Layout.PatternLayout
conversionPattern value=%date [%thread] %-5level %logger
[%property{NDC}] - %message%newline /
/layout
/appender
RE: New RollingFileAppender semantics
Maybe something like Date, Size, ApplicationUptime, RPCTrigger,
.. ?
RPCTrigger??
What you are really saying is that the new code needs to have enough
extension points that others could easily add new roll conditions. My
concern is in how to provide that functionality and get renaming and
deleting right.
I believe that I have to explain this further. My idea is to split the
rolling
into two parts:
* rolling condition
* file naming strategy
+1
This should ideally deprecate the different rolling strategies and
their hideous
behaviour by replacing it with a clear API and a configuration
semantic. This
here:
+1 on the goal
rollFileConfiguration
rollFileCondition when=daily /
rollFileNamingStrategy value=Date / /rollFileConfiguration
or this here (which is semantically the same, and would be just another
implementation of the condition interface):
rollFileConfiguration
rollFileCondition month=* day=* hour=24 minute=* /
rollFileNamingStrategy value=Date / /rollFileConfiguration
+1 or even +2 on the time stamp condition.
I like the time stamp condition idea, but does that mean at hour 24 or
24 hours from starting time? I think it means hour 24, so what symbol
could we add to make it relative to starting time. Maybe a + so that
+24 means 24 hours from when I started. Would a relative to start
time take away your requirement for a ApplicationUptime roll condition?
Questions about rollFileNamingStrategy.
Date makes sense, I assume that we would include the datePattern
somewhere as rollFileNameByDatePattern = yyMMdd/ etc.
How do we incorporate that with a size rollover to achieve names similar
to what are produced now (or should be produced now) as
File.datetime1.1
File.datetime1.2
File.datetime2.1
File.datetime2.2
It seems to me that we would need to pair a naming strategy with each
roll strategy and indicate where that part of the name goes into the
final file name.
This brings me to File= parameter that has {Date} and {Size} etc. to
indicate where date and size patterns are to be inserted. (Likewise for
any other rolling condition.) If this idea is acceptable, then we do not
have to worry about the date pattern separator or preserve extension
etc. because it is all handled by the file pattern. The biggest
downside that I see at this time is scanning the File= parameter to
determine what goes where to create the directory search, but it should
be doable without too much unfixable code.
Is much clearer than the old-fashioned thing:
Agreed
--
Roy Chastain
Re: New RollingFileAppender semantics
On 09/20/2011 05:06 PM, Roy Chastain wrote: I have already written code that could parse the old datepattern and determine where ? needed to go to do the file directory search. That code or a close relative can survive into the new model so, the directory search is still just a single pass with a mask. I even fixed that mask to take into account the size roll increment number also, which is why I wanted to make fixed width. I really think we need to provide for multiple name parts as proposed above. I think I should be able to keep the performance acceptable. Should I try to write down the API? It should be doable in a couple of hours and right now it looks like I have some free time tomorrow around 20°°-22°°. If so, is there somewhere a sandbox that I could use? -- Dominik Psenner ## OpenPGP Key Signature # # Key ID: B469318C # # Fingerprint: 558641995F7EC2D251354C3A49C7E3D1B469318C # ## signature.asc Description: OpenPGP digital signature
RE: New RollingFileAppender semantics
Should it be modifiable? I'm unsure. It would be easier to code if it were always 0, but there is a nicety to having it negative when only a few iterations of the file are being kept. That said, I would have no problem removing it as a configuration parameter. +1, but one should be able to change the spacer character. . is a nuisance to regex parse a logfile, where - is tangentially easier. Create a new DatePatternSeperator parameter that could be one or more characters. It could default to . or - (as we see fit). It would simply be placed to the left of resulting date before the resulting date is injected into the file name. rollFilePattern value=## / !-- see string.Format() -- The idea behind the fixed width field was to make finding the rolled files easier. Going to a ## type format would defeat that purpose. Allowing the user to specify a format would give them great flexibility, but would create yet more cases when rolling would fail simply because of the difficulty in determining the resultant file names. I am not sure that I can support a user defined format for a code point of view. What does it anyway? Enlighten me, please! :-) If true for size based rolling the current file is always the value of the File parameter and then the file is renamed to File.xx or File.xx.ext as needed. Otherwise the current file has its final name if countdirection is positive. For a date based rolling the file is renamed with its date portion appended and that is why the sample configuration from the bug report fails. The current code does not and has no reasonable way to detect the 'file name' in the DatePattern parameter. While having the fixed file name is great for humans because they can easily find it in the file list, it is terrible for performance and programs that might want to process the file list. I do not have an opinion one way or the other on your proposal for the grouped roll configuration. Please explain your reasoning for your preference so that I might understand better. -- Roy Chastain
Re: New RollingFileAppender semantics
On 2011-09-18, Roy Chastain wrote: After having spent two weekends looking at and playing with the code, I have decided that I do not have clear understanding of my target. Poor you. Given that it appears that I am going to break the internal contract for RFA and the ambiguity in the current implementation it appears that we should create a new RFA. (Perhaps called RollingFileAppender2??) with a clear definition of its semantics. +1 - I don't like the 2 suffix so we may want to search for a better name. As such I would like to propose 1) - Make CountDirection default to positive 2) - Make PreserveLogFileNameExtension default to true 3) - Any date portion in a file name be prefixed with a . as if it were an extension. +1 3) - Decide if PreserveLogFileNameExtension applies to both the datePattern and the size roll index or just to the size roll index. I propose that it applies to size roll index and a new parameter (PreserveLogFileBase - defaults to true) be used to apply to the date roll. [Oh, another item 3. Probably used RFA to number them ;-)] I like your proposal. 4) snip/ In other words, only allow date editing within the datePattern. The following config would achieve almost same file names. file value=./output/LoggerTest.log/ datePattern value=MMddHHmm/ PreserveLogFileBase value=true/ Generated file name would look like LogTest.201109181715.log If roll type become composite, then the files would look like LogTest.201109181715.1.log, LogTest.201109181715.2.log +1 for everything that makes things more predictable. 5) - I propose that in the case of a maximum fixed size of roll backups, that the roll index become a fixed width field with leading zeros. 6) - Include a new MaxDateRollBackups parameter to limit the number of date periods that will be +1 7) - Personally I would feel no loss if StaticFileName went away and was always treated as false, but I expect that many people would want to keep it. I know the Ops folks of one of my team's bigger deployments would want it. Stefan
New RollingFileAppender semantics
The current implementation of RFA allows configurations that are not meaningful and have non-intuitive semantics. Some of the issues come from the fact that the File parameter does not have to have a file portion when there is a datePattern. Example config from a bug report file value=./output// datePattern value='LoggerTest'MMddHHmm'.log'/ This configuration actually works in an intuitive manner if staticLogFileName is false. If staticLogFileName is true, it fails miserably and does not log anything due to file creation issues. Given the same configuration with composite rolling, where would you expect the roll index to go with and without PreserveLogFileNameExtension? Current code generates the same files with and without PreserveLogFileNameExtension. It generates LoggerTest201109181715.log LoggerTest201109181715.log.1 etc. To me that is not right. I believe that the .log in the datePattern should be treated as an extension and, if PreserveLogFileNameExtension = true, it should generate something like LoggerTest201109181715.1.log, LoggerTest201109181715.2.log. I do not believe that the current ignoring of PreserveLogFileNameExtension is intentional. It is just a case that was not considered. After having spent two weekends looking at and playing with the code, I have decided that I do not have clear understanding of my target. Given that it appears that I am going to break the internal contract for RFA and the ambiguity in the current implementation it appears that we should create a new RFA. (Perhaps called RollingFileAppender2??) with a clear definition of its semantics. As such I would like to propose 1) - Make CountDirection default to positive 2) - Make PreserveLogFileNameExtension default to true 3) - Any date portion in a file name be prefixed with a . as if it were an extension. 3) - Decide if PreserveLogFileNameExtension applies to both the datePattern and the size roll index or just to the size roll index. I propose that it applies to size roll index and a new parameter (PreserveLogFileBase - defaults to true) be used to apply to the date roll. 4) - Remove the ability to do the sample above. In other words, only allow date editing within the datePattern. The following config would achieve almost same file names. file value=./output/LoggerTest.log/ datePattern value=MMddHHmm/ PreserveLogFileBase value=true/ Generated file name would look like LogTest.201109181715.log If roll type become composite, then the files would look like LogTest.201109181715.1.log, LogTest.201109181715.2.log 5) - I propose that in the case of a maximum fixed size of roll backups, that the roll index become a fixed width field with leading zeros. Doing so will greatly simplify the deleting during rolling and make the process more robust because the pattern will be specific. Currently the code sort of guesses by employing too many string.StartsWith() calls. Example, if MaxSizeRollBackups is set to 10, then the file names above would be LogTest.201109181715.01.log, LogTest.201109181715.02.log 6) - Include a new MaxDateRollBackups parameter to limit the number of date periods that will be maintained. Doing so is (I believe) currently unfeasible, if not impossible, given the ability to include a file name in the base pattern and not have a file name in the File parameter. 7) - Personally I would feel no loss if StaticFileName went away and was always treated as false, but I expect that many people would want to keep it. -- Roy Chastain
