-
Notifications
You must be signed in to change notification settings - Fork 56
Log control #90
Log control #90
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package com.launchdarkly.client; | ||
|
||
import org.slf4j.LoggerFactory; | ||
|
||
|
||
public class DefaultLoggerFactoryImpl implements ILoggerFactory { | ||
public Logger getLogger(Class<?> classType) { | ||
return LoggerFactory.getLogger(classType); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.launchdarkly.client; | ||
|
||
import org.slf4j.Logger; | ||
|
||
public interface ILoggerFactory { | ||
Logger getLogger(Class<?> className); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package com.launchdarkly.client; | ||
|
||
import java.util.concurrent.atomic.AtomicReference; | ||
import org.slf4j.Logger; | ||
|
||
public class LoggerFactory { | ||
|
||
private static AtomicReference<ILoggerFactory> factory = new AtomicReference<>(); | ||
|
||
public static void install(ILoggerFactory factory) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @keynan What do you think about making the LoggerFactory part of the config? Then this method doesn't need to be public, and it makes configuring the Logger consistent with the rest of the LDClient setup. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh! I would much rather have this be part of a constructor somewhere. This way was about expediency to start the conversation. I can amend the PR on Monday. How long, do you think, before we'd see it in a release? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In many cases loggers where instantiated in classes that did not refrence the config so I've left them as is. Where possible it now goes to the LDConfig to get a logger instance. I couldn't get gradle to build locally so you should double check this all works before merging :/ sorry for the hassle |
||
LoggerFactory.factory.set(factory); | ||
} | ||
|
||
public static ILoggerFactory instance() { | ||
LoggerFactory.factory.compareAndSet(null, new DefaultLoggerFactoryImpl()); | ||
return LoggerFactory.factory.get(); | ||
} | ||
|
||
public static Logger getLogger(Class<?> className) { | ||
LoggerFactory.factory.compareAndSet(null, new DefaultLoggerFactoryImpl()); | ||
return LoggerFactory.factory.get().getLogger(className); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package com.launchdarkly.client; | ||
|
||
import java.util.concurrent.ConcurrentHashMap; | ||
import java.util.concurrent.ConcurrentMap; | ||
import org.slf4j.Logger; | ||
|
||
public class PerClassLoggerFactory implements ILoggerFactory { | ||
private final ILoggerFactory factory; | ||
private final ConcurrentMap<Class<?>, Logger> staticLoggers; | ||
public PerClassLoggerFactory(ILoggerFactory factory) { | ||
this.factory = factory; | ||
this.staticLoggers = new ConcurrentHashMap<>(); | ||
} | ||
|
||
public Logger getLogger(Class<?> klass){ | ||
return staticLoggers.computeIfAbsent(klass, factory::getLogger); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this change-- we've seen enough cases where an unknown FF is expected (e.g. test scenarios), so
debug
does feel appropriate to me.