Improve sql obfuscation regular expressions

[jsumners-nr]

node-newrelic/lib/util/sql/obfuscate.js

Lines 10 to 17 in d2fba9d

	// eslint-disable-next-line sonarjs/slow-regex
	const singleQuote = /'(?:''|[^'])*?(?:\\'.*|'(?!'))/
	// eslint-disable-next-line sonarjs/slow-regex
	const doubleQuote = /"(?:[^"]|"")*?(?:\\".*|"(?!"))/
	const dollarQuote = /(\$(?!\d)[^$]*?\$).*?(?:\1|$)/
	const oracleQuote = /q'\[.*?(?:\]'|$)|q'\{.*?(?:\}'|$)|q'<.*?(?:>'|$)|q'\(.*?(?:\)'|$)/
	// eslint-disable-next-line sonarjs/slow-regex
	const comment = /(?:#|--).*?(?=\r|\n|$)/

node-newrelic/lib/db/statement-matcher.js

Lines 8 to 10 in d2fba9d

	// ( ` database` . ` table ` )
	// eslint-disable-next-line sonarjs/slow-regex
	const CLEANER = /^\(?(?:([`'"]?)(.*?)\1\.)?([`'"]?)(.*?)\3\)?$/

These regular expressions are triggering sonar-js's "slow regex" rule. We need to investigate what the regular expressions do, and see if we can improve them.

[workato-integration]

https://new-relic.atlassian.net/browse/NR-354854

[jsumners-nr]

These may already be solved if we agree that #2714 is an acceptable change.

[jsumners-nr]

I wanted an easier to understand benchmark in order to compare our current implementation with the one I have proposed. I put it up at https://github.com/jsumners-nr/sql-parser-bench. The benchmark runs 10,000 iterations for each parser to parse all of our cross agent SQL parsing tests. This still obscures the finer details of each algorithm, but makes it easier to see the difference. The results are pretty consistently close to:

original parser duration: 144 ms
replacement parser duration: 1,519 ms
original is faster by: 1,374 ms

I want to call out that the replacement parser supports common table expressions (CTEs) whereas our current one does not.

If we instead run a 10,000 iteration test of a singular SQL statement that both support:

select
  foo,
  bar,
  baz
from some_db.some_table as a_table
where
    foo = 'foo'

We get a result like:

original parser duration: 5 ms
replacement parser duration: 44 ms
original is faster by: 38 ms

I think this is more representative of what would be encountered during normal operation.

I also think we could improve either algorithm with an LRU cache.

[jsumners-nr]

To be clear: I don't think there is anything we can do to the regular expressions to satisfy the linter rule. The linter rule is keying off "zero or more" and "one or more" operators. Those are necessary for the type of parsing being done by these rules. So we either solidify the exceptions with an explanation, or come up with a different way to do the parsing (which is what I have proposed and benchmarked).

[jsumners-nr]

Doing some more research, I have discovered:

1. Our sql parser and sql "obfuscator" are mutually exclusive tools.
2. My comments should be attached to https://github.com/newrelic/node-newrelic/blob/cb16516e90a3dc2cefb98e6131a7243412aefbfc/lib/db/query-parsers/sql.js
3. Obfuscation is only performed if a query meets our "slow query" threshold.
4. .Net does not use these regular expressions to obfuscate SQL statements. Instead, the parse the string character by character to do the replacement. At least in JavaScript, this would not be an ideal approach because values to redact could contain multibyte characters that would need to be detected. https://github.com/newrelic/newrelic-dotnet-agent/blob/4309938c5d0593029398d7658a37b5ebe78c283c/src/Agent/NewRelic/Agent/Core/Database/SqlObfuscator.cs?plain=1