Java Logging with a file per root package

Question

I have a set of classes across multiple packages, and I want any logging from within a single "com.name.root.xxx" package and all its child packages, to be logged to a different file. e.g:

//Package Name                    Log To     \\
//-------------------------------------------\\
com.name.root.router         C:\com\router.log
com.name.root.router.utils   C:\com\router.log
com.name.root.init           C:\com\init.log
com.name.root.database       C:\com\database.log
com.name.root.web            C:\com\web.log
com.name.root.web.rest       C:\com\web.log
com.name.root.web.http       C:\com\web.log

etc.

I have created a helper class which tracks which log file paths have been set up with file handlers already, and also which packages have already had their loggers set up; and provides accordingly.

I am interested in review for efficiency, and if there's a better (simpler/cleaner/more understandable) way to do this:

package com.name.root.util.log;

import java.io.IOException;
import java.util.HashMap;
import java.util.HashSet;
import java.util.logging.FileHandler;
import java.util.logging.Level;
import java.util.logging.Logger;

public class LogProvider
{
    private static final String rootPackageName = "com.name.root";
    private static final int rootPackageNameLength = rootPackageName.length();
    private static final HashMap<String,FileHandler> fileHandlersByPath = new HashMap<>();
    private static final HashSet<String> alreadyProvidedPackages = new HashSet<>();

    public static Logger getConfiguredLogger(Class<?> callingClass, String logPathIfNotAlreadySet)
    {
        return getConfiguredLogger(callingClass, logPathIfNotAlreadySet, Level.FINEST); // default log level
    }

    public static Logger getConfiguredLogger(Class<?> callingClass, String logPathIfNotAlreadySet, Level maxLogLevelIfNotAlreadySet)
    {
        String fqClassName = callingClass.getCanonicalName(); // e.g. com.name.root.router.utils
        String packageName = fqClassName; // default
        if (fqClassName.startsWith(rootPackageName))
        {
            // we want to just get as far as the main package after "com.name.root" - e.g. "com.name.root.router"
            packageName = fqClassName.substring(0,fqClassName.indexOf(".", rootPackageNameLength+1));
        }
        
        return getConfiguredLogger(packageName, logPathIfNotAlreadySet, maxLogLevelIfNotAlreadySet);
    }

    private static Logger getConfiguredLogger(String packageName, String logPathIfNotAlreadySet, Level maxLogLevelIfNotAlreadySet)
    {
        Logger logger = Logger.getLogger(packageName); // get the logger for the package
        if (alreadyProvidedPackages.contains(packageName))
        {
            return logger; // we've already configured this logger
        }
        else
        {
            alreadyProvidedPackages.add(packageName);

            logger.setLevel(maxLogLevelIfNotAlreadySet);
            
            String logPath = (logPathIfNotAlreadySet == null || logPathIfNotAlreadySet.isBlank() ? "C:\\com\\output.log" : logPathIfNotAlreadySet);
            
            try
            {
                // reuse an existing file handler if possible, so we don't get multiple output files if two packages want to log to the same file
                FileHandler fh = null;
                
                if (fileHandlersByPath.containsKey(logPath))
                {
                    fh = (fileHandlersByPath.get(logPath));
                }
                else
                {
                    fh = new FileHandler(logPath, false);
                    fh.setFormatter(new customSingleLineLogFormatter()); // The formatter itself is out of scope for review
                    fileHandlersByPath.put(logPath, fh);
                }
                
                logger.addHandler(fh);
            }           
            catch (SecurityException | IOException e)
            {
                e.printStackTrace();
            }
            
            return logger;
        }
    }
}

Example Usage:

For utility classes (e.g. static database access classes), I am just passing the logger into each method that uses it, because it can be used from multiple packages and should log as if it was a part of the calling class:

package com.name.root.util.database
public class StringUtils
{
    // just an example
    public static long parseStringToEpoch(String s, Logger logger)
    {
        logger.entering("parseStringToEpoch"); // should turn up in the calling class's log file
    }
}

but for all other classes, each class has its own static final Logger instance which is instantiated along with the class, calling the getConfiguredLogger method:

package com.name.root.router.base
public abstract class BaseRouter
{
    private static final Logger logger = LogProvider.getConfiguredLogger(BaseRouter.class, "C:\\com\\Router.log");`
// etc, including static methods that log
}

package com.name.root.router.impl
public class ChildRouter
{
    private static final Logger logger = LogProvider.getConfiguredLogger(ChildRouter.class, "C:\\com\\Router.log");`
// etc, including main and static methods that log

   // sample usage of utilities methods
   private static final long testEpoch = StringUtils.parseStringToEpoch("1234567",logger);
}

I've tried to create a Stack-based solution, and it is available as a prototype-snippet on GitLab. However, it has multiple shortcomings, for example that the cached Logger will never be removed, which is important if that Thread is being reused. I would not try to get a stack for every call, that should have a negative impact on the performance. — Bobby
– Bobby, Commented Oct 29, 2020 at 17:07

