Merge pull request #651 from Netflix/instrumentation

Property usage instrumentation initial framework

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 {

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.
+     * <p>
+     * Visitors should not have consequences based on specific key-value pairs and in general
+     * should be used primarily for logging purposes.
+     * <p>
+     * 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 <T>
+     */
     interface Visitor<T> {
         T visitKey(String key, Object value);
     }
@@ -51,9 +65,31 @@ interface Visitor<T> {
      */
     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<Object> getProperty(String key) { return Optional.ofNullable(getRawProperty(key)); }
-
+
+    @Override
+    default Optional<Object> 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<T> {
     default Iterable<String> keys() {
         return this::getKeys;
     }
-    
+
     /**
      * @return Return an interator to all prefixed property names owned by this config
      */

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 + "]";
+    }
+}

archaius2-api/src/main/java/com/netflix/archaius/api/PropertySource.java

@@ -13,12 +13,28 @@ public interface PropertySource {
      */
     default Optional<Object> getProperty(String key) { return Optional.empty(); }
 
+    /**
+     * Get the raw property value, but do not record any usage data.
+     * @param key
+     */
+    default Optional<Object> getPropertyUninstrumented(String key) {
+        return getProperty(key);
+    }
+
     /**
      * Mechanism for consuming all properties of the PropertySource
      * @param consumer
      */
     default void forEachProperty(BiConsumer<String, Object> consumer) {}
 
+    /**
+     * Mechanism for consuming all properties of the PropertySource that also avoids any usage tracking
+     * @param consumer
+     */
+    default void forEachPropertyUninstrumented(BiConsumer<String, Object> consumer) {
+        forEachProperty(consumer);
+    }
+
     /**
      * @return Name used to identify the source such as a filename.
      */

archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java

@@ -239,7 +239,9 @@ public Config getPrivateView() {
 
     @Override
     public <T> T accept(Visitor<T> 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;
     }
 

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<String> getKeys() {
+        return getState().getData().keySet().iterator();
+    }
+
+    @Override
+    public Iterable<String> keys() {
+        return getState().getData().keySet();
+    }
+
+    @Override
+    public void forEachProperty(BiConsumer<String, Object> 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<String, Object> 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);
+    }
+}

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<String, Object> data;
+    private final Map<String, Config> instrumentedKeys;
+
+    CachedState(Map<String, Object> data, Map<String, Config> instrumentedKeys) {
+        this.data = Collections.unmodifiableMap(data);
+        this.instrumentedKeys = Collections.unmodifiableMap(instrumentedKeys);
+    }
+
+    Map<String, Object> getData() {
+        return data;
+    }
+
+    Map<String, Config> getInstrumentedKeys() {
+        return instrumentedKeys;
+    }
+}