From 54d18c1c0407868b204e2a9b19ee6b2b24df1d49 Mon Sep 17 00:00:00 2001
From: Michael Davis <michael.davis@cern.ch>
Date: Mon, 5 Mar 2018 12:02:21 +0100
Subject: [PATCH] [xrd_ssi_pb] Changes all SSI/Protobuf debug logging to log to
 XRootD logfile

---
 cmdline/CtaAdminCmd.cpp                    |  4 +---
 cmdline/CtaAdminCmd.hpp                    |  2 ++
 cmdline/EosCtaStub.cpp                     |  8 ++------
 xroot_plugins/CMakeLists.txt               |  2 --
 xroot_plugins/XrdCtaArchiveFileLs.hpp      | 18 ++++++------------
 xroot_plugins/XrdSsiCtaRequestProc.cpp     | 19 ++-----------------
 xroot_plugins/XrdSsiCtaServiceProvider.cpp | 19 ++++---------------
 xroot_plugins/XrdSsiCtaServiceProvider.hpp | 14 ++++----------
 8 files changed, 21 insertions(+), 65 deletions(-)

diff --git a/cmdline/CtaAdminCmd.cpp b/cmdline/CtaAdminCmd.cpp
index 6f2775aecb..60c3718e41 100644
--- a/cmdline/CtaAdminCmd.cpp
+++ b/cmdline/CtaAdminCmd.cpp
@@ -301,9 +301,7 @@ int CtaAdminCmd::GetRequestTimeout() const
    // Use default if XRD_REQUESTTIMEOUT is not a valid positive integer
    if(request_timeout <= 0) request_timeout = DefaultRequestTimeout;
 
-#ifdef XRDSSI_DEBUG
-   std::cerr << "[DEBUG] CtaAdminCmd::GetRequestTimeout(): Request timeout = " << request_timeout << "s" << std::endl;
-#endif
+   XrdSsiPb::Log::Msg(XrdSsiPb::Log::DEBUG, LOG_SUFFIX, "GetRequestTimeout(): Request timeout = ", request_timeout, 's');
    return request_timeout;
 }
 
diff --git a/cmdline/CtaAdminCmd.hpp b/cmdline/CtaAdminCmd.hpp
index 97d70f888c..24c5285c55 100644
--- a/cmdline/CtaAdminCmd.hpp
+++ b/cmdline/CtaAdminCmd.hpp
@@ -65,6 +65,8 @@ private:
 
    std::string       m_execname;                        //!< Executable name of this program
    cta::xrd::Request m_request;                         //!< Protocol Buffer for the command and parameters
+
+   static constexpr const char* const LOG_SUFFIX  = "CtaAdminCmd";    //!< Identifier for log messages
 };
 
 }} // namespace cta::admin
diff --git a/cmdline/EosCtaStub.cpp b/cmdline/EosCtaStub.cpp
index 0ddefa61c8..9e4f654e43 100644
--- a/cmdline/EosCtaStub.cpp
+++ b/cmdline/EosCtaStub.cpp
@@ -117,9 +117,7 @@ void base64Decode(cta::eos::Notification &notification, const std::string &argva
       else if(key == "mode") notification.mutable_file()->set_mode(stoi(val));
       else if(key == "file") notification.mutable_file()->set_lpath(val);
       else {
-#ifdef XRDSSI_DEBUG
-         cout << "No match in protobuf for fmd:" << key << "=" << val << endl;
-#endif
+         XrdSsiPb::Log::Msg(XrdSsiPb::Log::ERROR, "base64Decode", "No match in protobuf for fmd:", key, '=', val);
       }
    }
 
@@ -163,9 +161,7 @@ void base64Decode(cta::eos::Notification &notification, const std::string &argva
          notification.mutable_directory()->mutable_xattr()->insert(google::protobuf::MapPair<string,string>(xattrn, val));
       }
       else {
-#ifdef XRDSSI_DEBUG
-         cout << "No match in protobuf for dmd:" << key << "=" << val << endl;
-#endif
+         XrdSsiPb::Log::Msg(XrdSsiPb::Log::ERROR, "base64Decode", "No match in protobuf for dmd:", key, '=', val);
       }
    }
 }
