ELY-1206: Added periodic-rotating and size-rotating audit endpoints #845
Conversation
|
Can one of the admins verify this patch? |
| * The suffix must be a string understood by the {@link java.time.format.DateTimeFormatter}. | ||
| * <p/> | ||
| * <b>Note:</b> Any files rotated with the suffix appended will not be deleted. The {@link #setMaxBackupIndex(int) | ||
| * maxBackupIndex} is not used for files with a suffix. |
There was a problem hiding this comment.
We should fix this documentation as it's not quite correct. Files with the same date suffix will be deleted.
| @Override | ||
| protected void write(byte[] bytes) throws IOException { | ||
| super.write(bytes); | ||
| } |
There was a problem hiding this comment.
Not really a big deal, but this could probably just be removed since it's only a super call.
| return; | ||
| } | ||
| closeStreams(); // close the original file (some OSes won't let you move/rename a file that is open) | ||
| final Path target = Paths.get(file.getAbsolutePath() + nextSuffix); |
There was a problem hiding this comment.
Is this safe on Windows? Maybe it would be safer to instead say something like (from memory):
final Path target = file.toPath().resolve(file.getName() + nextSuffix);
There was a problem hiding this comment.
+1 that's a good point and the above example should work.
There was a problem hiding this comment.
@jamezp I have added this modification. Just a minor note, in fact, nextSuffix here can be any pattern if it pass for a DateTimeFormatter, so to avoid possible problems defining suffixes on different operating systems, some characters in the suffix are always replaced by '_'
| * @throws IllegalArgumentException if the suffix is not valid | ||
| */ | ||
| public Builder setSuffix(String suffix) throws IllegalArgumentException { | ||
| format = DateTimeFormatter.ofPattern(suffix.replaceAll("[\\/:*?\"<>|]", "_")).withZone(timeZone); |
There was a problem hiding this comment.
@dmlloyd What do you think about this? I kind of think we should allow anything as different OS's allow for different characters in their paths.
There was a problem hiding this comment.
+1 This does seem a little over restrictive.
Also at some point this kind of check may be better handled in the management model, i.e. in the model we can either perform MODEL stage validation or correct values.
|
@darranl I have removed the file name restrictions when suffix is used |
|
@yersan Did you considered need of combined rollover? Dividing logs by individual days, but with automatic dividing of day's log file to more parts when it is too big in given day? |
|
@honza889 It was discussed with @jamezp and @bstansberry, final decision was not to include it here at the moment because this is just for audit and syslog files and there were complaints with that handler because it don't purge rotated files |
|
Since we're not using the |
|
@darranl It makes sense for me, |
|
Thanks, once that is in I will coordinate the two together. |
|
@jamezp @darranl , I have applied the changes to get rid of Java 7 date/time API not only in |
|
Looks good to me. 👍 |
Added two different rotating audit endpoints and removed the old one.
Jira issues:
https://issues.jboss.org/browse/ELY-1206
https://issues.jboss.org/browse/JBEAP-10700
depends on wildfly/wildfly-core#2425