diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9f9a53b..b7017ee 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -18,10 +18,10 @@ jobs: max-parallel: 100 matrix: spring_boot_version: - - 3.1.2 - - 3.0.7 - - 2.7.13 - - 2.6.15 + - 3.2.2 + - 3.1.6 + - 3.0.13 + - 2.7.18 env: SPRING_BOOT_VERSION: ${{ matrix.spring_boot_version }} services: diff --git a/CHANGELOG.md b/CHANGELOG.md index adcabcb..ea85cec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.15.0] - 2024-01-25 + +* Upgrading libraries, mainly the JSqlParser, to support more queries out of the box. + ## [2.14.0] - 2023-09-26 ### Added diff --git a/build.common.gradle b/build.common.gradle index b83483e..1bc4189 100644 --- a/build.common.gradle +++ b/build.common.gradle @@ -126,8 +126,8 @@ test { } tasks.findAll { it.name.startsWith("spotbugs") }*.configure { - effort = "max" excludeFilter = file('../spotbugs-exclude.xml') + reports { xml.required = true html.required = true diff --git a/build.gradle b/build.gradle index 5078060..1a53b5e 100644 --- a/build.gradle +++ b/build.gradle @@ -1,11 +1,13 @@ +import com.github.spotbugs.snom.Confidence +import com.github.spotbugs.snom.Effort import org.eclipse.jgit.api.errors.RefAlreadyExistsException plugins { - id "com.github.spotbugs" version "5.0.13" apply false + id "com.github.spotbugs" version "6.0.7" id "idea" - id 'org.ajoberstar.grgit' version '5.0.0' - id 'io.github.gradle-nexus.publish-plugin' version "1.1.0" - id 'com.github.johnrengelman.shadow' version '7.0.0' apply false + id 'org.ajoberstar.grgit' version '5.2.1' + id 'io.github.gradle-nexus.publish-plugin' version "1.3.0" + id 'com.github.johnrengelman.shadow' version '8.1.1' apply false } idea.project { @@ -39,3 +41,8 @@ nexusPublishing { } } } + +spotbugs { + effort = Effort.valueOf('MAX') + reportLevel = Confidence.valueOf('DEFAULT') +} diff --git a/build.libraries.gradle b/build.libraries.gradle index 24ab9e4..452bc25 100644 --- a/build.libraries.gradle +++ b/build.libraries.gradle @@ -1,20 +1,20 @@ ext { - springBootVersion = System.getenv("SPRING_BOOT_VERSION") ?: "2.6.15" + springBootVersion = System.getenv("SPRING_BOOT_VERSION") ?: "2.7.18" libraries = [ // explicit versions - guava : "com.google.guava:guava:31.1-jre", - jsqlParser : "com.github.jsqlparser:jsqlparser:4.6", + guava : "com.google.guava:guava:33.0.0-jre", + jsqlParser : "com.github.jsqlparser:jsqlparser:4.8", spotbugsAnnotations : "com.github.spotbugs:spotbugs-annotations:${spotbugs.toolVersion.get()}", springBootDependencies : "org.springframework.boot:spring-boot-dependencies:${springBootVersion}", - testContainersMariaDb : "org.testcontainers:mariadb:1.17.6", - twBaseUtils : "com.transferwise.common:tw-base-utils:1.10.1", + testContainersMariaDb : "org.testcontainers:mariadb:1.19.3", + twBaseUtils : "com.transferwise.common:tw-base-utils:1.12.3", twContext : "com.transferwise.common:tw-context:1.0.0", twContextStarter : "com.transferwise.common:tw-context-starter:1.0.0", - twGracefulShutdown : 'com.transferwise.common:tw-graceful-shutdown:2.11.0', - twGracefulShutdownInterfaces : 'com.transferwise.common:tw-graceful-shutdown-interfaces:2.11.0', - twSpyqlCore : "com.transferwise.common:tw-spyql-core:1.6.0", - twSpyqlStarter : "com.transferwise.common:tw-spyql-starter:1.6.0", + twGracefulShutdown : 'com.transferwise.common:tw-graceful-shutdown:2.14.2', + twGracefulShutdownInterfaces: 'com.transferwise.common:tw-graceful-shutdown-interfaces:2.14.2', + twSpyqlCore : "com.transferwise.common:tw-spyql-core:1.6.1", + twSpyqlStarter : "com.transferwise.common:tw-spyql-starter:1.6.1", // versions managed by spring-boot-dependencies platform caffeine : "com.github.ben-manes.caffeine:caffeine", @@ -24,6 +24,7 @@ ext { hikariCp : "com.zaxxer:HikariCP", junitJupiter : "org.junit.jupiter:junit-jupiter", + logbackClassic : "ch.qos.logback:logback-classic", lombok : "org.projectlombok:lombok", mariadbJavaClient : "org.mariadb.jdbc:mariadb-java-client", micrometerCore : "io.micrometer:micrometer-core", diff --git a/gradle.properties b/gradle.properties index 77685fc..6efb360 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1 +1 @@ -version=2.14.0 +version=2.15.0 diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar index ccebba7..c1962a7 100644 Binary files a/gradle/wrapper/gradle-wrapper.jar and b/gradle/wrapper/gradle-wrapper.jar differ diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 37aef8d..3499ded 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.1.1-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.5-bin.zip networkTimeout=10000 zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists diff --git a/gradlew b/gradlew index 79a61d4..aeb74cb 100755 --- a/gradlew +++ b/gradlew @@ -85,9 +85,6 @@ done APP_BASE_NAME=${0##*/} APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit -# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. -DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' - # Use the maximum available, or set MAX_FD != -1 to use that value. MAX_FD=maximum @@ -197,6 +194,10 @@ if "$cygwin" || "$msys" ; then done fi + +# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script. +DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' + # Collect all arguments for the java command; # * $DEFAULT_JVM_OPTS, $JAVA_OPTS, and $GRADLE_OPTS can contain fragments of # shell script including quotes and variable substitutions, so put them in diff --git a/tw-entrypoints-starter/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/TasFlywayConfigurationCustomizer.java b/tw-entrypoints-starter/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/TasFlywayConfigurationCustomizer.java index 5e71f18..c51563a 100644 --- a/tw-entrypoints-starter/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/TasFlywayConfigurationCustomizer.java +++ b/tw-entrypoints-starter/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/TasFlywayConfigurationCustomizer.java @@ -34,7 +34,7 @@ public boolean canHandleInTransaction(Event event, Context context) { @Override public void handle(Event event, Context context) { - if (event == Event.BEFORE_MIGRATE) { + if (event == Event.BEFORE_VALIDATE) { log.info("Disabling TAS query parsing before Flyway migration."); var twContext = TwContext.current().createSubContext().asEntryPoint("Flyway", "Flyway"); diff --git a/tw-entrypoints-starter/src/test/java/com/transferwise/common/entrypoints/test/BaseIntTest.java b/tw-entrypoints-starter/src/test/java/com/transferwise/common/entrypoints/test/BaseIntTest.java index b3a896c..b298a18 100644 --- a/tw-entrypoints-starter/src/test/java/com/transferwise/common/entrypoints/test/BaseIntTest.java +++ b/tw-entrypoints-starter/src/test/java/com/transferwise/common/entrypoints/test/BaseIntTest.java @@ -39,7 +39,7 @@ public void setup() { meterRegistry.getMeters().stream() .filter(m -> (m.getId().getName().startsWith("EntryPoints") || m.getId().getName().startsWith("database")) && !(m instanceof Gauge)) .forEach(m -> { - log.info("Removing metric: " + m.getId()); + log.debug("Removing metric: " + m.getId()); meterRegistry.remove(m); }); meterCache.clear(); diff --git a/tw-entrypoints/build.gradle b/tw-entrypoints/build.gradle index 5d09b08..7cecc42 100644 --- a/tw-entrypoints/build.gradle +++ b/tw-entrypoints/build.gradle @@ -22,17 +22,8 @@ dependencies { implementation libraries.twSpyqlCore implementation libraries.twGracefulShutdownInterfaces - shadow libraries.twContext - shadow libraries.commonsLang3 - shadow libraries.caffeine - shadow libraries.guava - shadow libraries.micrometerCore - shadow libraries.slf4jApi - shadow libraries.twBaseUtils - shadow libraries.twSpyqlCore - shadow libraries.twGracefulShutdownInterfaces - testImplementation libraries.junitJupiter + testRuntimeOnly libraries.logbackClassic } apply from: "build-idea-fix.gradle.kts" @@ -48,10 +39,10 @@ shadowJar { manifest { attributes 'Implementation-Version': "$project.version" } - relocate('net.sf.jsqlparser', 'com.transferwise.common.entrypoints.shadow') + relocate('net.sf.jsqlparser', 'com.transferwise.common.entrypoints.shadow.net.sf.jsqlparser') } -jar.enabled = true +jar.enabled = false jar.dependsOn shadowJar shadowJar { @@ -105,8 +96,8 @@ publishing { withXml { xml -> def dependenciesNode = xml.asNode().get('dependencies') ?: xml.asNode().appendNode('dependencies') - project.configurations.getByName("shadow").resolvedConfiguration.firstLevelModuleDependencies.forEach { - if (it.configuration != "platform-runtime") { + project.configurations.getByName("runtimeClasspath").resolvedConfiguration.firstLevelModuleDependencies.forEach { + if (it.configuration != "platform-runtime" && it.moduleName != 'jsqlparser') { def dependencyNode = dependenciesNode.appendNode('dependency') dependencyNode.appendNode('groupId', it.moduleGroup) dependencyNode.appendNode('artifactId', it.moduleName) diff --git a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/databaseaccessstatistics/DasUnknownCallsCollector.java b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/databaseaccessstatistics/DasUnknownCallsCollector.java index 76a074c..959d2ea 100644 --- a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/databaseaccessstatistics/DasUnknownCallsCollector.java +++ b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/databaseaccessstatistics/DasUnknownCallsCollector.java @@ -49,14 +49,14 @@ protected void registerUnknownCalls() { final long fetchedRows = das.getAndResetFetchedRowsCount(); TagsSet tagsSet = das.getTagsSet(); - UnknownCallMeters meters = meterCache.metersContainer(METRIC_PREFIX_DAS + "unknownCallMetrics", tagsSet, () -> { + UnknownCallMeters meters = meterCache.metersContainer(METRIC_PREFIX_DAS + "unknownCallMetrics", tagsSet, (name, tags) -> { UnknownCallMeters result = new UnknownCallMeters(); - result.commits = meterCache.counter(COUNTER_UNREGISTERED_COMMITS, tagsSet); - result.rollbacks = meterCache.counter(COUNTER_UNREGISTERED_ROLLBACKS, tagsSet); - result.nonTransactionalQueries = meterCache.counter(COUNTER_UNREGISTERED_NT_QUERIES, tagsSet); - result.transactionalQueries = meterCache.counter(COUNTER_UNREGISTERED_T_QUERIES, tagsSet); - result.timeTakenNs = meterCache.counter(COUNTER_UNREGISTERED_TIME_TAKEN_NS, tagsSet); - result.emptyTransactions = meterCache.counter(COUNTER_UNREGISTERED_EMPTY_TRANSACTIONS, tagsSet); + result.commits = meterCache.counter(COUNTER_UNREGISTERED_COMMITS, tags); + result.rollbacks = meterCache.counter(COUNTER_UNREGISTERED_ROLLBACKS, tags); + result.nonTransactionalQueries = meterCache.counter(COUNTER_UNREGISTERED_NT_QUERIES, tags); + result.transactionalQueries = meterCache.counter(COUNTER_UNREGISTERED_T_QUERIES, tags); + result.timeTakenNs = meterCache.counter(COUNTER_UNREGISTERED_TIME_TAKEN_NS, tags); + result.emptyTransactions = meterCache.counter(COUNTER_UNREGISTERED_EMPTY_TRANSACTIONS, tags); result.affectedRows = meterCache.counter(COUNTER_UNREGISTERED_AFFECTED_ROWS, tagsSet); result.fetchedRows = meterCache.counter(COUNTER_UNREGISTERED_FETCHED_ROWS, tagsSet); return result; diff --git a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/databaseaccessstatistics/DatabaseAccessStatisticsEntryPointInterceptor.java b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/databaseaccessstatistics/DatabaseAccessStatisticsEntryPointInterceptor.java index e5563c2..62d08a2 100644 --- a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/databaseaccessstatistics/DatabaseAccessStatisticsEntryPointInterceptor.java +++ b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/databaseaccessstatistics/DatabaseAccessStatisticsEntryPointInterceptor.java @@ -71,18 +71,18 @@ private void registerCall(TwContext context, Map { + CallMeters meters = meterCache.metersContainer(METRIC_PREFIX_DAS + "callMetrics", tagsSet, (name, tags) -> { CallMeters result = new CallMeters(); - result.commits = meterCache.summary(SUMMARY_REGISTERED_COMMITS, tagsSet); - result.rollbacks = meterCache.summary(SUMMARY_REGISTERED_ROLLBACKS, tagsSet); - result.nonTransactionalQueries = meterCache.summary(SUMMARY_REGISTERED_NT_QUERIES, tagsSet); - result.transactionalQueries = meterCache.summary(SUMMARY_REGISTERED_T_QUERIES, tagsSet); - result.maxConcurrentConnections = meterCache.summary(SUMMARY_REGISTERED_MAX_CONCURRENT_CONNECTIONS, tagsSet); - result.remainingOpenConnections = meterCache.summary(SUMMARY_REGISTERED_REMAINING_OPEN_CONNECTIONS, tagsSet); - result.emptyTransactions = meterCache.summary(SUMMARY_REGISTERED_EMPTY_TRANSACTIONS, tagsSet); - result.affectedRows = meterCache.summary(SUMMARY_REGISTERED_AFFECTED_ROWS, tagsSet); - result.fetchedRows = meterCache.summary(SUMMARY_REGISTERED_FETCHED_ROWS, tagsSet); - result.timeTakenNs = meterCache.timer(TIMER_REGISTERED_TIME_TAKEN, tagsSet); + result.commits = meterCache.summary(SUMMARY_REGISTERED_COMMITS, tags); + result.rollbacks = meterCache.summary(SUMMARY_REGISTERED_ROLLBACKS, tags); + result.nonTransactionalQueries = meterCache.summary(SUMMARY_REGISTERED_NT_QUERIES, tags); + result.transactionalQueries = meterCache.summary(SUMMARY_REGISTERED_T_QUERIES, tags); + result.maxConcurrentConnections = meterCache.summary(SUMMARY_REGISTERED_MAX_CONCURRENT_CONNECTIONS, tags); + result.remainingOpenConnections = meterCache.summary(SUMMARY_REGISTERED_REMAINING_OPEN_CONNECTIONS, tags); + result.emptyTransactions = meterCache.summary(SUMMARY_REGISTERED_EMPTY_TRANSACTIONS, tags); + result.affectedRows = meterCache.summary(SUMMARY_REGISTERED_AFFECTED_ROWS, tags); + result.fetchedRows = meterCache.summary(SUMMARY_REGISTERED_FETCHED_ROWS, tags); + result.timeTakenNs = meterCache.timer(TIMER_REGISTERED_TIME_TAKEN, tags); return result; }); diff --git a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/CustomTablesNamesFinder.java b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/CustomTablesNamesFinder.java index abe449d..0169e96 100644 --- a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/CustomTablesNamesFinder.java +++ b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/CustomTablesNamesFinder.java @@ -1,56 +1,33 @@ package com.transferwise.common.entrypoints.tableaccessstatistics; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import lombok.Getter; import net.sf.jsqlparser.schema.Table; -import net.sf.jsqlparser.statement.SetStatement; -import net.sf.jsqlparser.statement.ShowColumnsStatement; -import net.sf.jsqlparser.statement.Statements; -import net.sf.jsqlparser.statement.alter.Alter; -import net.sf.jsqlparser.statement.create.index.CreateIndex; -import net.sf.jsqlparser.statement.create.view.CreateView; -import net.sf.jsqlparser.statement.drop.Drop; -import net.sf.jsqlparser.statement.execute.Execute; import net.sf.jsqlparser.util.TablesNamesFinder; public class CustomTablesNamesFinder extends TablesNamesFinder { - @Override - protected String extractTableName(Table table) { - return table.getName(); - } - - @Override - public void visit(Drop drop) { - visit(drop.getName()); - } - - @Override - public void visit(CreateIndex createIndex) { - visit(createIndex.getTable()); - } - - @Override - public void visit(CreateView createView) { - visit(createView.getView()); - } + @Getter + private final List tables = new ArrayList<>(); + private final Set uniqueTables = new HashSet<>(); - @Override - public void visit(Alter alter) { - visit(alter.getTable()); - } - - @Override - public void visit(Statements stmts) { - } + /* + The super class loses the order of tables visited, as its tables list is based on Set. + We are providing here the option to get the ordered tables list. + */ @Override - public void visit(Execute execute) { - } + public void visit(Table tableName) { + String tableWholeName = extractTableName(tableName); - @Override - public void visit(SetStatement set) { - } + if (!uniqueTables.contains(tableWholeName)) { + uniqueTables.add(tableWholeName); + tables.add(tableWholeName); + } - @Override - public void visit(ShowColumnsStatement set) { + super.visit(tableName); } } diff --git a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/SqlParser.java b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/SqlParser.java index dbcb83c..8fcf66f 100644 --- a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/SqlParser.java +++ b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/SqlParser.java @@ -4,8 +4,6 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import net.sf.jsqlparser.JSQLParserException; import net.sf.jsqlparser.parser.CCJSqlParser; import net.sf.jsqlparser.parser.StringProvider; @@ -13,15 +11,6 @@ public class SqlParser { - /* - DATABASE is reserved keyword in sql parser, so having DATABASE() in sql will just throw error. - DATABASE() is however used by some of our internal libraries for MariaDb. - */ - private static final Pattern FUNCTION_REPLACEMENT_PATTERN = Pattern.compile("DATABASE()", - Pattern.LITERAL | Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE); - - private static final Pattern ON_CONFLICT_REPLACEMENT_PATTERN = Pattern.compile("on\\s*conflict\\s*\\(.*?\\)", - Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE | Pattern.DOTALL); private final ExecutorService executorService; public SqlParser(ExecutorService executorService) { @@ -29,39 +18,10 @@ public SqlParser(ExecutorService executorService) { } public Statements parse(String sql, Duration timeout) throws JSQLParserException { - sql = replaceFunctions(sql); - sql = replaceOnConflicts(sql); - CCJSqlParser parser = newParser(sql).withAllowComplexParsing(true); return parseStatementAsync(parser, timeout); } - // Sqlparser 4.6 does not support "on conflict" clause with multiple parameters. - // As a workaround, we change the query to a single parameter clause. - protected String replaceOnConflicts(String sql) { - var matcher = ON_CONFLICT_REPLACEMENT_PATTERN.matcher(sql); - - // 99.99% of sqls don't have it, so let's avoid new string creation for those. - if (matcher.find()) { - matcher.reset(); - return matcher.replaceAll(Matcher.quoteReplacement("on conflict (blah)")); - } - - return sql; - } - - protected String replaceFunctions(String sql) { - var matcher = FUNCTION_REPLACEMENT_PATTERN.matcher(sql); - - // 99.99% of sqls don't have it, so let's avoid new string creation for those. - if (matcher.find()) { - matcher.reset(); - return matcher.replaceAll(Matcher.quoteReplacement("UNSUPPORTED()")); - } - - return sql; - } - protected Statements parseStatementAsync(CCJSqlParser parser, Duration timeout) throws JSQLParserException { try { var future = executorService.submit(parser::Statements); diff --git a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/TableAccessStatisticsSpyqlListener.java b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/TableAccessStatisticsSpyqlListener.java index c7e872c..42379d8 100644 --- a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/TableAccessStatisticsSpyqlListener.java +++ b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/TableAccessStatisticsSpyqlListener.java @@ -14,6 +14,7 @@ import com.transferwise.common.entrypoints.EntryPointsMetrics; import com.transferwise.common.entrypoints.EntryPointsProperties; import com.transferwise.common.entrypoints.tableaccessstatistics.ParsedQuery.SqlOperation; +import com.transferwise.common.entrypoints.tableaccessstatistics.TasQueryParsingInterceptor.InterceptResult; import com.transferwise.common.entrypoints.tableaccessstatistics.TasQueryParsingInterceptor.InterceptResult.Decision; import com.transferwise.common.spyql.event.GetConnectionEvent; import com.transferwise.common.spyql.event.StatementExecuteEvent; @@ -32,7 +33,15 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; +import net.sf.jsqlparser.statement.SetStatement; import net.sf.jsqlparser.statement.Statement; +import net.sf.jsqlparser.statement.Statements; +import net.sf.jsqlparser.statement.UnsupportedStatement; +import net.sf.jsqlparser.statement.delete.Delete; +import net.sf.jsqlparser.statement.insert.Insert; +import net.sf.jsqlparser.statement.select.Select; +import net.sf.jsqlparser.statement.truncate.Truncate; +import net.sf.jsqlparser.statement.update.Update; import org.apache.commons.lang3.StringUtils; @Slf4j @@ -85,7 +94,7 @@ public TableAccessStatisticsSpyqlListener(IMeterCache meterCache, ExecutorServic this.tasQueryParsingInterceptor = tasQueryParsingInterceptor; this.tasQueryParsingListener = tasQueryParsingListener; - MeterRegistry meterRegistry = meterCache.getMeterRegistry(); + final MeterRegistry meterRegistry = meterCache.getMeterRegistry(); meterRegistry.config().meterFilter(new TasMeterFilter()); sqlParseResultsCache = Caffeine.newBuilder().maximumWeight(entryPointsProperties.getTas().getSqlParser().getCacheSizeMib() * MIB).recordStats() @@ -113,32 +122,40 @@ public SpyqlConnectionListener onGetConnection(GetConnectionEvent event) { } protected ParsedQuery parseSql(String sql, TwContext context) { - var interceptResult = tasQueryParsingInterceptor.intercept(sql); + final InterceptResult interceptResult = tasQueryParsingInterceptor.intercept(sql); if (interceptResult.getDecision() == Decision.CUSTOM_PARSED_QUERY) { return interceptResult.getParsedQuery(); } else if (interceptResult.getDecision() == Decision.SKIP) { return new ParsedQuery(); } - ParsedQuery result = new ParsedQuery(); - long startTimeMs = System.currentTimeMillis(); + final ParsedQuery result = new ParsedQuery(); + final long startTimeMs = System.currentTimeMillis(); try { - var stmts = sqlParser.parse(sql, entryPointsProperties.getTas().getSqlParser().getTimeout()); + final Statements stmts = sqlParser.parse(sql, entryPointsProperties.getTas().getSqlParser().getTimeout()); + + for (Statement stmt : stmts) { + if (stmt instanceof UnsupportedStatement) { + throw new IllegalStateException("Unsupported statement."); + } + } + + for (Statement stmt : stmts) { - for (Statement stmt : stmts.getStatements()) { // Intern() makes later equal checks much faster. - String opName = getOperationName(stmt).intern(); - CustomTablesNamesFinder tablesNamesFinder = new CustomTablesNamesFinder(); + final String opName = getOperationName(stmt).intern(); + final CustomTablesNamesFinder tablesNamesFinder = new CustomTablesNamesFinder(); List tableNames = null; try { - tableNames = tablesNamesFinder.getTableList(stmt); + tablesNamesFinder.getTables(stmt); + tableNames = tablesNamesFinder.getTables(); } catch (UnsupportedOperationException e) { // Some type of statements do not support finding table names. // For example a statement 'SHOW FULL TABLES IN ...'. log.debug("Unsupported query '{}'.", sql, e); } - ParsedQuery.SqlOperation sqlOp = result + final SqlOperation sqlOp = result .getOperations() .computeIfAbsent(opName, k -> new ParsedQuery.SqlOperation()); @@ -199,8 +216,21 @@ protected String trimTableName(String tableName) { } protected String getOperationName(Statement stmt) { - // class.getSimpleName() is very slow on JDK 8 - return StringUtils.substringAfterLast(stmt.getClass().getName(), ".").toLowerCase(); + if (stmt instanceof Select) { + return "select"; + } else if (stmt instanceof Update) { + return "update"; + } else if (stmt instanceof Insert) { + return "insert"; + } else if (stmt instanceof Delete) { + return "delete"; + } else if (stmt instanceof Truncate) { + return "truncate"; + } else if (stmt instanceof SetStatement) { + return "set"; + } + + return stmt.getClass().getSimpleName().toLowerCase(); } class ConnectionListener implements SpyqlConnectionListener { @@ -216,7 +246,7 @@ public void onStatementExecuteFailure(StatementExecuteFailureEvent event) { } protected void registerSql(String sql, boolean isInTransaction, boolean succeeded, long executionTimeNs) { - TwContext context = TwContext.current(); + final TwContext context = TwContext.current(); final Tag inTransactionTag = isInTransaction ? TAG_IN_TRANSACTION_TRUE : TAG_IN_TRANSACTION_FALSE; final Tag successTag = succeeded ? TAG_SUCCESS_TRUE : TAG_SUCCESS_FALSE; @@ -226,7 +256,7 @@ protected void registerSql(String sql, boolean isInTransaction, boolean succeede if (TasUtils.isQueryParsingEnabled(TwContext.current())) { parsedQuery = sqlParseResultsCache.get(sql, sqlForCache -> parseSql(sqlForCache, context)); } else { - var interceptResult = tasQueryParsingInterceptor.intercept(sql); + InterceptResult interceptResult = tasQueryParsingInterceptor.intercept(sql); if (interceptResult.getDecision() == Decision.CUSTOM_PARSED_QUERY) { parsedQuery = interceptResult.getParsedQuery(); } @@ -245,12 +275,12 @@ protected void registerSql(String sql, boolean isInTransaction, boolean succeede } for (Entry entry : parsedQuery.getOperations().entrySet()) { - String opName = entry.getKey(); - SqlOperation op = entry.getValue(); + final String opName = entry.getKey(); + final SqlOperation op = entry.getValue(); if (op.getTableNames() != null) { String firstTableName = null; for (String tableName : op.getTableNames()) { - TagsSet tagsSet = TagsSet.of( + final TagsSet tagsSet = TagsSet.of( dbTag.getKey(), dbTag.getValue(), TwContextMetricsTemplate.TAG_EP_GROUP, context.getGroup(), TwContextMetricsTemplate.TAG_EP_NAME, context.getName(), diff --git a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/transactionstatistics/TransactionsStatisticsSpyqlListener.java b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/transactionstatistics/TransactionsStatisticsSpyqlListener.java index 6a894c7..895e951 100644 --- a/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/transactionstatistics/TransactionsStatisticsSpyqlListener.java +++ b/tw-entrypoints/src/main/java/com/transferwise/common/entrypoints/transactionstatistics/TransactionsStatisticsSpyqlListener.java @@ -135,10 +135,10 @@ TAG_TRANSACTION_NAME, nullToUnknown(transactionDefinition.getName()), operationTag.getKey(), operationTag.getValue(), readOnlyTag.getKey(), readOnlyTag.getValue()); - TransactionMetrics metrics = meterCache.metersContainer(METRIC_COLLECTION_TRANSACTION_END, tagsSet, () -> { + TransactionMetrics metrics = meterCache.metersContainer(METRIC_COLLECTION_TRANSACTION_END, tagsSet, (name, tags) -> { TransactionMetrics result = new TransactionMetrics(); - result.completion = meterCache.timer(METRIC_TRANSACTION_COMPLETION, tagsSet); - result.finalization = meterCache.timer(METRIC_TRANSACTION_FINALIZATION, tagsSet); + result.completion = meterCache.timer(METRIC_TRANSACTION_COMPLETION, tags); + result.finalization = meterCache.timer(METRIC_TRANSACTION_FINALIZATION, tags); return result; }); diff --git a/tw-entrypoints/src/test/java/com/transferwise/common/entrypoints/tableaccessstatistics/SqlParserUtilsTest.java b/tw-entrypoints/src/test/java/com/transferwise/common/entrypoints/tableaccessstatistics/SqlParserUtilsTest.java index 638d16f..0e7de72 100644 --- a/tw-entrypoints/src/test/java/com/transferwise/common/entrypoints/tableaccessstatistics/SqlParserUtilsTest.java +++ b/tw-entrypoints/src/test/java/com/transferwise/common/entrypoints/tableaccessstatistics/SqlParserUtilsTest.java @@ -5,52 +5,92 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.Executors; +import lombok.Data; +import lombok.RequiredArgsConstructor; import lombok.SneakyThrows; +import lombok.extern.slf4j.Slf4j; import net.sf.jsqlparser.JSQLParserException; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +@Slf4j class SqlParserUtilsTest { @Test @SneakyThrows void testJSqlParser() { - List sqls = new ArrayList<>(); + final List testCases = new ArrayList<>(); - sqls.add("insert into fin_unique_tw_task_key(task_id,key_hash,key) values(?, ?, ?) on conflict (key_hash, key) do nothing"); + testCases.add(new TestCase("insert into fin_unique_tw_task_key(task_id,key_hash,key) values(?, ?, ?) on conflict (key_hash, key) do nothing", + List.of("fin_unique_tw_task_key"))); // DATABASE is a keyword for sql parser. - sqls.add("select DATABASE()"); + testCases.add(new TestCase("select DATABASE()", + List.of())); - sqls.add("delete tl from tag_links tl, tags t where t.name=? and tl.type=? and tl.tag_ref=? and tl.tag_id = t.id"); - sqls.add("select table_rows from information_schema.tables where table_schema=DATABASE() and table_name = 'tw_task'"); + testCases.add(new TestCase("delete tl from tag_links tl, tags t where t.name=? and tl.type=? and tl.tag_ref=? and tl.tag_id = t.id", + List.of("tag_links", "tags"))); + testCases.add(new TestCase("select table_rows from information_schema.tables where table_schema=DATABASE() and table_name = 'tw_task'", + List.of("information_schema.tables"))); - sqls.add("SHOW FULL TABLES IN fx WHERE TABLE_TYPE NOT LIKE 'VIEW'"); + testCases.add(new TestCase("SHOW FULL TABLES IN fx WHERE TABLE_TYPE NOT LIKE 'VIEW'", + List.of())); - sqls.add("SET statement_timeout TO '18000'"); - sqls.add("set idle_in_transaction_session_timeout to '600000'"); + testCases.add(new TestCase("SET statement_timeout TO '18000'", + List.of())); + testCases.add(new TestCase("set idle_in_transaction_session_timeout to '600000'", + List.of())); - sqls.add("insert into fin_unique_tw_task_key(task_id,key_hash,key) values(?, ?, ?) on conflict (key_hash, key) do nothing"); + testCases.add(new TestCase("insert into fin_unique_tw_task_key(task_id,key_hash,key) values(?, ?, ?) on conflict (key_hash, key) do nothing", + List.of("fin_unique_tw_task_key"))); + + testCases.add(new TestCase("UPDATE ninjas_wkp_partition_locks USE INDEX (uqidx1) set deadline=?", + List.of("ninjas_wkp_partition_locks"))); + + testCases.add(new TestCase("truncate table_a; truncate table_b", + List.of("table_a", "table_b"))); // Not supported by JSqlParser - // sqls.add("truncate table_a, table_b"); + // testCases.add(new TestCase("truncate table_a, table_b", + // List.of("table_a", "table_b"))); var sqlParser = new SqlParser(Executors.newCachedThreadPool()); var interceptor = new DefaultTasQueryParsingInterceptor(); - for (String sql : sqls) { + for (TestCase testCase : testCases) { try { - var interceptResult = interceptor.intercept(sql); + var interceptResult = interceptor.intercept(testCase.getSql()); if (interceptResult.getDecision() == Decision.SKIP) { continue; } - var statements = sqlParser.parse(sql, Duration.ofSeconds(5)); + var statements = sqlParser.parse(testCase.getSql(), Duration.ofSeconds(5)); + + List tables = new ArrayList<>(); + + for (var stmt : statements) { + var tablesNamesFinder = new CustomTablesNamesFinder(); + try { + tablesNamesFinder.getTables(stmt); + } catch (UnsupportedOperationException ignored) { + //ignored + } + tables.addAll(tablesNamesFinder.getTables()); + } + + Assertions.assertEquals(testCase.getExpectedTables(), tables); - statements.getStatements(); } catch (JSQLParserException e) { - throw new RuntimeException("Failed to parse sql '" + sql + "'.", e); + throw new RuntimeException("Failed to parse sql '" + testCase.getSql() + "'.", e); } } } + @RequiredArgsConstructor + @Data + private static class TestCase { + + private final String sql; + private final List expectedTables; + } }