Improved the query parsing for table access statistics (#31)

* Improvements for Table Access Statistics query parsing.

CHANGELOG.md

@@ -5,6 +5,27 @@ 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.11.0] - 2023-05-18
+
+### Changed
+
+* Implemented a timeout and interruption for TAS SQL parsing.
+ Complex queries in one of our services created long duration heavy CPU burn.
+
+* Query parsing will use `JSQLParser` complex parsing immediately.
+ Before, our implementation was using simple parsing. And `JSQLParser` implementation tried by default `simple` first
+ and then `complex`, if `simple` failed.
+ Performance tests showed `simple` parsing for simple queries, is not noticeably faster for simple queries.
+
+* Created a mechanism for a service to provide parsed query information itself and thus skip the query parsing.
+ It can be used for complex queries where parsing is slow or for queries which jsqlparser can not handle.
+ The mechanism can be used via `TableAccessStatisticsParsedQueryRegistry` interface.
+
+* Added more flexibility around query parsing via `TasQueryParsingListener` and `TasQueryParsingInterceptor`.
+
+* Supporting parsing queries with `on conflict (...)` clause with multiple parameters.
+ We can remove our own solution, when next `JSQLParser` version would support it.
+
 ## [2.10.0] - 2023-05-09
 
 ### Added

README.md

@@ -10,5 +10,3 @@
 Provides various metrics for service's databases usage.
 
 Reference the [documentation](docs/index.md) of the library for more information about adoption and features.
-
-

docs/index.md

@@ -1,6 +1,7 @@
 # Tw-EntryPoints documentation
 
 ## Table of Contents