diff --git a/xroot_plugins/CMakeLists.txt b/xroot_plugins/CMakeLists.txt
index 78ff9ebd57..1e0e99acb4 100644
--- a/xroot_plugins/CMakeLists.txt
+++ b/xroot_plugins/CMakeLists.txt
@@ -41,8 +41,6 @@ set_property (TARGET XrdSsiCta APPEND PROPERTY INSTALL_RPATH ${PROTOBUF3_RPATH})
 if (OCCI_SUPPORT)
   set_property (TARGET XrdSsiCta APPEND PROPERTY INSTALL_RPATH ${ORACLE-INSTANTCLIENT_RPATH})
 endif (OCCI_SUPPORT)
-# Get some extra debug messages on stderr
-#target_compile_definitions(XrdSsiCta PRIVATE XRDSSI_DEBUG)
 
 install(TARGETS XrdSsiCta DESTINATION usr/${CMAKE_INSTALL_LIBDIR})
 install(FILES cta-frontend-xrootd.conf DESTINATION ${CMAKE_INSTALL_SYSCONFDIR}/cta)
diff --git a/xroot_plugins/XrdCtaArchiveFileLs.hpp b/xroot_plugins/XrdCtaArchiveFileLs.hpp
index f00109e315..94416e4624 100644
--- a/xroot_plugins/XrdCtaArchiveFileLs.hpp
+++ b/xroot_plugins/XrdCtaArchiveFileLs.hpp
@@ -35,15 +35,11 @@ public:
       XrdSsiStream(XrdSsiStream::isActive),
       m_archiveFileItor(std::move(archiveFileItor))
    {
-#ifdef XRDSSI_DEBUG
-      std::cerr << "[DEBUG] ArchiveFileLsStream() constructor" << std::endl;
-#endif
+      XrdSsiPb::Log::Msg(XrdSsiPb::Log::DEBUG, LOG_SUFFIX, "ArchiveFileLsStream() constructor");
    }
 
    virtual ~ArchiveFileLsStream() {
-#ifdef XRDSSI_DEBUG
-      std::cerr << "[DEBUG] ArchiveFileLsStream() destructor" << std::endl;
-#endif
+      XrdSsiPb::Log::Msg(XrdSsiPb::Log::DEBUG, LOG_SUFFIX, "~ArchiveFileLsStream() destructor");
    }
 
    /*!
@@ -66,9 +62,7 @@ public:
     *                 last = false: A fatal error occurred, eRef has the reason.
     */
    virtual Buffer *GetBuff(XrdSsiErrInfo &eInfo, int &dlen, bool &last) {
-#ifdef XRDSSI_DEBUG
-      std::cerr << "[DEBUG] ArchiveFileLsStream::GetBuff(): XrdSsi buffer fill request (" << dlen << " bytes)" << std::endl;
-#endif
+      XrdSsiPb::Log::Msg(XrdSsiPb::Log::DEBUG, LOG_SUFFIX, "GetBuff(): XrdSsi buffer fill request (", dlen, " bytes)");
 
       XrdSsiPb::OStreamBuffer<cta::xrd::Data> *streambuf;
 
@@ -120,9 +114,7 @@ public:
             }
          }
          dlen = streambuf->Size();
-#ifdef XRDSSI_DEBUG
-         std::cerr << "[DEBUG] ArchiveFileLsStream::GetBuff(): Returning buffer with " << dlen << " bytes of data." << std::endl;
-#endif
+         XrdSsiPb::Log::Msg(XrdSsiPb::Log::DEBUG, LOG_SUFFIX, "GetBuff(): Returning buffer with ", dlen, " bytes of data.");
       } catch(cta::exception::Exception &ex) {
          throw std::runtime_error(ex.getMessage().str());
       } catch(std::exception &ex) {
@@ -141,6 +133,8 @@ public:
 
 private:
    catalogue::ArchiveFileItor m_archiveFileItor;
+
+   static constexpr const char* const LOG_SUFFIX  = "ArchiveFileLsStream";    //!< Identifier for log messages
 };
 
 }} // namespace cta::xrd
