From f6cec41dd83fe828186ccba990be4c0cc7036345 Mon Sep 17 00:00:00 2001
From: Esa Korhonen <esa.korhonen@mariadb.com>
Date: Mon, 21 Jan 2019 15:51:53 +0200
Subject: [PATCH] MXS-2271 Monitor config name and instance name are 
 parameters of createInstance()

Also adds/moves some comments from previous entrypoints. Name and module
are now constant fields.
---
 include/maxscale/monitor.hh                   | 22 ++++++++++++-------
 server/core/monitor.cc                        | 17 +++++++-------
 server/modules/monitor/auroramon/auroramon.cc |  7 +++---
 server/modules/monitor/auroramon/auroramon.hh |  4 ++--
 .../monitor/clustrixmon/clustrixmonitor.cc    |  7 +++---
 .../monitor/clustrixmon/clustrixmonitor.hh    |  4 ++--
 server/modules/monitor/csmon/csmon.cc         |  7 +++---
 server/modules/monitor/csmon/csmon.hh         |  4 ++--
 server/modules/monitor/galeramon/galeramon.cc |  9 ++++----
 server/modules/monitor/galeramon/galeramon.hh |  6 ++---
 server/modules/monitor/grmon/grmon.cc         |  7 +++---
 server/modules/monitor/grmon/grmon.hh         |  4 ++--
 .../modules/monitor/mariadbmon/mariadbmon.cc  |  7 +++---
 .../modules/monitor/mariadbmon/mariadbmon.hh  |  7 +++---
 .../mariadbmon/test/test_cycle_find.cc        |  2 +-
 server/modules/monitor/mmmon/mmmon.cc         |  9 ++++----
 server/modules/monitor/mmmon/mmmon.hh         |  4 ++--
 .../monitor/ndbclustermon/ndbclustermon.cc    |  7 +++---
 .../monitor/ndbclustermon/ndbclustermon.hh    |  4 ++--
 19 files changed, 75 insertions(+), 63 deletions(-)

diff --git a/include/maxscale/monitor.hh b/include/maxscale/monitor.hh
index 002d6cc7d..3e88cd9a9 100644
--- a/include/maxscale/monitor.hh
+++ b/include/maxscale/monitor.hh
@@ -51,9 +51,11 @@ struct MXS_MONITOR_API
      * If the function fails, MaxScale will not start. The returned object must inherit from
      * the abstract base monitor class and implement the missing methods.
      *
+     * @param name Configuration name of the monitor
+     * @param module Module name of the monitor
      * @return Monitor object
      */
-    Monitor* (* createInstance)();
+    Monitor* (* createInstance)(const std::string& name, const std::string& module);
 };
 
 /**
@@ -164,7 +166,7 @@ inline mxb::intrusive_slist_iterator<MXS_MONITORED_SERVER> end(MXS_MONITORED_SER
 class Monitor
 {
 public:
-    Monitor();
+    Monitor(const std::string& name, const std::string& module);
     virtual ~Monitor();
     virtual bool configure(const MXS_CONFIG_PARAMETER* params) = 0;
 
@@ -198,8 +200,8 @@ public:
      */
     virtual json_t* diagnostics_json() const = 0;
 
-    char*             name;         /**< Monitor instance name */
-    std::string       module_name;  /**< Name of the monitor module */
+    const char* const name;         /**< Monitor instance name. TODO: change to string */
+    const std::string module_name;  /**< Name of the monitor module */
     Monitor*          next;         /**< Next monitor in the linked list */
 
     mutable std::mutex    lock;
