diff --git a/archaius2-api/build.gradle b/archaius2-api/build.gradle index 4e5796518..078c58baa 100644 --- a/archaius2-api/build.gradle +++ b/archaius2-api/build.gradle @@ -18,6 +18,7 @@ apply plugin: 'java-library' dependencies { api 'javax.inject:javax.inject:1' + implementation 'org.slf4j:slf4j-api:1.7.36' } eclipse { diff --git a/archaius2-api/src/main/java/com/netflix/archaius/api/Config.java b/archaius2-api/src/main/java/com/netflix/archaius/api/Config.java index 76731ea24..dc7701bce 100644 --- a/archaius2-api/src/main/java/com/netflix/archaius/api/Config.java +++ b/archaius2-api/src/main/java/com/netflix/archaius/api/Config.java @@ -26,6 +26,20 @@ * Core API for reading a configuration. The API is read only. */ public interface Config extends PropertySource { + + /** + * Interface for a visitor visiting all key, value pairs. + *

+ * Visitors should not have consequences based on specific key-value pairs and in general + * should be used primarily for logging purposes. + *

+ * Notably, instrumentation is by default disabled on visitors, meaning that if there are + * visitors that result in consequences based on specific key-value pairs, it is possible + * that they are still registered as unused and cleaned up, resulting in an unintended + * code behavior change. + * + * @param + */ interface Visitor { T visitKey(String key, Object value); } @@ -51,9 +65,31 @@ interface Visitor { */ Object getRawProperty(String key); + /** + * Returns the raw object associated with a key, but without reporting on its usage. Only relevant for configs that + * support property instrumentation. + * @param key + */ + default Object getRawPropertyUninstrumented(String key) { return getRawProperty(key); } + + @Override default Optional getProperty(String key) { return Optional.ofNullable(getRawProperty(key)); } - + + @Override + default Optional getPropertyUninstrumented(String key) { + return Optional.ofNullable(getRawPropertyUninstrumented(key)); + } + + default void recordUsage(PropertyDetails propertyDetails) { + throw new UnsupportedOperationException("Property usage instrumentation not supported for this config type."); + } + + /** Returns whether a config is recording usage on the standard property endpoints. */ + default boolean instrumentationEnabled() { + return false; + } + /** * Parse the property as a long. * @param key @@ -145,7 +181,7 @@ interface Visitor { default Iterable keys() { return this::getKeys; } - + /** * @return Return an interator to all prefixed property names owned by this config */ diff --git a/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyDetails.java b/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyDetails.java new file mode 100644 index 000000000..8b55baecb --- /dev/null +++ b/archaius2-api/src/main/java/com/netflix/archaius/api/PropertyDetails.java @@ -0,0 +1,43 @@ +package com.netflix.archaius.api; + +import java.util.Objects; + +/** + * Container class for any information about the property at usage time that is relevant for instrumentation purposes. + */ +public class PropertyDetails { + private final String key; + private final String id; + private final Object value; + public PropertyDetails(String key, String id, Object value) { + this.key = key; + this.id = id; + this.value = value; + } + + public String getKey() { + return key; + } + + public String getId() { + return id; + } + + public Object getValue() { + return value; + } + + public boolean equals(Object o) { + if (!(o instanceof PropertyDetails)) { + return false; + } + PropertyDetails pd = (PropertyDetails) o; + return Objects.equals(key, pd.key) + && Objects.equals(id, pd.id) + && Objects.equals(value, pd.value); + } + + public String toString() { + return "[key: " + key + ", id: " + id + ", value: " + value + "]"; + } +} diff --git a/archaius2-api/src/main/java/com/netflix/archaius/api/PropertySource.java b/archaius2-api/src/main/java/com/netflix/archaius/api/PropertySource.java index a8988bdd5..7ac1b43aa 100644 --- a/archaius2-api/src/main/java/com/netflix/archaius/api/PropertySource.java +++ b/archaius2-api/src/main/java/com/netflix/archaius/api/PropertySource.java @@ -13,12 +13,28 @@ public interface PropertySource { */ default Optional getProperty(String key) { return Optional.empty(); } + /** + * Get the raw property value, but do not record any usage data. + * @param key + */ + default Optional getPropertyUninstrumented(String key) { + return getProperty(key); + } + /** * Mechanism for consuming all properties of the PropertySource * @param consumer */ default void forEachProperty(BiConsumer consumer) {} + /** + * Mechanism for consuming all properties of the PropertySource that also avoids any usage tracking + * @param consumer + */ + default void forEachPropertyUninstrumented(BiConsumer consumer) { + forEachProperty(consumer); + } + /** * @return Name used to identify the source such as a filename. */ diff --git a/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java b/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java index e1dbaea8c..9e0c70648 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java @@ -239,7 +239,9 @@ public Config getPrivateView() { @Override public T accept(Visitor visitor) { - forEachProperty(visitor::visitKey); + // The general visitor pattern is not expected to have consequences at the individual key-value pair level, + // so we choose to leave visitors uninstrumented as they otherwise represent a large source of noisy data. + forEachPropertyUninstrumented(visitor::visitKey); return null; } diff --git a/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractDependentConfig.java b/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractDependentConfig.java new file mode 100644 index 000000000..4eea616ee --- /dev/null +++ b/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractDependentConfig.java @@ -0,0 +1,95 @@ +package com.netflix.archaius.config; + +import com.netflix.archaius.api.PropertyDetails; + +import java.util.Iterator; +import java.util.function.BiConsumer; + +/** + * Extendable base class for the dependent config paradigm. Dependent configs are assumed to be configs which inherit + * values from some number (1+) of parent configs. Dependent configs hold onto caches of the property data and operate + * independently of the parent configs except in cases of propagation of instrumentation data and property value + * changes. + * + * TODO: Move DependentConfigListener logic here as well? + */ +public abstract class AbstractDependentConfig extends AbstractConfig { + + public AbstractDependentConfig(String name) { + super(name); + } + + public AbstractDependentConfig() { + super(); + } + + abstract CachedState getState(); + + @Override + public Object getRawProperty(String key) { + Object value = getState().getData().get(key); + if (getState().getInstrumentedKeys().containsKey(key)) { + getState().getInstrumentedKeys().get(key).recordUsage(createPropertyDetails(key, value)); + } + return value; + } + + @Override + public Object getRawPropertyUninstrumented(String key) { + return getState().getData().get(key); + } + + /** Return a set of all unique keys tracked by any child of this composite. */ + @Override + public Iterator getKeys() { + return getState().getData().keySet().iterator(); + } + + @Override + public Iterable keys() { + return getState().getData().keySet(); + } + + @Override + public void forEachProperty(BiConsumer consumer) { + getState().getData().forEach((k, v) -> { + if (getState().getInstrumentedKeys().containsKey(k)) { + getState().getInstrumentedKeys().get(k).recordUsage(createPropertyDetails(k, v)); + } + consumer.accept(k, v); + }); + } + + @Override + public void forEachPropertyUninstrumented(BiConsumer consumer) { + getState().getData().forEach(consumer); + } + + @Override + public boolean containsKey(String key) { + return getState().getData().containsKey(key); + } + + @Override + public boolean isEmpty() { + return getState().getData().isEmpty(); + } + + @Override + public void recordUsage(PropertyDetails propertyDetails) { + if (getState().getInstrumentedKeys().containsKey(propertyDetails.getKey())) { + getState().getInstrumentedKeys().get(propertyDetails.getKey()).recordUsage(propertyDetails); + } + } + + @Override + public boolean instrumentationEnabled() { + // In the case of dependent configs, instrumentation needs to be propagated. + // So, if any of the parent configs are instrumented, we mark this config as instrumented as well. + return !getState().getInstrumentedKeys().isEmpty(); + } + + protected PropertyDetails createPropertyDetails(String key, Object value) { + return new PropertyDetails(key, null, value); + } +} diff --git a/archaius2-core/src/main/java/com/netflix/archaius/config/CachedState.java b/archaius2-core/src/main/java/com/netflix/archaius/config/CachedState.java new file mode 100644 index 000000000..157d05679 --- /dev/null +++ b/archaius2-core/src/main/java/com/netflix/archaius/config/CachedState.java @@ -0,0 +1,25 @@ +package com.netflix.archaius.config; + +import com.netflix.archaius.api.Config; + +import java.util.Collections; +import java.util.Map; + +/** Represents an immutable, current view of a dependent config over its parent configs. */ +class CachedState { + private final Map data; + private final Map instrumentedKeys; + + CachedState(Map data, Map instrumentedKeys) { + this.data = Collections.unmodifiableMap(data); + this.instrumentedKeys = Collections.unmodifiableMap(instrumentedKeys); + } + + Map getData() { + return data; + } + + Map getInstrumentedKeys() { + return instrumentedKeys; + } +} diff --git a/archaius2-core/src/main/java/com/netflix/archaius/config/DefaultCompositeConfig.java b/archaius2-core/src/main/java/com/netflix/archaius/config/DefaultCompositeConfig.java index 9b1b5f686..d4379de9a 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/config/DefaultCompositeConfig.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/config/DefaultCompositeConfig.java @@ -18,13 +18,10 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.BiConsumer; -import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,7 +41,7 @@ * TODO: Resolve method to collapse all the child configurations into a single config * TODO: Combine children and lookup into a single LinkedHashMap */ -public class DefaultCompositeConfig extends AbstractConfig implements com.netflix.archaius.api.config.CompositeConfig { +public class DefaultCompositeConfig extends AbstractDependentConfig implements com.netflix.archaius.api.config.CompositeConfig { private static final Logger LOG = LoggerFactory.getLogger(DefaultCompositeConfig.class); /** @@ -77,13 +74,33 @@ public static com.netflix.archaius.api.config.CompositeConfig create() throws Co private class State { private final Map children; - private final Map data; + private final CachedState cachedState; public State(Map children, int size) { this.children = children; Map data = new HashMap<>(size); - children.values().forEach(child -> child.forEachProperty(data::putIfAbsent)); - this.data = Collections.unmodifiableMap(data); + Map instrumentedKeys = new HashMap<>(); + for (Config child : children.values()) { + boolean instrumented = child.instrumentationEnabled(); + child.forEachPropertyUninstrumented( + (k, v) -> updateData(data, instrumentedKeys, k, v, child, instrumented)); + } + this.cachedState = new CachedState(data, instrumentedKeys); + } + + private void updateData( + Map data, + Map instrumentedKeys, + String key, + Object value, + Config childConfig, + boolean instrumented) { + if (!data.containsKey(key)) { + if (instrumented) { + instrumentedKeys.put(key, childConfig); + } + data.put(key, value); + } } State addConfig(String name, Config config) { @@ -95,20 +112,20 @@ State addConfig(String name, Config config) { children.putAll(this.children); children.put(name, config); } - return new State(children, data.size() + 16); + return new State(children, cachedState.getData().size() + 16); } State removeConfig(String name) { if (children.containsKey(name)) { LinkedHashMap children = new LinkedHashMap<>(this.children); children.remove(name); - return new State(children, data.size()); + return new State(children, cachedState.getData().size()); } return this; } public State refresh() { - return new State(children, data.size()); + return new State(children, cachedState.getData().size()); } @@ -157,7 +174,6 @@ public void onSourceError(Throwable error, DefaultCompositeConfig dcc, Config co private final ConfigListener listener; private final boolean reversed; private volatile State state; - public DefaultCompositeConfig() { this(false); } @@ -169,6 +185,11 @@ public DefaultCompositeConfig(boolean reversed) { this.state = new State(Collections.emptyMap(), 0); } + @Override + CachedState getState() { + return state.cachedState; + } + private void refreshState() { this.state = state.refresh(); } @@ -253,29 +274,6 @@ public Config getConfig(String name) { return state.children.get(name); } - - @Override - public Object getRawProperty(String key) { - return state.data.get(key); - } - - /** - * Return a set of all unique keys tracked by any child of this composite. - * This can be an expensive operations as it requires iterating through all of - * the children. - * - * TODO: Cache keys - */ - @Override - public Iterator getKeys() { - return state.data.keySet().iterator(); - } - - @Override - public Iterable keys() { - return state.data.keySet(); - } - @Override public synchronized T accept(Visitor visitor) { AtomicReference result = new AtomicReference<>(null); @@ -285,7 +283,7 @@ public synchronized T accept(Visitor visitor) { result.set(cv.visitChild(key, config)); }); } else { - state.data.forEach(visitor::visitKey); + state.cachedState.getData().forEach(visitor::visitKey); } return result.get(); } @@ -297,21 +295,6 @@ public static com.netflix.archaius.api.config.CompositeConfig from(LinkedHashMap } return builder.build(); } - - @Override - public boolean containsKey(String key) { - return state.data.containsKey(key); - } - - @Override - public boolean isEmpty() { - return state.data.isEmpty(); - } - - @Override - public void forEachProperty(BiConsumer consumer) { - this.state.data.forEach(consumer); - } @Override public String toString() { diff --git a/archaius2-core/src/main/java/com/netflix/archaius/config/DefaultLayeredConfig.java b/archaius2-core/src/main/java/com/netflix/archaius/config/DefaultLayeredConfig.java index 3b6889f7d..0b15a7136 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/config/DefaultLayeredConfig.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/config/DefaultLayeredConfig.java @@ -13,12 +13,10 @@ import java.util.Collections; import java.util.Comparator; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; -import java.util.function.BiConsumer; import java.util.stream.Collectors; /** @@ -28,7 +26,7 @@ * * Runtime -> Environment -> System -> Application -> Library -> Defaults */ -public class DefaultLayeredConfig extends AbstractConfig implements LayeredConfig { +public class DefaultLayeredConfig extends AbstractDependentConfig implements LayeredConfig { private static final Logger LOG = LoggerFactory.getLogger(DefaultLayeredConfig.class); private final ConfigListener listener; @@ -80,11 +78,6 @@ private void refreshState() { this.state = state.refresh(); } - @Override - public boolean isEmpty() { - return state.data.isEmpty(); - } - @Override public synchronized void addConfig(Layer layer, Config config) { addConfig(layer, config, insertionOrderCounter.incrementAndGet()); @@ -123,11 +116,6 @@ public synchronized Optional removeConfig(Layer layer, String name) { } return previous; } - - @Override - public void forEachProperty(BiConsumer consumer) { - this.state.data.forEach(consumer); - } private static final AtomicInteger insertionOrderCounter = new AtomicInteger(1); @@ -206,14 +194,34 @@ public String toString() { */ private static final class ImmutableCompositeState { private final List children; - private final Map data; + private final CachedState cachedState; ImmutableCompositeState(List entries) { this.children = entries; this.children.sort(ByPriorityAndInsertionOrder); Map data = new HashMap<>(); - this.children.forEach(child -> child.config.forEachProperty(data::putIfAbsent)); - this.data = Collections.unmodifiableMap(data); + Map instrumentedKeys = new HashMap<>(); + for (LayerAndConfig child : children) { + boolean instrumented = child.config.instrumentationEnabled(); + child.config.forEachPropertyUninstrumented( + (k, v) -> updateData(data, instrumentedKeys, k, v, child.config, instrumented)); + } + this.cachedState = new CachedState(data, instrumentedKeys); + } + + private void updateData( + Map data, + Map instrumentedKeys, + String key, + Object value, + Config childConfig, + boolean instrumented) { + if (!data.containsKey(key)) { + if (instrumented) { + instrumentedKeys.put(key, childConfig); + } + data.put(key, value); + } } public ImmutableCompositeState addChild(LayerAndConfig layerAndConfig) { @@ -245,27 +253,7 @@ ImmutableCompositeState refresh() { } @Override - public Optional getProperty(String key) { - return Optional.ofNullable(state.data.get(key)); - } - - @Override - public Object getRawProperty(String key) { - return state.data.get(key); - } - - @Override - public boolean containsKey(String key) { - return state.data.containsKey(key); - } - - @Override - public Iterator getKeys() { - return state.data.keySet().iterator(); - } - - @Override - public Iterable keys() { - return state.data.keySet(); + public CachedState getState() { + return state.cachedState; } } diff --git a/archaius2-core/src/main/java/com/netflix/archaius/config/PollingDynamicConfig.java b/archaius2-core/src/main/java/com/netflix/archaius/config/PollingDynamicConfig.java index e31954b59..94a085c61 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/config/PollingDynamicConfig.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/config/PollingDynamicConfig.java @@ -27,7 +27,9 @@ import org.slf4j.LoggerFactory; import com.netflix.archaius.api.config.PollingStrategy; +import com.netflix.archaius.api.PropertyDetails; import com.netflix.archaius.config.polling.PollingResponse; +import com.netflix.archaius.instrumentation.AccessMonitorUtil; /** * Special DynamicConfig that reads an entire snapshot of the configuration @@ -38,15 +40,25 @@ public class PollingDynamicConfig extends AbstractConfig { private static final Logger LOG = LoggerFactory.getLogger(PollingDynamicConfig.class); private volatile Map current = Collections.emptyMap(); + private volatile Map currentIds = Collections.emptyMap(); private final AtomicBoolean busy = new AtomicBoolean(); private final Callable reader; private final AtomicLong updateCounter = new AtomicLong(); private final AtomicLong errorCounter = new AtomicLong(); private final PollingStrategy strategy; - + // Holds the AccessMonitorUtil and whether instrumentation is enabled. This is encapsulated to avoid + // race conditions while also allowing for on-the-fly enabling and disabling of instrumentation. + private volatile Instrumentation instrumentation; + public PollingDynamicConfig(Callable reader, PollingStrategy strategy) { + this(reader, strategy, null); + } + + public PollingDynamicConfig( + Callable reader, PollingStrategy strategy, AccessMonitorUtil accessMonitorUtil) { this.reader = reader; this.strategy = strategy; + this.instrumentation = new Instrumentation(accessMonitorUtil, accessMonitorUtil != null); strategy.execute(new Runnable() { @Override public void run() { @@ -71,6 +83,15 @@ public boolean isEmpty() { @Override public Object getRawProperty(String key) { + Object rawProperty = current.get(key); + if (instrumentationEnabled() && rawProperty != null) { + recordUsage(new PropertyDetails(key, currentIds.get(key), rawProperty)); + } + return rawProperty; + } + + @Override + public Object getRawPropertyUninstrumented(String key) { return current.get(key); } @@ -82,6 +103,7 @@ private void update() throws Exception { PollingResponse response = reader.call(); if (response.hasData()) { current = Collections.unmodifiableMap(response.getToAdd()); + currentIds = Collections.unmodifiableMap(response.getNameToIdsMap()); notifyConfigUpdated(this); } } @@ -127,6 +149,57 @@ public Iterable keys() { @Override public void forEachProperty(BiConsumer consumer) { + boolean instrumentationEnabled = instrumentationEnabled(); + current.forEach((k, v) -> { + if (instrumentationEnabled) { + recordUsage(new PropertyDetails(k, currentIds.get(k), v)); + } + consumer.accept(k, v); + }); + } + + @Override + public void forEachPropertyUninstrumented(BiConsumer consumer) { current.forEach(consumer); } + + @Override + public void recordUsage(PropertyDetails propertyDetails) { + // Once the instrumentation object is being actively changed (i.e. we have a runtime-disabling mechanism), + // there is a potential race condition here. Ensure that this is addressed when this is being changed. + if (instrumentationEnabled()) { + // Instrumentation calls from outside PollingDynamicConfig may not have ids populated, so we replace the id + // here if the id isn't present. + if (propertyDetails.getId() == null) { + propertyDetails = new PropertyDetails( + propertyDetails.getKey(), + currentIds.get(propertyDetails.getKey()), + propertyDetails.getValue()); + } + instrumentation.getAccessMonitorUtil().registerUsage(propertyDetails); + } + } + + @Override + public boolean instrumentationEnabled() { + return instrumentation.getEnabled() && instrumentation.getAccessMonitorUtil() != null; + } + + private static class Instrumentation { + private final AccessMonitorUtil accessMonitorUtil; + private final boolean enabled; + + Instrumentation(AccessMonitorUtil accessMonitorUtil, boolean enabled) { + this.accessMonitorUtil = accessMonitorUtil; + this.enabled = enabled; + } + + private AccessMonitorUtil getAccessMonitorUtil() { + return accessMonitorUtil; + } + + private boolean getEnabled() { + return enabled; + } + } } diff --git a/archaius2-core/src/main/java/com/netflix/archaius/config/PrefixedViewConfig.java b/archaius2-core/src/main/java/com/netflix/archaius/config/PrefixedViewConfig.java index 10ebad777..9a442540c 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/config/PrefixedViewConfig.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/config/PrefixedViewConfig.java @@ -15,16 +15,14 @@ */ package com.netflix.archaius.config; -import java.util.Collections; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; import java.util.Map.Entry; -import java.util.function.BiConsumer; import com.netflix.archaius.api.Config; import com.netflix.archaius.api.ConfigListener; import com.netflix.archaius.api.Decoder; +import com.netflix.archaius.api.PropertyDetails; import com.netflix.archaius.api.StrInterpolator; import com.netflix.archaius.api.StrInterpolator.Lookup; import com.netflix.archaius.interpolate.ConfigStrLookup; @@ -35,25 +33,11 @@ * This class is meant to work with dynamic Config object that may have properties * added and removed. */ -public class PrefixedViewConfig extends AbstractConfig { +public class PrefixedViewConfig extends AbstractDependentConfig { private final Config config; private final String prefix; private final Lookup nonPrefixedLookup; - private volatile State state; - - private static class State { - final Map data; - - public State(Config config, String prefix) { - Map data = new LinkedHashMap<>(); - config.forEachProperty((k, v) -> { - if (k.startsWith(prefix)) { - data.put(k.substring(prefix.length()), v); - } - }); - this.data = Collections.unmodifiableMap(data); - } - } + private volatile CachedState state; /** Listener to update the state of the PrefixedViewConfig on any changes in the source config. */ private static class PrefixedViewConfigListener extends DependentConfigListener { @@ -85,47 +69,43 @@ public PrefixedViewConfig(final String prefix, final Config config) { this.config = config; this.prefix = prefix.endsWith(".") ? prefix : prefix + "."; this.nonPrefixedLookup = ConfigStrLookup.from(config); - this.state = new State(config, this.prefix); + this.state = createState(config); this.config.addListener(new PrefixedViewConfigListener(this)); } private void updateState(Config config) { - this.state = new State(config, prefix); - } - - @Override - public Iterator getKeys() { - return state.data.keySet().iterator(); - } - - @Override - public Iterable keys() { - return state.data.keySet(); - } - - @Override - public boolean containsKey(String key) { - return state.data.containsKey(key); + this.state = createState(config); + } + + private CachedState createState(Config config) { + Map data = new LinkedHashMap<>(); + Map instrumentedKeys = new LinkedHashMap<>(); + boolean instrumented = config.instrumentationEnabled(); + config.forEachPropertyUninstrumented((k, v) -> { + if (k.startsWith(prefix)) { + String key = k.substring(prefix.length()); + data.put(key, v); + if (instrumented) { + instrumentedKeys.put(key, config); + } + } + }); + return new CachedState(data, instrumentedKeys); } @Override - public boolean isEmpty() { - return state.data.isEmpty(); + public CachedState getState() { + return state; } @Override public T accept(Visitor visitor) { T t = null; - for (Entry entry : state.data.entrySet()) { + for (Entry entry : state.getData().entrySet()) { t = visitor.visitKey(entry.getKey(), entry.getValue()); } return t; } - - @Override - public Object getRawProperty(String key) { - return state.data.get(key); - } @Override protected Lookup getLookup() { @@ -157,7 +137,7 @@ public synchronized void removeListener(ConfigListener listener) { } @Override - public void forEachProperty(BiConsumer consumer) { - this.state.data.forEach(consumer); + protected PropertyDetails createPropertyDetails(String key, Object value) { + return new PropertyDetails(prefix + key, null, value); } } diff --git a/archaius2-core/src/main/java/com/netflix/archaius/config/PrivateViewConfig.java b/archaius2-core/src/main/java/com/netflix/archaius/config/PrivateViewConfig.java index 026047ee6..7800906dd 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/config/PrivateViewConfig.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/config/PrivateViewConfig.java @@ -15,8 +15,6 @@ */ package com.netflix.archaius.config; -import java.util.Collections; -import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; import java.util.Map.Entry; @@ -32,17 +30,7 @@ *

* This class is meant to work with dynamic Config object that may have properties added and removed. */ -public class PrivateViewConfig extends AbstractConfig { - - private static class State { - final Map data; - - public State(Config config) { - Map data = new LinkedHashMap<>(); - config.forEachProperty(data::put); - this.data = Collections.unmodifiableMap(data); - } - } +public class PrivateViewConfig extends AbstractDependentConfig { /** Listener to update our own state on upstream changes and then propagate the even to our own listeners. */ private static class ViewConfigListener extends DependentConfigListener { @@ -73,48 +61,41 @@ public void onSourceError(Throwable error, PrivateViewConfig pvc, Config config) } } - private volatile State state; + private volatile CachedState state; private void updateState(Config config) { - this.state = new State(config); - } - - public PrivateViewConfig(final Config wrappedConfig) { - this.state = new State(wrappedConfig); - wrappedConfig.addListener(new ViewConfigListener(this)); - } - - @Override - public Iterator getKeys() { - return state.data.keySet().iterator(); + this.state = createState(config); } - @Override - public Iterable keys() { - return state.data.keySet(); + private CachedState createState(Config config) { + Map data = new LinkedHashMap<>(); + Map instrumentedKeys = new LinkedHashMap<>(); + boolean instrumented = config.instrumentationEnabled(); + config.forEachPropertyUninstrumented((k, v) -> { + data.put(k, v); + if (instrumented) { + instrumentedKeys.put(k, config); + } + }); + return new CachedState(data, instrumentedKeys); } @Override - public boolean containsKey(String key) { - return state.data.containsKey(key); + public CachedState getState() { + return state; } - @Override - public boolean isEmpty() { - return state.data.isEmpty(); + public PrivateViewConfig(final Config wrappedConfig) { + this.state = createState(wrappedConfig); + wrappedConfig.addListener(new ViewConfigListener(this)); } @Override public T accept(Visitor visitor) { T t = null; - for (Entry entry : state.data.entrySet()) { + for (Entry entry : state.getData().entrySet()) { t = visitor.visitKey(entry.getKey(), entry.getValue()); } return t; } - - @Override - public Object getRawProperty(String key) { - return state.data.get(key); - } } diff --git a/archaius2-core/src/main/java/com/netflix/archaius/config/polling/PollingResponse.java b/archaius2-core/src/main/java/com/netflix/archaius/config/polling/PollingResponse.java index c1a19b1db..9959a9075 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/config/polling/PollingResponse.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/config/polling/PollingResponse.java @@ -5,6 +5,31 @@ import java.util.Map; public abstract class PollingResponse { + + public static PollingResponse forSnapshot(final Map values, final Map ids) { + return new PollingResponse() { + @Override + public Map getToAdd() { + return values; + } + + @Override + public Collection getToRemove() { + return Collections.emptyList(); + } + + @Override + public boolean hasData() { + return true; + } + + @Override + public Map getNameToIdsMap() { + return ids; + } + }; + } + public static PollingResponse forSnapshot(final Map values) { return new PollingResponse() { @Override @@ -46,5 +71,8 @@ public boolean hasData() { public abstract Map getToAdd(); public abstract Collection getToRemove(); - public abstract boolean hasData(); + public abstract boolean hasData(); + public Map getNameToIdsMap() { + return Collections.emptyMap(); + } } diff --git a/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/AccessMonitorUtil.java b/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/AccessMonitorUtil.java new file mode 100644 index 000000000..207130544 --- /dev/null +++ b/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/AccessMonitorUtil.java @@ -0,0 +1,165 @@ +package com.netflix.archaius.instrumentation; + +import com.netflix.archaius.api.PropertyDetails; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; + +/** Tracks property usage data and flushes the data periodically to a sink. */ +public class AccessMonitorUtil implements AutoCloseable { + private static final Logger LOG = LoggerFactory.getLogger(AccessMonitorUtil.class); + + // Map from property id to property usage data + private final ConcurrentHashMap propertyUsageMap; + + // Map from stack trace to how many times that stack trace appeared + private final ConcurrentHashMap stackTrace; + + private static final AtomicInteger counter = new AtomicInteger(); + + private final ScheduledExecutorService executor; + + private final Consumer dataFlushConsumer; + private final boolean recordStackTrace; + + public static class Builder { + private Consumer dataFlushConsumer = null; + private boolean recordStackTrace = false; + private int initialFlushDelaySeconds = 30; + private int flushPeriodSeconds = 120; + + public Builder setDataFlushConsumer(Consumer dataFlushConsumer) { + this.dataFlushConsumer = dataFlushConsumer; + return this; + } + + public Builder setRecordStackTrace(boolean recordStackTrace) { + this.recordStackTrace = recordStackTrace; + return this; + } + + public Builder setInitialFlushDelaySeconds(int initialFlushDelaySeconds) { + this.initialFlushDelaySeconds = initialFlushDelaySeconds; + return this; + } + + public Builder setFlushPeriodSeconds(int flushPeriodSeconds) { + this.flushPeriodSeconds = flushPeriodSeconds; + return this; + } + + public AccessMonitorUtil build() { + AccessMonitorUtil accessMonitorUtil = new AccessMonitorUtil(dataFlushConsumer, recordStackTrace); + accessMonitorUtil.startFlushing(initialFlushDelaySeconds, flushPeriodSeconds); + return accessMonitorUtil; + } + } + + public static Builder builder() { + return new Builder(); + } + + private AccessMonitorUtil( + Consumer dataFlushConsumer, + boolean recordStackTrace) { + this.propertyUsageMap = new ConcurrentHashMap<>(); + this.stackTrace = new ConcurrentHashMap<>(); + this.dataFlushConsumer = dataFlushConsumer; + this.recordStackTrace = recordStackTrace; + this.executor = Executors.newSingleThreadScheduledExecutor( + runnable -> { + Thread thread = Executors.defaultThreadFactory().newThread(runnable); + thread.setDaemon(true); + thread.setName(String.format("Archaius-Instrumentation-Flusher-%d", counter.incrementAndGet())); + return thread; + }); + } + + private void startFlushing(int initialDelay, int period) { + if (!flushingEnabled()) { + LOG.info("Property usage data is being captured, but not flushed as there is no consumer specified."); + } else { + executor.scheduleWithFixedDelay(this::flushUsageData, initialDelay, period, TimeUnit.SECONDS); + } + } + + private void flushUsageData() { + try { + if (flushingEnabled()) { + dataFlushConsumer.accept(new PropertiesInstrumentationData(getAndClearUsageMap())); + } + } catch (Exception e) { + LOG.warn("Failed to flush property instrumentation data", e); + } + } + + /** Merge the results of given accessMonitorUtil into this one. */ + public void merge(AccessMonitorUtil accessMonitorUtil) { + for (Map.Entry entry : accessMonitorUtil.propertyUsageMap.entrySet()) { + propertyUsageMap.putIfAbsent(entry.getKey(), entry.getValue()); + } + for (Map.Entry entry : accessMonitorUtil.stackTrace.entrySet()) { + stackTrace.merge(entry.getKey(), entry.getValue(), Integer::sum); + } + } + + public void registerUsage(PropertyDetails propertyDetails) { + // Initially, we limit the number of events we keep to one event per property id per flush. + propertyUsageMap.putIfAbsent( + propertyDetails.getId(), + new PropertyUsageData(createEventList(new PropertyUsageEvent(System.currentTimeMillis())))); + + // Very crude and will have a very noticeable performance impact, but is + // particularly useful for finding out call sites that iterate over all + // properties. + if (recordStackTrace) { + String trace = Arrays.toString(Thread.currentThread().getStackTrace()); + stackTrace.merge(trace, 1, (v1, v2) -> v1 + 1); + } + } + + private List createEventList(PropertyUsageEvent event) { + List list = new ArrayList<>(); + list.add(event); + return list; + } + + private Map getAndClearUsageMap() { + synchronized (propertyUsageMap) { + Map ret = getUsageMapImmutable(); + propertyUsageMap.clear(); + return ret; + } + } + + public Map getUsageMapImmutable() { + return Collections.unmodifiableMap(new HashMap<>(propertyUsageMap)); + } + + public Map getStackTracesImmutable() { + return Collections.unmodifiableMap(new HashMap<>(stackTrace)); + } + + public boolean flushingEnabled() { + return dataFlushConsumer != null; + } + + @Override + public void close() { + executor.shutdown(); + flushUsageData(); + } +} diff --git a/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/PropertiesInstrumentationData.java b/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/PropertiesInstrumentationData.java new file mode 100644 index 000000000..3717120a4 --- /dev/null +++ b/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/PropertiesInstrumentationData.java @@ -0,0 +1,16 @@ +package com.netflix.archaius.instrumentation; + +import java.util.Map; + +/** Instrumentation data snapshot for usages captured since the last flush. */ +public class PropertiesInstrumentationData { + private final Map idToUsageDataMap; + + public PropertiesInstrumentationData(Map idToUsageDataMap) { + this.idToUsageDataMap = idToUsageDataMap; + } + + public Map getIdToUsageDataMap() { + return idToUsageDataMap; + } +} diff --git a/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/PropertyUsageData.java b/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/PropertyUsageData.java new file mode 100644 index 000000000..f1bc91b45 --- /dev/null +++ b/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/PropertyUsageData.java @@ -0,0 +1,15 @@ +package com.netflix.archaius.instrumentation; + +import java.util.List; + +/** Container for all usages of a specific property in a flush cycle. */ +public class PropertyUsageData { + private List propertyUsageEvents; + public PropertyUsageData(List propertyUsageEvents) { + this.propertyUsageEvents = propertyUsageEvents; + } + + public List getPropertyUsageEvents() { + return propertyUsageEvents; + } +} diff --git a/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/PropertyUsageEvent.java b/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/PropertyUsageEvent.java new file mode 100644 index 000000000..0d0d86bbd --- /dev/null +++ b/archaius2-core/src/main/java/com/netflix/archaius/instrumentation/PropertyUsageEvent.java @@ -0,0 +1,14 @@ +package com.netflix.archaius.instrumentation; + +/** Container for data about a single property usage event. */ +public class PropertyUsageEvent { + private final long usageTimeMillis; + + public PropertyUsageEvent(long usageTimeMillis) { + this.usageTimeMillis = usageTimeMillis; + } + + public long getUsageTimeMillis() { + return this.usageTimeMillis; + } +} diff --git a/archaius2-core/src/test/java/com/netflix/archaius/config/CompositeConfigTest.java b/archaius2-core/src/test/java/com/netflix/archaius/config/CompositeConfigTest.java index 4d15771ed..5f3cae500 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/config/CompositeConfigTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/config/CompositeConfigTest.java @@ -18,11 +18,18 @@ import java.lang.ref.Reference; import java.lang.ref.WeakReference; import java.util.Collection; +import java.util.HashMap; import java.util.Iterator; +import java.util.Map; import java.util.Properties; +import java.util.concurrent.Callable; import com.netflix.archaius.api.Config; import com.netflix.archaius.api.config.SettableConfig; +import com.netflix.archaius.config.polling.ManualPollingStrategy; +import com.netflix.archaius.config.polling.PollingResponse; +import com.netflix.archaius.instrumentation.AccessMonitorUtil; +import com.netflix.archaius.api.PropertyDetails; import org.junit.Assert; import org.junit.Test; @@ -33,6 +40,11 @@ import static com.netflix.archaius.TestUtils.set; import static com.netflix.archaius.TestUtils.size; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class CompositeConfigTest { @Test @@ -201,4 +213,84 @@ public void unusedCompositeConfigIsGarbageCollected() throws ConfigException { System.gc(); Assert.assertNull(weakReference.get()); } + + @Test + public void instrumentationNotEnabled() throws Exception { + com.netflix.archaius.api.config.CompositeConfig composite = new DefaultCompositeConfig(); + + composite.addConfig("polling", createPollingDynamicConfig("a1", "1", "b1", "2", null)); + + Assert.assertFalse(composite.instrumentationEnabled()); + Assert.assertEquals(composite.getRawProperty("a1"), "1"); + Assert.assertEquals(composite.getRawProperty("b1"), "2"); + } + + @Test + public void instrumentationPropagation() throws Exception { + com.netflix.archaius.api.config.CompositeConfig composite = new DefaultCompositeConfig(); + AccessMonitorUtil accessMonitorUtil = spy(AccessMonitorUtil.builder().build()); + + PollingDynamicConfig outerPollingDynamicConfig = createPollingDynamicConfig("a1", "1", "b1", "2", accessMonitorUtil); + composite.addConfig("outer", outerPollingDynamicConfig); + + com.netflix.archaius.api.config.CompositeConfig innerComposite = new DefaultCompositeConfig(); + PollingDynamicConfig nestedPollingDynamicConfig = createPollingDynamicConfig("b1", "1", "c1", "3", accessMonitorUtil); + innerComposite.addConfig("polling", nestedPollingDynamicConfig); + composite.addConfig("innerComposite", innerComposite); + + composite.addConfig("d", MapConfig.builder().put("c1", "4").put("d1", "5").build()); + + // Properties (a1: 1) and (b1: 2) are covered by the first polling config + Assert.assertEquals(composite.getRawProperty("a1"), "1"); + verify(accessMonitorUtil).registerUsage(eq(new PropertyDetails("a1", "a1", "1"))); + + Assert.assertEquals(composite.getRawPropertyUninstrumented("a1"), "1"); + verify(accessMonitorUtil, times(1)).registerUsage(any()); + + Assert.assertEquals(composite.getRawProperty("b1"), "2"); + verify(accessMonitorUtil).registerUsage(eq(new PropertyDetails("b1", "b1", "2"))); + + Assert.assertEquals(composite.getRawPropertyUninstrumented("b1"), "2"); + verify(accessMonitorUtil, times(2)).registerUsage(any()); + + // Property (c1: 3) is covered by the composite config over the polling config + Assert.assertEquals(composite.getRawProperty("c1"), "3"); + verify(accessMonitorUtil).registerUsage(eq(new PropertyDetails("c1", "c1", "3"))); + + Assert.assertEquals(composite.getRawPropertyUninstrumented("c1"), "3"); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + + // Property (d1: 5) is covered by the final, uninstrumented MapConfig + Assert.assertEquals(composite.getRawProperty("d1"), "5"); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + + Assert.assertEquals(composite.getRawPropertyUninstrumented("d1"), "5"); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + + // The instrumented forEachProperty endpoint updates the counts for every property + composite.forEachProperty((k, v) -> {}); + verify(accessMonitorUtil, times(2)).registerUsage(eq(new PropertyDetails("a1", "a1", "1"))); + verify(accessMonitorUtil, times(2)).registerUsage(eq(new PropertyDetails("b1", "b1", "2"))); + verify(accessMonitorUtil, times(2)).registerUsage(eq(new PropertyDetails("c1", "c1", "3"))); + verify(accessMonitorUtil, times(6)).registerUsage((any())); + + // The uninstrumented forEachProperty leaves the counts unchanged + composite.forEachPropertyUninstrumented((k, v) -> {}); + verify(accessMonitorUtil, times(6)).registerUsage((any())); + } + + private PollingDynamicConfig createPollingDynamicConfig( + String key1, String value1, String key2, String value2, AccessMonitorUtil accessMonitorUtil) throws Exception { + ManualPollingStrategy strategy = new ManualPollingStrategy(); + Map props = new HashMap<>(); + props.put(key1, value1); + props.put(key2, value2); + Map propIds = new HashMap<>(); + propIds.put(key1, key1); + propIds.put(key2, key2); + Callable reader = () -> PollingResponse.forSnapshot(props, propIds); + PollingDynamicConfig config = new PollingDynamicConfig(reader, strategy, accessMonitorUtil); + strategy.fire(); + return config; + } } diff --git a/archaius2-core/src/test/java/com/netflix/archaius/config/DefaultLayeredConfigTest.java b/archaius2-core/src/test/java/com/netflix/archaius/config/DefaultLayeredConfigTest.java index ed7d00baa..80896458e 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/config/DefaultLayeredConfigTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/config/DefaultLayeredConfigTest.java @@ -3,9 +3,13 @@ import com.netflix.archaius.Layers; import com.netflix.archaius.api.Config; import com.netflix.archaius.api.ConfigListener; +import com.netflix.archaius.api.PropertyDetails; import com.netflix.archaius.api.config.LayeredConfig; import com.netflix.archaius.api.config.SettableConfig; +import com.netflix.archaius.config.polling.ManualPollingStrategy; +import com.netflix.archaius.config.polling.PollingResponse; +import com.netflix.archaius.instrumentation.AccessMonitorUtil; import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; @@ -13,10 +17,18 @@ import java.lang.ref.Reference; import java.lang.ref.WeakReference; import java.util.Collection; +import java.util.HashMap; import java.util.Iterator; +import java.util.Map; +import java.util.concurrent.Callable; import static com.netflix.archaius.TestUtils.set; import static com.netflix.archaius.TestUtils.size; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class DefaultLayeredConfigTest { @Test @@ -231,4 +243,84 @@ public void testKeysIterableModificationThrows() { Assert.assertThrows(UnsupportedOperationException.class, config.keys().iterator()::remove); Assert.assertThrows(UnsupportedOperationException.class, ((Collection) config.keys())::clear); } + + @Test + public void instrumentationNotEnabled() throws Exception { + LayeredConfig config = new DefaultLayeredConfig(); + + config.addConfig(Layers.DEFAULT, createPollingDynamicConfig("a1", "1", "b1", "2", null)); + + Assert.assertFalse(config.instrumentationEnabled()); + Assert.assertEquals(config.getRawProperty("a1"), "1"); + Assert.assertEquals(config.getRawProperty("b1"), "2"); + } + + @Test + public void instrumentationPropagation() throws Exception { + com.netflix.archaius.api.config.LayeredConfig layered = new DefaultLayeredConfig(); + AccessMonitorUtil accessMonitorUtil = spy(AccessMonitorUtil.builder().build()); + + PollingDynamicConfig outerPollingDynamicConfig = createPollingDynamicConfig("a1", "1", "b1", "2", accessMonitorUtil); + layered.addConfig(Layers.RUNTIME, outerPollingDynamicConfig); + + com.netflix.archaius.api.config.CompositeConfig innerComposite = new DefaultCompositeConfig(); + PollingDynamicConfig nestedPollingDynamicConfig = createPollingDynamicConfig("b1", "1", "c1", "3", accessMonitorUtil); + innerComposite.addConfig("polling", nestedPollingDynamicConfig); + layered.addConfig(Layers.SYSTEM, innerComposite); + + layered.addConfig(Layers.ENVIRONMENT, MapConfig.builder().put("c1", "4").put("d1", "5").build()); + + // Properties (a1: 1) and (b1: 2) are covered by the first polling config + Assert.assertEquals(layered.getRawProperty("a1"), "1"); + verify(accessMonitorUtil).registerUsage(eq(new PropertyDetails("a1", "a1", "1"))); + + Assert.assertEquals(layered.getRawPropertyUninstrumented("a1"), "1"); + verify(accessMonitorUtil, times(1)).registerUsage(any()); + + Assert.assertEquals(layered.getRawProperty("b1"), "2"); + verify(accessMonitorUtil).registerUsage(eq(new PropertyDetails("b1", "b1", "2"))); + + Assert.assertEquals(layered.getRawPropertyUninstrumented("b1"), "2"); + verify(accessMonitorUtil, times(2)).registerUsage(any()); + + // Property (c1: 3) is covered by the composite config over the polling config + Assert.assertEquals(layered.getRawProperty("c1"), "3"); + verify(accessMonitorUtil).registerUsage(eq(new PropertyDetails("c1", "c1", "3"))); + + Assert.assertEquals(layered.getRawPropertyUninstrumented("c1"), "3"); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + + // Property (d1: 5) is covered by the final, uninstrumented MapConfig + Assert.assertEquals(layered.getRawProperty("d1"), "5"); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + + Assert.assertEquals(layered.getRawPropertyUninstrumented("d1"), "5"); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + + // The instrumented forEachProperty endpoint updates the counts for every property + layered.forEachProperty((k, v) -> {}); + verify(accessMonitorUtil, times(2)).registerUsage(eq(new PropertyDetails("a1", "a1", "1"))); + verify(accessMonitorUtil, times(2)).registerUsage(eq(new PropertyDetails("b1", "b1", "2"))); + verify(accessMonitorUtil, times(2)).registerUsage(eq(new PropertyDetails("c1", "c1", "3"))); + verify(accessMonitorUtil, times(6)).registerUsage((any())); + + // The uninstrumented forEachProperty leaves the counts unchanged + layered.forEachPropertyUninstrumented((k, v) -> {}); + verify(accessMonitorUtil, times(6)).registerUsage((any())); + } + + private PollingDynamicConfig createPollingDynamicConfig( + String key1, String value1, String key2, String value2, AccessMonitorUtil accessMonitorUtil) throws Exception { + ManualPollingStrategy strategy = new ManualPollingStrategy(); + Map props = new HashMap<>(); + props.put(key1, value1); + props.put(key2, value2); + Map propIds = new HashMap<>(); + propIds.put(key1, key1); + propIds.put(key2, key2); + Callable reader = () -> PollingResponse.forSnapshot(props, propIds); + PollingDynamicConfig config = new PollingDynamicConfig(reader, strategy, accessMonitorUtil); + strategy.fire(); + return config; + } } diff --git a/archaius2-core/src/test/java/com/netflix/archaius/config/PollingDynamicConfigTest.java b/archaius2-core/src/test/java/com/netflix/archaius/config/PollingDynamicConfigTest.java index 6e8665dd4..963252d5c 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/config/PollingDynamicConfigTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/config/PollingDynamicConfigTest.java @@ -25,7 +25,9 @@ import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicInteger; +import com.netflix.archaius.api.PropertyDetails; import com.netflix.archaius.config.polling.PollingResponse; +import com.netflix.archaius.instrumentation.AccessMonitorUtil; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; @@ -41,6 +43,11 @@ import static com.netflix.archaius.TestUtils.set; import static com.netflix.archaius.TestUtils.size; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class PollingDynamicConfigTest { @@ -284,4 +291,38 @@ public void testKeysIterableModificationThrows() throws Exception { Assert.assertThrows(UnsupportedOperationException.class, config.keys().iterator()::remove); Assert.assertThrows(UnsupportedOperationException.class, ((Collection) config.keys())::clear); } + + @Test + public void testInstrumentation() throws Exception { + ManualPollingStrategy strategy = new ManualPollingStrategy(); + Callable reader = () -> { + Map props = new HashMap<>(); + props.put("foo", "foo-value"); + props.put("bar", "bar-value"); + Map propIds = new HashMap<>(); + propIds.put("foo", "1"); + propIds.put("bar", "2"); + return PollingResponse.forSnapshot(props, propIds); + }; + AccessMonitorUtil accessMonitorUtil = spy(AccessMonitorUtil.builder().build()); + PollingDynamicConfig config = new PollingDynamicConfig(reader, strategy, accessMonitorUtil); + strategy.fire(); + + Assert.assertTrue(config.instrumentationEnabled()); + + config.getRawProperty("foo"); + verify(accessMonitorUtil).registerUsage(eq(new PropertyDetails("foo", "1", "foo-value"))); + verify(accessMonitorUtil, times(1)).registerUsage(any()); + + config.getRawPropertyUninstrumented("bar"); + verify(accessMonitorUtil, times(1)).registerUsage(any()); + + config.forEachProperty((k, v) -> {}); + verify(accessMonitorUtil, times(2)).registerUsage(eq(new PropertyDetails("foo", "1", "foo-value"))); + verify(accessMonitorUtil, times(1)).registerUsage(eq(new PropertyDetails("bar", "2", "bar-value"))); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + + config.forEachPropertyUninstrumented((k, v) -> {}); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + } } diff --git a/archaius2-core/src/test/java/com/netflix/archaius/config/PrefixedViewTest.java b/archaius2-core/src/test/java/com/netflix/archaius/config/PrefixedViewTest.java index 8e16673d3..bfa5e29b8 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/config/PrefixedViewTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/config/PrefixedViewTest.java @@ -1,23 +1,37 @@ package com.netflix.archaius.config; +import com.netflix.archaius.Layers; import com.netflix.archaius.api.Config; import com.netflix.archaius.api.ConfigListener; +import com.netflix.archaius.api.PropertyDetails; +import com.netflix.archaius.api.config.LayeredConfig; import com.netflix.archaius.api.config.SettableConfig; import com.netflix.archaius.api.exceptions.ConfigException; import java.lang.ref.Reference; import java.lang.ref.WeakReference; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.Map; import java.util.Set; +import java.util.concurrent.Callable; +import com.netflix.archaius.config.polling.ManualPollingStrategy; +import com.netflix.archaius.config.polling.PollingResponse; +import com.netflix.archaius.instrumentation.AccessMonitorUtil; import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; import static com.netflix.archaius.TestUtils.set; import static com.netflix.archaius.TestUtils.size; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class PrefixedViewTest { @Test @@ -147,4 +161,53 @@ public void testKeysIterableModificationThrows() { Assert.assertThrows(UnsupportedOperationException.class, config.keys().iterator()::remove); Assert.assertThrows(UnsupportedOperationException.class, ((Collection) config.keys())::clear); } + + @Test + public void instrumentationNotEnabled() throws Exception { + Config config = MapConfig.builder() + .put("foo.prop1", "value1") + .put("foo.prop2", "value2") + .build() + .getPrefixedView("foo"); + + Assert.assertFalse(config.instrumentationEnabled()); + Assert.assertEquals(config.getRawProperty("prop1"), "value1"); + Assert.assertEquals(config.getRawProperty("prop2"), "value2"); + } + + @Test + public void instrumentation() throws Exception { + ManualPollingStrategy strategy = new ManualPollingStrategy(); + Callable reader = () -> { + Map props = new HashMap<>(); + props.put("foo.prop1", "foo-value"); + props.put("foo.prop2", "bar-value"); + Map propIds = new HashMap<>(); + propIds.put("foo.prop1", "1"); + propIds.put("foo.prop2", "2"); + return PollingResponse.forSnapshot(props, propIds); + }; + AccessMonitorUtil accessMonitorUtil = spy(AccessMonitorUtil.builder().build()); + PollingDynamicConfig baseConfig = new PollingDynamicConfig(reader, strategy, accessMonitorUtil); + strategy.fire(); + + Config config = baseConfig.getPrefixedView("foo"); + + Assert.assertTrue(config.instrumentationEnabled()); + + config.getRawProperty("prop1"); + verify(accessMonitorUtil).registerUsage(eq(new PropertyDetails("foo.prop1", "1", "foo-value"))); + verify(accessMonitorUtil, times(1)).registerUsage(any()); + + config.getRawPropertyUninstrumented("prop2"); + verify(accessMonitorUtil, times(1)).registerUsage(any()); + + config.forEachProperty((k, v) -> {}); + verify(accessMonitorUtil, times(2)).registerUsage(eq(new PropertyDetails("foo.prop1", "1", "foo-value"))); + verify(accessMonitorUtil, times(1)).registerUsage(eq(new PropertyDetails("foo.prop2", "2", "bar-value"))); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + + config.forEachPropertyUninstrumented((k, v) -> {}); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + } } diff --git a/archaius2-core/src/test/java/com/netflix/archaius/config/PrivateViewTest.java b/archaius2-core/src/test/java/com/netflix/archaius/config/PrivateViewTest.java index 1f7735fed..8eb0043c7 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/config/PrivateViewTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/config/PrivateViewTest.java @@ -3,8 +3,12 @@ import com.netflix.archaius.api.Config; import com.netflix.archaius.api.ConfigListener; import com.netflix.archaius.api.Decoder; +import com.netflix.archaius.api.PropertyDetails; import com.netflix.archaius.api.config.SettableConfig; import com.netflix.archaius.api.exceptions.ConfigException; +import com.netflix.archaius.config.polling.ManualPollingStrategy; +import com.netflix.archaius.config.polling.PollingResponse; +import com.netflix.archaius.instrumentation.AccessMonitorUtil; import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; @@ -12,15 +16,23 @@ import java.lang.ref.Reference; import java.lang.ref.WeakReference; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.Map; import java.util.Set; +import java.util.concurrent.Callable; import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertSame; import static com.netflix.archaius.TestUtils.set; import static com.netflix.archaius.TestUtils.size; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class PrivateViewTest { @@ -159,4 +171,53 @@ public void testKeysIterableModificationThrows() { Assert.assertThrows(UnsupportedOperationException.class, config.keys().iterator()::remove); Assert.assertThrows(UnsupportedOperationException.class, ((Collection) config.keys())::clear); } + + @Test + public void instrumentationNotEnabled() throws Exception { + Config config = MapConfig.builder() + .put("foo.prop1", "value1") + .put("foo.prop2", "value2") + .build() + .getPrivateView(); + + Assert.assertFalse(config.instrumentationEnabled()); + Assert.assertEquals(config.getRawProperty("foo.prop1"), "value1"); + Assert.assertEquals(config.getRawProperty("foo.prop2"), "value2"); + } + + @Test + public void testInstrumentation() throws Exception { + ManualPollingStrategy strategy = new ManualPollingStrategy(); + Callable reader = () -> { + Map props = new HashMap<>(); + props.put("foo.prop1", "foo-value"); + props.put("foo.prop2", "bar-value"); + Map propIds = new HashMap<>(); + propIds.put("foo.prop1", "1"); + propIds.put("foo.prop2", "2"); + return PollingResponse.forSnapshot(props, propIds); + }; + AccessMonitorUtil accessMonitorUtil = spy(AccessMonitorUtil.builder().build()); + PollingDynamicConfig baseConfig = new PollingDynamicConfig(reader, strategy, accessMonitorUtil); + strategy.fire(); + + Config config = baseConfig.getPrivateView(); + + Assert.assertTrue(config.instrumentationEnabled()); + + config.getRawProperty("foo.prop1"); + verify(accessMonitorUtil).registerUsage(eq(new PropertyDetails("foo.prop1", "1", "foo-value"))); + verify(accessMonitorUtil, times(1)).registerUsage(any()); + + config.getRawPropertyUninstrumented("foo.prop2"); + verify(accessMonitorUtil, times(1)).registerUsage(any()); + + config.forEachProperty((k, v) -> {}); + verify(accessMonitorUtil, times(2)).registerUsage(eq(new PropertyDetails("foo.prop1", "1", "foo-value"))); + verify(accessMonitorUtil, times(1)).registerUsage(eq(new PropertyDetails("foo.prop2", "2", "bar-value"))); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + + config.forEachPropertyUninstrumented((k, v) -> {}); + verify(accessMonitorUtil, times(3)).registerUsage(any()); + } } diff --git a/archaius2-guice/src/main/java/com/netflix/archaius/guice/ArchaiusModule.java b/archaius2-guice/src/main/java/com/netflix/archaius/guice/ArchaiusModule.java index b0b85114e..49d07e55e 100644 --- a/archaius2-guice/src/main/java/com/netflix/archaius/guice/ArchaiusModule.java +++ b/archaius2-guice/src/main/java/com/netflix/archaius/guice/ArchaiusModule.java @@ -20,12 +20,14 @@ import com.google.inject.AbstractModule; import com.google.inject.binder.LinkedBindingBuilder; import com.google.inject.multibindings.Multibinder; +import com.google.inject.multibindings.OptionalBinder; import com.google.inject.name.Names; import com.netflix.archaius.api.CascadeStrategy; import com.netflix.archaius.api.Config; import com.netflix.archaius.api.inject.DefaultLayer; import com.netflix.archaius.api.inject.RemoteLayer; import com.netflix.archaius.config.MapConfig; +import com.netflix.archaius.instrumentation.AccessMonitorUtil; /** * Guice Module for enabling archaius and making its components injectable. Installing this @@ -242,7 +244,8 @@ protected void bindApplicationConfigurationOverrideResource(String overrideResou @Override protected final void configure() { install(new InternalArchaiusModule()); - + OptionalBinder.newOptionalBinder(binder(), AccessMonitorUtil.class); + configureArchaius(); // TODO: Remove in next release diff --git a/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/DefaultPersisted2ClientConfig.java b/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/DefaultPersisted2ClientConfig.java index b2218d99d..466b4807c 100644 --- a/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/DefaultPersisted2ClientConfig.java +++ b/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/DefaultPersisted2ClientConfig.java @@ -17,6 +17,7 @@ public class DefaultPersisted2ClientConfig implements Persisted2ClientConfig { private Map scopes = new HashMap<>(); private boolean skipPropsWithExtraScopes = false; private boolean isEnabled = true; + private boolean instrumentationEnabled = false; public DefaultPersisted2ClientConfig withRefreshRate(int refreshRate) { this.refreshRate = refreshRate; diff --git a/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/JsonPersistedV2Reader.java b/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/JsonPersistedV2Reader.java index 8800cb23a..7248d2dbb 100644 --- a/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/JsonPersistedV2Reader.java +++ b/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/JsonPersistedV2Reader.java @@ -51,7 +51,8 @@ public class JsonPersistedV2Reader implements Callable { private final static List DEFAULT_ORDERED_SCOPES = Arrays.asList("serverId", "asg", "ami", "cluster", "appId", "env", "countries", "stack", "zone", "region"); private final static String DEFAULT_KEY_FIELD = "key"; private final static String DEFAULT_VALUE_FIELD = "value"; - private final static List DEFAULT_PATH = Arrays.asList("persistedproperties", "properties", "property"); + private final static String DEFAULT_ID_FIELD = "propertyId"; + private final static List DEFAULT_PATH = Arrays.asList("persistedproperties", "properties", "property"); public static class Builder { private final Callable reader; @@ -59,8 +60,10 @@ public static class Builder { private List scopeFields = DEFAULT_ORDERED_SCOPES; private String keyField = DEFAULT_KEY_FIELD; private String valueField = DEFAULT_VALUE_FIELD; + private String idField = DEFAULT_ID_FIELD; private ScopePredicate predicate = ScopePredicates.alwaysTrue(); private ScopedValueResolver resolver = new ScopePriorityPropertyValueResolver(); + private boolean readIdField = false; public Builder(Callable reader) { this.reader = reader; @@ -96,11 +99,21 @@ public Builder withValueField(String valueField) { this.valueField = valueField; return this; } + + public Builder withIdField(String idField) { + this.idField = idField; + return this; + } public Builder withValueResolver(ScopedValueResolver resolver) { this.resolver = resolver; return this; } + + public Builder withReadIdField(boolean readIdField) { + this.readIdField = readIdField; + return this; + } public JsonPersistedV2Reader build() { return new JsonPersistedV2Reader(this); @@ -118,8 +131,10 @@ public static Builder builder(Callable reader) { private final ObjectMapper mapper = new ObjectMapper(); private final List scopeFields; private final String keyField; + private final String idField; private final String valueField; private final List path; + private final boolean readIdField; private JsonPersistedV2Reader(Builder builder) { this.reader = builder.reader; @@ -127,13 +142,16 @@ private JsonPersistedV2Reader(Builder builder) { this.valueResolver = builder.resolver; this.keyField = builder.keyField; this.valueField = builder.valueField; + this.idField = builder.idField; this.scopeFields = builder.scopeFields; this.path = builder.path; + this.readIdField = builder.readIdField; } @Override public PollingResponse call() throws Exception { Map> props = new HashMap>(); + Map> propIds = new HashMap<>(); InputStream is = reader.call(); if (is == null) { @@ -171,6 +189,11 @@ public PollingResponse call() throws Exception { props.put(key, variations); } variations.add(new ScopedValue(value, scopes)); + if (readIdField) { + propIds.putIfAbsent(key, new ArrayList<>()); + propIds.get(key).add( + new ScopedValue(property.has(idField) ? property.get(idField).asText() : "", scopes)); + } } catch (Exception e) { LOG.warn("Unable to process property '{}'", key); @@ -191,6 +214,14 @@ public PollingResponse call() throws Exception { for (Entry> entry : props.entrySet()) { result.put(entry.getKey(), valueResolver.resolve(entry.getKey(), entry.getValue())); } + + if (readIdField) { + final Map idResult = new HashMap<>(); + for (Entry> entry : propIds.entrySet()) { + idResult.put(entry.getKey(), valueResolver.resolve(entry.getKey(), entry.getValue())); + } + return PollingResponse.forSnapshot(result, idResult); + } return PollingResponse.forSnapshot(result); } diff --git a/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/Persisted2ConfigProvider.java b/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/Persisted2ConfigProvider.java index 0ea310bc8..2c5cb3609 100644 --- a/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/Persisted2ConfigProvider.java +++ b/archaius2-persisted2/src/main/java/com/netflix/archaius/persisted2/Persisted2ConfigProvider.java @@ -4,6 +4,7 @@ import java.net.URLEncoder; import java.util.Map; import java.util.Map.Entry; +import java.util.Optional; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -18,6 +19,7 @@ import com.netflix.archaius.config.EmptyConfig; import com.netflix.archaius.config.PollingDynamicConfig; import com.netflix.archaius.config.polling.FixedPollingStrategy; +import com.netflix.archaius.instrumentation.AccessMonitorUtil; import com.netflix.archaius.persisted2.loader.HTTPStreamLoader; /** @@ -60,11 +62,15 @@ public class Persisted2ConfigProvider implements Provider { private final Logger LOG = LoggerFactory.getLogger(Persisted2ConfigProvider.class); private final Provider config; + private final Optional accessMonitorUtilOptional; private volatile PollingDynamicConfig dynamicConfig; - + @Inject - public Persisted2ConfigProvider(Provider config) throws Exception { + public Persisted2ConfigProvider( + Provider config, + Optional accessMonitorUtilOptional) throws Exception { this.config = config; + this.accessMonitorUtilOptional = accessMonitorUtilOptional; } public static String getFilterString(Map> scopes) { @@ -123,9 +129,17 @@ public Config get() { .withPath("propertiesList") .withScopes(clientConfig.getPrioritizedScopes()) .withPredicate(ScopePredicates.fromMap(clientConfig.getScopes())) + // If instrumentation flushing is enabled, we need to read the id fields as well to uniquely + // identify the property being used. + .withReadIdField(accessMonitorUtilOptional.isPresent()) .build(); - return dynamicConfig = new PollingDynamicConfig(reader, new FixedPollingStrategy(clientConfig.getRefreshRate(), TimeUnit.SECONDS)); + dynamicConfig = + new PollingDynamicConfig( + reader, + new FixedPollingStrategy(clientConfig.getRefreshRate(), TimeUnit.SECONDS), + accessMonitorUtilOptional.orElse(null)); + return dynamicConfig; } catch (Exception e1) { throw new RuntimeException(e1); } diff --git a/archaius2-persisted2/src/test/java/com/netflix/archaius/persisted2/JsonPersistedV2ReaderTest.java b/archaius2-persisted2/src/test/java/com/netflix/archaius/persisted2/JsonPersistedV2ReaderTest.java new file mode 100644 index 000000000..d99949613 --- /dev/null +++ b/archaius2-persisted2/src/test/java/com/netflix/archaius/persisted2/JsonPersistedV2ReaderTest.java @@ -0,0 +1,92 @@ +package com.netflix.archaius.persisted2; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.netflix.archaius.config.polling.PollingResponse; +import org.junit.Assert; +import org.junit.Test; + +import java.io.ByteArrayInputStream; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +public class JsonPersistedV2ReaderTest { + @Test + public void idFieldReturnedWhenPresent() throws Exception { + List propertyList = new ArrayList<>(); + propertyList.add(new TestProperty("key1", "value3", "id3", "app1", "")); + // The next two properties are the only two that are actually resolved, as the other two are overridden due to + // the presence of the region field. + propertyList.add(new TestProperty("key1", "value1", "id1", "app1", "region1")); + propertyList.add(new TestProperty("key2", "value2", "id2", "app1", "region1")); + propertyList.add(new TestProperty("key2", "value4", "id4", "app1", "")); + TestPropertyList properties = new TestPropertyList(propertyList); + + JsonPersistedV2Reader reader = + JsonPersistedV2Reader.builder( + () -> new ByteArrayInputStream( + new ObjectMapper().writeValueAsBytes(properties))) + .withPath("propertiesList") + .withReadIdField(true) + .build(); + + PollingResponse response = reader.call(); + Map props = response.getToAdd(); + Assert.assertEquals(2, props.size()); + Assert.assertEquals("value1", props.get("key1")); + Assert.assertEquals("value2", props.get("key2")); + Map propIds = response.getNameToIdsMap(); + Assert.assertEquals(2, propIds.size()); + Assert.assertEquals("id1", propIds.get("key1")); + Assert.assertEquals("id2", propIds.get("key2")); + } + @Test + public void idFieldAbsent() throws Exception { + List propertyList = new ArrayList<>(); + propertyList.add(new TestProperty("key1", "value3", "id3", "app1", "")); + // The next two properties are the only two that are actually resolved, as the other two are overridden due to + // the presence of the region field. + propertyList.add(new TestProperty("key1", "value1", "id1", "app1", "region1")); + propertyList.add(new TestProperty("key2", "value2", "id2", "app1", "region1")); + propertyList.add(new TestProperty("key2", "value4", "id4", "app1", "")); + TestPropertyList properties = new TestPropertyList(propertyList); + + JsonPersistedV2Reader reader = + JsonPersistedV2Reader.builder( + () -> new ByteArrayInputStream( + new ObjectMapper().writeValueAsBytes(properties))) + .withPath("propertiesList") + .build(); + + PollingResponse response = reader.call(); + Map props = response.getToAdd(); + Assert.assertEquals(2, props.size()); + Assert.assertEquals("value1", props.get("key1")); + Assert.assertEquals("value2", props.get("key2")); + Assert.assertTrue(response.getNameToIdsMap().isEmpty()); + } + + public static class TestPropertyList { + public List propertiesList; + public TestPropertyList(List propertiesList) { + this.propertiesList = propertiesList; + } + } + + public static class TestProperty { + public String key; + public String value; + public String propertyId; + public String appId; + public String region; + + public TestProperty(String key, String value, String propertyId, String appId, String region) { + this.key = key; + this.value = value; + this.propertyId = propertyId; + this.appId = appId; + this.region = region; + } + } +}