Merge pull request #698 from Netflix/1.x-instrumentation

Update instrumentation with more call sites and instrumentation clearing endpoint
Netflix · Jan 19, 2024 · ee954c2 · ee954c2
2 parents 45feec7 + 29d5b06
commit ee954c2
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 9 deletions.
diff --git a/archaius-core/src/main/java/com/netflix/config/ConcurrentCompositeConfiguration.java b/archaius-core/src/main/java/com/netflix/config/ConcurrentCompositeConfiguration.java
@@ -30,6 +30,7 @@
 import java.util.HashSet;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.commons.configuration.AbstractConfiguration;
 import org.apache.commons.configuration.Configuration;
@@ -109,15 +110,19 @@ public class ConcurrentCompositeConfiguration extends ConcurrentMapConfiguration
     private final boolean enableStackTrace = Boolean.parseBoolean(System.getProperty(ENABLE_STACK_TRACE));
     private final boolean enableInstrumentation = Boolean.parseBoolean(System.getProperty(ENABLE_INSTRUMENTATION));
 
-    private Map<String, AbstractConfiguration> namedConfigurations = new ConcurrentHashMap<String, AbstractConfiguration>();
+    private Map<String, AbstractConfiguration> namedConfigurations = new ConcurrentHashMap<>();
 
     private final Map<String, Integer> stackTraces = new ConcurrentHashMap<>();
-    private final Set<String> usedProperties = ConcurrentHashMap.newKeySet();
+    private final AtomicReference<Set<String>> usedPropertiesRef = new AtomicReference<>(ConcurrentHashMap.newKeySet());
 
     public Set<String> getUsedProperties() {
-        return usedProperties;
+        return Collections.unmodifiableSet(new HashSet<>(usedPropertiesRef.get()));
     }
 
+    public Set<String> getAndClearUsedProperties() {
+        Set<String> ret = usedPropertiesRef.getAndSet(ConcurrentHashMap.newKeySet());
+        return Collections.unmodifiableSet(ret);
+    }
 
     private List<AbstractConfiguration> configList = new CopyOnWriteArrayList<AbstractConfiguration>();
 
@@ -546,6 +551,9 @@ public Object getPropertyUninstrumented(String key) {
     private Object getProperty(String key, boolean instrument)
     {
         if (overrideProperties.containsKey(key)) {
+            if (instrument) {
+                recordUsage(key);
+            }
             return overrideProperties.getProperty(key);
         }
         Configuration firstMatchingConfiguration = null;
@@ -580,7 +588,7 @@ private Object getProperty(String key, boolean instrument)
      */
     public void recordUsage(String key) {
         if (enableInstrumentation) {
-            usedProperties.add(key);
+            usedPropertiesRef.get().add(key);
             if (enableStackTrace) {
                 String trace = Arrays.toString(Thread.currentThread().getStackTrace());
                 stackTraces.merge(trace, 1, (v1, v2) -> v1 + 1);
@@ -922,12 +930,13 @@ public Configuration getSource(String key)
      * @param config the configuration to query
      * @param key the key of the property
      */
-    private static void appendListProperty(List<Object> dest, Configuration config,
+    private void appendListProperty(List<Object> dest, Configuration config,
             String key)
     {
         Object value = config.getProperty(key);
         if (value != null)
         {
+            recordUsage(key);
             if (value instanceof Collection)
             {
                 Collection<?> col = (Collection<?>) value;

diff --git a/archaius-core/src/test/java/com/netflix/config/ConcurrentCompositeConfigurationTest.java b/archaius-core/src/test/java/com/netflix/config/ConcurrentCompositeConfigurationTest.java
@@ -18,6 +18,7 @@
 import static org.junit.Assert.*;
 
 import java.util.List;
+import java.util.Set;
 
 import org.apache.commons.configuration.AbstractConfiguration;
 import org.apache.commons.configuration.BaseConfiguration;
@@ -80,11 +81,24 @@ public void testInstrumentation() {
         config.addProperty("prop1", "val1");
         config.addProperty("prop2", "val2");
         assertEquals(config.getProperty("prop1"), "val1");
-        assertEquals(config.getUsedProperties().size(), 1);
-        assertEquals(config.getUsedProperties().iterator().next(), "prop1");
+
+        // Confirm that the usage is captured
+        Set<String> usedProperties = config.getUsedProperties();
+        assertEquals(usedProperties.size(), 1);
+        assertEquals(usedProperties.iterator().next(), "prop1");
+
+        // Confirm that an uninstrumented call is ignored
         assertEquals(config.getPropertyUninstrumented("prop2"), "val2");
-        assertEquals(config.getUsedProperties().size(), 1);
-        assertEquals(config.getUsedProperties().iterator().next(), "prop1");
+        usedProperties = config.getAndClearUsedProperties();
+        assertEquals(usedProperties.size(), 1);
+        assertEquals(usedProperties.iterator().next(), "prop1");
+
+        // Confirm that both usedProperties endpoints respect when the properties have been cleared
+        usedProperties = config.getUsedProperties();
+        assertTrue(usedProperties.isEmpty());
+
+        usedProperties = config.getAndClearUsedProperties();
+        assertTrue(usedProperties.isEmpty());
 
         System.clearProperty(ConcurrentCompositeConfiguration.ENABLE_INSTRUMENTATION);
     }