@@ -429,6 +431,9 @@ void monitor_debug_wait();
 namespace maxscale
 {
 
+/**
+ * An abstract class which helps implement a monitor based on a maxbase::Worker thread.
+ */
 class MonitorWorker : public Monitor
                     , protected maxbase::Worker
 {
@@ -525,7 +530,7 @@ public:
     static int64_t get_time_ms();
 
 protected:
-    MonitorWorker();
+    MonitorWorker(const std::string& name, const std::string& module);
 
     /**
      * @brief Should the monitor shut down?
@@ -649,7 +654,8 @@ public:
     MonitorWorkerSimple& operator=(const MonitorWorkerSimple&) = delete;
 
 protected:
-    MonitorWorkerSimple()
+    MonitorWorkerSimple(const std::string& name, const std::string& module)
+        : MonitorWorker(name, module)
     {
     }
 
@@ -707,10 +713,10 @@ public:
     MonitorApi(const MonitorApi&) = delete;
     MonitorApi& operator=(const MonitorApi&) = delete;
 
-    static Monitor* createInstance()
+    static Monitor* createInstance(const std::string& name, const std::string& module)
     {
         MonitorInstance* pInstance = NULL;
-        MXS_EXCEPTION_GUARD(pInstance = MonitorInstance::create());
+        MXS_EXCEPTION_GUARD(pInstance = MonitorInstance::create(name, module));
         return pInstance;
     }
 
diff --git a/server/core/monitor.cc b/server/core/monitor.cc
index 69388a9f9..5c22a5a38 100644
--- a/server/core/monitor.cc
+++ b/server/core/monitor.cc
@@ -106,7 +106,7 @@ Monitor* MonitorManager::create_monitor(const string& name, const string& module
         return NULL;
     }
 
-    Monitor* mon = api->createInstance();
+    Monitor* mon = api->createInstance(name, module);
     if (!mon)
     {
         MXS_ERROR("Unable to create monitor instance for '%s', using module '%s'.",
@@ -114,10 +114,6 @@ Monitor* MonitorManager::create_monitor(const string& name, const string& module
         return NULL;
     }
 
-    // These initializations are kept outside the constructor to keep it simple.
-    mon->name = MXS_STRDUP_A(name.c_str());
-    mon->module_name = module;
-
     if (mon->configure_base(params)) // TODO: Move derived class configure() here
     {
         std::lock_guard<std::mutex> guard(monLock);
@@ -132,7 +128,9 @@ Monitor* MonitorManager::create_monitor(const string& name, const string& module
     return mon;
 }
 
-Monitor::Monitor()
+Monitor::Monitor(const string& name, const string& module)
+    : name(MXS_STRDUP_A(name.c_str()))
+    , module_name(module)
 {
     memset(journal_hash, 0, sizeof(journal_hash));
 }
@@ -210,7 +208,7 @@ Monitor::~Monitor()
     delete disk_space_threshold;
     config_parameter_free(parameters);
     monitor_server_free_all(monitored_servers);
-    MXS_FREE(name);
+    MXS_FREE((const_cast<char*>(name)));
 }
 
 void MonitorManager::destroy_all_monitors()
@@ -2429,8 +2427,9 @@ void monitor_debug_wait()
 namespace maxscale
 {
 
-MonitorWorker::MonitorWorker()
-    : m_monitor(this)
+MonitorWorker::MonitorWorker(const string& name, const string& module)
+    : Monitor(name, module)
+    , m_monitor(this)
     , m_master(NULL)
     , m_thread_running(false)
     , m_shutdown(0)
diff --git a/server/modules/monitor/auroramon/auroramon.cc b/server/modules/monitor/auroramon/auroramon.cc
index 6111347e1..2bcd797cd 100644
--- a/server/modules/monitor/auroramon/auroramon.cc
+++ b/server/modules/monitor/auroramon/auroramon.cc
@@ -25,7 +25,8 @@
 #include <maxscale/mysql_utils.hh>
 
 
-AuroraMonitor::AuroraMonitor()
+AuroraMonitor::AuroraMonitor(const std::string& name, const std::string& module)
+    : MonitorWorkerSimple(name, module)
 {
 }
 
@@ -34,9 +35,9 @@ AuroraMonitor::~AuroraMonitor()
 }
 
 // static
-AuroraMonitor* AuroraMonitor::create()
+AuroraMonitor* AuroraMonitor::create(const std::string& name, const std::string& module)
 {
-    return new AuroraMonitor();
+    return new AuroraMonitor(name, module);
 }
 
 bool AuroraMonitor::has_sufficient_permissions() const
diff --git a/server/modules/monitor/auroramon/auroramon.hh b/server/modules/monitor/auroramon/auroramon.hh
index 52dba2e34..709f78dc8 100644
--- a/server/modules/monitor/auroramon/auroramon.hh
+++ b/server/modules/monitor/auroramon/auroramon.hh
@@ -26,12 +26,12 @@ public:
     AuroraMonitor& operator=(const AuroraMonitor&) = delete;
 
     ~AuroraMonitor();
-    static AuroraMonitor* create();
+    static AuroraMonitor* create(const std::string& name, const std::string& module);
 
 protected:
     bool has_sufficient_permissions() const;
     void update_server_status(MXS_MONITORED_SERVER* monitored_server);
 
 private:
-    AuroraMonitor();
+    AuroraMonitor(const std::string& name, const std::string& module);
 };
diff --git a/server/modules/monitor/clustrixmon/clustrixmonitor.cc b/server/modules/monitor/clustrixmon/clustrixmonitor.cc
index 7f6db1e4d..1f9cf30cd 100644
--- a/server/modules/monitor/clustrixmon/clustrixmonitor.cc
+++ b/server/modules/monitor/clustrixmon/clustrixmonitor.cc
@@ -27,7 +27,8 @@ const int DEFAULT_HEALTH_PORT = 3581;
 
 }
 
-ClustrixMonitor::ClustrixMonitor()
+ClustrixMonitor::ClustrixMonitor(const string& name, const string& module)
+    : MonitorWorker(name, module)
 {
 }
 
@@ -36,9 +37,9 @@ ClustrixMonitor::~ClustrixMonitor()
 }
 
 //static
-ClustrixMonitor* ClustrixMonitor::create()
+ClustrixMonitor* ClustrixMonitor::create(const string& name, const string& module)
 {
-    return new ClustrixMonitor();
+    return new ClustrixMonitor(name, module);
 }
 
 bool ClustrixMonitor::configure(const MXS_CONFIG_PARAMETER* pParams)
diff --git a/server/modules/monitor/clustrixmon/clustrixmonitor.hh b/server/modules/monitor/clustrixmon/clustrixmonitor.hh
index a10963b6f..d2a237086 100644
--- a/server/modules/monitor/clustrixmon/clustrixmonitor.hh
+++ b/server/modules/monitor/clustrixmon/clustrixmonitor.hh
@@ -60,12 +60,12 @@ public:
 
     ~ClustrixMonitor();
 
-    static ClustrixMonitor* create();
+    static ClustrixMonitor* create(const std::string& name, const std::string& module);
 
     bool configure(const MXS_CONFIG_PARAMETER* pParams) override;
 
 private:
-    ClustrixMonitor();
+    ClustrixMonitor(const std::string& name, const std::string& module);
 
     void pre_loop() override;
     void post_loop() override;
diff --git a/server/modules/monitor/csmon/csmon.cc b/server/modules/monitor/csmon/csmon.cc
index 2c93baa66..7f42d0dba 100644
--- a/server/modules/monitor/csmon/csmon.cc
+++ b/server/modules/monitor/csmon/csmon.cc
@@ -71,7 +71,8 @@ int get_cs_version(MXS_MONITORED_SERVER* srv)
 }
 }
 
-CsMonitor::CsMonitor()
+CsMonitor::CsMonitor(const std::string& name, const std::string& module)
+    : MonitorWorkerSimple(name, module)
 {
 }
 
@@ -80,9 +81,9 @@ CsMonitor::~CsMonitor()
 }
 
 // static
-CsMonitor* CsMonitor::create()
+CsMonitor* CsMonitor::create(const std::string& name, const std::string& module)
 {
-    return new CsMonitor();
+    return new CsMonitor(name, module);
 }
 
 bool CsMonitor::has_sufficient_permissions() const
diff --git a/server/modules/monitor/csmon/csmon.hh b/server/modules/monitor/csmon/csmon.hh
index b4fc39b30..9ed36881b 100644
--- a/server/modules/monitor/csmon/csmon.hh
+++ b/server/modules/monitor/csmon/csmon.hh
@@ -22,14 +22,14 @@ public:
     CsMonitor& operator=(const CsMonitor&) = delete;
 
     ~CsMonitor();
-    static CsMonitor* create();
+    static CsMonitor* create(const std::string& name, const std::string& module);
 
 protected:
     bool has_sufficient_permissions() const;
     void update_server_status(MXS_MONITORED_SERVER* monitored_server);
 
 private:
-    CsMonitor();
+    CsMonitor(const std::string& name, const std::string& module);
     bool configure(const MXS_CONFIG_PARAMETER* pParams) override;
 
     SERVER* m_primary;
diff --git a/server/modules/monitor/galeramon/galeramon.cc b/server/modules/monitor/galeramon/galeramon.cc
index e30057c0b..261cf00ac 100644
--- a/server/modules/monitor/galeramon/galeramon.cc
+++ b/server/modules/monitor/galeramon/galeramon.cc
@@ -42,8 +42,9 @@ static int                   compare_node_index(const void*, const void*);
 static int                   compare_node_priority(const void*, const void*);
 static bool                  using_xtrabackup(MXS_MONITORED_SERVER* database, const char* server_string);
 
-GaleraMonitor::GaleraMonitor()
-    : m_disableMasterFailback(0)
+GaleraMonitor::GaleraMonitor(const std::string& name, const std::string& module)
+    : MonitorWorkerSimple(name, module)
+    , m_disableMasterFailback(0)
     , m_availableWhenDonor(0)
     , m_disableMasterRoleSetting(0)
     , m_root_node_as_master(false)
@@ -59,9 +60,9 @@ GaleraMonitor::~GaleraMonitor()
 }
 
 // static
-GaleraMonitor* GaleraMonitor::create()
+GaleraMonitor* GaleraMonitor::create(const std::string& name, const std::string& module)
 {
-    return new GaleraMonitor();
+    return new GaleraMonitor(name, module);
 }
 
 void GaleraMonitor::diagnostics(DCB* dcb) const
diff --git a/server/modules/monitor/galeramon/galeramon.hh b/server/modules/monitor/galeramon/galeramon.hh
index 0ec9c37e3..85738a59d 100644
--- a/server/modules/monitor/galeramon/galeramon.hh
+++ b/server/modules/monitor/galeramon/galeramon.hh
@@ -17,9 +17,7 @@
  */
 
 #include <maxscale/ccdefs.hh>
-
 #include <unordered_map>
-
 #include <maxscale/monitor.hh>
 
 /**
@@ -43,7 +41,7 @@ public:
     GaleraMonitor& operator=(const GaleraMonitor&) = delete;
 
     ~GaleraMonitor();
-    static GaleraMonitor* create();
+    static GaleraMonitor* create(const std::string& name, const std::string& module);
     void                  diagnostics(DCB* dcb) const;
     json_t*               diagnostics_json() const;
 
@@ -68,7 +66,7 @@ private:
     NodeMap     m_info;                         /**< Contains Galera Cluster variables of all nodes */
     int         m_cluster_size;                 /**< How many nodes in the cluster */
 
-    GaleraMonitor();
+    GaleraMonitor(const std::string& name, const std::string& module);
 
     bool detect_cluster_size(const int n_nodes,
                              const char* candidate_uuid,
diff --git a/server/modules/monitor/grmon/grmon.cc b/server/modules/monitor/grmon/grmon.cc
index 04da29bd7..118f96bba 100644
--- a/server/modules/monitor/grmon/grmon.cc
+++ b/server/modules/monitor/grmon/grmon.cc
@@ -26,7 +26,8 @@
 #include <mysqld_error.h>
 
 
-GRMon::GRMon()
+GRMon::GRMon(const std::string& name, const std::string& module)
+    : MonitorWorkerSimple(name, module)
 {
 }
 
@@ -34,9 +35,9 @@ GRMon::~GRMon()
 {
 }
 
-GRMon* GRMon::create()
+GRMon* GRMon::create(const std::string& name, const std::string& module)
 {
-    return new GRMon();
+    return new GRMon(name, module);
 }
 
 bool GRMon::has_sufficient_permissions() const
diff --git a/server/modules/monitor/grmon/grmon.hh b/server/modules/monitor/grmon/grmon.hh
index a5eea2a81..2f4787f3c 100644
--- a/server/modules/monitor/grmon/grmon.hh
+++ b/server/modules/monitor/grmon/grmon.hh
@@ -26,12 +26,12 @@ public:
     GRMon& operator&(const GRMon&) = delete;
 
     ~GRMon();
-    static GRMon* create();
+    static GRMon* create(const std::string& name, const std::string& module);
 
 protected:
     bool has_sufficient_permissions() const;
     void update_server_status(MXS_MONITORED_SERVER* monitored_server);
 
 private:
-    GRMon();
+    GRMon(const std::string& name, const std::string& module);
 };
diff --git a/server/modules/monitor/mariadbmon/mariadbmon.cc b/server/modules/monitor/mariadbmon/mariadbmon.cc
index b456c9e62..9d84ece56 100644
--- a/server/modules/monitor/mariadbmon/mariadbmon.cc
+++ b/server/modules/monitor/mariadbmon/mariadbmon.cc
@@ -59,7 +59,8 @@ static const char CN_REPLICATION_PASSWORD[] = "replication_password";
 static const char DIAG_ERROR[] = "Internal error, could not print diagnostics. "
                                  "Check log for more information.";
 
-MariaDBMonitor::MariaDBMonitor()
+MariaDBMonitor::MariaDBMonitor(const string& name, const string& module)
+    : MonitorWorker(name, module)
 {
 }
 
@@ -172,9 +173,9 @@ bool MariaDBMonitor::set_replication_credentials(const MXS_CONFIG_PARAMETER* par
     return rval;
 }
 
-MariaDBMonitor* MariaDBMonitor::create()
+MariaDBMonitor* MariaDBMonitor::create(const string& name, const string& module)
 {
-    return new MariaDBMonitor();
+    return new MariaDBMonitor(name, module);
 }
 
 /**
diff --git a/server/modules/monitor/mariadbmon/mariadbmon.hh b/server/modules/monitor/mariadbmon/mariadbmon.hh
index a240d6f8b..7351c9a96 100644
--- a/server/modules/monitor/mariadbmon/mariadbmon.hh
+++ b/server/modules/monitor/mariadbmon/mariadbmon.hh
@@ -48,10 +48,11 @@ public:
     /**
      * Create the monitor instance and return the instance data.
      *
-     * @param monitor Generic monitor data
+     * @param name Monitor config name
+     * @param module Module name
      * @return MariaDBMonitor instance
      */
-    static MariaDBMonitor* create();
+    static MariaDBMonitor* create(const std::string& name, const std::string& module);
 
     ~MariaDBMonitor();
 
@@ -236,7 +237,7 @@ private:
                                                  * gtid:s? */
 
     // Base methods
-    MariaDBMonitor();
+    MariaDBMonitor(const std::string& name, const std::string& module);
     bool configure(const MXS_CONFIG_PARAMETER* params);
     bool set_replication_credentials(const MXS_CONFIG_PARAMETER* params);
     void reset_server_info();
diff --git a/server/modules/monitor/mariadbmon/test/test_cycle_find.cc b/server/modules/monitor/mariadbmon/test/test_cycle_find.cc
index 0fa2c7ae3..08e21eb0e 100644
--- a/server/modules/monitor/mariadbmon/test/test_cycle_find.cc
+++ b/server/modules/monitor/mariadbmon/test/test_cycle_find.cc
@@ -90,7 +90,7 @@ int main()
 }
 
 MariaDBMonitor::Test::Test(bool use_hostnames)
-    : m_monitor(new MariaDBMonitor())
+    : m_monitor(new MariaDBMonitor("TestMonitor", MXS_MODULE_NAME))
     , m_use_hostnames(use_hostnames)
 {
 }
diff --git a/server/modules/monitor/mmmon/mmmon.cc b/server/modules/monitor/mmmon/mmmon.cc
index 7ff76521c..e320c7d2e 100644
--- a/server/modules/monitor/mmmon/mmmon.cc
+++ b/server/modules/monitor/mmmon/mmmon.cc
@@ -34,8 +34,9 @@
 static void detectStaleMaster(void*, int);
 static bool isMySQLEvent(mxs_monitor_event_t event);
 
-MMMonitor::MMMonitor()
-    : m_detectStaleMaster(false)
+MMMonitor::MMMonitor(const std::string& name, const std::string& module)
+    : MonitorWorkerSimple(name, module)
+    , m_detectStaleMaster(false)
 {
 }
 
@@ -44,9 +45,9 @@ MMMonitor::~MMMonitor()
 }
 
 // static
