From ab9a5a72e610183a14d55fecd9c2bc2b228397c3 Mon Sep 17 00:00:00 2001 From: kpratt Date: Fri, 10 Mar 2017 15:36:03 +1100 Subject: [PATCH 1/2] This commit adds a little indirection around logger construction. We found LD was to verbose, especially with warn messages. This will let us (your customers) have fine grained controll of what gets logged. --- .../java/com/launchdarkly/client/Clause.java | 1 - .../client/DefaultLoggerFactoryImpl.java | 10 ++++++++++ .../com/launchdarkly/client/EventProcessor.java | 1 - .../com/launchdarkly/client/FeatureFlag.java | 1 - .../launchdarkly/client/FeatureRequestor.java | 1 - .../com/launchdarkly/client/ILoggerFactory.java | 8 ++++++++ .../client/InMemoryFeatureStore.java | 1 - .../java/com/launchdarkly/client/LDClient.java | 3 +-- .../java/com/launchdarkly/client/LDConfig.java | 1 - .../java/com/launchdarkly/client/LDUser.java | 1 - .../com/launchdarkly/client/LoggerFactory.java | 17 +++++++++++++++++ .../launchdarkly/client/NewRelicReflector.java | 1 - .../launchdarkly/client/PollingProcessor.java | 1 - .../launchdarkly/client/RedisFeatureStore.java | 1 - .../client/RedisFeatureStoreBuilder.java | 1 - .../launchdarkly/client/StreamProcessor.java | 1 - 16 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 src/main/java/com/launchdarkly/client/DefaultLoggerFactoryImpl.java create mode 100644 src/main/java/com/launchdarkly/client/ILoggerFactory.java create mode 100644 src/main/java/com/launchdarkly/client/LoggerFactory.java diff --git a/src/main/java/com/launchdarkly/client/Clause.java b/src/main/java/com/launchdarkly/client/Clause.java index 53c315bb5..22897ed35 100644 --- a/src/main/java/com/launchdarkly/client/Clause.java +++ b/src/main/java/com/launchdarkly/client/Clause.java @@ -4,7 +4,6 @@ import com.google.gson.JsonElement; import com.google.gson.JsonPrimitive; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.List; diff --git a/src/main/java/com/launchdarkly/client/DefaultLoggerFactoryImpl.java b/src/main/java/com/launchdarkly/client/DefaultLoggerFactoryImpl.java new file mode 100644 index 000000000..d3c3d87d9 --- /dev/null +++ b/src/main/java/com/launchdarkly/client/DefaultLoggerFactoryImpl.java @@ -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); + } +} diff --git a/src/main/java/com/launchdarkly/client/EventProcessor.java b/src/main/java/com/launchdarkly/client/EventProcessor.java index a578e771c..62c3c82a2 100644 --- a/src/main/java/com/launchdarkly/client/EventProcessor.java +++ b/src/main/java/com/launchdarkly/client/EventProcessor.java @@ -9,7 +9,6 @@ import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.Closeable; import java.io.IOException; diff --git a/src/main/java/com/launchdarkly/client/FeatureFlag.java b/src/main/java/com/launchdarkly/client/FeatureFlag.java index 7dbaa0810..9040e0f3b 100644 --- a/src/main/java/com/launchdarkly/client/FeatureFlag.java +++ b/src/main/java/com/launchdarkly/client/FeatureFlag.java @@ -4,7 +4,6 @@ import com.google.gson.JsonElement; import com.google.gson.reflect.TypeToken; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.lang.reflect.Type; import java.util.ArrayList; diff --git a/src/main/java/com/launchdarkly/client/FeatureRequestor.java b/src/main/java/com/launchdarkly/client/FeatureRequestor.java index 543c93b91..404f0133b 100644 --- a/src/main/java/com/launchdarkly/client/FeatureRequestor.java +++ b/src/main/java/com/launchdarkly/client/FeatureRequestor.java @@ -11,7 +11,6 @@ import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.util.EntityUtils; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.Map; diff --git a/src/main/java/com/launchdarkly/client/ILoggerFactory.java b/src/main/java/com/launchdarkly/client/ILoggerFactory.java new file mode 100644 index 000000000..0e839ce99 --- /dev/null +++ b/src/main/java/com/launchdarkly/client/ILoggerFactory.java @@ -0,0 +1,8 @@ +package com.launchdarkly.client; + +import org.slf4j.Logger; + +public interface ILoggerFactory { + Logger getLogger(Class className); +} + diff --git a/src/main/java/com/launchdarkly/client/InMemoryFeatureStore.java b/src/main/java/com/launchdarkly/client/InMemoryFeatureStore.java index 3dd3aa32b..b746ecc74 100644 --- a/src/main/java/com/launchdarkly/client/InMemoryFeatureStore.java +++ b/src/main/java/com/launchdarkly/client/InMemoryFeatureStore.java @@ -1,7 +1,6 @@ package com.launchdarkly.client; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.HashMap; diff --git a/src/main/java/com/launchdarkly/client/LDClient.java b/src/main/java/com/launchdarkly/client/LDClient.java index 50fc98628..d0814a4fc 100644 --- a/src/main/java/com/launchdarkly/client/LDClient.java +++ b/src/main/java/com/launchdarkly/client/LDClient.java @@ -7,7 +7,6 @@ import org.apache.commons.codec.binary.Hex; import org.apache.http.annotation.ThreadSafe; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; @@ -344,7 +343,7 @@ private JsonElement evaluate(String featureKey, LDUser user, JsonElement default try { FeatureFlag featureFlag = config.featureStore.get(featureKey); if (featureFlag == null) { - logger.warn("Unknown feature flag " + featureKey + "; returning default value"); + logger.debug("Unknown feature flag " + featureKey + "; returning default value"); sendFlagRequestEvent(featureKey, user, defaultValue, defaultValue, null); return defaultValue; } diff --git a/src/main/java/com/launchdarkly/client/LDConfig.java b/src/main/java/com/launchdarkly/client/LDConfig.java index ad7ab7837..8c3ff9f9e 100644 --- a/src/main/java/com/launchdarkly/client/LDConfig.java +++ b/src/main/java/com/launchdarkly/client/LDConfig.java @@ -5,7 +5,6 @@ import org.apache.http.client.methods.HttpPost; import org.apache.http.client.utils.URIBuilder; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.net.URI; diff --git a/src/main/java/com/launchdarkly/client/LDUser.java b/src/main/java/com/launchdarkly/client/LDUser.java index 270d9a978..fb21736d7 100644 --- a/src/main/java/com/launchdarkly/client/LDUser.java +++ b/src/main/java/com/launchdarkly/client/LDUser.java @@ -4,7 +4,6 @@ import com.google.gson.JsonElement; import com.google.gson.JsonPrimitive; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.HashMap; import java.util.List; diff --git a/src/main/java/com/launchdarkly/client/LoggerFactory.java b/src/main/java/com/launchdarkly/client/LoggerFactory.java new file mode 100644 index 000000000..f254a9a70 --- /dev/null +++ b/src/main/java/com/launchdarkly/client/LoggerFactory.java @@ -0,0 +1,17 @@ +package com.launchdarkly.client; + +import java.util.concurrent.atomic.AtomicReference; +import org.slf4j.Logger; + +public class LoggerFactory { + + private static AtomicReference factory = new AtomicReference<>(); + + public static void install(ILoggerFactory factory) { + LoggerFactory.factory.set(factory); + } + + public static Logger getLogger(Class className) { + LoggerFactory.factory.compareAndSet(null, new DefaultLoggerFactoryImpl()); + } +} diff --git a/src/main/java/com/launchdarkly/client/NewRelicReflector.java b/src/main/java/com/launchdarkly/client/NewRelicReflector.java index f01832d6f..7b11b2838 100644 --- a/src/main/java/com/launchdarkly/client/NewRelicReflector.java +++ b/src/main/java/com/launchdarkly/client/NewRelicReflector.java @@ -1,7 +1,6 @@ package com.launchdarkly.client; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.lang.reflect.Method; diff --git a/src/main/java/com/launchdarkly/client/PollingProcessor.java b/src/main/java/com/launchdarkly/client/PollingProcessor.java index 134f955d7..74ecf285e 100644 --- a/src/main/java/com/launchdarkly/client/PollingProcessor.java +++ b/src/main/java/com/launchdarkly/client/PollingProcessor.java @@ -2,7 +2,6 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.concurrent.*; diff --git a/src/main/java/com/launchdarkly/client/RedisFeatureStore.java b/src/main/java/com/launchdarkly/client/RedisFeatureStore.java index 8694d403d..e6cc9e75d 100644 --- a/src/main/java/com/launchdarkly/client/RedisFeatureStore.java +++ b/src/main/java/com/launchdarkly/client/RedisFeatureStore.java @@ -11,7 +11,6 @@ import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import redis.clients.jedis.Jedis; import redis.clients.jedis.JedisPool; import redis.clients.jedis.JedisPoolConfig; diff --git a/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java b/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java index 6eeb85de7..cc87db184 100644 --- a/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java +++ b/src/main/java/com/launchdarkly/client/RedisFeatureStoreBuilder.java @@ -2,7 +2,6 @@ import org.apache.http.client.utils.URIBuilder; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import redis.clients.jedis.JedisPoolConfig; import redis.clients.jedis.Protocol; diff --git a/src/main/java/com/launchdarkly/client/StreamProcessor.java b/src/main/java/com/launchdarkly/client/StreamProcessor.java index a27d444db..79bad9be5 100644 --- a/src/main/java/com/launchdarkly/client/StreamProcessor.java +++ b/src/main/java/com/launchdarkly/client/StreamProcessor.java @@ -11,7 +11,6 @@ import org.apache.http.HttpHost; import org.joda.time.DateTime; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.net.URI; From c2e653fee877b2ecec978f563b349b931b81270a Mon Sep 17 00:00:00 2001 From: kpratt Date: Mon, 13 Mar 2017 12:00:15 +1100 Subject: [PATCH 2/2] Redirected logger construction through LDConfig where possible. Couldn't build locally, so please confirm compilation. --- .../java/com/launchdarkly/client/LDClient.java | 3 ++- .../java/com/launchdarkly/client/LDConfig.java | 13 +++++++++++-- .../com/launchdarkly/client/LoggerFactory.java | 6 ++++++ .../client/PerClassLoggerFactory.java | 18 ++++++++++++++++++ .../launchdarkly/client/PollingProcessor.java | 3 ++- .../launchdarkly/client/StreamProcessor.java | 3 ++- 6 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 src/main/java/com/launchdarkly/client/PerClassLoggerFactory.java diff --git a/src/main/java/com/launchdarkly/client/LDClient.java b/src/main/java/com/launchdarkly/client/LDClient.java index d0814a4fc..f0ccb13ad 100644 --- a/src/main/java/com/launchdarkly/client/LDClient.java +++ b/src/main/java/com/launchdarkly/client/LDClient.java @@ -29,7 +29,7 @@ */ @ThreadSafe public class LDClient implements LDClientInterface { - private static final Logger logger = LoggerFactory.getLogger(LDClient.class); + private final Logger logger; private static final String HMAC_ALGORITHM = "HmacSHA256"; protected static final String CLIENT_VERSION = getClientVersion(); @@ -57,6 +57,7 @@ public LDClient(String sdkKey) { * @param config a client configuration object */ public LDClient(String sdkKey, LDConfig config) { + this.logger = config.getLogger(LDClient.class); this.config = config; this.sdkKey = sdkKey; this.requestor = createFeatureRequestor(sdkKey, config); diff --git a/src/main/java/com/launchdarkly/client/LDConfig.java b/src/main/java/com/launchdarkly/client/LDConfig.java index 8c3ff9f9e..1749c8924 100644 --- a/src/main/java/com/launchdarkly/client/LDConfig.java +++ b/src/main/java/com/launchdarkly/client/LDConfig.java @@ -12,7 +12,7 @@ * This class exposes advanced configuration options for the {@link LDClient}. Instances of this class must be constructed with a {@link com.launchdarkly.client.LDConfig.Builder}. * */ -public final class LDConfig { +public final class LDConfig implements ILoggerFactory{ private static final URI DEFAULT_BASE_URI = URI.create("https://app.launchdarkly.com"); private static final URI DEFAULT_EVENTS_URI = URI.create("https://events.launchdarkly.com"); private static final URI DEFAULT_STREAM_URI = URI.create("https://stream.launchdarkly.com"); @@ -24,7 +24,8 @@ public final class LDConfig { private static final long DEFAULT_START_WAIT_MILLIS = 5000L; private static final int DEFAULT_SAMPLING_INTERVAL = 0; private static final long DEFAULT_RECONNECT_TIME_MILLIS = 1000; - private static final Logger logger = LoggerFactory.getLogger(LDConfig.class); + private final ILoggerFactory loggerFactory; + private final Logger logger; protected static final LDConfig DEFAULT = new Builder().build(); @@ -46,6 +47,9 @@ public final class LDConfig { final long reconnectTimeMs; protected LDConfig(Builder builder) { + this.loggerFactory = new PerClassLoggerFactory(builder.loggerFactory); + this.logger = this.loggerFactory.getLogger(LDConfig.class); + this.baseURI = builder.baseURI; this.eventsURI = builder.eventsURI; this.capacity = builder.capacity; @@ -68,6 +72,10 @@ protected LDConfig(Builder builder) { this.reconnectTimeMs = builder.reconnectTimeMs; } + public Logger getLogger(Class klass) { + return this.getLogger(klass); + } + /** * A builder that helps construct {@link com.launchdarkly.client.LDConfig} objects. Builder * calls can be chained, enabling the following pattern: @@ -98,6 +106,7 @@ public static class Builder { private long startWaitMillis = DEFAULT_START_WAIT_MILLIS; private int samplingInterval = DEFAULT_SAMPLING_INTERVAL; private long reconnectTimeMs = DEFAULT_RECONNECT_TIME_MILLIS; + private ILoggerFactory loggerFactory = LoggerFactory. /** * Creates a builder with all configuration parameters set to the default diff --git a/src/main/java/com/launchdarkly/client/LoggerFactory.java b/src/main/java/com/launchdarkly/client/LoggerFactory.java index f254a9a70..f01f94c0f 100644 --- a/src/main/java/com/launchdarkly/client/LoggerFactory.java +++ b/src/main/java/com/launchdarkly/client/LoggerFactory.java @@ -11,7 +11,13 @@ public static void install(ILoggerFactory factory) { 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); } } diff --git a/src/main/java/com/launchdarkly/client/PerClassLoggerFactory.java b/src/main/java/com/launchdarkly/client/PerClassLoggerFactory.java new file mode 100644 index 000000000..b197f212b --- /dev/null +++ b/src/main/java/com/launchdarkly/client/PerClassLoggerFactory.java @@ -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, Logger> staticLoggers; + public PerClassLoggerFactory(ILoggerFactory factory) { + this.factory = factory; + this.staticLoggers = new ConcurrentHashMap<>(); + } + + public Logger getLogger(Class klass){ + return staticLoggers.computeIfAbsent(klass, factory::getLogger); + } +} diff --git a/src/main/java/com/launchdarkly/client/PollingProcessor.java b/src/main/java/com/launchdarkly/client/PollingProcessor.java index 74ecf285e..288d67537 100644 --- a/src/main/java/com/launchdarkly/client/PollingProcessor.java +++ b/src/main/java/com/launchdarkly/client/PollingProcessor.java @@ -8,7 +8,7 @@ import java.util.concurrent.atomic.AtomicBoolean; public class PollingProcessor implements UpdateProcessor { - private static final Logger logger = LoggerFactory.getLogger(PollingProcessor.class); + private final Logger logger; private final FeatureRequestor requestor; private final LDConfig config; @@ -19,6 +19,7 @@ public class PollingProcessor implements UpdateProcessor { PollingProcessor(LDConfig config, FeatureRequestor requestor) { this.requestor = requestor; this.config = config; + this.logger = config.getLogger(PollingProcessor.class); this.store = config.featureStore; } diff --git a/src/main/java/com/launchdarkly/client/StreamProcessor.java b/src/main/java/com/launchdarkly/client/StreamProcessor.java index 79bad9be5..4e5ccd1ca 100644 --- a/src/main/java/com/launchdarkly/client/StreamProcessor.java +++ b/src/main/java/com/launchdarkly/client/StreamProcessor.java @@ -23,7 +23,7 @@ class StreamProcessor implements UpdateProcessor { private static final String DELETE = "delete"; private static final String INDIRECT_PUT = "indirect/put"; private static final String INDIRECT_PATCH = "indirect/patch"; - private static final Logger logger = LoggerFactory.getLogger(StreamProcessor.class); + private final Logger logger; private static final int DEAD_CONNECTION_INTERVAL_SECONDS = 300; private final FeatureStore store; @@ -39,6 +39,7 @@ class StreamProcessor implements UpdateProcessor { StreamProcessor(String sdkKey, LDConfig config, FeatureRequestor requestor) { this.store = config.featureStore; this.config = config; + this.logger = config.getLogger(StreamProcessor.class); this.sdkKey = sdkKey; this.requestor = requestor; ThreadFactory threadFactory = new ThreadFactoryBuilder()