Merge pull request #2490 from newrelic/disable-sql-regex

Adds a new config option under the transaction_tracer stanza to disable the execution of the call and exec SQL parser regular expressions.

Author: jtduffy

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

@@ -24,6 +24,16 @@ public interface TransactionTracerConfig {
      */
     String getRecordSql();
 
+    /**
+     * For large SQL statements, executing the regular expressions that attempt to parse exec and
+     * call statements can take a significant amount of time. Setting this to true will disable the
+     * execution of these complex regular expressions.
+     * Default is false.
+     *
+     * @return true if the exec and call regular expression execution is disabled
+     */
+    boolean isExecCallSqlRegexDisabled();
+
     /**
      * The set of modules that are allowed to send up obfuscated slow query information when high_security
      * mode is enabled. If high_security mode is disabled this setting is ignored.

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

@@ -46,6 +46,8 @@ public final class TransactionTracerConfigImpl extends BaseConfig implements Tra
     public static final String TOKEN_LIMIT = "token_limit";
     public static final String TOP_N = "top_n";
     public static final String TRANSACTION_THRESHOLD = "transaction_threshold";
+    public static final String EXEC_CALL_SQL_REGEX_DISABLED = "exec_call_sql_regex_disabled";
+
     public static final boolean DEFAULT_COLLECT_TRACES = false;
     public static final boolean DEFAULT_ENABLED = true;
     public static final boolean DEFAULT_EXPLAIN_ENABLED = true;
@@ -61,6 +63,7 @@ public final class TransactionTracerConfigImpl extends BaseConfig implements Tra
     public static final String DEFAULT_TRANSACTION_THRESHOLD = APDEX_F;
     public static final int DEFAULT_TOKEN_LIMIT = 3000;
     public static final int DEFAULT_TOP_N = 20;
+    public static final boolean DEFAULT_EXEC_CALL_SQL_REGEX_DISABLED = false;
     public static final int APDEX_F_MULTIPLE = 4;
     public static final String SYSTEM_PROPERTY_ROOT = "newrelic.config.transaction_tracer.";
     public static final String CATEGORY_REQUEST_SYSTEM_PROPERTY_ROOT = "newrelic.config.transaction_tracer.category." + REQUEST_CATEGORY_NAME + ".";
@@ -84,6 +87,7 @@ public final class TransactionTracerConfigImpl extends BaseConfig implements Tra
     private final int maxExplainPlans;
     private final int maxTokens;
     private final int topN;
+    private final boolean isExecCallSqlRegexDisabled;
     protected final String inheritedFromSystemPropertyRoot;
 
     private TransactionTracerConfigImpl(String systemPropertyRoot, String inheritedFromSystemPropertyRoot,
@@ -109,6 +113,7 @@ private TransactionTracerConfigImpl(String systemPropertyRoot, String inheritedF
         maxExplainPlans = getIntProperty(MAX_EXPLAIN_PLANS, DEFAULT_MAX_EXPLAIN_PLANS);
         maxTokens = getIntProperty(TOKEN_LIMIT, DEFAULT_TOKEN_LIMIT);
         topN = getIntProperty(TOP_N, DEFAULT_TOP_N);
+        isExecCallSqlRegexDisabled = getProperty(EXEC_CALL_SQL_REGEX_DISABLED, DEFAULT_EXEC_CALL_SQL_REGEX_DISABLED);
     }
 
     private boolean initEnabled() {
@@ -241,6 +246,11 @@ protected Object getPropertyFromSystemEnvironment(String name, Object defaultVal
         return inheritedKey == null ? null : parseValue(SystemPropertyFactory.getSystemPropertyProvider().getEnvironmentVariable(inheritedKey));
     }
 
+    @Override
+    public boolean isExecCallSqlRegexDisabled() {
+        return isExecCallSqlRegexDisabled;
+    }
+
     @Override
     public double getExplainThresholdInMillis() {
         return explainThreshold;

newrelic-agent/src/main/java/com/newrelic/agent/database/DefaultDatabaseStatementParser.java

@@ -93,7 +93,7 @@ public ParsedDatabaseStatement getParsedDatabaseStatement(
                         return new ParsedDatabaseStatement(tableName.toLowerCase(), SELECT_OPERATION, true);
                     }
                 }
-            } catch (Exception e) {
+            } catch (Exception ignored) {
             }
         }
         return parseStatement(statement);

newrelic-agent/src/main/java/com/newrelic/agent/database/DefaultStatementFactory.java

@@ -8,10 +8,16 @@
 package com.newrelic.agent.database;
 
 import com.newrelic.agent.Agent;
+import com.newrelic.agent.config.ConfigService;
+import com.newrelic.agent.config.TransactionTracerConfig;
+import com.newrelic.agent.service.ServiceFactory;
 import com.newrelic.agent.util.Strings;
 import org.apache.commons.lang3.StringUtils;
 
 import java.text.MessageFormat;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.logging.Level;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -21,19 +27,24 @@ class DefaultStatementFactory implements StatementFactory {
     private final DefaultStatementFactory backupPattern;
     protected final String key;
     private final boolean generateMetric;
+    private final boolean isExecCallSqlRegexDisabled;
+
+    private final Set<String> DISABLEABLE_REGEX = new HashSet<>(Arrays.asList("call", "exec"));
 
     public DefaultStatementFactory(String key, Pattern pattern, boolean generateMetric) {
         this.key = key;
         this.pattern = pattern;
         this.generateMetric = generateMetric;
         this.backupPattern = null;
+        isExecCallSqlRegexDisabled = getExecCallSqlRegexDisabled();
     }
 
     public DefaultStatementFactory(String key, Pattern pattern, boolean generateMetric, Pattern backupPattern) {
         this.key = key;
         this.pattern = pattern;
         this.generateMetric = generateMetric;
         this.backupPattern = new DefaultStatementFactory(key, backupPattern, generateMetric);
+        isExecCallSqlRegexDisabled = getExecCallSqlRegexDisabled();
     }
 
     protected boolean isMetricGenerator() {
@@ -42,7 +53,10 @@ protected boolean isMetricGenerator() {
 
     @Override
     public ParsedDatabaseStatement parseStatement(String statement) {
-        // Optimization to prevent running complex regex when we don't need to
+        // Optimizations to prevent running complex regex when we don't need to
+        if (isExecCallSqlRegexDisabled && DISABLEABLE_REGEX.contains(getOperation())) {
+            return null;
+        }
         if (!StringUtils.containsIgnoreCase(statement, key)) {
             return null;
         }
@@ -86,4 +100,12 @@ ParsedDatabaseStatement createParsedDatabaseStatement(String model) {
     public String getOperation() {
         return key;
     }
+
+    private boolean getExecCallSqlRegexDisabled() {
+        ConfigService configService = ServiceFactory.getConfigService();
+        TransactionTracerConfig transactionTracerConfig = ServiceFactory.getConfigService()
+                .getTransactionTracerConfig(configService.getDefaultAgentConfig().getApplicationName());
+
+        return transactionTracerConfig.isExecCallSqlRegexDisabled();
+    }
 }

newrelic-agent/src/main/resources/newrelic.yml

@@ -225,6 +225,12 @@ common: &default_settings
     # Default is apdex_f.
     transaction_threshold: apdex_f
 
+    # For large SQL statements, executing the regular expressions that attempt to parse exec and
+    # call statements can take a significant amount of time. Setting this to true will disable the
+    # execution of these complex regular expressions.
+    # Default is false.
+    exec_call_sql_regex_disabled: false
+
     # When transaction tracer is on, SQL statements can optionally be
     # recorded. The recorder has three modes, "off" which sends no
     # SQL, "raw" which sends the SQL statement in its original form,

newrelic-agent/src/test/java/com/newrelic/agent/database/DatabaseStatementResponseParserTest.java

@@ -8,9 +8,12 @@
 package com.newrelic.agent.database;
 
 import com.google.common.io.Files;
+import com.newrelic.agent.MockConfigService;
 import com.newrelic.agent.MockCoreService;
+import com.newrelic.agent.MockServiceManager;
 import com.newrelic.agent.bridge.datastore.DatastoreVendor;
 import com.newrelic.agent.bridge.datastore.UnknownDatabaseVendor;
+import com.newrelic.agent.config.AgentConfig;
 import com.newrelic.agent.service.ServiceFactory;
 import com.newrelic.agent.service.ServiceManager;
 import org.junit.AfterClass;
@@ -30,12 +33,23 @@
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 public class DatabaseStatementResponseParserTest {
     DatabaseStatementParser parser;
+    private static AgentConfig agentConfig;
 
     @BeforeClass
     public static void beforeClass() throws Exception {
         MockCoreService.getMockAgentAndBootstrapTheServiceManager();
+
+        MockServiceManager sm = new MockServiceManager();
+        ServiceFactory.setServiceManager(sm);
+        agentConfig = mock(AgentConfig.class, RETURNS_DEEP_STUBS);
+        MockConfigService configService = new MockConfigService(agentConfig);
+        sm.setConfigService(configService);
     }
 
     @AfterClass
@@ -49,6 +63,7 @@ public static void afterClass() throws Exception {
     @Before
     public void before() {
         parser = new DefaultDatabaseStatementParser();
+        when(agentConfig.getTransactionTracerConfig().isExecCallSqlRegexDisabled()).thenReturn(false);
     }
 
     @Test
@@ -536,6 +551,39 @@ public void testLargeSqlSlowdown() throws Exception {
         assertEquals("jxu7wns.djs_project_test", parsedStatement.getModel());
     }
 
+    @Test
+    public void testExecCallRegexDisabled() {
+        when(agentConfig.getTransactionTracerConfig().isExecCallSqlRegexDisabled()).thenReturn(true);
+        parser = new DefaultDatabaseStatementParser();
+
+        // These should return an unparaseable statement instance
+        ParsedDatabaseStatement parsedStatement = parseStatement("call dude(?)");
+        assertEquals("other", parsedStatement.getOperation());
+        assertNull(parsedStatement.getModel());
+
+        parsedStatement = parseStatement("exec dude(?)");
+        assertEquals("other", parsedStatement.getOperation());
+        assertNull(parsedStatement.getModel());
+
+        // These should parse normally even with the disabled flag on
+        parsedStatement = parseStatement("Select * from metrics");
+        assertEquals("select", parsedStatement.getOperation());
+        assertEquals("metrics", parsedStatement.getModel());
+
+        parsedStatement = parseStatement("DROP PROCEDURE IF EXISTS agent_count_all");
+        assertEquals("drop", parsedStatement.getOperation());
+        assertEquals("Procedure", parsedStatement.getModel());
+
+        parsedStatement = parseStatement("insert into dude(1,2,3)");
+        assertEquals("insert", parsedStatement.getOperation());
+        assertEquals("dude", parsedStatement.getModel());
+
+        parsedStatement = parseStatement("delete from  dude");
+        assertEquals("delete", parsedStatement.getOperation());
+        assertEquals("dude", parsedStatement.getModel());
+
+    }
+
     private ParsedDatabaseStatement parseStatement(String statement, ResultSetMetaData metaData) {
         return parser.getParsedDatabaseStatement(UnknownDatabaseVendor.INSTANCE, statement, metaData);
     }