-MMMonitor* MMMonitor::create()
+MMMonitor* MMMonitor::create(const std::string& name, const std::string& module)
 {
-    return new MMMonitor();
+    return new MMMonitor(name, module);
 }
 
 void MMMonitor::diagnostics(DCB* dcb) const
diff --git a/server/modules/monitor/mmmon/mmmon.hh b/server/modules/monitor/mmmon/mmmon.hh
index 0fc92c7f0..db6819c7e 100644
--- a/server/modules/monitor/mmmon/mmmon.hh
+++ b/server/modules/monitor/mmmon/mmmon.hh
@@ -26,7 +26,7 @@ public:
     MMMonitor& operator=(const MMMonitor&) = delete;
 
     ~MMMonitor();
-    static MMMonitor* create();
+    static MMMonitor* create(const std::string& name, const std::string& module);
     void              diagnostics(DCB* dcb) const;
     json_t*           diagnostics_json() const;
 
@@ -39,7 +39,7 @@ protected:
 private:
     int           m_detectStaleMaster;  /**< Monitor flag for Stale Master detection */
 
-    MMMonitor();
+    MMMonitor(const std::string& name, const std::string& module);
 
     MXS_MONITORED_SERVER* get_current_master();
 };
diff --git a/server/modules/monitor/ndbclustermon/ndbclustermon.cc b/server/modules/monitor/ndbclustermon/ndbclustermon.cc
index 64ef8f1ef..4748adc25 100644
--- a/server/modules/monitor/ndbclustermon/ndbclustermon.cc
+++ b/server/modules/monitor/ndbclustermon/ndbclustermon.cc
@@ -22,7 +22,8 @@
 #include <maxscale/mysql_utils.hh>
 
 
-NDBCMonitor::NDBCMonitor()
+NDBCMonitor::NDBCMonitor(const std::string& name, const std::string& module)
+    : MonitorWorkerSimple(name, module)
 {
 }
 
@@ -31,9 +32,9 @@ NDBCMonitor::~NDBCMonitor()
 }
 
 // static
-NDBCMonitor* NDBCMonitor::create()
+NDBCMonitor* NDBCMonitor::create(const std::string& name, const std::string& module)
 {
-    return new NDBCMonitor();
+    return new NDBCMonitor(name, module);
 }
 
 bool NDBCMonitor::has_sufficient_permissions() const
diff --git a/server/modules/monitor/ndbclustermon/ndbclustermon.hh b/server/modules/monitor/ndbclustermon/ndbclustermon.hh
index 695b86388..f216d0128 100644
--- a/server/modules/monitor/ndbclustermon/ndbclustermon.hh
+++ b/server/modules/monitor/ndbclustermon/ndbclustermon.hh
@@ -26,12 +26,12 @@ public:
     NDBCMonitor& operator=(const NDBCMonitor&) = delete;
 
     ~NDBCMonitor();
-    static NDBCMonitor* create();
+    static NDBCMonitor* create(const std::string& name, const std::string& module);
 
 protected:
     bool has_sufficient_permissions() const;
     void update_server_status(MXS_MONITORED_SERVER* monitored_server);
 
 private:
-    NDBCMonitor();
+    NDBCMonitor(const std::string& name, const std::string& module);
 };