Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add configurable rate limiter for jar collector processing #183

Merged
merged 2 commits into from
Jan 20, 2021
Merged

add configurable rate limiter for jar collector processing #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f11a070
add configurable rate limiter for jar collector processing
jack-berg Jan 7, 2021
d3cc487
remove unused max_class_loaders config
jack-berg Jan 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

}
25 changes: 18 additions & 7 deletions newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfigImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
33 changes: 17 additions & 16 deletions ...c-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorServiceProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
*/
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we disable if 0 and use the default if negative?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could do that. I was thinking through that question myself and came to the conclusion that you could effectively disable by setting the config to really high number. Like, if someone set it to 1_000_000_000, there's no way the single thread processing these jars could run into that bound.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with leaving this behavior as is and providing public documentation for the jar_collector config so that customers can disable it if desired.

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) {
Expand All @@ -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;
Expand Down
Loading