+
 * [Introduction](#intro)
 * [Setup](#setup)
 * [Integration tests](#integration-tests)
@@ -18,13 +19,11 @@ Is built on top of
 
 Integrates nicely with Transferwise Entrypoints system.
 
-
 Example dashboards:
 
 - [EntryPoints Database Access V3](https://dashboards.tw.ee/d/f6l4lrUWz/entrypoints-database-access-v3?orgId=1)
 - [EntryPoints Table Access V2](https://dashboards.tw.ee/d/dyp0u9UZz/entrypoints-table-access-v2?orgId=1)
 
-
 ## Setup
 
 Assuming, your data sources are using [HikariCP](https://github.com/brettwooldridge/HikariCP), only thing you need to do, is to add a dependency.
@@ -59,6 +58,75 @@ You would need to add a dependency for this as well:
 testImplementation "com.transferwise.common:tw-entrypoints"
 ```
 
+## Table access statistics and `JSqlParser` library
+
+We are using (JSqlParser)[https://github.com/JSQLParser/JSqlParser] library to parse table names from queries.
+
+The library is pretty good, but some services have few queries, it can not parse. Also, sometimes the parsing can take so long,
+that it will create latency spikes or cpu burns.
+
+In those case, you can override/control the parsing via `TasQueryParsingInterceptor` and `TasParsedQueryRegistry`.
+
+Example for `TasQueryParsingInterceptor`.
+
+<!-- @formatter:off -->
+```java
+public class MyTasQueryParsingInterceptor extends DefaultTasQueryParsingInterceptor {
+
+ @Override
+ public InterceptResult intercept(String sql) {
+ if (StringUtils.startsWithIgnoreCase(sql, "SET fancy_variable TO")) {
+ return InterceptResult.doSkip();
+ }
+
+ else if (sql.equals(knownUnParseableSql)){
+ return InterceptResult.returnParsedQuery(new ParsedQuery()
+ .addOperation("insert",new SqlOperation()
+ // Main table should always be first, as we register "first-table" metrics by that.
+ .addTable("transfer")
+ .addTable("payout")));
+ }
+
+ return super.intercept(sql);
+ }
+}
+```
+<!-- @formatter:on -->
+
+Example for `TasParsedQueryRegistry`.
+
+<!-- @formatter:off -->
+```java
+@Autowired
+private TasParsedQueryRegistry registry;
+
+public void registerBadSqls(){
+ registry.register(knownUnParseableSql,new ParsedQuery()
+ .addOperation("insert",new SqlOperation()
+ .addTable("transfer")
+ .addTable("payout")));
+}
+```
+<!-- @formatter:on -->
+
+In case where failed parsing will create too much logs noise, you have an option to override `TasQueryParsingListener`.
+
+For example:
+
+```java
+public class MyTasQueryParsingListener extends DefaultTasQueryParsingListener {
+
+ @Override
+ public void parsingFailed(String sql, Duration timeTaken, Throwable t) {
+ if (sql.equals(knownProblematicQuery)) {
+ // ignore
+ } else {
+ super.parsingFailed(sql, timeTaken, t);
+ }
+ }
+}
+```
+
 ## License
 
 Copyright 2021 TransferWise Ltd.

gradle.properties

@@ -1 +1 @@
-version=2.10.0
+version=2.11.0

tw-entrypoints-starter/src/main/java/com/transferwise/common/entrypoints/EntryPointsAutoConfiguration.java

@@ -9,19 +9,27 @@
 import com.transferwise.common.entrypoints.databaseaccessstatistics.DatabaseAccessStatisticsBeanPostProcessor;
 import com.transferwise.common.entrypoints.databaseaccessstatistics.DatabaseAccessStatisticsEntryPointInterceptor;
 import com.transferwise.common.entrypoints.executionstatistics.ExecutionStatisticsEntryPointInterceptor;
+import com.transferwise.common.entrypoints.tableaccessstatistics.DefaultTasParsedQueryRegistry;
+import com.transferwise.common.entrypoints.tableaccessstatistics.DefaultTasQueryParsingInterceptor;
+import com.transferwise.common.entrypoints.tableaccessstatistics.DefaultTasQueryParsingListener;
 import com.transferwise.common.entrypoints.tableaccessstatistics.TableAccessStatisticsBeanPostProcessor;
+import com.transferwise.common.entrypoints.tableaccessstatistics.TasParsedQueryRegistry;
+import com.transferwise.common.entrypoints.tableaccessstatistics.TasQueryParsingInterceptor;
+import com.transferwise.common.entrypoints.tableaccessstatistics.TasQueryParsingListener;
 import com.transferwise.common.entrypoints.transactionstatistics.TransactionStatisticsBeanPostProcessor;
 import io.micrometer.core.instrument.MeterRegistry;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
+import org.springframework.boot.context.properties.ConfigurationProperties;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
 @Configuration
 public class EntryPointsAutoConfiguration {
 
  @Bean
+ @ConfigurationProperties(value = "tw-entrypoints", ignoreUnknownFields = false)
  public EntryPointsProperties twEntryPointsProperties() {
  return new EntryPointsProperties();
  }
@@ -49,6 +57,27 @@ public TableAccessStatisticsBeanPostProcessor twEntryPointsTableAccessStatistics
  return new TableAccessStatisticsBeanPostProcessor(beanFactory);
  }
 
+ @Bean
+ @ConditionalOnProperty(name = "tw-entrypoints.tas.enabled", havingValue = "true", matchIfMissing = true)
+ @ConditionalOnMissingBean(TasParsedQueryRegistry.class)
+ public DefaultTasParsedQueryRegistry twEntryPointsTableAccessStatisticsParsedQueryRegistry() {
+ return new DefaultTasParsedQueryRegistry();
+ }
+
+ @Bean
+ @ConditionalOnProperty(name = "tw-entrypoints.tas.enabled", havingValue = "true", matchIfMissing = true)
+ @ConditionalOnMissingBean(TasQueryParsingInterceptor.class)
+ public DefaultTasQueryParsingInterceptor twEntryPointsTableAccessStatisticsQueryParsingInterceptor() {
+ return new DefaultTasQueryParsingInterceptor();
+ }
+
+ @Bean
+ @ConditionalOnProperty(name = "tw-entrypoints.tas.enabled", havingValue = "true", matchIfMissing = true)
+ @ConditionalOnMissingBean(TasQueryParsingListener.class)
+ public DefaultTasQueryParsingListener twEntryPointsTableAccessStatisticsQueryParsingListener(EntryPointsProperties entryPointsProperties) {
+ return new DefaultTasQueryParsingListener(entryPointsProperties);
+ }
+
  @Bean
  @ConditionalOnProperty(name = "tw-entrypoints.ts.enabled", havingValue = "true", matchIfMissing = true)
  @ConditionalOnMissingBean

tw-entrypoints-starter/src/main/java/com/transferwise/common/entrypoints/tableaccessstatistics/TableAccessStatisticsBeanPostProcessor.java

@@ -4,16 +4,12 @@
 import com.transferwise.common.baseutils.concurrency.ThreadNamingExecutorServiceWrapper;
 import com.transferwise.common.baseutils.meters.cache.IMeterCache;
 import com.transferwise.common.entrypoints.BaseEntryPointsBeanProcessor;
+import com.transferwise.common.entrypoints.EntryPointsProperties;
 import com.transferwise.common.spyql.SpyqlDataSource;
-import java.util.concurrent.ExecutorService;
 import org.springframework.beans.factory.BeanFactory;
-import org.springframework.beans.factory.annotation.Value;
 
 public class TableAccessStatisticsBeanPostProcessor extends BaseEntryPointsBeanProcessor {
 
- @Value("${tw-entrypoints.tas.sql-parser.cache-size-mib:50}")
- private int sqlParserCacheSizeMib;
-
  private final BeanFactory beanFactory;
 
  public TableAccessStatisticsBeanPostProcessor(BeanFactory beanFactory) {
@@ -23,17 +19,25 @@ public TableAccessStatisticsBeanPostProcessor(BeanFactory beanFactory) {
  @Override
  protected void instrument(SpyqlDataSource spyqlDataSource, String databaseName) {
  boolean isAlreadyAttached = spyqlDataSource.getDataSourceListeners().stream().anyMatch(
- l -> l instanceof TableAccessStatisticsSpyqlListener);
+ TableAccessStatisticsSpyqlListener.class::isInstance);
 
  if (isAlreadyAttached) {
  return;
  }
 
- IMeterCache meterCache = beanFactory.getBean(IMeterCache.class);
- ExecutorService executorService = new ThreadNamingExecutorServiceWrapper("eptas", beanFactory
+ var entryPointsProperties = beanFactory.getBean(EntryPointsProperties.class);
+ var meterCache = beanFactory.getBean(IMeterCache.class);
+ var executorService = new ThreadNamingExecutorServiceWrapper("eptas", beanFactory
  .getBean(IExecutorServicesProvider.class).getGlobalExecutorService());
 
+ var tableAccessStatisticsParsedQueryRegistry = beanFactory.getBean(
+ TasParsedQueryRegistry.class);
+
+ var tasQueryParsingInterceptor = beanFactory.getBean(TasQueryParsingInterceptor.class);
+ var tasQueryParsingListener = beanFactory.getBean(TasQueryParsingListener.class);
+
  spyqlDataSource.addListener(
- new TableAccessStatisticsSpyqlListener(meterCache, executorService, databaseName, sqlParserCacheSizeMib));
+ new TableAccessStatisticsSpyqlListener(meterCache, executorService, tableAccessStatisticsParsedQueryRegistry, databaseName,
+ entryPointsProperties, tasQueryParsingListener, tasQueryParsingInterceptor));
  }
 }

tw-entrypoints-starter/src/test/java/com/transferwise/common/entrypoints/tableaccessstatistics/TableAccessStatisticsIntTest.java

@@ -4,6 +4,7 @@
 
 import com.transferwise.common.baseutils.ExceptionUtils;
 import com.transferwise.common.context.TwContext;
+import com.transferwise.common.entrypoints.tableaccessstatistics.ParsedQuery.SqlOperation;
 import com.transferwise.common.entrypoints.test.BaseIntTest;
 import com.transferwise.common.spyql.SpyqlDataSource;
 import io.micrometer.core.instrument.Counter;
@@ -19,22 +20,29 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.jdbc.core.JdbcTemplate;
 
-public class TableAccessStatisticsIntTest extends BaseIntTest {
+class TableAccessStatisticsIntTest extends BaseIntTest {
+
+ @Autowired
+ private TestTasQueryParsingInterceptor testTasQueryParsingInterceptor;
 
  @Autowired
  private DataSource dataSource;
 
+ @Autowired
+ private DefaultTasParsedQueryRegistry tableAccessStatisticsParsedQueryRegistry;
+
  private JdbcTemplate jdbcTemplate;
 
  @BeforeEach
  public void setup() {
  super.setup();
  jdbcTemplate = new JdbcTemplate(dataSource);
  invalidateParserCache();
+ testTasQueryParsingInterceptor.setParsedQuery(null);
  }
 
  @Test
- public void selectToNotExistingTableGetsCorrectlyRegistered() {
+ void selectToNotExistingTableGetsCorrectlyRegistered() {
  TwContext.current().createSubContext().asEntryPoint("Test", "myEntryPoint").execute(() -> {
  try {
  jdbcTemplate.queryForObject("select id from not_existing_table", Long.class);
@@ -71,6 +79,58 @@ public void selectToNotExistingTableGetsCorrectlyRegistered() {
  assertThat(firstTableAccessMeter.mean(TimeUnit.NANOSECONDS)).isGreaterThan(0);
  }
 
+ @Test
+ void parsedQueryRegistryCanOverrideParsing() {
+ String sql = "select id from not_existing_table limit 1234";
+
+ tableAccessStatisticsParsedQueryRegistry.register(sql, new ParsedQuery().addOperation("upsert", new SqlOperation().addTable("my_custom_table")));
+
+ TwContext.current().createSubContext().asEntryPoint("Test", "myEntryPoint").execute(() -> {
+ try {
+ jdbcTemplate.queryForObject("select id from not_existing_table limit 1234", Long.class);
+ } catch (Exception ignored) {
+ //ignored
+ }
+ });
+
+ List<Meter> meters = getTableAccessMeters();
+
+ assertThat(meters.size()).isEqualTo(1);
+ var counter = (Counter) meters.get(0);
+
+ assertThat(counter.getId().getTag("operation")).isEqualTo("upsert");
+ assertThat(counter.getId().getTag("table")).isEqualTo("my_custom_table");
+
+ var firstTableAccessMeter = (Timer) getMeter("EntryPoints_Tas_FirstTableAccess");
+
+ assertThat(firstTableAccessMeter.getId().getTag("operation")).isEqualTo("upsert");
+ assertThat(firstTableAccessMeter.getId().getTag("table")).isEqualTo("my_custom_table");
+ }
+
+ @Test
+ void interceptorCanProvideItsOwnParsedQuery() {
+ testTasQueryParsingInterceptor.setParsedQuery(new ParsedQuery().addOperation("insert",
+ new SqlOperation().addTable("my_custom_table123")));
+
+ String sql = "select id from not_existing_table limit 1234";
+
+ TwContext.current().createSubContext().asEntryPoint("Test", "myEntryPoint").execute(() -> {
+ try {
+ jdbcTemplate.queryForObject(sql, Long.class);
+ } catch (Exception ignored) {
+ //ignored
+ }
+ });
+
+ List<Meter> meters = getTableAccessMeters();
+
+ assertThat(meters.size()).isEqualTo(1);
+ var counter = (Counter) meters.get(0);
+
+ assertThat(counter.getId().getTag("operation")).isEqualTo("insert");
+ assertThat(counter.getId().getTag("table")).isEqualTo("my_custom_table123");
+ }
+
  @Test
  public void workingUpdateSqlGetCorrectlyRegistered() {
  TwContext.current().createSubContext().asEntryPoint("Test", "myEntryPoint").execute(() -> {

tw-entrypoints-starter/src/test/java/com/transferwise/common/entrypoints/tableaccessstatistics/TestTasQueryParsingInterceptor.java

@@ -0,0 +1,21 @@
+package com.transferwise.common.entrypoints.tableaccessstatistics;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.springframework.stereotype.Component;
+
+@Component
+public class TestTasQueryParsingInterceptor extends DefaultTasQueryParsingInterceptor {
+
+ @Getter
+ @Setter
+ private ParsedQuery parsedQuery;
+
+ @Override
+ public InterceptResult intercept(String sql) {
+ if (parsedQuery != null) {
+ return InterceptResult.returnParsedQuery(parsedQuery);
+ }
+ return super.intercept(sql);
+ }
+}

tw-entrypoints-starter/src/test/java/com/transferwise/common/entrypoints/test/TestApplication.java

@@ -1,6 +1,7 @@
 package com.transferwise.common.entrypoints.test;
 
 import com.transferwise.common.baseutils.transactionsmanagement.TransactionsHelper;
+import com.transferwise.common.entrypoints.tableaccessstatistics.TestTasQueryParsingInterceptor;
 import com.transferwise.common.spyql.SpyqlDataSource;
 import com.zaxxer.hikari.HikariConfig;
 import com.zaxxer.hikari.HikariDataSource;
@@ -37,4 +38,9 @@ public DataSource dataSource() {
  public TransactionsHelper transactionsHelper() {
  return new TransactionsHelper();
  }
+
+ @Bean
+ public TestTasQueryParsingInterceptor testTasQueryParsingInterceptor() {
+ return new TestTasQueryParsingInterceptor();
+ }
 }

tw-entrypoints/build-idea-fix.gradle.kts

@@ -0,0 +1,6 @@
+gradle.taskGraph.whenReady {
+ val task = this.allTasks.find { it.name.endsWith(".main()") } as? JavaExec // or whatever other method your Main class runs
+ task?.let {
+ it.setExecutable(it.javaLauncher.get().executablePath.asFile.absolutePath)
+ }
+}

tw-entrypoints/build.gradle

@@ -20,5 +20,6 @@ dependencies {
  implementation libraries.twGracefulShutdownInterfaces
 
  testImplementation libraries.junitJupiter
-
 }
+
+apply from: "build-idea-fix.gradle.kts"