Log JdbcClient invocations

presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/JdbcModule.java

@@ -19,11 +19,15 @@
 import com.google.inject.Provides;
 import com.google.inject.Scopes;
 import com.google.inject.Singleton;
+import io.airlift.log.Logger;
+import io.prestosql.plugin.base.util.LoggingInvocationHandler;
+import io.prestosql.plugin.base.util.LoggingInvocationHandler.ParameterNamesProvider;
 import io.prestosql.plugin.jdbc.jmx.StatisticsAwareConnectionFactory;
 import io.prestosql.plugin.jdbc.jmx.StatisticsAwareJdbcClient;
 import io.prestosql.spi.connector.ConnectorAccessControl;
 import io.prestosql.spi.procedure.Procedure;
 
+import static com.google.common.reflect.Reflection.newProxy;
 import static com.google.inject.multibindings.Multibinder.newSetBinder;
 import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder;
 import static io.airlift.configuration.ConfigBinder.configBinder;
@@ -62,7 +66,16 @@ public void configure(Binder binder)
     @InternalBaseJdbc
     public static JdbcClient createJdbcClientWithStats(JdbcClient client)
     {
-        return new StatisticsAwareJdbcClient(client);
+        StatisticsAwareJdbcClient statisticsAwareJdbcClient = new StatisticsAwareJdbcClient(client);
+
+        Logger logger = Logger.get(JdbcClient.class);
+        if (!logger.isDebugEnabled()) {
+            return statisticsAwareJdbcClient;
+        }
+
+        ParameterNamesProvider parameterNamesProvider = new LoggingInvocationHandler.AirliftParameterNamesProvider(JdbcClient.class, StatisticsAwareJdbcClient.class);
+        LoggingInvocationHandler loggingInvocationHandler = new LoggingInvocationHandler(statisticsAwareJdbcClient, parameterNamesProvider, logger::debug);
+        return newProxy(JdbcClient.class, loggingInvocationHandler);
     }
 
     @Provides

presto-hive/pom.xml

@@ -98,12 +98,6 @@
             <artifactId>configuration</artifactId>
         </dependency>
 
-        <dependency>
-            <groupId>io.airlift</groupId>
-            <artifactId>parameternames</artifactId>
-            <version>1.4</version>
-        </dependency>
-
         <dependency>
             <groupId>com.google.guava</groupId>
             <artifactId>guava</artifactId>

presto-hive/src/main/java/io/prestosql/plugin/hive/metastore/thrift/ThriftHiveMetastoreClient.java

@@ -15,9 +15,9 @@
 
 import com.google.common.collect.ImmutableList;
 import io.airlift.log.Logger;
-import io.prestosql.plugin.hive.util.LoggingInvocationHandler;
-import io.prestosql.plugin.hive.util.LoggingInvocationHandler.AirliftParameterNamesProvider;
-import io.prestosql.plugin.hive.util.LoggingInvocationHandler.ParameterNamesProvider;
+import io.prestosql.plugin.base.util.LoggingInvocationHandler;
+import io.prestosql.plugin.base.util.LoggingInvocationHandler.AirliftParameterNamesProvider;
+import io.prestosql.plugin.base.util.LoggingInvocationHandler.ParameterNamesProvider;
 import org.apache.hadoop.hive.metastore.api.ColumnStatistics;
 import org.apache.hadoop.hive.metastore.api.ColumnStatisticsDesc;
 import org.apache.hadoop.hive.metastore.api.ColumnStatisticsObj;

presto-hive/src/test/java/io/prestosql/plugin/hive/util/TestLoggingInvocationHandlerWithHiveMetastore.java

@@ -0,0 +1,56 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.prestosql.plugin.hive.util;
+
+import io.prestosql.plugin.base.util.LoggingInvocationHandler;
+import io.prestosql.plugin.base.util.LoggingInvocationHandler.AirliftParameterNamesProvider;
+import org.apache.hadoop.hive.metastore.api.ThriftHiveMetastore;
+import org.testng.annotations.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static com.google.common.reflect.Reflection.newProxy;
+import static java.lang.reflect.Proxy.newProxyInstance;
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class TestLoggingInvocationHandlerWithHiveMetastore
+{
+    private static final String DURATION_PATTERN = "\\d+(\\.\\d+)?\\w{1,2}";
+
+    @Test
+    public void testWithThriftHiveMetastoreClient()
+            throws Exception
+    {
+        List<String> messages = new ArrayList<>();
+        // LoggingInvocationHandler is used e.g. with ThriftHiveMetastore.Iface. Since the logging is reflection-based,
+        // we test it with this interface as well.
+        ThriftHiveMetastore.Iface proxy = newProxy(ThriftHiveMetastore.Iface.class, new LoggingInvocationHandler(
+                dummyThriftHiveMetastoreClient(),
+                new AirliftParameterNamesProvider(ThriftHiveMetastore.Iface.class, ThriftHiveMetastore.Client.class),
+                messages::add));
+        proxy.get_table("some_database", "some_table_name");
+        assertThat(messages)
+                .hasSize(1)
+                .element(0).matches(message -> message.matches("\\QInvocation of get_table(dbname='some_database', tbl_name='some_table_name') succeeded in\\E " + DURATION_PATTERN));
+    }
+
+    private static ThriftHiveMetastore.Iface dummyThriftHiveMetastoreClient()
+    {
+        return (ThriftHiveMetastore.Iface) newProxyInstance(
+                TestLoggingInvocationHandlerWithHiveMetastore.class.getClassLoader(),
+                new Class<?>[] {ThriftHiveMetastore.Iface.class},
+                (proxy, method, args) -> null);
+    }
+}

