logback-android icon indicating copy to clipboard operation
logback-android copied to clipboard

SizeBasedTriggeringPolicy fails for relative file paths

Open fberlakovich opened this issue 8 years ago • 1 comments

I wrote a custom RollingPolicy based on the FixedWindowRollingPolicy for my app and combined it with the SizeBasedTriggeringPolicy. During testing I noted that the rollover never occurred. However, the base logfile (client.log) was created and filled. Further investigation of the problem revealed that the FixedWindowRollingPolicy seems to be affected by #30. As soon as I override the getActiveFIleName method in my RollingPolicy like so

    /**
     * Gets the absolute path to the filename, starting from the app's
     * "files" directory
     *
     * @param filename filename to evaluate
     * @return absolute path to the filename
     */
    private String getAbsoluteFilePath(String filename) {
        // In Android, relative paths created with File() are relative
        // to root, so fix it by prefixing the path to the app's "files"
        // directory.
        String dataDir = context.getProperty(CoreConstants.DATA_DIR_KEY);
        return FileUtil.prefixRelativePath(dataDir, filename);
    }

    @Override
    public String getActiveFileName() {
        return getAbsoluteFilePath(super.getActiveFileName());
    }

everything works as expected. I can provide a patch, but I don't know where to do the fixing. Should the RollingFileAppender fix the path received from the RollingPolicy or should the path returned by the RollingPolicy be already correct?

For reference here is the appender configuration I used:

    <appender name="MyCustomAppender" class="ch.qos.logback.core.rolling.RollingFileAppender">
        <!-- the current logfile name -->
        <file>client.log</file>

        <rollingPolicy class="MyCustomRollingPolicy">
            <fileNamePattern>client.%i.log</fileNamePattern>
        </rollingPolicy>

        <triggeringPolicy class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
            <!-- small logfile size used for testing -->
            <maxFileSize>10</maxFileSize>
        </triggeringPolicy>

        <encoder>
            <pattern>%-4relative [%thread] %-5level %logger{35} - %msg%n</pattern>
        </encoder>

    </appender>

fberlakovich avatar Nov 30 '16 10:11 fberlakovich

To be consistent with FileAppender, which changes the relative path to absolute, I think the path fix should be in RollingFileAppender (instead of the RollingPolicy).

Note the wiki currently indicates the absolute file paths should be specified for both FileAppender and RollingFileAppender.

tony19 avatar Mar 06 '18 04:03 tony19