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

Add pluggable logging for #426 v2 #476

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

Add pluggable logging for #426 v2 #476

wants to merge 2 commits into from

Conversation

agentgt
Copy link

@agentgt agentgt commented Jan 25, 2017

See original issue. I'm of the high opinion that at least core configuration should not require logging otherwise you can't configure logging!!!

This change adds a private logging facade for archaius. I have set the default to NoOp but one could easily write a SLF4J and have it be the default. We could also use the ServiceLoader mechanism to load up a default logging implementation (I actually helped @mattrjacobs setup something similar for Hystrix plugins).

Of course the possibly simpler and perhaps better option is to not do any logging in archaius2-core and fail fast and configuration to ignore errors... etc.

Ignore the checked in pom file (I needed it because I'm mirroring in a corp environment with mercurial as a forked version and I can't edit the commit... yet). I will squash/edit the commit and remove it if there is interest in this addition.

@agentgt
Copy link
Author

agentgt commented Mar 17, 2017

I figured out a simpler way that requires less code if you want to get the dependency to SLF4j:

package com.netflix.archaius.api.log;

import java.util.ServiceLoader;

import org.slf4j.ILoggerFactory;
import org.slf4j.Logger;
import org.slf4j.helpers.NOPLoggerFactory;

public interface LoggerService {
	
	public static class InternalLoggerFactory {

		/*
		 * Simply prefix Internal in front of LoggerFactory:
		 * So instead of:
		 * Logger logger = LoggerFactory.getLogger(MyClass.class);
		 * It should be:
		 * Logger logger = InternalLoggerFactory.getLogger(MyClass.class);
		 */
		public static Logger getLogger(Class<?> clazz) {
			return getILoggerFactory().getLogger(clazz.getName());
		}
		
		public static ILoggerFactory getILoggerFactory() {
			return LazyHolder.INSTANCE;
		}
		
	    //We should not load unless we are requested to. This avoids accidental initialization. @agentgt
	    //See https://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom
		private static class LazyHolder { 
			private static final ILoggerFactory INSTANCE = InternalLoggerFactory.createFactory();
		}
		
		private static ILoggerFactory createFactory() {
			/*
			 * You could also use a system property or static variable for resolution as well.
			 * The following looks for a file located here:
			 *  /META-INF/services/LoggerService.getPackage().getName()
			 * With the name of the class as a single line that implements LoggerService.
			 */
			ServiceLoader<LoggerService> loader = ServiceLoader.load(LoggerService.class);
			for (LoggerService ls : loader) {
				return ls.createLoggerFactory();
			}
			return new NOPLoggerFactory();
		}

	}
	
	public ILoggerFactory createLoggerFactory();

}

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

Successfully merging this pull request may close these issues.

2 participants