Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SizeBasedTriggeringPolicy fails for relative file paths #135

Open
fberlakovich opened this issue Nov 30, 2016 · 1 comment
Open

SizeBasedTriggeringPolicy fails for relative file paths #135

fberlakovich opened this issue Nov 30, 2016 · 1 comment

Comments

@fberlakovich
Copy link

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 fberlakovich changed the title SIzeBasedTriggeringPolicy fails for relative file paths SizeBasedTriggeringPolicy fails for relative file paths Nov 30, 2016
@tony19
Copy link
Owner

tony19 commented Mar 6, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants