Converted Recorder into a singleton and handled other comments

Signed-off-by: Vivek Reddy Karri <[email protected]>

lib/recorder.cpp

@@ -11,9 +11,13 @@ const std::string Recorder::SWSS_FNAME = "swss.rec";
 const std::string Recorder::SAIREDIS_FNAME = "sairedis.rec";
 const std::string Recorder::RESPPUB_FNAME = "responsepublisher.rec";
 
-std::unique_ptr<SwSSRec> Recorder::swss = std::make_unique<SwSSRec>();
-std::unique_ptr<SaiRedisRec> Recorder::sairedis = std::make_unique<SaiRedisRec>();
-std::unique_ptr<ResPubRec> Recorder::respub = std::make_unique<ResPubRec>();
+
+Recorder& Recorder::Instance()
+{
+    static Recorder m_recorder;
+    return m_recorder;
+}
+
 
 SwSSRec::SwSSRec() 
 {

lib/recorder.h

@@ -10,32 +10,32 @@ namespace swss {
 
 class RecBase {
 public:
-    RecBase() {}
+    RecBase() = default;
     /* Setters */
-    void setRecord(bool record)  { enable_rec = record; }
-    void setRotate(bool rotate)  { log_rotate = rotate; }
-    void setLocation(const std::string& loc) { location = loc; }
-    void setFileName(const std::string& name) { filename = name; }
+    void setRecord(bool record)  { m_recording = record; }
+    void setRotate(bool rotate)  { m_rotate = rotate; }
+    void setLocation(const std::string& loc) { m_location = loc; }
+    void setFileName(const std::string& name) { m_filename = name; }
     void setName(const std::string& name)  { m_name = name; }
 
     /* getters */
-    bool isRecord()  { return enable_rec; }
-    bool isRotate()  { return log_rotate; }
-    std::string getLoc() { return location; }
-    std::string getFile() { return filename; }
+    bool isRecord()  { return m_recording; }
+    bool isRotate()  { return m_rotate; }
+    std::string getLoc() { return m_location; }
+    std::string getFile() { return m_filename; }
     std::string getName() { return m_name; }
 
 private:
-    bool enable_rec;
-    bool log_rotate;
-    std::string location;
-    std::string filename;
+    bool m_recording;
+    bool m_rotate;
+    std::string m_location;
+    std::string m_filename;
     std::string m_name;
 };
 
 class RecWriter : public RecBase {
 public:
-    RecWriter() : RecBase() {}
+    RecWriter() = default;
     virtual ~RecWriter();
     void startRec(bool exit_if_failure);
     void record(const std::string& val);
@@ -67,15 +67,18 @@ class SaiRedisRec : public RecBase {
 /* Interface to access recorder classes */
 class Recorder {
 public:
+    static Recorder& Instance();
     static const std::string DEFAULT_DIR;
     static const std::string REC_START;
     static const std::string SWSS_FNAME;
     static const std::string SAIREDIS_FNAME;
     static const std::string RESPPUB_FNAME;
-
-    static std::unique_ptr<SwSSRec> swss;
-    static std::unique_ptr<SaiRedisRec> sairedis;
-    static std::unique_ptr<ResPubRec> respub;
+
+    Recorder() = default;
+    /* Individual Handlers */
+    SwSSRec swss;
+    SaiRedisRec sairedis;
+    ResPubRec respub;
 };
 
 }

orchagent/main.cpp

@@ -96,9 +96,9 @@ void sighup_handler(int signo)
     /*
      * Don't do any logging since they are using mutexes.
      */
-    Recorder::swss->setRotate(true);
-    Recorder::sairedis->setRotate(true);
-    Recorder::respub->setRotate(true);
+    Recorder::Instance().swss.setRotate(true);
+    Recorder::Instance().sairedis.setRotate(true);
+    Recorder::Instance().respub.setRotate(true);
 }
 
 void syncd_apply_view()
@@ -427,31 +427,31 @@ int main(int argc, char **argv)
     SWSS_LOG_NOTICE("--- Starting Orchestration Agent ---");
 
     /* Initialize sairedis recording parameters */
-    Recorder::sairedis->setRecord(
+    Recorder::Instance().sairedis.setRecord(
         (record_type & SAIREDIS_RECORD_ENABLE) == SAIREDIS_RECORD_ENABLE
     );
-    Recorder::sairedis->setLocation(record_location);
-    Recorder::sairedis->setFileName(sairedis_rec_filename);
+    Recorder::Instance().sairedis.setLocation(record_location);
+    Recorder::Instance().sairedis.setFileName(sairedis_rec_filename);
 
     /* Initialize sairedis */
     initSaiApi();
     initSaiRedis();
 
     /* Initialize remaining recorder parameters  */
-    Recorder::swss->setRecord(
+    Recorder::Instance().swss.setRecord(
         (record_type & SWSS_RECORD_ENABLE) == SWSS_RECORD_ENABLE
     );
-    Recorder::swss->setLocation(record_location);
-    Recorder::swss->setFileName(swss_rec_filename);
-    Recorder::swss->startRec(true);
+    Recorder::Instance().swss.setLocation(record_location);
+    Recorder::Instance().swss.setFileName(swss_rec_filename);
+    Recorder::Instance().swss.startRec(true);
 
-    Recorder::respub->setRecord(
+    Recorder::Instance().respub.setRecord(
         (record_type & RESPONSE_PUBLISHER_RECORD_ENABLE) ==
         RESPONSE_PUBLISHER_RECORD_ENABLE
     );
-    Recorder::respub->setLocation(record_location);
-    Recorder::respub->setFileName(responsepublisher_rec_filename);
-    Recorder::respub->startRec(false);
+    Recorder::Instance().respub.setLocation(record_location);
+    Recorder::Instance().respub.setFileName(responsepublisher_rec_filename);
+    Recorder::Instance().respub.startRec(false);
 
     sai_attribute_t attr;
     vector<sai_attribute_t> attrs;

orchagent/orch.cpp

@@ -62,7 +62,7 @@ void ConsumerBase::addToSync(const KeyOpFieldsValuesTuple &entry)
     string op  = kfvOp(entry);
 
     /* Record incoming tasks */
-    Recorder::swss->record(dumpTuple(entry));
+    Recorder::Instance().swss.record(dumpTuple(entry));
 
     /*
     * m_toSync is a multimap which will allow one key with multiple values,

orchagent/orch.h

@@ -220,7 +220,7 @@ class Orch
     Orch(swss::DBConnector *db, const std::vector<std::string> &tableNames);
     Orch(swss::DBConnector *db, const std::vector<table_name_with_pri_t> &tableNameWithPri);
     Orch(const std::vector<TableConnector>& tables);
-    virtual ~Orch() {};
+    virtual ~Orch() = default;
 
     std::vector<swss::Selectable*> getSelectables();
 

orchagent/orchdaemon.cpp

@@ -711,7 +711,7 @@ void OrchDaemon::start()
 {
     SWSS_LOG_ENTER();
 
-    Recorder::sairedis->setRotate(false);
+    Recorder::Instance().sairedis.setRotate(false);
 
     for (Orch *o : m_orchList)
     {
@@ -757,10 +757,10 @@ void OrchDaemon::start()
         }
 
         // check if logroate is requested
-        if (Recorder::sairedis->isRotate())
+        if (Recorder::Instance().sairedis.isRotate())
         {
-            SWSS_LOG_NOTICE("performing log rotate");
-            Recorder::sairedis->setRotate(false);
+            SWSS_LOG_NOTICE("Performing %s log rotate", Recorder::Instance().sairedis.getName().c_str());
+            Recorder::Instance().sairedis.setRotate(false);
             logRotate();
         }
 

orchagent/response_publisher.cpp

@@ -31,7 +31,7 @@ std::string PrependedComponent(const ReturnCode &status)
 void RecordDBWrite(const std::string &table, const std::string &key, const std::vector<swss::FieldValueTuple> &attrs,
                    const std::string &op)
 {
-    if (!swss::Recorder::respub->isRecord())
+    if (!swss::Recorder::Instance().respub.isRecord())
     {
         return;
     }
@@ -42,15 +42,15 @@ void RecordDBWrite(const std::string &table, const std::string &key, const std::
         s += "|" + fvField(attr) + ":" + fvValue(attr);
     }
 
-    swss::Recorder::respub->record(s);
+    swss::Recorder::Instance().respub.record(s);
 }
 
 void RecordResponse(const std::string &response_channel, const std::string &key,
                     const std::vector<swss::FieldValueTuple> &attrs, const std::string &status)
 {
-    if (!swss::Recorder::respub->isRecord())
+    if (!swss::Recorder::Instance().respub.isRecord())
     {
-        return;
+       return;
     }
 
     std::string s = response_channel + ":" + key + "|" + status;
@@ -59,7 +59,7 @@ void RecordResponse(const std::string &response_channel, const std::string &key,
         s += "|" + fvField(attr) + ":" + fvValue(attr);
     }
 
-    swss::Recorder::respub->record(s);
+    swss::Recorder::Instance().respub.record(s);
 }
 
 } // namespace

orchagent/saihelper.cpp

@@ -251,11 +251,11 @@ void initSaiRedis()
     sai_attribute_t attr;
     sai_status_t status;
 
-    auto record_filename = Recorder::sairedis->getFile();
-    auto record_location = Recorder::sairedis->getLoc();
+    auto record_filename = Recorder::Instance().sairedis.getFile();
+    auto record_location = Recorder::Instance().sairedis.getLoc();
 
     /* set recording dir before enable recording */
-    if (Recorder::sairedis->isRecord())
+    if (Recorder::Instance().sairedis.isRecord())
     {
         attr.id = SAI_REDIS_SWITCH_ATTR_RECORDING_OUTPUT_DIR;
         attr.value.s8list.count = (uint32_t)record_location.size();
@@ -285,13 +285,13 @@ void initSaiRedis()
 
     /* Disable/enable SAI Redis recording */
     attr.id = SAI_REDIS_SWITCH_ATTR_RECORD;
-    attr.value.booldata = Recorder::sairedis->isRecord();
+    attr.value.booldata = Recorder::Instance().sairedis.isRecord();
 
     status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);
     if (status != SAI_STATUS_SUCCESS)
     {
         SWSS_LOG_ERROR("Failed to %s SAI Redis recording, rv:%d",
-            Recorder::sairedis->isRecord() ? "enable" : "disable", status);
+            Recorder::Instance().sairedis.isRecord() ? "enable" : "disable", status);
         exit(EXIT_FAILURE);
     }