Created attachment 28344 [details] Diff of AccessLogValve patch The current AccessLogValve implementation uses a custom file writer instead of the normal log mechanism. There are circumstances where it would be nice to have the power and flexibility of the standard logging capabilities. This was briefly discussed in a email thread with subject 'AccessLogValve enhancement' from 2.15.2012 Attached is a proposed patch to add this capability, however I think it is still up for debate if this is a good enhancement. This would be an option that people wouldn't have to use. I believe it will work for me and is hopefully generally useful (otherwise it doesn't make a good enhancement and is just clutter.) Earlier, I was trying to see how to do this by searching on the web and I found several others asking the same thing. So, I think there are many others who at least think they want this feature. Issues: * (Potential performance, as mentioned previously) * "common" pattern for AccessLogValve includes timestamp, which is also often included in log record headers. * header messages (#VERSION, #FIELDS:, #SOFTWARE) won't repeat in log files, and are not necessarily useful in the initial one. (#FIELD is null, presumably because its attribute was later in my server.xml file.)
Created attachment 28345 [details] Diff of ExtendedAccessLogValve
You can always create your own class that inherits from AccessLogValve() and overrides - open() - close() - log(String) I think it could be possible to refactor AccessLogValve to extract output file handling into helper class. Redirection to a logger might be an alternative implementation of such helper. I see the following weaknesses of proposed patch: 1. It does not prevent creation of access log files by the valve. It just prints nothing to them, but the files will be created as usual. 2. Those "Fields/Version/Software" headers are specific to ExtendedAccessLogValve, because they are part of "W3c Extended Log File Format" format that it implements. They are not needed in AccessLogValve. If you want to add them there, it would be a separate feature. For reference - see discussion on dev@ regarding viability of this approach, "AccessLogValve enhancement" - http://marc.info/?t=132934123800005&r=1&w=2 - http://tomcat.markmail.org/thread/iz67c2puilxv4leg Note that using this feature requires specific logging configuration.
Created attachment 28515 [details] Diff of changed needed to delete older files
Created attachment 28516 [details] One error message was added for when a file could not be deleted
Created attachment 28517 [details] Two new parameters to support configuration of old file deletion
What I really want to be able to do is delete older access log files. I was thinking that using a standard logger in the AccessLogValve was the way to do that. However, this idea received some resistance because of the potential performance problem. (Note: In tests that I could mock up here, the change was not really noticeable... the average time was slightly less than 1 millis using the current mechanism and slightly more than 1 millis using a standard logger. I did not have a good way to test what would happen under a heavy load...which is the only case in which the performance would really matter.) However, there are some other things that the AccessLogValve does that I could not account for. For instance, the ExtendedAccessLogValve puts a header at the top of every file; something it can do since it controls the creation of the files. The header contains the format for the entries, presumably for automated parsing. Since I did not have easy access to good testing facilities, and I thought that deleting old files would be a generally useful feature, I did a smaller patch that targets the problem a bit better. I felt this would encounter less resistance and would hopefully be adopted sooner and with less risk. The original attachments are no longer in my purposed solution. The new solution simply adds the capability to delete old files to the existing logging mechanism. The feature is off by default so that the behavior will be backwards compatible. One error message was added to .properties file, although I did not translate it to the other supported languages. I believe I added the parameters appropriately to the mbeans descriptor file. Once thing thing that I was a bit unsure about was that the files are sorted by their last modified times to determine which files are to be deleted. This seemed like a good way to do it, although other ways exist (like parsing the date extension of each file and sorting by Dates).
Another thing I realized (that I don't want to forget) is that there are 2 predefined log output formats: common and combined. These have date/time stamps defined in them. The standard logging mechanisms I have seen have a date/time as well. I am not sure if this can be turned off in the standard loggers. However, if it can't (or maybe you don't want to), then the predefined loggers cannot be used without outputting redundant information. This is, perhaps, not really important, but it is something that needs to be at least addressed in the docs if not handled in another way.
I also realized that the default tomcat JULI FileHandler and AsyncFileHandler do not support deleting old files. So, I would either need to change these as well, or need to configure my installation of tomcat to use something else (like log4j). These things are not difficult, but it started to seem like I was on a pretty circuitous route to get the small feature I wanted.
Since this was opened, the maxDays attribute was added to the JULI FileHandler. I have therefore added a similar attribute to the AccessLogValve. Fixed in: - trunk for 9.0.7 onwards - 8.5.x for 8.5.30 onwards - 7.0.x for 7.0.86 onwards Since this is a new feature and 8.0.x is close to end of life, I haven't implemented it for 8.0.x.