Logging - set default value for quarkus.log.file.rotation.max-file-size

[mkouba]

• also change the default value for
  quarkus.log.file.rotation.max-backup-index

The current behavior is a bit misleading. The docs say that quarkus.log.file.rotation.max-file-size has the default value 10 which is not correct, because no default value is set and so no rotation happens (and the log file is growing and growing..) unless you set the quarkus.log.file.rotation.file-suffix, but then the default value from the org.jboss.logmanager.handlers.PeriodicSizeRotatingFileHandler is taken which is 0xa0000, i.e. approx. 650KB (it's actually a bug, because the comment is clear that it should be 0xa00000). Therefore, we set the default max-file-size to 10M, so the file size is always limited.

We also change the default value for max-backup-index from 1 to 5 which is IMO a more reasonable default.

[mkouba]

CC @dmlloyd

[machi1990]

This supersedes #24267, correct?
/cc @gsmet

[mkouba]

This supersedes #24267, correct? /cc @gsmet

I didn't know about this one but I think that the change in the LoggingSetupRecorder is needed as well.

[geoand]

The test failures are related to the change

[mkouba]

The test is fixed.

[mkouba]

I wonder if we should consider this a breaking change in the sense that we drop the support of org.jboss.logmanager.handlers.FileHandler which just appends messages to a file (i.e. limits are filesystem-specific). Such a handler IMO does not make a lot of sense in practice.

[mkouba]

@michalszynkiewicz is it (#25072 (comment)) a known flaky test?

[gsmet]

Please let me think a bit about it before merging. I’ll have a look on Monday.

[michalszynkiewicz]

@michalszynkiewicz is it (#25072 (comment)) a known flaky test?

Not known to me

[gsmet]

So my main problem with this is:

• what happens when you want to set fileSuffix and not rotate on size?
• in this case, you usually want one log per day and keep 5 days for instance

My guess is that with this patch, it won't work anymore and things will get broken (and you can't even fix it given the value for the file size is not optional anymore).

[mkouba]

So my main problem with this is:

* what happens when you want to set `fileSuffix` and not rotate on size?

* in this case, you usually want one log per day and keep 5 days for instance

My guess is that with this patch, it won't work anymore and things will get broken (and you can't even fix it given the value for the file size is not optional anymore).

In theory, you can set the max-file-size to a value that is not likely to be reached, e.g. 2T. We could switch back to Optional but in that case you'd have to supply the empty config value anyway, i.e. something like quarkus.log.file.rotation.max-file-size=.

However, this would not work even now - see my comment: if you set file-suffix and not the max-file-size then the default value (~ 650KB) is used (note that rotate-on-boot is true by default).

In any case, I think that the current behavior is wrong and the unlimited file handler is not a reasonable default.

CC @gsmet

[dmlloyd]

I think I have a config type somewhere which is a type-or-enum which would allow a size to be specified or a keyword like "unlimited". I would personally prefer this over special values.