Merge pull request #183 from newrelic/rate-limit-jar-collector

add configurable rate limiter for jar collector processing

newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfig.java

@@ -19,10 +19,17 @@ public interface JarCollectorConfig {
     boolean isEnabled();
 
     /**
-     * The number of class loaders which we should grab jars from.
+     * True if temp jars should be skipped.
      *
-     * @return The max number of class loaders to look at for jar information.
+     * @return <code>true</code> if temp jars should be skipped.
      */
-    int getMaxClassLoaders();
+    boolean skipTempJars();
+
+    /**
+     * The maximum number of jars to process per second. Must be positive.
+     *
+     * @return The number of jars to process per second.
+     */
+    int getJarsPerSecond();
 
 }

newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfigImpl.java

@@ -13,22 +13,28 @@
 
 public class JarCollectorConfigImpl extends BaseConfig implements JarCollectorConfig {
 
-    public static final Integer DEFAULT_MAX_CLASS_LOADERS = 5000;
     public static final String ENABLED = "enabled";
-    public static final String MAX_CLASS_LOADERS = "max_class_loaders";
-    public static final Boolean DEFAULT_ENABLED = Boolean.TRUE;
+    public static final String SKIP_TEMP_JARS = "skip_temp_jars";
+    public static final String JARS_PER_SECOND = "jars_per_second";
+
+    public static final boolean DEFAULT_ENABLED = Boolean.TRUE;
+    public static final boolean DEFAULT_SKIP_TEMP_JARS = Boolean.TRUE;
+    public static final int DEFAULT_JARS_PER_SECOND = 10;
+
     // The newrelic.config.module root shouldn't be used but is kept for backwards compatibility
     public static final String SYSTEM_PROPERTY_ROOT_DEPRECATED = "newrelic.config.module."; // NEW_RELIC_MODULE_
     public static final String SYSTEM_PROPERTY_ROOT = "newrelic.config.jar_collector."; // NEW_RELIC_JAR_COLLECTOR_
     private static final AtomicBoolean isUsingDeprecatedConfigSettings = new AtomicBoolean(false);
 
     private final boolean isEnabled;
-    private final int maxClassLoaders;
+    private final boolean skipTempJars;
+    private final Integer jarsPerSecond;
 
     public JarCollectorConfigImpl(Map<String, Object> pProps) {
         super(pProps, SYSTEM_PROPERTY_ROOT);
         isEnabled = getProperty(ENABLED, DEFAULT_ENABLED);
-        maxClassLoaders = getProperty(MAX_CLASS_LOADERS, DEFAULT_MAX_CLASS_LOADERS);
+        skipTempJars = getProperty(SKIP_TEMP_JARS, DEFAULT_SKIP_TEMP_JARS);
+        jarsPerSecond = getProperty(JARS_PER_SECOND, DEFAULT_JARS_PER_SECOND);
     }
 
     // This method gets hit multiple times due to merging local and server side configs
@@ -45,8 +51,13 @@ public boolean isEnabled() {
     }
 
     @Override
-    public int getMaxClassLoaders() {
-        return maxClassLoaders;
+    public boolean skipTempJars() {
+        return skipTempJars;
+    }
+
+    @Override
+    public int getJarsPerSecond() {
+        return jarsPerSecond;
     }
 
     @Override

newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorServiceProcessor.java

@@ -7,11 +7,12 @@
 
 package com.newrelic.agent.service.module;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
+import com.google.common.util.concurrent.RateLimiter;
 import com.newrelic.Function;
 import com.newrelic.agent.bridge.ManifestUtils;
 import com.newrelic.agent.config.AgentConfig;
-import com.newrelic.api.agent.Config;
 import com.newrelic.api.agent.Logger;
 
 import java.io.File;
@@ -29,6 +30,8 @@
 import java.util.jar.Manifest;
 import java.util.logging.Level;
 
+import static com.newrelic.agent.config.JarCollectorConfigImpl.DEFAULT_JARS_PER_SECOND;
+
 /**
  * Attempts to open jars and obtain version information from manifests.
  */
@@ -55,32 +58,29 @@ public class JarCollectorServiceProcessor implements Function<URL, JarData> {
                     Attributes.Name.IMPLEMENTATION_VENDOR.toString(),
                     Attributes.Name.IMPLEMENTATION_VENDOR_ID.toString());
 
-    private final boolean skipTempJars; // default true
     private final Logger logger;
-
-    /**
-     * The list of jars to ignore.
-     */
+    private final boolean skipTempJars;
     private final List<String> ignoreJars;
+    private final RateLimiter processUrlRateLimiter;
 
     public JarCollectorServiceProcessor(Logger logger, AgentConfig agentConfig) {
-        this(agentConfig, agentConfig.getIgnoreJars(), logger);
-    }
-
-    /**
-     * Creates this JarCollectorServiceProcessor.
-     */
-    JarCollectorServiceProcessor(Config config, List<String> ignoreJars, Logger logger) {
-        this.ignoreJars = new ArrayList<>(ignoreJars);
         this.logger = logger;
-        this.skipTempJars = config.getValue("jar_collector.skip_temp_jars", true);
+        this.skipTempJars = agentConfig.getJarCollectorConfig().skipTempJars();
         if (!skipTempJars) {
             logger.log(Level.FINEST, "temporary jars will be transmitted to the host");
         }
+        this.ignoreJars = new ArrayList<>(agentConfig.getIgnoreJars());
+        int jarsPerSecond = agentConfig.getJarCollectorConfig().getJarsPerSecond();
+        if (jarsPerSecond <= 0) {
+            logger.log(Level.INFO, "Jars per second must be greater than 0. Defaulting to {0}.", DEFAULT_JARS_PER_SECOND);
+            jarsPerSecond = DEFAULT_JARS_PER_SECOND;
+        }
+        this.processUrlRateLimiter = RateLimiter.create(jarsPerSecond);
     }
 
     @Override
     public JarData apply(URL url) {
+        processUrlRateLimiter.acquire(1);
         try {
             return tryProcessSingleURL(url);
         } catch (Throwable t) {
@@ -89,7 +89,8 @@ public JarData apply(URL url) {
         }
     }
 
-    private JarData tryProcessSingleURL(URL url) throws URISyntaxException {
+    @VisibleForTesting
+    JarData tryProcessSingleURL(URL url) throws URISyntaxException {
         if (skipTempJars && isTempFile(url)) {
             logger.log(Level.FINE, "{0} Skipping temp jar file", url);
             return null;