Add configuration for JMX base name

pom.xml

@@ -56,6 +56,7 @@
         <dep.testng.version>6.10</dep.testng.version>
         <dep.assertj-core.version>3.8.0</dep.assertj-core.version>
         <dep.logback.version>1.2.3</dep.logback.version>
+        <dep.jmxutils.version>1.20</dep.jmxutils.version>
 
         <!--
           America/Bahia_Banderas has:

presto-hive/src/main/java/io/prestosql/plugin/hive/ConnectorObjectNameGeneratorModule.java

@@ -0,0 +1,117 @@
+/*
+ * 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;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.inject.Binder;
+import com.google.inject.Module;
+import com.google.inject.Provides;
+import io.airlift.configuration.Config;
+import org.weakref.jmx.ObjectNameBuilder;
+import org.weakref.jmx.ObjectNameGenerator;
+
+import java.util.Map;
+
+import static com.google.common.base.MoreObjects.firstNonNull;
+import static io.airlift.configuration.ConfigBinder.configBinder;
+import static java.util.Objects.requireNonNull;
+
+// Note: There are multiple copies of this class in the codebase.  If you change one you, should change them all.
+public class ConnectorObjectNameGeneratorModule
+        implements Module
+{
+    private static final String CONNECTOR_PACKAGE_NAME = "io.prestosql.plugin.hive";
+    private static final String DEFAULT_DOMAIN_BASE = "presto.plugin.hive";
+
+    private final String catalogName;
+
+    public ConnectorObjectNameGeneratorModule(String catalogName)
+    {
+        this.catalogName = requireNonNull(catalogName, "catalogName is null");
+    }
+
+    @Override
+    public void configure(Binder binder)
+    {
+        configBinder(binder).bindConfig(ConnectorObjectNameGeneratorConfig.class);
+    }
+
+    @Provides
+    ObjectNameGenerator createPrefixObjectNameGenerator(ConnectorObjectNameGeneratorConfig config)
+    {
+        String domainBase = firstNonNull(config.getDomainBase(), DEFAULT_DOMAIN_BASE);
+        return new ConnectorObjectNameGenerator(domainBase, catalogName);
+    }
+
+    public static class ConnectorObjectNameGeneratorConfig
+    {
+        private String domainBase;
+
+        public String getDomainBase()
+        {
+            return domainBase;
+        }
+
+        @Config("jmx.base-name")
+        public ConnectorObjectNameGeneratorConfig setDomainBase(String domainBase)
+        {
+            this.domainBase = domainBase;
+            return this;
+        }
+    }
+
+    public static final class ConnectorObjectNameGenerator
+            implements ObjectNameGenerator
+    {
+        private final String domainBase;
+        private final String catalogName;
+
+        public ConnectorObjectNameGenerator(String domainBase, String catalogName)
+        {
+            this.domainBase = domainBase;
+            this.catalogName = catalogName;
+        }
+
+        @Override
+        public String generatedNameOf(Class<?> type)
+        {
+            return new ObjectNameBuilder(toDomain(type))
+                    .withProperties(ImmutableMap.<String, String>builder()
+                    .put("type", type.getSimpleName())
+                    .put("name", catalogName)
+                    .build())
+                    .build();
+        }
+
+        @Override
+        public String generatedNameOf(Class<?> type, Map<String, String> properties)
+        {
+            return new ObjectNameBuilder(toDomain(type))
+                    .withProperties(ImmutableMap.<String, String>builder()
+                    .putAll(properties)
+                    .put("catalog", catalogName)
+                    .build())
+                    .build();
+        }
+
+        private String toDomain(Class<?> type)
+        {
+            String domain = type.getPackage().getName();
+            if (domain.startsWith(CONNECTOR_PACKAGE_NAME)) {
+                domain = domainBase + domain.substring(CONNECTOR_PACKAGE_NAME.length());
+            }
+            return domain;
+        }
+    }
+}

presto-hive/src/main/java/io/prestosql/plugin/hive/HiveClientModule.java

@@ -43,7 +43,6 @@
 import static io.airlift.json.JsonCodecBinder.jsonCodecBinder;
 import static java.util.concurrent.Executors.newCachedThreadPool;
 import static java.util.concurrent.Executors.newFixedThreadPool;
-import static org.weakref.jmx.ObjectNames.generatedNameOf;
 import static org.weakref.jmx.guice.ExportBinder.newExporter;
 
 public class HiveClientModule
@@ -73,7 +72,7 @@ public void configure(Binder binder)
         binder.bind(HiveTableProperties.class).in(Scopes.SINGLETON);
 
         binder.bind(NamenodeStats.class).in(Scopes.SINGLETON);
-        newExporter(binder).export(NamenodeStats.class).as(generatedNameOf(NamenodeStats.class, connectorId));
+        newExporter(binder).export(NamenodeStats.class).withGeneratedName();
 
         binder.bind(PrestoS3ClientFactory.class).in(Scopes.SINGLETON);
 
@@ -82,7 +81,7 @@ public void configure(Binder binder)
         recordCursorProviderBinder.addBinding().to(GenericHiveRecordCursorProvider.class).in(Scopes.SINGLETON);
 
         binder.bind(HiveWriterStats.class).in(Scopes.SINGLETON);
-        newExporter(binder).export(HiveWriterStats.class).as(generatedNameOf(HiveWriterStats.class, connectorId));
+        newExporter(binder).export(HiveWriterStats.class).withGeneratedName();
 
         newSetBinder(binder, EventClient.class).addBinding().to(HiveEventClient.class).in(Scopes.SINGLETON);
         binder.bind(HivePartitionManager.class).in(Scopes.SINGLETON);
@@ -92,15 +91,15 @@ public void configure(Binder binder)
         binder.bind(new TypeLiteral<Supplier<TransactionalMetadata>>() {}).to(HiveMetadataFactory.class).in(Scopes.SINGLETON);
         binder.bind(HiveTransactionManager.class).in(Scopes.SINGLETON);
         binder.bind(ConnectorSplitManager.class).to(HiveSplitManager.class).in(Scopes.SINGLETON);
-        newExporter(binder).export(ConnectorSplitManager.class).as(generatedNameOf(HiveSplitManager.class, connectorId));
+        newExporter(binder).export(ConnectorSplitManager.class).as(generator -> generator.generatedNameOf(HiveSplitManager.class));
         binder.bind(ConnectorPageSourceProvider.class).to(HivePageSourceProvider.class).in(Scopes.SINGLETON);
         binder.bind(ConnectorPageSinkProvider.class).to(HivePageSinkProvider.class).in(Scopes.SINGLETON);
         binder.bind(ConnectorNodePartitioningProvider.class).to(HiveNodePartitioningProvider.class).in(Scopes.SINGLETON);
 
         jsonCodecBinder(binder).bindJsonCodec(PartitionUpdate.class);
 
         binder.bind(FileFormatDataSourceStats.class).in(Scopes.SINGLETON);
-        newExporter(binder).export(FileFormatDataSourceStats.class).as(generatedNameOf(FileFormatDataSourceStats.class, connectorId));
+        newExporter(binder).export(FileFormatDataSourceStats.class).withGeneratedName();
 
         Multibinder<HivePageSourceFactory> pageSourceFactoryBinder = newSetBinder(binder, HivePageSourceFactory.class);
         pageSourceFactoryBinder.addBinding().to(OrcPageSourceFactory.class).in(Scopes.SINGLETON);
@@ -110,7 +109,7 @@ public void configure(Binder binder)
 
         Multibinder<HiveFileWriterFactory> fileWriterFactoryBinder = newSetBinder(binder, HiveFileWriterFactory.class);
         binder.bind(OrcFileWriterFactory.class).in(Scopes.SINGLETON);
-        newExporter(binder).export(OrcFileWriterFactory.class).as(generatedNameOf(OrcFileWriterFactory.class, connectorId));
+        newExporter(binder).export(OrcFileWriterFactory.class).withGeneratedName();
         configBinder(binder).bindConfig(OrcFileWriterConfig.class);
         fileWriterFactoryBinder.addBinding().to(OrcFileWriterFactory.class).in(Scopes.SINGLETON);
         fileWriterFactoryBinder.addBinding().to(RcFileFileWriterFactory.class).in(Scopes.SINGLETON);

presto-hive/src/main/java/io/prestosql/plugin/hive/HiveConnectorFactory.java

@@ -96,10 +96,11 @@ public Connector create(String catalogName, Map<String, String> config, Connecto
             Bootstrap app = new Bootstrap(
                     new EventModule(),
                     new MBeanModule(),
+                    new ConnectorObjectNameGeneratorModule(catalogName),
                     new JsonModule(),
                     new HiveClientModule(catalogName),
-                    new HiveS3Module(catalogName),
-                    new HiveMetastoreModule(catalogName, metastore),
+                    new HiveS3Module(),
+                    new HiveMetastoreModule(metastore),
                     new HiveSecurityModule(),
                     new HiveAuthenticationModule(),
                     new HiveProcedureModule(),

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

@@ -27,12 +27,10 @@
 public class HiveMetastoreModule
         extends AbstractConfigurationAwareModule
 {
-    private final String connectorId;
     private final Optional<ExtendedHiveMetastore> metastore;
 
-    public HiveMetastoreModule(String connectorId, Optional<ExtendedHiveMetastore> metastore)
+    public HiveMetastoreModule(Optional<ExtendedHiveMetastore> metastore)
     {
-        this.connectorId = connectorId;
         this.metastore = metastore;
     }
 
@@ -43,9 +41,9 @@ protected void setup(Binder binder)
             binder.bind(ExtendedHiveMetastore.class).toInstance(metastore.get());
         }
         else {
-            bindMetastoreModule("thrift", new ThriftMetastoreModule(connectorId));
-            bindMetastoreModule("file", new FileMetastoreModule(connectorId));
-            bindMetastoreModule("glue", new GlueMetastoreModule(connectorId));
+            bindMetastoreModule("thrift", new ThriftMetastoreModule());
+            bindMetastoreModule("file", new FileMetastoreModule());
+            bindMetastoreModule("glue", new GlueMetastoreModule());
         }
     }
 

presto-hive/src/main/java/io/prestosql/plugin/hive/metastore/file/FileMetastoreModule.java

@@ -21,27 +21,18 @@
 import io.prestosql.plugin.hive.metastore.ExtendedHiveMetastore;
 
 import static io.airlift.configuration.ConfigBinder.configBinder;
-import static java.util.Objects.requireNonNull;
-import static org.weakref.jmx.ObjectNames.generatedNameOf;
 import static org.weakref.jmx.guice.ExportBinder.newExporter;
 
 public class FileMetastoreModule
         implements Module
 {
-    private final String connectorId;
-
-    public FileMetastoreModule(String connectorId)
-    {
-        this.connectorId = requireNonNull(connectorId, "connectorId is null");
-    }
-
     @Override
     public void configure(Binder binder)
     {
         configBinder(binder).bindConfig(FileHiveMetastoreConfig.class);
         binder.bind(ExtendedHiveMetastore.class).annotatedWith(ForCachingHiveMetastore.class).to(FileHiveMetastore.class).in(Scopes.SINGLETON);
         binder.bind(ExtendedHiveMetastore.class).to(CachingHiveMetastore.class).in(Scopes.SINGLETON);
         newExporter(binder).export(ExtendedHiveMetastore.class)
-                .as(generatedNameOf(CachingHiveMetastore.class, connectorId));
+                .as(generator -> generator.generatedNameOf(CachingHiveMetastore.class));
     }
 }

presto-hive/src/main/java/io/prestosql/plugin/hive/metastore/glue/GlueMetastoreModule.java

@@ -19,26 +19,17 @@
 import io.prestosql.plugin.hive.metastore.ExtendedHiveMetastore;
 
 import static io.airlift.configuration.ConfigBinder.configBinder;
-import static java.util.Objects.requireNonNull;
-import static org.weakref.jmx.ObjectNames.generatedNameOf;
 import static org.weakref.jmx.guice.ExportBinder.newExporter;
 
 public class GlueMetastoreModule
         implements Module
 {
-    private final String connectorId;
-
-    public GlueMetastoreModule(String connectorId)
-    {
-        this.connectorId = requireNonNull(connectorId, "connectorId is null");
-    }
-
     @Override
     public void configure(Binder binder)
     {
         configBinder(binder).bindConfig(GlueHiveMetastoreConfig.class);
         binder.bind(ExtendedHiveMetastore.class).to(GlueHiveMetastore.class).in(Scopes.SINGLETON);
         newExporter(binder).export(ExtendedHiveMetastore.class)
-                .as(generatedNameOf(GlueHiveMetastore.class, connectorId));
+                .as(generator -> generator.generatedNameOf(GlueHiveMetastore.class));
     }
 }

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

@@ -24,20 +24,11 @@
 import io.prestosql.plugin.hive.metastore.RecordingHiveMetastore;
 
 import static io.airlift.configuration.ConfigBinder.configBinder;
-import static java.util.Objects.requireNonNull;
-import static org.weakref.jmx.ObjectNames.generatedNameOf;
 import static org.weakref.jmx.guice.ExportBinder.newExporter;
 
 public class ThriftMetastoreModule
         extends AbstractConfigurationAwareModule
 {
-    private final String connectorId;
-
-    public ThriftMetastoreModule(String connectorId)
-    {
-        this.connectorId = requireNonNull(connectorId, "connectorId is null");
-    }
-
     @Override
     protected void setup(Binder binder)
     {
@@ -57,8 +48,7 @@ protected void setup(Binder binder)
                     .to(RecordingHiveMetastore.class)
                     .in(Scopes.SINGLETON);
             binder.bind(RecordingHiveMetastore.class).in(Scopes.SINGLETON);
-            newExporter(binder).export(RecordingHiveMetastore.class)
-                    .as(generatedNameOf(RecordingHiveMetastore.class, connectorId));
+            newExporter(binder).export(RecordingHiveMetastore.class);
         }
         else {
             binder.bind(ExtendedHiveMetastore.class)
@@ -69,8 +59,8 @@ protected void setup(Binder binder)
 
         binder.bind(ExtendedHiveMetastore.class).to(CachingHiveMetastore.class).in(Scopes.SINGLETON);
         newExporter(binder).export(HiveMetastore.class)
-                .as(generatedNameOf(ThriftHiveMetastore.class, connectorId));
+                .as(generator -> generator.generatedNameOf(ThriftHiveMetastore.class));
         newExporter(binder).export(ExtendedHiveMetastore.class)
-                .as(generatedNameOf(CachingHiveMetastore.class, connectorId));
+                .as(generator -> generator.generatedNameOf(CachingHiveMetastore.class));
     }
 }

presto-hive/src/main/java/io/prestosql/plugin/hive/s3/HiveS3Module.java

@@ -21,22 +21,13 @@
 import org.apache.hadoop.hive.common.JavaUtils;
 
 import static io.airlift.configuration.ConfigBinder.configBinder;
-import static java.util.Objects.requireNonNull;
-import static org.weakref.jmx.ObjectNames.generatedNameOf;
 import static org.weakref.jmx.guice.ExportBinder.newExporter;
 
 public class HiveS3Module
         extends AbstractConfigurationAwareModule
 {
     private static final String EMR_FS_CLASS_NAME = "com.amazon.ws.emr.hadoop.fs.EmrFileSystem";
 
-    private final String connectorId;
-
-    public HiveS3Module(String connectorId)
-    {
-        this.connectorId = requireNonNull(connectorId, "connectorId is null");
-    }
-
     @Override
     protected void setup(Binder binder)
     {
@@ -46,7 +37,8 @@ protected void setup(Binder binder)
             configBinder(binder).bindConfig(HiveS3Config.class);
 
             binder.bind(PrestoS3FileSystemStats.class).toInstance(PrestoS3FileSystem.getFileSystemStats());
-            newExporter(binder).export(PrestoS3FileSystemStats.class).as(generatedNameOf(PrestoS3FileSystem.class, connectorId));
+            newExporter(binder).export(PrestoS3FileSystemStats.class)
+                    .as(generator -> generator.generatedNameOf(PrestoS3FileSystem.class));
         }
         else if (type == S3FileSystemType.EMRFS) {
             validateEmrFsClass();

presto-main/etc/catalog/thrift.properties

@@ -0,0 +1,9 @@
+#
+# WARNING
+# ^^^^^^^
+# This configuration file is for development only and should NOT be used be
+# used in production. For example configuration, see the Presto documentation.
+#
+
+connector.name=presto-thrift
+presto.thrift.client.addresses=127.0.0.1:1234

presto-main/etc/config.properties

@@ -39,6 +39,7 @@ plugin.bundles=\
   ../presto-mysql/pom.xml,\
   ../presto-sqlserver/pom.xml, \
   ../presto-postgresql/pom.xml, \
+  ../presto-thrift/pom.xml, \
   ../presto-tpcds/pom.xml
 
 presto.version=testversion

presto-main/src/main/java/io/prestosql/connector/ConnectorManager.java

@@ -169,7 +169,7 @@ public synchronized ConnectorId createConnection(String catalogName, String conn
     {
         requireNonNull(connectorName, "connectorName is null");
         ConnectorFactory connectorFactory = connectorFactories.get(connectorName);
-        checkArgument(connectorFactory != null, "No factory for connector %s", connectorName);
+        checkArgument(connectorFactory != null, "No factory for connector [%s].  Available factories: %s", connectorName, connectorFactories.keySet());
         return createConnection(catalogName, connectorFactory, properties);
     }