[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 Violeta Georgieva changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #14 from Violeta Georgieva --- Hi, The new functionality was back ported to: - 8.5.x for 8.5.16 onwards - 8.0.x for 8.0.45 onwards - 7.0.x for 7.0.79 onwards The default will be: keep the log files forever. Regards, Violeta -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 --- Comment #13 from Matt Farwell --- It would great to see this functionality backported. Bringing it into Tomcat 8.5 and set it as disabled by default makes sense to me. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 --- Comment #12 from Mark Thomas --- Happy to see it back-ported. I'd disable by default though so there is no change of behaviour in stable versions. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 --- Comment #11 from Violeta Georgieva --- The change is applied and will be available in 9.0.0.M22 onwards. The log files will be kept by default 90 days. Do we want this change in the previous versions? May be with different default value? Thanks, Violeta -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105
--- Comment #10 from Violeta Georgieva ---
(In reply to Konstantin Kolinko from comment #9)
> (In reply to Violeta Georgieva from comment #8)
> > Any comments?
>
> Generally: I like it.
>
> 1. Typo in method name: obtainDateFormPath s/Form/From/
Fixed
> 2. Building a pattern,
>
> > pattern = Pattern.compile("^(" + prefix + ")\\d{4}-\\d{1,2}-\\d{1,2}(" +
> > suffix + ")$");
>
> This should use (Pattern.quote(prefix) + "..." + Pattern.quote(suffix))
>
> Prefix and suffix can contain special characters, e.g. '.' = any character.
> Wrapping them with Pattern.quote() solves this issue.
>
Fixed, added a test also
> 3. Temporary directory handling in unit test
>
> There is a base Test class that provides support for temporary directories,
>
> https://github.com/apache/tomcat/blob/trunk/test/org/apache/catalina/startup/
> LoggingBaseTest.java
>
> Differences:
> - It respects system property "tomcat.test.temp"
> - It uses creates a random directory for the test, to allow running several
> tests in parallel
> tempDir = Files.createTempDirectory(tempBasePath, "test").toFile();
>
> Maybe it is not a good idea to use LoggingBaseTest directly as a base class,
> as it initializes logging and this test tests logging, but it can be used to
> copy some code.
Fixed
Thanks for the review,
Violeta
--
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105
--- Comment #9 from Konstantin Kolinko ---
(In reply to Violeta Georgieva from comment #8)
> Any comments?
Generally: I like it.
1. Typo in method name: obtainDateFormPath s/Form/From/
2. Building a pattern,
> pattern = Pattern.compile("^(" + prefix + ")\\d{4}-\\d{1,2}-\\d{1,2}(" +
> suffix + ")$");
This should use (Pattern.quote(prefix) + "..." + Pattern.quote(suffix))
Prefix and suffix can contain special characters, e.g. '.' = any character.
Wrapping them with Pattern.quote() solves this issue.
3. Temporary directory handling in unit test
There is a base Test class that provides support for temporary directories,
https://github.com/apache/tomcat/blob/trunk/test/org/apache/catalina/startup/LoggingBaseTest.java
Differences:
- It respects system property "tomcat.test.temp"
- It uses creates a random directory for the test, to allow running several
tests in parallel
tempDir = Files.createTempDirectory(tempBasePath, "test").toFile();
Maybe it is not a good idea to use LoggingBaseTest directly as a base class,
as it initializes logging and this test tests logging, but it can be used to
copy some code.
--
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 --- Comment #8 from Violeta Georgieva --- Any comments? -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105
--- Comment #7 from Violeta Georgieva ---
Hi,
(In reply to Konstantin Kolinko from comment #5)
> (In reply to Violeta Georgieva from comment #3)
> > Hi,
> >
> > What do you think about this approach?
> > https://github.com/apache/tomcat/pull/60
> >
>
> +public static final int DEFAULT_MAX_DAYS = 90;
> +private int maxDays = DEFAULT_MAX_DAYS;
>
> I do not like the idea of built-in default limit in java code.
>
> I am open to discuss whether it is feasible for Tomcat 9,
> but such built-in limit cannot be backported to stable versions (8.5 and
> earlier).
>
> I think it is better to add limits explicitly to the default
> logging.properties configuration.
>
>
> +String sMaxDays = getProperty(className + ".maxDays",
> String.valueOf(DEFAULT_MAX_DAYS));
> +if (maxDays <= 0) {
> +try {
> +maxDays = Integer.parseInt(sMaxDays);
> +} catch (NumberFormatException ignore) {
> +// no-op
> +}
> +}
>
> I think the above try/catch block is never executed, as "if (maxDays <= 0)"
> is always false, as maxDays is "90" by default.
>
> +private DirectoryStream streamFilesForDelete() throws IOException
> {
> +FileTime maxDaysOffset = FileTime.from(Instant.now().minus(maxDays,
> ChronoUnit.DAYS));
> +return Files.newDirectoryStream(new File(directory).toPath(), path
> -> {
> +String fileName = path.getFileName().toString();
> +return fileName.startsWith(prefix) && fileName.endsWith(suffix)
> +&&
> Files.getLastModifiedTime(path).compareTo(maxDaysOffset) < 0;
> +});
> +}
>
> I do not like the above.
>
> 1. "fileName.startsWith(prefix)" will result in false positives.
>
> 2. I'd prefer to test the date in the file name, not file modification date.
>
>
> BTW, for access logs I usually have an empty prefix, grouping the files into
> separate directories by month:
> fileDateFormat="-MM'/webappname.'-MM-dd"
> prefix=""
> suffix=".access.log"
>
> Such feature is not implemented for JULI logging yet. If it were, the
> "fileName.startsWith(prefix)" here would be true for every file.
Thanks for the review. I prepared a new patch where I applied all your
recommendations. Can you take a look at it?
Violeta
--
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 --- Comment #6 from Huxing Zhang --- Hi, > There was no concern expressed about the log files that are currently not > rolled (generally, I suspect, because well written apps won;t trigger content > to those files). We do have concerns about rotate the output to stdout/stderr. In most of our cases, this is due to logging framework conflict between log4j and logback in a web application. The default behavior is that all the logging content are eventually gone to catalina.out. Most of the users even won't be aware of it, until being alerted by running out of the disk space (The web application may run for months). To avoid this, we actually have implemented a feature in Tomcat to rotate catalina.out on a daily basis. Under the hood we use a customized PrintStream to replace System.out/System.err, capture the content, and output to JULI. Since it is rotated by day, it make us easier to keep the latest N files. I know the best solution will be solving the conflict, but according to our experience, most of the user don't know there is a conflict. In there any interest in adding this feature to Tomcat? -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [Bug 61105] Roll log files by default
Hi, > There was no concern expressed about the log files that are currently not > rolled (generally, I suspect, because well written apps won;t trigger content > to those files). We do have concerns about rotate the output to stdout/stderr. In most of our cases, this is due to logging framework conflict between log4j and logback in a web application. The default behavior is that all the logging content are eventually gone to catalina.out. Most of the users even won't be aware of it, until being alerted by running out of the disk space (The web application may run for months). To avoid this, we actually have implemented a feature in Tomcat to rotate catalina.out on a daily basis. Under the hood we use a customized PrintStream to replace System.out/System.err, capture the content, and output to JULI. Since it is rotated by day, it make us easier to keep the latest N files. I know the best solution will be solving the conflict, but according to our experience, most of the user don't know there is a conflict. In there any interest in adding this feature to Tomcat? -- From:bugzilla Time:2017 Jun 6 (Tue) 03:45 To:dev Subject:[Bug 61105] Roll log files by default https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 --- Comment #2 from Mark Thomas --- The conversation at TomcatCon was around putting a (relatively large) limit on the number of files that are kept by default. Picking a number of of thin air, how does 90 days sound? There was no concern expressed about the log files that are currently not rolled (generally, I suspect, because well written apps won;t trigger content to those files). -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105
--- Comment #5 from Konstantin Kolinko ---
(In reply to Violeta Georgieva from comment #3)
> Hi,
>
> What do you think about this approach?
> https://github.com/apache/tomcat/pull/60
>
+public static final int DEFAULT_MAX_DAYS = 90;
+private int maxDays = DEFAULT_MAX_DAYS;
I do not like the idea of built-in default limit in java code.
I am open to discuss whether it is feasible for Tomcat 9,
but such built-in limit cannot be backported to stable versions (8.5 and
earlier).
I think it is better to add limits explicitly to the default logging.properties
configuration.
+String sMaxDays = getProperty(className + ".maxDays",
String.valueOf(DEFAULT_MAX_DAYS));
+if (maxDays <= 0) {
+try {
+maxDays = Integer.parseInt(sMaxDays);
+} catch (NumberFormatException ignore) {
+// no-op
+}
+}
I think the above try/catch block is never executed, as "if (maxDays <= 0)" is
always false, as maxDays is "90" by default.
+private DirectoryStream streamFilesForDelete() throws IOException {
+FileTime maxDaysOffset = FileTime.from(Instant.now().minus(maxDays,
ChronoUnit.DAYS));
+return Files.newDirectoryStream(new File(directory).toPath(), path ->
{
+String fileName = path.getFileName().toString();
+return fileName.startsWith(prefix) && fileName.endsWith(suffix)
+&&
Files.getLastModifiedTime(path).compareTo(maxDaysOffset) < 0;
+});
+}
I do not like the above.
1. "fileName.startsWith(prefix)" will result in false positives.
2. I'd prefer to test the date in the file name, not file modification date.
BTW, for access logs I usually have an empty prefix, grouping the files into
separate directories by month:
fileDateFormat="-MM'/webappname.'-MM-dd"
prefix=""
suffix=".access.log"
Such feature is not implemented for JULI logging yet. If it were, the
"fileName.startsWith(prefix)" here would be true for every file.
--
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 --- Comment #4 from Violeta Georgieva --- Hi, In the PR [1] there is a proposal for merging the proposed functionality with https://github.com/apache/tomee/blob/master/tomee/tomee-juli/src/main/java/org/apache/tomee/jul/handler/rotating/LocalFileHandler.java I checked the TomEE's LocalFileHandler and it provides many useful features. If there is a demand I can port it to Tomcat. Regards, Violeta [1] https://github.com/apache/tomcat/pull/60 -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 --- Comment #3 from Violeta Georgieva --- Hi, What do you think about this approach? https://github.com/apache/tomcat/pull/60 Thanks, Violeta -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 --- Comment #2 from Mark Thomas --- The conversation at TomcatCon was around putting a (relatively large) limit on the number of files that are kept by default. Picking a number of of thin air, how does 90 days sound? There was no concern expressed about the log files that are currently not rolled (generally, I suspect, because well written apps won;t trigger content to those files). -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 61105] Roll log files by default
https://bz.apache.org/bugzilla/show_bug.cgi?id=61105 --- Comment #1 from Violeta Georgieva --- Hi, Currently the log files (except catalina.out which is available on OSes != Windows) are rotated by default based on the date. We can introduce in addition a configuration for the file size and also how many files to keep. Do we want to archive files when rotate them or just a simple renaming is enough? What about catalina.out file? Do we want a rotation for this file also? There is already enhancement about something similar https://bz.apache.org/bugzilla/show_bug.cgi?id=53930 What about the log files created by the Tomcat service on Windows? Do we want to have something similar for them also? Thanks, Violeta -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
