Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Difficult to customize logging behaviour #102

Closed
ulfj opened this issue Apr 11, 2012 · 7 comments
Closed

Difficult to customize logging behaviour #102

ulfj opened this issue Apr 11, 2012 · 7 comments

Comments

@ulfj
Copy link

ulfj commented Apr 11, 2012

I would like to suggest a small change to make it simpler to integrate logging with an application-wide logging facility.
The websocket++ logger class can be "replaced" by passing a parameter to the websocketpp::endpoint template:

typedef websocketpp::endpoint<websocketpp::role::server, websocketpp::socket::plain, my_logger> server;

This is good but there are some issues that may need some attention for this to reach its full usefullness.

  1. The logger template class is instantiated twice in the endpoint (alog and elog) with template parameters log::alevel::value and log::elevel::value. These types resolves to the same uint16_t type making it difficult to use template specialization to give elog and alog different behaviour (at least on Windows using VS2010). This can be fixed in different ways. The very least I am looking for is to know if the level bits are alevel::value or elevel::value. This is because the logging filter will be in the outer logging framework.

  2. Logging is done like this (example from library code):

        elog().at(log::elevel::FATAL) 
            << "Tried to switch to a NULL handler." << log::endl;
    

The problem I have with this is the "log::endl" which is statement that causes the log to be printed. The log::endl function looks like:

template <typename level_type>
logger<level_type>& endl(logger<level_type>& out)
{
    return out.print();
}

This means that I see no way to get my own "print" function called short of making changes to the library (making print virtual) and deriving my logger class from the one in the library. A better way may perhaps be to replace log::endl with a function defined by the logger class (no virtual function call overhead) - example:

        elog().at(log::elevel::FATAL) 
            << "Tried to switch to a NULL handler." << elogger_type::endl;

This way the custom logger should be possible to customize without changes to the library. I have not tested the changes I suggest so they may not be working.

Regards,
Ulf

@zaphoyd
Copy link
Owner

zaphoyd commented Apr 24, 2012

The difficulty of customizing logging is noted. I'll try and address these in the next revision of the logging system. If anyone else has feedback on custom logging please leave them here.

@ulfj
Copy link
Author

ulfj commented Apr 25, 2012

Thanks for the response. It is in no way urgent or critical. I have made it work by making "print" virtual and created a derived class. This works for now.

@SemiConscious
Copy link

ulfj's 'temporary' solution (overriding print()) works fine (and requires few changes) if you code the alevel and elevel constants as enums - you can then specialize on the 2 cases as ulf wanted.

Here's a diff of the changes I made to make it possible to create a subclassed logger of my own. As far as I can see, few changes are required in the library code and no changes to library clients.

Index: endpoint.hpp
===================================================================
--- endpoint.hpp    (revision 20601)
+++ endpoint.hpp    (working copy)
@@ -120,8 +120,8 @@
     friend socket_type;
     friend connection_type;
 #else
-    friend class role< endpoint<role,socket> >;
-    friend class socket< endpoint<role,socket> >;
+    friend class role< endpoint<role,socket,logger> >;
+    friend class socket< endpoint<role,socket,logger> >;
     friend class connection<type,role< type >::template connection,socket< type >::template connection>;
 #endif