diff --git a/xroot_plugins/XrdSsiCtaRequestProc.cpp b/xroot_plugins/XrdSsiCtaRequestProc.cpp
index 077646fa62..8c5b20e5d5 100644
--- a/xroot_plugins/XrdSsiCtaRequestProc.cpp
+++ b/xroot_plugins/XrdSsiCtaRequestProc.cpp
@@ -19,9 +19,7 @@
 #include "common/dataStructures/ArchiveRequest.hpp"
 #include "common/exception/Exception.hpp"
 
-#ifdef XRDSSI_DEBUG
-#include "XrdSsiPbDebug.hpp"
-#endif
+#include "XrdSsiPbLog.hpp"
 #include "XrdSsiPbException.hpp"
 #include "XrdSsiPbRequestProc.hpp"
 
@@ -60,11 +58,8 @@ void RequestProc<cta::xrd::Request, cta::xrd::Response, cta::xrd::Alert>::Execut
          throw cta::exception::Exception("XRootD Service is not a CTA Service");
       }
 
-#ifdef XRDSSI_DEBUG
       // Output message in Json format
-
-      OutputJsonString(std::cerr, &m_request);
-#endif
+      Log::DumpProtobuf(Log::PROTOBUF, &m_request);
 
       cta::xrd::RequestMessage request_msg(*(m_resource.client), cta_service_ptr);
       request_msg.process(m_request, m_metadata, m_response_stream_ptr);
@@ -75,16 +70,6 @@ void RequestProc<cta::xrd::Request, cta::xrd::Response, cta::xrd::Alert>::Execut
       m_metadata.set_type(cta::xrd::Response::RSP_ERR_PROTOBUF);
       m_metadata.set_message_txt(ex.what());
    } catch(std::exception &ex) {
-#ifdef XRDSSI_DEBUG
-      // Serialize and send a log message
-
-      cta::xrd::Alert alert_msg;
-
-      alert_msg.set_audience(cta::xrd::Alert::LOG);
-      alert_msg.set_message_txt("Something bad happened");
-
-      Alert(alert_msg);
-#endif
       m_metadata.set_type(cta::xrd::Response::RSP_ERR_CTA);
       m_metadata.set_message_txt(ex.what());
    }
diff --git a/xroot_plugins/XrdSsiCtaServiceProvider.cpp b/xroot_plugins/XrdSsiCtaServiceProvider.cpp
index f31cd337dc..1e68aa3aef 100644
--- a/xroot_plugins/XrdSsiCtaServiceProvider.cpp
+++ b/xroot_plugins/XrdSsiCtaServiceProvider.cpp
@@ -16,10 +16,6 @@
  *                 along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifdef XRDSSI_DEBUG
-#include <iostream>
-#endif
-
 #include "XrdSsiPbAlert.hpp"
 #include "XrdSsiPbService.hpp"
 #include "cta_frontend.pb.h"