Bobby · Accepted Answer · 2020-10-29 12:16:09Z

public class LogProvider

That class should most likely be final and have a private constructor, to make it clear that it is a static utility.

    private static final HashMap<String,FileHandler> fileHandlersByPath = new HashMap<>();
    private static final HashSet<String> alreadyProvidedPackages = new HashSet<>();

Always try to use the lowest common class or interface when declaring variables, in this case Map and Set.

I'm unsure of your names, normally I'd expect static final variables to constants, therefor named with UPPER_SNAKE_CASE.

public class LogProvider

The class does not provide logs, it does provide loggers, so it should be named LoggerProvider.

More commonly is the name LoggerFactory for such classes, though.

public static Logger getConfiguredLogger(Class<?> callingClass, String logPathIfNotAlreadySet)

I'd drop the "Configured" from the name, as there is no way to get a "not configured" logger, and it also does not matter to the user of the API whether it is configured or not. You might even get away with only having a get method, like this:

Logger LOGGER = LoggerFactory.get(SomeClass.class);

String fqClassName = callingClass.getCanonicalName();

The name of the variable is incorrect, it's the canonical name, not the fully-qualified name. They might or might not be the same.

String packageName = fqClassName;

This one is also incorrect, it's not the package name (retrieved through Class.getPackageName()) but is the canonical name.

if (fqClassName.startsWith(rootPackageName))

That check is incorrect, it would also apply to com.name.rootbutdifferentpackage.sub.Class.

            catch (SecurityException | IOException e)
            {
                e.printStackTrace();
            }

You should use a logger to log that...but actually, you most likely want to fail, because in this case you can't provide logging capabilities anymore, which might jeopardize the operation of the application.

Yes, not being to log information is a serious problem in enterprise environments, in desktop application not at all, but if you have a datacenter with 10k instances running, and 5k of those don't log, you have a problem.

As far as I can see, your class is not thread-safe, which might lead to failure or that a FileHandler is created twice for the same file, possibly corrupting the log data.

You need a concept of when to synchronize the access to the stored information. The easiest way might be to use a synchronized list by using a wrapper created by Collections.synchronized*. However, that will only get rid of possible exceptions, it might still corrupt the stored state. You will need to synchronize on something, a lock-object to make sure that you never create the same file twice.

// Requires to be synchronized to allow adding and checking at the same time.
private static Map<String,FileHandler> fileHandlersByPath = Collections.synchronizedMap(new HashMap<>());

private static Object insertionLockObject = new Object();

// ...

// Assuming that this method is thread-safe and only delivers the same
// instance once for the same parameter.
Logger logger = Logger.getLogger(packageName);

// First "cheap" check to see if it is set.
if (!alreadyProvidedPackages.contains(packageName)) {
    // If it is not, we must acquire the lock to insert it.
    synchronized(insertionLockObject) {
        // Second check, because another thread could have acquired
        // the lock before us, and already did all the set up.
        if (!alreadyProvidedPackages.contains(packageName)) {
            // Code goes here.
        }
    }
}

return logger;

For utility classes (e.g. static database access classes), I am just passing the logger into each method that uses it, because it can be used from multiple packages and should log as if it was a part of the calling class:

That smells. But I see why you are doing it.

There is no easy way to do what you want here, one could come up with a solution to parse the current StackTrace to retrieve the calling class and package, but that might be a fragile solution without giving it really much thought.

private static final Logger logger = LogProvider.getConfiguredLogger(BaseRouter.class, "C:\\com\\Router.log");

As said before, I would expect that variable to UPPER_SNAKE_CASE.

I missed this before, but why are you passing the whole path? It would be much better if you wouldn't pass a path at all, but instead set one in LogProvider and then create a path/file for this based on the passed in class. That would be much less fragile and deterministic.

Thanks, the synchronisation stuff especially was a helpful reminder. — simonalexander2005
– simonalexander2005, Commented Oct 30, 2020 at 11:06

Zoran Jankov · Accepted Answer · 2020-10-28 20:46:39Z

0

I am not completely sure what are you trying to accomplish here but you should store all your resources, namely your log files, in a resource source folder, and in code you can access those resources with:

// the stream holding the file content
InputStream is = getClass().getClassLoader().getResourceAsStream("file.txt");

// for static access, uses the class name directly
InputStream is = JavaClassName.class.getClassLoader().getResourceAsStream("file.txt");

answered Oct 28, 2020 at 20:46

Zoran Jankov

7592 gold badges7 silver badges26 bronze badges

\$\begingroup\$ I'm trying to use java.util.logging across multiple packages without creating new Logger and FileHandler instances for each class \$\endgroup\$

simonalexander2005
– simonalexander2005

2020-10-29 09:50:28 +00:00
Commented Oct 29, 2020 at 9:50

Add a comment |

Stack Exchange Network

Java Logging with a file per root package

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Java Logging with a file per root package

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions