feat(cache): metrics on putCacheResult. (spinnaker#2790)

Adds logging and metrics on putCacheResult capturing rate of change on a type from a specific agent.
cfieber · Jul 25, 2018 · e78a1ae · e78a1ae
1 parent 6510c29
commit e78a1ae
Showing 8 changed files with 149 additions and 25 deletions.
diff --git a/cats/cats-core/cats-core.gradle b/cats/cats-core/cats-core.gradle
@@ -1,6 +1,7 @@
 dependencies {
   compile spinnaker.dependency('slf4jApi')
   compile spinnaker.dependency('jacksonAnnotations')
+  compile spinnaker.dependency('spectatorApi')
 
   testCompile project(":cats:cats-test")
 }
diff --git a/cats/cats-core/src/main/java/com/netflix/spinnaker/cats/module/CatsModule.java b/cats/cats-core/src/main/java/com/netflix/spinnaker/cats/module/CatsModule.java
@@ -16,6 +16,8 @@
 
 package com.netflix.spinnaker.cats.module;
 
+import com.netflix.spectator.api.NoopRegistry;
+import com.netflix.spectator.api.Registry;
 import com.netflix.spinnaker.cats.agent.AgentScheduler;
 import com.netflix.spinnaker.cats.agent.CompositeExecutionInstrumentation;
 import com.netflix.spinnaker.cats.agent.DefaultAgentScheduler;
@@ -53,6 +55,7 @@ public static class Builder {
         private NamedCacheFactory cacheFactory;
         private AgentScheduler scheduler;
         private Collection<ExecutionInstrumentation> instrumentations = new LinkedList<>();
+        private Registry registry = new NoopRegistry();
 
         public Builder scheduler(AgentScheduler agentScheduler) {
             if (this.scheduler != null) {
@@ -87,6 +90,11 @@ public Builder cacheFactory(NamedCacheFactory namedCacheFactory) {
             return this;
         }
 
+        public Builder registry(Registry registry) {
+            this.registry = registry;
+            return this;
+        }
+
         public CatsModule build(Provider... providers) {
             return build(Arrays.asList(providers));
         }
@@ -106,7 +114,7 @@ public CatsModule build(Collection<Provider> providers) {
             if (cacheFactory == null) {
                 cacheFactory = new InMemoryNamedCacheFactory();
             }
-            return new DefaultCatsModule(providers, cacheFactory, scheduler, instrumentation);
+            return new DefaultCatsModule(providers, cacheFactory, scheduler, instrumentation, registry);
         }
     }
 

diff --git a/cats/cats-core/src/main/java/com/netflix/spinnaker/cats/module/DefaultCatsModule.java b/cats/cats-core/src/main/java/com/netflix/spinnaker/cats/module/DefaultCatsModule.java
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.cats.module;
 
+import com.netflix.spectator.api.Registry;
 import com.netflix.spinnaker.cats.agent.AgentController;
 import com.netflix.spinnaker.cats.agent.AgentScheduler;
 import com.netflix.spinnaker.cats.agent.ExecutionInstrumentation;
@@ -35,9 +36,9 @@ public class DefaultCatsModule implements CatsModule {
     private final Cache view;
     private final ExecutionInstrumentation executionInstrumentation;
 
-    public DefaultCatsModule(Collection<Provider> providers, NamedCacheFactory namedCacheFactory, AgentScheduler agentScheduler, ExecutionInstrumentation executionInstrumentation) {
+    public DefaultCatsModule(Collection<Provider> providers, NamedCacheFactory namedCacheFactory, AgentScheduler agentScheduler, ExecutionInstrumentation executionInstrumentation, Registry registry) {
         this.namedCacheFactory = namedCacheFactory;
-        providerRegistry = new DefaultProviderRegistry(providers, namedCacheFactory);
+        providerRegistry = new DefaultProviderRegistry(providers, namedCacheFactory, registry);
         this.agentScheduler = agentScheduler;
 
         if (agentScheduler instanceof CatsModuleAware) {

diff --git a/cats/cats-core/src/main/java/com/netflix/spinnaker/cats/provider/DefaultProviderCache.java b/cats/cats-core/src/main/java/com/netflix/spinnaker/cats/provider/DefaultProviderCache.java
@@ -16,11 +16,15 @@
 
 package com.netflix.spinnaker.cats.provider;
 
+import com.netflix.spectator.api.Counter;
+import com.netflix.spectator.api.Registry;
 import com.netflix.spinnaker.cats.agent.CacheResult;
 import com.netflix.spinnaker.cats.cache.CacheData;
 import com.netflix.spinnaker.cats.cache.CacheFilter;
 import com.netflix.spinnaker.cats.cache.DefaultCacheData;
 import com.netflix.spinnaker.cats.cache.WriteableCache;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -40,15 +44,19 @@
  */
 public class DefaultProviderCache implements ProviderCache {
 
+    private final Logger log = LoggerFactory.getLogger(DefaultProviderCache.class);
+
     private static final String ALL_ID = "_ALL_"; //dirty = true
     private static final Map<String, Object> ALL_ATTRIBUTE = Collections.unmodifiableMap(new HashMap<String, Object>(1) {{
         put("id", ALL_ID);
     }});
 
     private final WriteableCache backingStore;
+    private final Registry registry;
 
-    public DefaultProviderCache(WriteableCache backingStore) {
+    public DefaultProviderCache(WriteableCache backingStore, Registry registry) {
         this.backingStore = backingStore;
+        this.registry = registry;
     }
 
     @Override
@@ -133,23 +141,88 @@ public void putCacheResult(String sourceAgentType, Collection<String> authoritat
         Map<String, Collection<String>> evictions = new HashMap<>();
 
         for (String type : allTypes) {
-            final Collection<String> previousSet;
-            if (authoritativeTypes.contains(type)) {
-                previousSet = getExistingSourceIdentifiers(type, sourceAgentType);
-            } else {
-                previousSet = new HashSet<>();
-            }
+            final boolean authoritative = authoritativeTypes.contains(type);
+            final Set<String> previousIdentifiers = getExistingSourceIdentifiers(type, sourceAgentType);
+            final int originalSetSize = previousIdentifiers.size();
+            int newItems = 0;
+            final int cacheResultSize;
             if (cacheResult.getCacheResults().containsKey(type)) {
-                cacheDataType(type, sourceAgentType, cacheResult.getCacheResults().get(type));
-                for (CacheData data : cacheResult.getCacheResults().get(type)) {
-                    previousSet.remove(data.getId());
+                final Collection<CacheData> cacheResultsForType = cacheResult.getCacheResults().get(type);
+                cacheResultSize = cacheResultsForType.size();
+                cacheDataType(type, sourceAgentType, cacheResultsForType);
+                for (CacheData data : cacheResultsForType) {
+                    if (!previousIdentifiers.remove(data.getId())) {
+                      newItems++;
+                    }
                 }
+            } else {
+              cacheResultSize = 0;
             }
+            final Set<String> evictionsForType = new HashSet<>();
+            final int explicitEvictionSize;
             if (cacheResult.getEvictions().containsKey(type)) {
-              previousSet.addAll(cacheResult.getEvictions().get(type));
+                evictionsForType.addAll(cacheResult.getEvictions().get(type));
+                explicitEvictionSize = evictions.size();
+            } else {
+                explicitEvictionSize = 0;
             }
-            if (!previousSet.isEmpty()) {
-              evictions.put(type, previousSet);
+
+            final int noLongerPresentItemsCount;
+            if (authoritative) {
+                noLongerPresentItemsCount = previousIdentifiers.size();
+                evictionsForType.addAll(previousIdentifiers);
+            } else {
+                noLongerPresentItemsCount = -1;
+            }
+
+            if (!evictionsForType.isEmpty()) {
+                evictions.put(type, evictionsForType);
+            }
+
+            final int totalEvictions = evictionsForType.size();
+            final int changedItems = totalEvictions + newItems;
+            final int changedPercentage = originalSetSize > 0 ? (int) Math.round(((double) changedItems / (double) originalSetSize) * 100d) : -1;
+            final boolean highChangePercentage = changedPercentage > 5;
+            final String authoritativeLabel = authoritative ? "authoritative" : "informative";
+
+            Object[] params = {
+                authoritativeLabel,
+                sourceAgentType,
+                type,
+                originalSetSize,
+                cacheResultSize,
+                newItems,
+                explicitEvictionSize,
+                noLongerPresentItemsCount,
+                totalEvictions,
+                changedItems,
+                changedPercentage
+            };
+
+            Map<String, String> metricTags = new HashMap<>();
+            metricTags.put("authoritative", authoritativeLabel);
+            metricTags.put("sourceAgentType", sourceAgentType);
+            metricTags.put("type", type);
+            metricTags.put("highChangePercentage", Boolean.toString(highChangePercentage));
+
+            MetricBuilder mb = new MetricBuilder(registry, metricTags, "cats.defaultProviderCache.putCacheResult.");
+
+            mb.counter("cacheResultSize").increment(cacheResultSize);
+            mb.counter("newItems").increment(newItems);
+            mb.counter("explicitEvictions").increment(explicitEvictionSize);
+            if (authoritative) {
+                mb.counter("noLongerPresentItems").increment(noLongerPresentItemsCount);
+            }
+            mb.counter("totalEvictions").increment(totalEvictions);
+            mb.counter("changedItems").increment(changedItems);
+
+            if (changedItems > 0) {
+                if (highChangePercentage) {
+                    mb.counter("highChangePercentage").increment();
+                    log.warn("High changed percentage. " + DEBUG_METRICS, params);
+                } else {
+                    log.debug(DEBUG_METRICS, params);
+                }
             }
         }
 
@@ -158,6 +231,32 @@ public void putCacheResult(String sourceAgentType, Collection<String> authoritat
         }
     }
 
+    private static final String DEBUG_METRICS = "{} cache results for sourceAgentType {} type {}. " +
+        "originalDataSetSize={}," +
+        "cacheResultSize={}," +
+        "newItems={}," +
+        "explicitEvictions={}," +
+        "noLongerPresentItems={}," +
+        "totalEvictions={}," +
+        "changedItems={}," +
+        "changedPercentage={}";
+
+    private static class MetricBuilder {
+        private final Registry registry;
+        private final Map<String, String> baseTags;
+        private final String metricPrefix;
+
+        public MetricBuilder(Registry registry, Map<String, String> baseTags, String metricPrefix) {
+            this.registry = registry;
+            this.baseTags = baseTags;
+            this.metricPrefix = metricPrefix;
+        }
+
+        public Counter counter(String metricName) {
+            return registry.counter(registry.createId(metricPrefix + metricName, baseTags));
+        }
+    }
+
     @Override
     public void putCacheData(String sourceAgentType, CacheData cacheData) {
         backingStore.merge(sourceAgentType, cacheData);
@@ -193,7 +292,7 @@ private Collection<CacheData> buildResponse(Collection<CacheData> source) {
         return Collections.unmodifiableCollection(response);
     }
 
-    private Collection<String> getExistingSourceIdentifiers(String type, String sourceAgentType) {
+    private Set<String> getExistingSourceIdentifiers(String type, String sourceAgentType) {
         CacheData all = backingStore.get(type, ALL_ID);
         if (all == null) {
             return new HashSet<>();
@@ -202,7 +301,10 @@ private Collection<String> getExistingSourceIdentifiers(String type, String sour
         if (relationship == null) {
             return new HashSet<>();
         }
-        return relationship;
+        if (relationship instanceof Set) {
+          return (Set<String>) relationship;
+        }
+        return new HashSet<>(relationship);
     }
 
     private void cacheDataType(String type, String sourceAgentType, Collection<CacheData> items) {

diff --git a/.../cats-core/src/main/java/com/netflix/spinnaker/cats/provider/DefaultProviderRegistry.java b/.../cats-core/src/main/java/com/netflix/spinnaker/cats/provider/DefaultProviderRegistry.java
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.cats.provider;
 
+import com.netflix.spectator.api.Registry;
 import com.netflix.spinnaker.cats.cache.Cache;
 import com.netflix.spinnaker.cats.cache.NamedCacheFactory;
 
@@ -28,10 +29,10 @@ public class DefaultProviderRegistry implements ProviderRegistry {
     private final ConcurrentMap<String, ProviderCache> providerCaches = new ConcurrentHashMap<>();
     private final Collection<Provider> providers;
 
-    public DefaultProviderRegistry(Collection<Provider> providers, NamedCacheFactory cacheFactory) {
+    public DefaultProviderRegistry(Collection<Provider> providers, NamedCacheFactory cacheFactory, Registry registry) {
         this.providers = Collections.unmodifiableCollection(providers);
         for (Provider provider : providers) {
-            providerCaches.put(provider.getProviderName(), new DefaultProviderCache(cacheFactory.getCache(provider.getProviderName())));
+            providerCaches.put(provider.getProviderName(), new DefaultProviderCache(cacheFactory.getCache(provider.getProviderName()), registry));
         }
     }
 

diff --git a/...-core/src/test/groovy/com/netflix/spinnaker/cats/provider/DefaultProviderCacheSpec.groovy b/...-core/src/test/groovy/com/netflix/spinnaker/cats/provider/DefaultProviderCacheSpec.groovy
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.cats.provider
 
+import com.netflix.spectator.api.NoopRegistry
 import com.netflix.spinnaker.cats.agent.CacheResult
 import com.netflix.spinnaker.cats.agent.DefaultCacheResult
 import com.netflix.spinnaker.cats.cache.*
@@ -28,7 +29,7 @@ class DefaultProviderCacheSpec extends CacheSpec {
     @Override
     Cache getSubject() {
         backingStore = new InMemoryCache()
-        new DefaultProviderCache(backingStore)
+        new DefaultProviderCache(backingStore, new NoopRegistry())
     }
 
     void populateOne(String type, String id, CacheData cacheData = createData(id)) {

diff --git a/clouddriver-core/src/main/groovy/com/netflix/spinnaker/clouddriver/cache/CacheConfig.groovy b/clouddriver-core/src/main/groovy/com/netflix/spinnaker/clouddriver/cache/CacheConfig.groovy
@@ -74,8 +74,17 @@ class CacheConfig {
 
   @Bean
   @ConditionalOnMissingBean(CatsModule)
-  CatsModule catsModule(List<Provider> providers, List<ExecutionInstrumentation> executionInstrumentation, NamedCacheFactory cacheFactory, AgentScheduler agentScheduler) {
-    new CatsModule.Builder().cacheFactory(cacheFactory).scheduler(agentScheduler).instrumentation(executionInstrumentation).build(providers)
+  CatsModule catsModule(List<Provider> providers,
+                        List<ExecutionInstrumentation> executionInstrumentation,
+                        NamedCacheFactory cacheFactory,
+                        AgentScheduler agentScheduler,
+                        Registry registry) {
+    return new CatsModule.Builder()
+      .cacheFactory(cacheFactory)
+      .scheduler(agentScheduler)
+      .instrumentation(executionInstrumentation)
+      .registry(registry)
+      .build(providers)
   }
 
   @Bean

diff --git a/...urity/src/test/groovy/com/netflix/spinnaker/clouddriver/security/ProviderUtilsSpec.groovy b/...urity/src/test/groovy/com/netflix/spinnaker/clouddriver/security/ProviderUtilsSpec.groovy
@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.clouddriver.security
 
+import com.netflix.spectator.api.NoopRegistry
 import com.netflix.spinnaker.cats.agent.NoopExecutionInstrumentation
 import com.netflix.spinnaker.cats.mem.InMemoryNamedCacheFactory
 import com.netflix.spinnaker.cats.module.DefaultCatsModule
@@ -119,7 +120,7 @@ class ProviderUtilsSpec extends Specification {
       def executionInstrumentation = new NoopExecutionInstrumentation()
 
     when:
-      new DefaultCatsModule([agentSchedulerAwareProvider], namedCacheFactory, scheduler, executionInstrumentation)
+      new DefaultCatsModule([agentSchedulerAwareProvider], namedCacheFactory, scheduler, executionInstrumentation, new NoopRegistry())
 
     then:
       scheduler.scheduled.collect { it.agent } == [testAgent1]
@@ -162,7 +163,7 @@ class ProviderUtilsSpec extends Specification {
       def catsModule
 
     when:
-      catsModule = new DefaultCatsModule([agentSchedulerAwareProvider], namedCacheFactory, scheduler, executionInstrumentation)
+      catsModule = new DefaultCatsModule([agentSchedulerAwareProvider], namedCacheFactory, scheduler, executionInstrumentation, new NoopRegistry())
 
     then:
       scheduler.scheduled.collect { it.agent } == [testAgent1, testAgent2, testAgent3, testAgent4, testAgent5]