@@ -58,9 +54,7 @@ bool XrdSsiCtaServiceProvider::Init(XrdSsiLogger *logP, XrdSsiCluster *clsP, con
 {
    using namespace cta;
 
-#ifdef XRDSSI_DEBUG
-   std::cout << "[DEBUG] Called Init(" << cfgFn << "," << parms << ")" << std::endl;
-#endif
+   XrdSsiPb::Log::Msg(XrdSsiPb::Log::INFO, LOG_SUFFIX, "Called Init(", cfgFn, ',', parms, ')');
   
    // Instantiate the logging system
 
@@ -129,9 +123,7 @@ bool XrdSsiCtaServiceProvider::Init(XrdSsiLogger *logP, XrdSsiCluster *clsP, con
 
 XrdSsiService* XrdSsiCtaServiceProvider::GetService(XrdSsiErrInfo &eInfo, const std::string &contact, int oHold)
 {
-#ifdef XRDSSI_DEBUG
-   std::cout << "[DEBUG] Called GetService(" << contact << "," << oHold << ")" << std::endl;
-#endif
+   XrdSsiPb::Log::Msg(XrdSsiPb::Log::INFO, LOG_SUFFIX, "Called GetService(", contact, ',', oHold, ')');
 
    XrdSsiService *ptr = new XrdSsiPb::Service<cta::xrd::Request, cta::xrd::Response, cta::xrd::Alert>;
 
@@ -161,11 +153,8 @@ XrdSsiProvider::rStat XrdSsiCtaServiceProvider::QueryResource(const char *rName,
    XrdSsiProvider::rStat resourcePresence = (strcmp(rName, "/ctafrontend") == 0) ?
                                             XrdSsiProvider::isPresent : XrdSsiProvider::notPresent;
 
-#ifdef XRDSSI_DEBUG
-   std::cout << "[DEBUG] XrdSsiCtaServiceProvider::QueryResource(" << rName << "): "
-             << ((resourcePresence == XrdSsiProvider::isPresent) ? "isPresent" : "notPresent")
-             << std::endl;
-#endif
+   XrdSsiPb::Log::Msg(XrdSsiPb::Log::INFO, LOG_SUFFIX, "QueryResource(", rName, "): ",
+                     ((resourcePresence == XrdSsiProvider::isPresent) ? "isPresent" : "notPresent"));
 
    return resourcePresence;
 }
diff --git a/xroot_plugins/XrdSsiCtaServiceProvider.hpp b/xroot_plugins/XrdSsiCtaServiceProvider.hpp
index 526dc339a8..e61717fd39 100644
--- a/xroot_plugins/XrdSsiCtaServiceProvider.hpp
+++ b/xroot_plugins/XrdSsiCtaServiceProvider.hpp
@@ -18,10 +18,6 @@
 
 #pragma once
 
-#ifdef XRDSSI_DEBUG
-#include <iostream>
-#endif
-
 #include <XrdSsi/XrdSsiProvider.hh>
 
 #include "common/Configuration.hpp"
@@ -50,15 +46,11 @@ class XrdSsiCtaServiceProvider : public XrdSsiProvider
 public:
    XrdSsiCtaServiceProvider() : m_ctaConf("/etc/cta/cta-frontend.conf")
    {
-#ifdef XRDSSI_DEBUG
-      std::cout << "[DEBUG] XrdSsiCtaServiceProvider() constructor" << std::endl;
-#endif
+      XrdSsiPb::Log::Msg(XrdSsiPb::Log::DEBUG, LOG_SUFFIX, "XrdSsiCtaServiceProvider() constructor");
    }
 
    virtual ~XrdSsiCtaServiceProvider() {
-#ifdef XRDSSI_DEBUG
-      std::cout << "[DEBUG] ~XrdSsiCtaServiceProvider() destructor" << std::endl;
-#endif
+      XrdSsiPb::Log::Msg(XrdSsiPb::Log::DEBUG, LOG_SUFFIX, "~XrdSsiCtaServiceProvider() destructor");
    }
 
    /*!
@@ -125,5 +117,7 @@ private:
    std::unique_ptr<cta::Scheduler>                     m_scheduler;           //!< The scheduler
    std::unique_ptr<cta::log::Logger>                   m_log;                 //!< The logger
    UniquePtrAgentHeartbeatThread                       m_agentHeartbeat;      //!< Agent heartbeat thread
+
+   static constexpr const char* const LOG_SUFFIX = "XrdSsiCtaServiceProvider";    //!< Identifier for log messages
 };
 
-- 
GitLab