Skip to content

Add org.apache.logging.log4j.core.appender.FileManager.getPath() #3855

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Open

Add org.apache.logging.log4j.core.appender.FileManager.getPath() #3855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,19 +246,20 @@ public static FileManager getFileManager(
protected OutputStream createOutputStream() throws IOException {
final String filename = getFileName();
LOGGER.debug("Now writing to {} at {}", filename, new Date());
final File file = new File(filename);
final Path path = getPath();
final File file = path.toFile();
createParentDir(file);
final FileOutputStream fos = new FileOutputStream(file, isAppend);
if (file.exists() && file.length() == 0) {
try {
final FileTime now = FileTime.fromMillis(System.currentTimeMillis());
Files.setAttribute(file.toPath(), "creationTime", now);
Files.setAttribute(path, "creationTime", now);
} catch (Exception ex) {
LOGGER.warn("Unable to set current file time for {}", filename);
}
writeHeader(fos);
}
defineAttributeView(Paths.get(filename));
defineAttributeView(path);
return fos;
}

Expand Down Expand Up @@ -343,6 +344,16 @@ protected synchronized void writeToDestination(final byte[] bytes, final int off
public String getFileName() {
return getName();
}

/**
* Returns the Path of the file being managed.
* @return The name of the file being managed.
* @since 2.26.0
*/
public Path getPath() {
Copy link
Contributor

@ppkarwasz ppkarwasz Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small naming suggestion: since we already have a getFileName() method, it might be clearer to name this new method getFilePath() instead of getPath().

It would also help to document the expected characteristics of the returned Path, for example:

  • Whether the returned path is absolute and normalized. Since multiple appenders (even across different logger contexts) may share a single FileManager, using an absolute, normalized Path avoids ambiguity and makes comparisons safer.
  • Whether the value may change over time. In particular, RollingFileManager can update its active file during rollover (especially with the DirectWriteRolloverStrategy), so it would be useful to document that the returned path may not be stable across invocations.

Copy link
Member Author

@garydgregory garydgregory Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small naming suggestion: since we already have a getFileName() method, it might be clearer to name this new method getFilePath() instead of getPath().

Names are important and I'm glad we're discussing them.

IMO, "getFilePath()" is bad because it implies this is a Path to a file:// only, and since a Path is an interface for locating a file in a file system, we should not set expectations that the implementation of the Path is only for file-schemed Paths.

It doesn't matter that there's already an API called "getFileName()", a file name is very different from a Path, where the expectation (from a NIO Path POV) is that the file name is the last segment, the "bar.txt" in "/foo/bar.txt". Having a common prefix between these two methods has no benefit.

Copy link
Contributor

@ppkarwasz ppkarwasz Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was interpreting filePath as “path of the file” as opposed to the path to its parent directory or similar.

I agree that “path with file: scheme” would be the wrong interpretation, even if currently that is the only scheme we support.

return Paths.get(getFileName());
}

/**
* Returns the append status.
* @return true if the file will be appended to, false if it is overwritten.
Expand Down
7 changes: 7 additions & 0 deletions src/changelog/.2.x.x/add_FileManager_getPath_.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://logging.apache.org/xml/ns"
xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
type="added">
<description format="asciidoc">Add org.apache.logging.log4j.core.appender.FileManager.getPath().</description>
</entry>
Loading