presto-plugin-toolkit/pom.xml

@@ -32,6 +32,12 @@
             <artifactId>configuration</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>io.airlift</groupId>
+            <artifactId>parameternames</artifactId>
+            <version>1.4</version>
+        </dependency>
+
         <dependency>
             <groupId>com.google.inject</groupId>
             <artifactId>guice</artifactId>

presto-hive/src/main/java/io/prestosql/plugin/hive/util/LoggingInvocationHandler.java → presto-plugin-toolkit/src/main/java/io/prestosql/plugin/base/util/LoggingInvocationHandler.java

@@ -11,7 +11,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package io.prestosql.plugin.hive.util;
+package io.prestosql.plugin.base.util;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;

presto-hive/src/test/java/io/prestosql/plugin/hive/util/TestLoggingInvocationHandler.java → presto-plugin-toolkit/src/test/java/io/prestosql/plugin/base/util/TestLoggingInvocationHandler.java

@@ -11,19 +11,16 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package io.prestosql.plugin.hive.util;
+package io.prestosql.plugin.base.util;
 
-import io.prestosql.plugin.hive.util.LoggingInvocationHandler.AirliftParameterNamesProvider;
-import io.prestosql.plugin.hive.util.LoggingInvocationHandler.ReflectiveParameterNamesProvider;
-import org.apache.hadoop.hive.metastore.api.ThriftHiveMetastore;
+import io.prestosql.plugin.base.util.LoggingInvocationHandler.ReflectiveParameterNamesProvider;
 import org.testng.annotations.Test;
 
 import java.lang.reflect.InvocationHandler;
 import java.util.ArrayList;
 import java.util.List;
 
 import static com.google.common.reflect.Reflection.newProxy;
-import static java.lang.reflect.Proxy.newProxyInstance;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
@@ -63,31 +60,6 @@ public void run(boolean ok, String s)
                 });
     }
 
-    @Test
-    public void testWithThriftHiveMetastoreClient()
-            throws Exception
-    {
-        List<String> messages = new ArrayList<>();
-        // LoggingInvocationHandler is used e.g. with ThriftHiveMetastore.Iface. Since the logging is reflection-based,
-        // we test it with this interface as well.
-        ThriftHiveMetastore.Iface proxy = newProxy(ThriftHiveMetastore.Iface.class, new LoggingInvocationHandler(
-                dummyThriftHiveMetastoreClient(),
-                new AirliftParameterNamesProvider(ThriftHiveMetastore.Iface.class, ThriftHiveMetastore.Client.class),
-                messages::add));
-        proxy.get_table("some_database", "some_table_name");
-        assertThat(messages)
-                .hasSize(1)
-                .element(0).matches(message -> message.matches("\\QInvocation of get_table(dbname='some_database', tbl_name='some_table_name') succeeded in\\E " + DURATION_PATTERN));
-    }
-
-    private static ThriftHiveMetastore.Iface dummyThriftHiveMetastoreClient()
-    {
-        return (ThriftHiveMetastore.Iface) newProxyInstance(
-                TestLoggingInvocationHandler.class.getClassLoader(),
-                new Class<?>[] {ThriftHiveMetastore.Iface.class},
-                (proxy, method, args) -> null);
-    }
-
     private interface SomeInterface
     {
         default void run(boolean ok, String s) {}

presto-thrift-testing-server/pom.xml

@@ -26,6 +26,12 @@
         <dependency>
             <groupId>io.airlift.drift</groupId>
             <artifactId>drift-server</artifactId>
+            <exclusions>
+                <exclusion>
+                    <groupId>io.airlift</groupId>
+                    <artifactId>parameternames</artifactId>
+                </exclusion>
+            </exclusions>
         </dependency>
 
         <dependency>