From f11a0700379e0daff77bdd171c9c29a928a5ff1f Mon Sep 17 00:00:00 2001 From: jack-berg <34418638+jack-berg@users.noreply.github.com> Date: Thu, 7 Jan 2021 09:11:09 -0600 Subject: [PATCH 1/2] add configurable rate limiter for jar collector processing --- .../agent/config/JarCollectorConfig.java | 14 ++++ .../agent/config/JarCollectorConfigImpl.java | 24 ++++++- .../module/JarCollectorServiceProcessor.java | 33 ++++----- .../config/JarCollectorConfigImplTest.java | 2 +- .../JarCollectorServiceProcessorTest.java | 69 +++++++++++++------ 5 files changed, 101 insertions(+), 41 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfig.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfig.java index 3d7d86d222..90439ede52 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfig.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfig.java @@ -18,6 +18,13 @@ public interface JarCollectorConfig { */ boolean isEnabled(); + /** + * True if temp jars should be skipped. + * + * @return true if temp jars should be skipped. + */ + boolean skipTempJars(); + /** * The number of class loaders which we should grab jars from. * @@ -25,4 +32,11 @@ public interface JarCollectorConfig { */ int getMaxClassLoaders(); + /** + * The maximum number of jars to process per second. Must be positive. + * + * @return The number of jars to process per second. + */ + int getJarsPerSecond(); + } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfigImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfigImpl.java index 1d2d1a7371..abac91c43d 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfigImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfigImpl.java @@ -13,22 +13,32 @@ 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 SKIP_TEMP_JARS = "skip_temp_jars"; public static final String MAX_CLASS_LOADERS = "max_class_loaders"; - public static final Boolean DEFAULT_ENABLED = Boolean.TRUE; + 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_MAX_CLASS_LOADERS = 5000; + 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 boolean skipTempJars; private final int maxClassLoaders; + private final Integer jarsPerSecond; public JarCollectorConfigImpl(Map pProps) { super(pProps, SYSTEM_PROPERTY_ROOT); isEnabled = getProperty(ENABLED, DEFAULT_ENABLED); + skipTempJars = getProperty(SKIP_TEMP_JARS, DEFAULT_SKIP_TEMP_JARS); maxClassLoaders = getProperty(MAX_CLASS_LOADERS, DEFAULT_MAX_CLASS_LOADERS); + jarsPerSecond = getProperty(JARS_PER_SECOND, DEFAULT_JARS_PER_SECOND); } // This method gets hit multiple times due to merging local and server side configs @@ -44,11 +54,21 @@ public boolean isEnabled() { return isEnabled; } + @Override + public boolean skipTempJars() { + return skipTempJars; + } + @Override public int getMaxClassLoaders() { return maxClassLoaders; } + @Override + public int getJarsPerSecond() { + return jarsPerSecond; + } + @Override protected Object getPropertyFromSystemEnvironment(String name, Object defaultVal) { return getMergedValue(name, true); diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorServiceProcessor.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorServiceProcessor.java index 1d5a9757b8..0c6848dcc8 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorServiceProcessor.java +++ b/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 { 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 ignoreJars; + private final RateLimiter processUrlRateLimiter; public JarCollectorServiceProcessor(Logger logger, AgentConfig agentConfig) { - this(agentConfig, agentConfig.getIgnoreJars(), logger); - } - - /** - * Creates this JarCollectorServiceProcessor. - */ - JarCollectorServiceProcessor(Config config, List 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; diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/config/JarCollectorConfigImplTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/config/JarCollectorConfigImplTest.java index defbe9612b..be0f1173d9 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/config/JarCollectorConfigImplTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/config/JarCollectorConfigImplTest.java @@ -58,7 +58,7 @@ public void testDefaultConfigValues() { jarCollectorConfig = new JarCollectorConfigImpl(configProps); assertEquals(DEFAULT_ENABLED, jarCollectorConfig.isEnabled()); - assertEquals(DEFAULT_MAX_CLASS_LOADERS.intValue(), jarCollectorConfig.getMaxClassLoaders()); + assertEquals(DEFAULT_MAX_CLASS_LOADERS, jarCollectorConfig.getMaxClassLoaders()); } @Test diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceProcessorTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceProcessorTest.java index 75711c0830..b9804f943b 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceProcessorTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceProcessorTest.java @@ -8,10 +8,11 @@ package com.newrelic.agent.service.module; import com.google.common.io.ByteStreams; -import com.newrelic.api.agent.Config; +import com.newrelic.agent.config.AgentConfig; +import com.newrelic.agent.config.JarCollectorConfig; import com.newrelic.api.agent.Logger; import org.junit.Test; -import org.mockito.internal.stubbing.answers.ReturnsArgumentAt; +import org.mockito.ArgumentMatchers; import javax.servlet.jsp.JspPage; import java.io.File; @@ -29,9 +30,11 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.anyBoolean; -import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class JarCollectorServiceProcessorTest { @@ -68,7 +71,7 @@ static URL getURL(String path) { } static URL getJarURLInsideWar() throws IOException { - File tmpFile = File.createTempFile("embedded_war", '.' + "war"); + File tmpFile = File.createTempFile("embedded_war", ".war"); tmpFile.deleteOnExit(); URL embeddedJarURL = JarCollectorServiceProcessorTest.getURL(EMBEDDED_JAR); try (FileOutputStream out = new FileOutputStream(tmpFile); @@ -83,15 +86,35 @@ static URL getEmbeddedJarURL() throws MalformedURLException { return new URL(JarCollectorServiceProcessorTest.getURL(EMBEDDED_JAR).toExternalForm() + "!/lib/test-jar1-1.2.3.jar"); } - private Config getMockConfig() { - Config mockConfig = mock(Config.class); - when(mockConfig.getValue(anyString(), anyBoolean())).thenAnswer(new ReturnsArgumentAt(-1)); - return mockConfig; + private AgentConfig getMockConfig() { + AgentConfig agentConfig = mock(AgentConfig.class); + JarCollectorConfig jarCollectorConfig = mock(JarCollectorConfig.class); + when(agentConfig.getJarCollectorConfig()).thenReturn(jarCollectorConfig); + return agentConfig; + } + + @Test + public void applyWithRateLimit() throws URISyntaxException { + AgentConfig config = getMockConfig(); + when(config.getJarCollectorConfig().getJarsPerSecond()).thenReturn(10); + JarCollectorServiceProcessor target = spy(new JarCollectorServiceProcessor(mock(Logger.class), config)); + doReturn(mock(JarData.class)).when(target).tryProcessSingleURL(ArgumentMatchers.any()); + + long startMillis = System.currentTimeMillis(); + for (int i = 0; i < 50; i++) { + target.apply(getURL(TXT_FILE)); + } + long elapsedMillis = System.currentTimeMillis() - startMillis; + + // 50 urls at 10 per second should take 5 second, but the rate limiter has a warm up period + // preventing perfect accuracy so we give it some leniency for more reliable testing. + assertTrue(elapsedMillis > 4000); + verify(target, times(50)).apply(ArgumentMatchers.any()); } @Test public void parseJarNameWithJarProtocol() throws MalformedURLException, URISyntaxException { - JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); URL url = new URL( "jar:file:/Users/sdaubin/servers/jboss-as-7.1.1.Final/modules/org/apache/xerces/main/xercesImpl-2.9.1-jbossas-1.jar!/"); assertEquals("xercesImpl-2.9.1-jbossas-1.jar", target.parseJarName(url)); @@ -99,7 +122,7 @@ public void parseJarNameWithJarProtocol() throws MalformedURLException, URISynta @Test public void parseJarNameWithoutJarProtocolCurrentDir() throws MalformedURLException, URISyntaxException { - JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); URL url = new URL( "ftp:xercesImpl-2.9.1-jbossas-1.jar!/"); assertEquals("xercesImpl-2.9.1-jbossas-1.jar", target.parseJarName(url)); @@ -107,7 +130,7 @@ public void parseJarNameWithoutJarProtocolCurrentDir() throws MalformedURLExcept @Test public void parseJarNameWithoutJarProtocolRootDir() throws MalformedURLException, URISyntaxException { - JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); URL url = new URL( "ftp:/xercesImpl-2.9.1-jbossas-1.jar!/"); assertEquals("xercesImpl-2.9.1-jbossas-1.jar", target.parseJarName(url)); @@ -117,7 +140,7 @@ public void parseJarNameWithoutJarProtocolRootDir() throws MalformedURLException public void testProcessJar1() { URL jarURL = ClassLoader.getSystemClassLoader().getResource(JAR_PATH); - JarCollectorServiceProcessor task = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor task = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); JarData jarData = task.apply(jarURL); assertEquals("jarTest.jar", jarData.getName()); @@ -128,7 +151,7 @@ public void testProcessJar1() { public void testProcessJar2() { URL jarURL = getURL(JAR_PATH_2); - JarCollectorServiceProcessor task = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor task = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); JarData jarData = task.apply(jarURL); assertEquals("anotherJar.jar", jarData.getName()); assertEquals("5.0", jarData.getVersion()); @@ -148,7 +171,7 @@ public void isTemp() throws IOException { @Test public void embeddedJar() throws IOException { - JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); JarInfo jarInfo = target.getJarInfoSafe(getEmbeddedJarURL()); assertEquals("1.2.3", jarInfo.version); @@ -160,7 +183,7 @@ public void embeddedJar() throws IOException { @Test public void embeddedWar() throws IOException { - JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); URL url = getJarURLInsideWar(); assertTrue(url.toString().contains(".war!/")); JarInfo jarInfo = target.getJarInfoSafe(url); @@ -174,7 +197,7 @@ public void embeddedWar() throws IOException { @Test public void getJarInfo_withoutPom() { - JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); JarInfo jarInfo = target.getJarInfoSafe(getURL(JAR_PATH)); assertEquals("b82b735bc9ddee35c7fe6780d68f4a0256c4bd7a", jarInfo.attributes.get("sha1Checksum")); @@ -184,7 +207,7 @@ public void getJarInfo_withoutPom() { @Test public void getJarInfo_noVersion() { - JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); JarInfo jarInfo = target.getJarInfoSafe(getURL(THREADILIZER_PATH)); assertEquals("2cd63bbdc83562c6a26d7c96f13d11522541e352", jarInfo.attributes.get("sha1Checksum")); @@ -193,7 +216,7 @@ public void getJarInfo_noVersion() { @Test public void getJarInfo_withPomProperties() { - JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor target = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); JarInfo jarInfo = target.getJarInfoSafe(getURL(POM_PROPS_JAR_PATH)); assertEquals("com.newrelic.pom.props", jarInfo.attributes.get("groupId")); assertEquals("pom-props", jarInfo.attributes.get("artifactId")); @@ -209,18 +232,20 @@ public void testProcessEmptyJar() throws Exception { JarOutputStream out = new JarOutputStream(new FileOutputStream(jar)); out.close(); - JarCollectorServiceProcessor processor = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor processor = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); assertNotNull(processor.addJarAndVersion(jar.toURI().toURL(), null)); // this hits a slightly different code path in addJarAndVersion - processor = new JarCollectorServiceProcessor(getMockConfig(), Collections.singletonList(jar.getName()), mock(Logger.class)); + AgentConfig agentConfig = getMockConfig(); + when(agentConfig.getIgnoreJars()).thenReturn(Collections.singletonList(jar.getName())); + processor = new JarCollectorServiceProcessor(mock(Logger.class), agentConfig); assertNull(processor.addJarAndVersion(jar.toURI().toURL(), null)); } @Test public void textFilesReturnNull() { URL txtURL = getURL(TXT_FILE); - JarCollectorServiceProcessor task = new JarCollectorServiceProcessor(getMockConfig(), Collections.emptyList(), mock(Logger.class)); + JarCollectorServiceProcessor task = new JarCollectorServiceProcessor(mock(Logger.class), getMockConfig()); assertNull(task.apply(txtURL)); } From d3cc487be71240db8829995be023c593416b5618 Mon Sep 17 00:00:00 2001 From: jack-berg <34418638+jack-berg@users.noreply.github.com> Date: Wed, 20 Jan 2021 09:49:52 -0600 Subject: [PATCH 2/2] remove unused max_class_loaders config --- .../agent/config/JarCollectorConfig.java | 7 - .../agent/config/JarCollectorConfigImpl.java | 9 -- .../config/JarCollectorConfigImplTest.java | 138 ++++++++---------- 3 files changed, 58 insertions(+), 96 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfig.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfig.java index 90439ede52..50435cf5f6 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfig.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfig.java @@ -25,13 +25,6 @@ public interface JarCollectorConfig { */ boolean skipTempJars(); - /** - * The number of class loaders which we should grab jars from. - * - * @return The max number of class loaders to look at for jar information. - */ - int getMaxClassLoaders(); - /** * The maximum number of jars to process per second. Must be positive. * diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfigImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfigImpl.java index abac91c43d..c86ceaddb1 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfigImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/config/JarCollectorConfigImpl.java @@ -15,12 +15,10 @@ public class JarCollectorConfigImpl extends BaseConfig implements JarCollectorCo public static final String ENABLED = "enabled"; public static final String SKIP_TEMP_JARS = "skip_temp_jars"; - public static final String MAX_CLASS_LOADERS = "max_class_loaders"; 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_MAX_CLASS_LOADERS = 5000; public static final int DEFAULT_JARS_PER_SECOND = 10; // The newrelic.config.module root shouldn't be used but is kept for backwards compatibility @@ -30,14 +28,12 @@ public class JarCollectorConfigImpl extends BaseConfig implements JarCollectorCo private final boolean isEnabled; private final boolean skipTempJars; - private final int maxClassLoaders; private final Integer jarsPerSecond; public JarCollectorConfigImpl(Map pProps) { super(pProps, SYSTEM_PROPERTY_ROOT); isEnabled = getProperty(ENABLED, DEFAULT_ENABLED); skipTempJars = getProperty(SKIP_TEMP_JARS, DEFAULT_SKIP_TEMP_JARS); - maxClassLoaders = getProperty(MAX_CLASS_LOADERS, DEFAULT_MAX_CLASS_LOADERS); jarsPerSecond = getProperty(JARS_PER_SECOND, DEFAULT_JARS_PER_SECOND); } @@ -59,11 +55,6 @@ public boolean skipTempJars() { return skipTempJars; } - @Override - public int getMaxClassLoaders() { - return maxClassLoaders; - } - @Override public int getJarsPerSecond() { return jarsPerSecond; diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/config/JarCollectorConfigImplTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/config/JarCollectorConfigImplTest.java index be0f1173d9..a285eefbce 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/config/JarCollectorConfigImplTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/config/JarCollectorConfigImplTest.java @@ -16,34 +16,29 @@ import java.util.HashMap; import java.util.Properties; +import static com.newrelic.agent.SaveSystemPropertyProviderRule.TestEnvironmentFacade; +import static com.newrelic.agent.SaveSystemPropertyProviderRule.TestSystemProps; import static com.newrelic.agent.config.JarCollectorConfigImpl.DEFAULT_ENABLED; -import static com.newrelic.agent.config.JarCollectorConfigImpl.DEFAULT_MAX_CLASS_LOADERS; +import static com.newrelic.agent.config.JarCollectorConfigImpl.DEFAULT_JARS_PER_SECOND; +import static com.newrelic.agent.config.JarCollectorConfigImpl.DEFAULT_SKIP_TEMP_JARS; import static com.newrelic.agent.config.JarCollectorConfigImpl.ENABLED; -import static com.newrelic.agent.config.JarCollectorConfigImpl.MAX_CLASS_LOADERS; +import static com.newrelic.agent.config.JarCollectorConfigImpl.JARS_PER_SECOND; +import static com.newrelic.agent.config.JarCollectorConfigImpl.SKIP_TEMP_JARS; import static com.newrelic.agent.config.JarCollectorConfigImpl.SYSTEM_PROPERTY_ROOT; import static com.newrelic.agent.config.JarCollectorConfigImpl.SYSTEM_PROPERTY_ROOT_DEPRECATED; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; public class JarCollectorConfigImplTest { private JarCollectorConfig jarCollectorConfig; private HashMap configProps; // Preferred environment variables - private final String NEW_RELIC_JAR_COLLECTOR_ENABLED = "NEW_RELIC_JAR_COLLECTOR_ENABLED"; - private final String NEW_RELIC_JAR_COLLECTOR_MAX_CLASS_LOADERS = "NEW_RELIC_JAR_COLLECTOR_MAX_CLASS_LOADERS"; + private static final String NEW_RELIC_JAR_COLLECTOR_ENABLED = "NEW_RELIC_JAR_COLLECTOR_ENABLED"; + private static final String NEW_RELIC_JAR_COLLECTOR_SKIP_TEMP_JARS = "NEW_RELIC_JAR_COLLECTOR_SKIP_TEMP_JARS"; + private static final String NEW_RELIC_JAR_COLLECTOR_JARS_PER_SECOND = "NEW_RELIC_JAR_COLLECTOR_JARS_PER_SECOND"; // Deprecated environment variables - private final String NEW_RELIC_MODULE_ENABLED = "NEW_RELIC_MODULE_ENABLED"; - private final String NEW_RELIC_MODULE_MAX_CLASS_LOADERS = "NEW_RELIC_MODULE_MAX_CLASS_LOADERS"; - - // Preferred system properties - private final String enabledSystemProperty = SYSTEM_PROPERTY_ROOT + ENABLED; - private final String maxClassLoadersSystemProperty = SYSTEM_PROPERTY_ROOT + MAX_CLASS_LOADERS; - - // Deprecated system properties - private final String enabledSystemPropertyDeprecated = SYSTEM_PROPERTY_ROOT_DEPRECATED + ENABLED; - private final String maxClassLoadersSystemPropertyDeprecated = SYSTEM_PROPERTY_ROOT_DEPRECATED + MAX_CLASS_LOADERS; + private static final String NEW_RELIC_MODULE_ENABLED = "NEW_RELIC_MODULE_ENABLED"; @Rule public SaveSystemPropertyProviderRule saveSystemPropertyProviderRule = new SaveSystemPropertyProviderRule(); @@ -58,159 +53,142 @@ public void testDefaultConfigValues() { jarCollectorConfig = new JarCollectorConfigImpl(configProps); assertEquals(DEFAULT_ENABLED, jarCollectorConfig.isEnabled()); - assertEquals(DEFAULT_MAX_CLASS_LOADERS, jarCollectorConfig.getMaxClassLoaders()); + assertEquals(DEFAULT_SKIP_TEMP_JARS, jarCollectorConfig.skipTempJars()); + assertEquals(DEFAULT_JARS_PER_SECOND, jarCollectorConfig.getJarsPerSecond()); } @Test public void testLocalConfigValues() { // Local config props - configProps.put(ENABLED, true); - configProps.put(MAX_CLASS_LOADERS, 42); + configProps.put(ENABLED, !DEFAULT_ENABLED); + configProps.put(SKIP_TEMP_JARS, !DEFAULT_SKIP_TEMP_JARS); + configProps.put(JARS_PER_SECOND, 5); jarCollectorConfig = new JarCollectorConfigImpl(configProps); - assertEquals(configProps.get(ENABLED), jarCollectorConfig.isEnabled()); - assertEquals(configProps.get(MAX_CLASS_LOADERS), jarCollectorConfig.getMaxClassLoaders()); + assertEquals(!DEFAULT_ENABLED, jarCollectorConfig.isEnabled()); + assertEquals(!DEFAULT_SKIP_TEMP_JARS, jarCollectorConfig.skipTempJars()); + assertEquals(5, jarCollectorConfig.getJarsPerSecond()); } @Test public void testServerConfigValues() { // Server config props - ServerProp enabledServerProp = ServerProp.createPropObject(!DEFAULT_ENABLED); - ServerProp maxClassLoadersServerProp = ServerProp.createPropObject(DEFAULT_MAX_CLASS_LOADERS - 1000); - - configProps.put(ENABLED, enabledServerProp); - configProps.put(MAX_CLASS_LOADERS, maxClassLoadersServerProp); + configProps.put(ENABLED, ServerProp.createPropObject(!DEFAULT_ENABLED)); + configProps.put(SKIP_TEMP_JARS, ServerProp.createPropObject(!DEFAULT_SKIP_TEMP_JARS)); + configProps.put(JARS_PER_SECOND, ServerProp.createPropObject(5)); jarCollectorConfig = new JarCollectorConfigImpl(configProps); - assertEquals(enabledServerProp.getValue(), jarCollectorConfig.isEnabled()); - assertEquals(maxClassLoadersServerProp.getValue(), jarCollectorConfig.getMaxClassLoaders()); + assertEquals(!DEFAULT_ENABLED, jarCollectorConfig.isEnabled()); + assertEquals(!DEFAULT_SKIP_TEMP_JARS, jarCollectorConfig.skipTempJars()); + assertEquals(5, jarCollectorConfig.getJarsPerSecond()); } @Test public void testEnvironmentVariables() { - SaveSystemPropertyProviderRule.TestEnvironmentFacade environmentFacade = new SaveSystemPropertyProviderRule.TestEnvironmentFacade(ImmutableMap.of( - NEW_RELIC_JAR_COLLECTOR_ENABLED, "false", - NEW_RELIC_JAR_COLLECTOR_MAX_CLASS_LOADERS, "321" + TestEnvironmentFacade environmentFacade = new TestEnvironmentFacade(ImmutableMap.of( + NEW_RELIC_JAR_COLLECTOR_ENABLED, String.valueOf(!DEFAULT_ENABLED), + NEW_RELIC_JAR_COLLECTOR_SKIP_TEMP_JARS, String.valueOf(!DEFAULT_SKIP_TEMP_JARS), + NEW_RELIC_JAR_COLLECTOR_JARS_PER_SECOND, "5" )); SystemPropertyFactory.setSystemPropertyProvider(new SystemPropertyProvider( - new SaveSystemPropertyProviderRule.TestSystemProps(), + new TestSystemProps(), environmentFacade )); jarCollectorConfig = new JarCollectorConfigImpl(configProps); - assertEquals(Boolean.parseBoolean(environmentFacade.getenv(NEW_RELIC_JAR_COLLECTOR_ENABLED)), jarCollectorConfig.isEnabled()); - assertEquals(Integer.parseInt(environmentFacade.getenv(NEW_RELIC_JAR_COLLECTOR_MAX_CLASS_LOADERS)), jarCollectorConfig.getMaxClassLoaders()); + assertEquals(!DEFAULT_ENABLED, jarCollectorConfig.isEnabled()); + assertEquals(!DEFAULT_SKIP_TEMP_JARS, jarCollectorConfig.skipTempJars()); + assertEquals(5, jarCollectorConfig.getJarsPerSecond()); } @Test public void testDeprecatedEnvironmentVariables() { - SaveSystemPropertyProviderRule.TestEnvironmentFacade environmentFacade = new SaveSystemPropertyProviderRule.TestEnvironmentFacade(ImmutableMap.of( - NEW_RELIC_MODULE_ENABLED, "false", - NEW_RELIC_MODULE_MAX_CLASS_LOADERS, "321" + TestEnvironmentFacade environmentFacade = new TestEnvironmentFacade(ImmutableMap.of( + NEW_RELIC_MODULE_ENABLED, String.valueOf(!DEFAULT_ENABLED) )); SystemPropertyFactory.setSystemPropertyProvider(new SystemPropertyProvider( - new SaveSystemPropertyProviderRule.TestSystemProps(), + new TestSystemProps(), environmentFacade )); jarCollectorConfig = new JarCollectorConfigImpl(configProps); - assertEquals(Boolean.parseBoolean(environmentFacade.getenv(NEW_RELIC_MODULE_ENABLED)), jarCollectorConfig.isEnabled()); - assertEquals(Integer.parseInt(environmentFacade.getenv(NEW_RELIC_MODULE_MAX_CLASS_LOADERS)), jarCollectorConfig.getMaxClassLoaders()); + assertEquals(!DEFAULT_ENABLED, jarCollectorConfig.isEnabled()); } @Test public void testEnvironmentVariablePrecedence() { - SaveSystemPropertyProviderRule.TestEnvironmentFacade environmentFacade = new SaveSystemPropertyProviderRule.TestEnvironmentFacade(ImmutableMap.of( + TestEnvironmentFacade environmentFacade = new TestEnvironmentFacade(ImmutableMap.of( // Preferred config settings - NEW_RELIC_JAR_COLLECTOR_ENABLED, "true", - NEW_RELIC_JAR_COLLECTOR_MAX_CLASS_LOADERS, "321", + NEW_RELIC_JAR_COLLECTOR_ENABLED, String.valueOf(DEFAULT_ENABLED), // Deprecated config settings - NEW_RELIC_MODULE_ENABLED, "false", - NEW_RELIC_MODULE_MAX_CLASS_LOADERS, "123" + NEW_RELIC_MODULE_ENABLED, String.valueOf(!DEFAULT_ENABLED) )); SystemPropertyFactory.setSystemPropertyProvider(new SystemPropertyProvider( - new SaveSystemPropertyProviderRule.TestSystemProps(), + new TestSystemProps(), environmentFacade )); jarCollectorConfig = new JarCollectorConfigImpl(configProps); // Preferred config settings take precedence over the deprecated settings when both are present - assertEquals(Boolean.parseBoolean(environmentFacade.getenv(NEW_RELIC_JAR_COLLECTOR_ENABLED)), jarCollectorConfig.isEnabled()); - assertEquals(Integer.parseInt(environmentFacade.getenv(NEW_RELIC_JAR_COLLECTOR_MAX_CLASS_LOADERS)), jarCollectorConfig.getMaxClassLoaders()); - - // Deprecated settings aren't applied - assertNotEquals(Boolean.parseBoolean(environmentFacade.getenv(NEW_RELIC_MODULE_ENABLED)), jarCollectorConfig.isEnabled()); - assertNotEquals(Integer.parseInt(environmentFacade.getenv(NEW_RELIC_MODULE_MAX_CLASS_LOADERS)), jarCollectorConfig.getMaxClassLoaders()); + assertEquals(DEFAULT_ENABLED, jarCollectorConfig.isEnabled()); } @Test public void testSystemProperties() { Properties props = new Properties(); - props.setProperty(enabledSystemProperty, "true"); - props.setProperty(maxClassLoadersSystemProperty, "77"); - - SaveSystemPropertyProviderRule.TestSystemProps testSystemProps = new SaveSystemPropertyProviderRule.TestSystemProps(props); + props.setProperty(SYSTEM_PROPERTY_ROOT + ENABLED, String.valueOf(!DEFAULT_ENABLED)); + props.setProperty(SYSTEM_PROPERTY_ROOT + SKIP_TEMP_JARS, String.valueOf(!DEFAULT_SKIP_TEMP_JARS)); + props.setProperty(SYSTEM_PROPERTY_ROOT + JARS_PER_SECOND, "5"); SystemPropertyFactory.setSystemPropertyProvider(new SystemPropertyProvider( - testSystemProps, - new SaveSystemPropertyProviderRule.TestEnvironmentFacade() + new TestSystemProps(props), + new TestEnvironmentFacade() )); jarCollectorConfig = new JarCollectorConfigImpl(configProps); - assertEquals(Boolean.parseBoolean(testSystemProps.getSystemProperty(enabledSystemProperty)), jarCollectorConfig.isEnabled()); - assertEquals(Integer.parseInt(testSystemProps.getSystemProperty(maxClassLoadersSystemProperty)), jarCollectorConfig.getMaxClassLoaders()); + assertEquals(!DEFAULT_ENABLED, jarCollectorConfig.isEnabled()); + assertEquals(!DEFAULT_SKIP_TEMP_JARS, jarCollectorConfig.skipTempJars()); + assertEquals(5, jarCollectorConfig.getJarsPerSecond()); } @Test public void testDeprecatedSystemProperties() { Properties props = new Properties(); - props.setProperty(enabledSystemPropertyDeprecated, "false"); - props.setProperty(maxClassLoadersSystemPropertyDeprecated, "4999"); + props.setProperty(SYSTEM_PROPERTY_ROOT_DEPRECATED + ENABLED, String.valueOf(!DEFAULT_ENABLED)); - SaveSystemPropertyProviderRule.TestSystemProps testSystemProps = new SaveSystemPropertyProviderRule.TestSystemProps(props); SystemPropertyFactory.setSystemPropertyProvider(new SystemPropertyProvider( - testSystemProps, - new SaveSystemPropertyProviderRule.TestEnvironmentFacade() + new TestSystemProps(props), + new TestEnvironmentFacade() )); jarCollectorConfig = new JarCollectorConfigImpl(configProps); - assertEquals(Boolean.parseBoolean(testSystemProps.getSystemProperty(enabledSystemPropertyDeprecated)), jarCollectorConfig.isEnabled()); - assertEquals(Integer.parseInt(testSystemProps.getSystemProperty(maxClassLoadersSystemPropertyDeprecated)), jarCollectorConfig.getMaxClassLoaders()); + assertEquals(!DEFAULT_ENABLED, jarCollectorConfig.isEnabled()); } @Test public void testSystemPropertyPrecedence() { Properties props = new Properties(); // Preferred config settings - props.setProperty(enabledSystemProperty, "true"); - props.setProperty(maxClassLoadersSystemProperty, "77"); - + props.setProperty(SYSTEM_PROPERTY_ROOT + ENABLED, String.valueOf(DEFAULT_ENABLED)); // Deprecated config settings - props.setProperty(enabledSystemPropertyDeprecated, "false"); - props.setProperty(maxClassLoadersSystemPropertyDeprecated, "4999"); + props.setProperty(SYSTEM_PROPERTY_ROOT_DEPRECATED + ENABLED, String.valueOf(!DEFAULT_ENABLED)); - SaveSystemPropertyProviderRule.TestSystemProps testSystemProps = new SaveSystemPropertyProviderRule.TestSystemProps(props); SystemPropertyFactory.setSystemPropertyProvider(new SystemPropertyProvider( - testSystemProps, - new SaveSystemPropertyProviderRule.TestEnvironmentFacade() + new TestSystemProps(props), + new TestEnvironmentFacade() )); jarCollectorConfig = new JarCollectorConfigImpl(configProps); // Preferred config settings take precedence over the deprecated settings when both are present - assertEquals(Boolean.parseBoolean(testSystemProps.getSystemProperty(enabledSystemProperty)), jarCollectorConfig.isEnabled()); - assertEquals(Integer.parseInt(testSystemProps.getSystemProperty(maxClassLoadersSystemProperty)), jarCollectorConfig.getMaxClassLoaders()); - - // Deprecated settings aren't applied - assertNotEquals(Boolean.parseBoolean(testSystemProps.getSystemProperty(enabledSystemPropertyDeprecated)), jarCollectorConfig.isEnabled()); - assertNotEquals(Integer.parseInt(testSystemProps.getSystemProperty(maxClassLoadersSystemPropertyDeprecated)), jarCollectorConfig.getMaxClassLoaders()); + assertEquals(DEFAULT_ENABLED, jarCollectorConfig.isEnabled()); } }