Index: logger/logger.hpp
===================================================================
--- logger/logger.hpp   (revision 20601)
+++ logger/logger.hpp   (working copy)
@@ -37,57 +37,61 @@
 namespace log {

 namespace alevel {
-    typedef uint16_t value;
+typedef enum {

-    static const value OFF = 0x0;
+    OFF = 0x0,

     // A single line on connect with connecting ip, websocket version, 
     // request resource, user agent, and the response code.
-    static const value CONNECT = 0x1;
+    CONNECT = 0x1,
     // A single line on disconnect with wasClean status and local and remote
     // close codes and reasons.
-    static const value DISCONNECT = 0x2;
+    DISCONNECT = 0x2,
     // A single line on incoming and outgoing control messages.
-    static const value CONTROL = 0x4;
+    CONTROL = 0x4,
     // A single line on incoming and outgoing frames with full frame headers
-    static const value FRAME_HEADER = 0x10;
+    FRAME_HEADER = 0x10,
     // Adds payloads to frame logs. Note these can be long!
-    static const value FRAME_PAYLOAD = 0x20;
+    FRAME_PAYLOAD = 0x20,
     // A single line on incoming and outgoing messages with metadata about type,
     // length, etc
-    static const value MESSAGE_HEADER = 0x40;
+    MESSAGE_HEADER = 0x40,
     // Adds payloads to message logs. Note these can be long!
-    static const value MESSAGE_PAYLOAD = 0x80;
+    MESSAGE_PAYLOAD = 0x80,

     // Notices about internal endpoint operations
-    static const value ENDPOINT = 0x100;
+    ENDPOINT = 0x100,

     // DEBUG values
-    static const value DEBUG_HANDSHAKE = 0x8000;
-    static const value DEBUG_CLOSE = 0x4000;
-    static const value DEVEL = 0x2000;
+    DEBUG_HANDSHAKE = 0x8000,
+    DEBUG_CLOSE = 0x4000,
+    DEVEL = 0x2000,

-    static const value ALL = 0xFFFF;
+    ALL = 0xFFFF
+} value;
 }

 namespace elevel {
-    typedef uint16_t value;
+typedef enum {

-    static const value OFF = 0x0;
+   OFF = 0x0,

-    static const value DEVEL = 0x1;     // debugging
-    static const value LIBRARY = 0x2;   // library usage exceptions
-    static const value INFO = 0x4;      // 
-    static const value WARN = 0x8;      //
-    static const value RERROR = 0x10;    // recoverable error
-    static const value FATAL = 0x20;    // unrecoverable error
+   DEVEL = 0x1,     // debugging
+   LIBRARY = 0x2,   // library usage exceptions
+   INFO = 0x4,      //
+   WARN = 0x8,      //
+   RERROR = 0x10,    // recoverable error
+   FATAL = 0x20,    // unrecoverable error

-    static const value ALL = 0xFFFF;
+   ALL = 0xFFFF
+} value;
 }

 template <typename level_type>
 class logger {
 public:
+   virtual ~logger<level_type>() {}
+
     template <typename T>
     logger<level_type>& operator<<(T a) {
         if (test_level(m_write_level)) {
@@ -105,7 +109,7 @@
     }

     void set_level(level_type l) {
-        m_level |= l;
+        m_level = static_cast<level_type>(m_level | l);
     }

     void set_levels(level_type l1, level_type l2) {
@@ -129,7 +133,7 @@
         }
     }

-    logger<level_type>& print() {
+    virtual logger<level_type>& print() {
         if (test_level(m_write_level)) {
             std::cout << m_prefix << 
                 boost::posix_time::to_iso_extended_string(
@@ -145,7 +149,7 @@
         m_write_level = l;
         return *this;
     }
-private:
+protected:
     std::ostringstream m_oss;
     level_type m_write_level;
     level_type m_level;

Here's a sample of how I implemented it:

namespace websocketpp
{
    namespace log
    {
        template<typename level_type>
        class my_logger : public logger<level_type>
        {
            virtual logger<level_type>& print();
        };

        template<> 
        logger<alevel::value>& my_logger<alevel::value>::print()
        {
            if (test_level(m_write_level))
            {
                LogVerbose ("WSK: " << m_oss.str());
                m_oss.str("");
            }

            return *this;
        }

        template<> 
        logger<elevel::value>& my_logger<elevel::value>::print()
        {
            if (test_level(m_write_level))
            {
                switch(m_write_level)
                {
                    case elevel::OFF:
                        break;
                    case elevel::DEVEL:
                        LogDebug ("WSK: " << m_oss.str());
                        break;
                    case elevel::LIBRARY:
                    case elevel::INFO:
                        LogVerbose ("WSK: " << m_oss.str());
                        break;
                    case elevel::WARN:
                        LogMessage ("WSK: " << m_oss.str());
                        break;
                    case elevel::RERROR:
                    case elevel::FATAL:
                        LogWarning ("WSK: " << m_oss.str());
                        break;
                    default:
                        LogWarning ("WSK: [unknown elevel " << 
                            m_write_level << "]: " << m_oss.str());
                        break;
                }

                m_oss.str("");
            }

            return *this;
        }
    }

    typedef websocketpp::endpoint<
        websocketpp::role::server, 
        websocketpp::socket::plain, 
        log::my_logger> server;
}

@zarnce
Copy link

zarnce commented Jun 19, 2012

How about adding support for external logging libraries. pion-net gives you the ability to build with support for several libraries.

From git project page: https://github.com/cloudmeter/pion-net/tree/4.0.11
"For logging, Pion may be configured to:

a) use std::cout and std::cerr for logging (the default configuration)
b) use one of the following open source libraries: log4cxx, log4cpp or
log4cplus (configure using one of --with-log4cxx, --with-log4cpp or
--with-log4cplus, respectively; also may be auto-detected)
c) disable logging entirely (configure --disable-logging)"

Brian

@zaphoyd
Copy link
Owner

zaphoyd commented Jun 29, 2012

some of these external logging libraries look quite interesting. I am going to try and get some preliminary support for external logging in version 0.3

@SemiConscious
Copy link

Hmm - I ended up building a client app after my fixes and one or 2 small
things were broken. Would you like me to submit a patch, or do you want to
go down a different route?

IMHO - if you give us good support for applying our own logging, maybe you
don't need to add support (or dependencies) for any other libs; it was the
work of 2 minutes to add support for the boost logging libs for us.

Best, Jim

On 29 June 2012 21:27, Peter Thorson <
reply@reply.github.com

wrote:

some of these external logging libraries look quite interesting. I am
going to try and get some preliminary support for external logging in
version 0.3


Reply to this email directly or view it on GitHub:
#102 (comment)

@zaphoyd
Copy link
Owner

zaphoyd commented Mar 25, 2014

This thread refers to issues related to the design of the logging policies in the 0.2.x branch, which will not be fixed. The 0.3.x+ logging policies were completely re-done and should allow for better logging customization. If anyone has further suggestions or issues with the customizability of 0.3.x+ logging they should open a new issue.

@zaphoyd zaphoyd closed this as completed Mar 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants