Upgrade/logging (#44)

* Dub - Switched to `gogga` for logging * birchwood.logging - Added logging facilities * Client - Use new logging facility * Receiver - Use new logging facility * Messages - Use new logging facility * Messages - Cleaned up * Dub - Bumped * Dub - Bumped * Revert "Dub" This reverts commit a7049d1. * Reapply "Dub" This reverts commit 44b9b19.
deavmi · May 12, 2024 · b1e3f24 · b1e3f24
1 parent 23b5b39
commit b1e3f24
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 63 deletions.
diff --git a/dub.json b/dub.json
@@ -5,7 +5,7 @@
 	],
 	"copyright": "Copyright © 2023, Tristan B. Kildaire",
 	"dependencies": {
-		"dlog": ">=0.3.19",
+		"gogga" : ">=3.1.1",
 		"eventy": ">=0.4.0"
 	},
 	"description": "A sane IRC framework for the D language",

diff --git a/source/birchwood/client/client.d b/source/birchwood/client/client.d
@@ -19,14 +19,7 @@ import birchwood.protocol.constants : ReplyType;
 import birchwood.client.receiver : ReceiverThread;
 import birchwood.client.sender : SenderThread;
 import birchwood.client.events;
-
-import dlog;
-
-package __gshared Logger logger;
-__gshared static this()
-{
-    logger = new DefaultLogger();
-}
+import birchwood.logging;
 
 // TODO: Make abstract and for unit tests make a `DefaultClient`
 // ... which logs outputs for the `onX()` handler functions
@@ -136,7 +129,7 @@ public class Client : Thread
     public void onChannelMessage(Message fullMessage, string channel, string msgBody)
     {
         /* Default implementation */
-        logger.log("Channel("~channel~"): "~msgBody);
+        DEBUG("Channel("~channel~"): "~msgBody);
     }
 
     /** 
@@ -150,7 +143,7 @@ public class Client : Thread
     public void onDirectMessage(Message fullMessage, string nickname, string msgBody)
     {
         /* Default implementation */
-        logger.log("DirectMessage("~nickname~"): "~msgBody);
+        DEBUG("DirectMessage("~nickname~"): "~msgBody);
     }
 
     /** 
@@ -162,7 +155,7 @@ public class Client : Thread
     public void onGenericCommand(Message message)
     {
         /* Default implementation */
-        logger.log("Generic("~message.getCommand()~", "~message.getFrom()~"): "~message.getParams());
+        DEBUG("Generic("~message.getCommand()~", "~message.getFrom()~"): "~message.getParams());
     }
 
     // TODO: Hook certain ones default style with an implemenation
@@ -179,29 +172,29 @@ public class Client : Thread
         // ... state
 
         /* Default implementation */
-        logger.log("Response("~to!(string)(commandReply.getReplyType())~", "~commandReply.getFrom()~"): "~commandReply.toString());
+        DEBUG("Response("~to!(string)(commandReply.getReplyType())~", "~commandReply.getFrom()~"): "~commandReply.toString());
 
         if(commandReply.getReplyType() == ReplyType.RPL_ISUPPORT)
         {
             // TODO: Testing code was here
-            // logger.log();
-            // logger.log("<<<>>>");
+            // DEBUG();
+            // DEBUG("<<<>>>");
 
-            // logger.log("Take a look:\n\n"~commandReply.getParams());
+            // DEBUG("Take a look:\n\n"~commandReply.getParams());
 
-            // logger.log("And here is key-value pairs: ", commandReply.getKVPairs());
-            // logger.log("And here is array: ", commandReply.getPairs());
+            // DEBUG("And here is key-value pairs: ", commandReply.getKVPairs());
+            // DEBUG("And here is array: ", commandReply.getPairs());
 
             // // TODO: DLog bug, this prints nothing
-            // logger.log("And here is trailing: ", commandReply.getTrailing());
+            // DEBUG("And here is trailing: ", commandReply.getTrailing());
 
             // import std.stdio;
             // writeln("Trailer: "~commandReply.getTrailing());
 
             // writeln(cast(ubyte[])commandReply.getTrailing());
 
-            // logger.log("<<<>>>");
-            // logger.log();
+            // DEBUG("<<<>>>");
+            // DEBUG();
 
             import std.stdio;
             writeln("Support stuff: ", commandReply.getKVPairs());
@@ -213,7 +206,7 @@ public class Client : Thread
                 /* Update the db */
                 string value = receivedKV[key];
                 connInfo.updateDB(key, value);
-                logger.log("Updated key in db '"~key~"' with value '"~value~"'");
+                DEBUG("Updated key in db '"~key~"' with value '"~value~"'");
             }
 
         }
@@ -225,7 +218,7 @@ public class Client : Thread
     public void onConnectionClosed()
     {
         // TODO: Add log as default behaviour?
-        logger.log("Connection was closed, not doing anything");
+        DEBUG("Connection was closed, not doing anything");
     }
 
     /** 
@@ -773,7 +766,7 @@ public class Client : Thread
                 assert(ircEvent); //Should never fail, unless some BOZO regged multiple handles for 1 - wait idk does eventy do that even mmm
 
                 // NOTE: Enable this when debugging
-                // logger.log("IRCEvent(message): "~ircEvent.getMessage().toString());
+                // DEBUG("IRCEvent(message): "~ircEvent.getMessage().toString());
 
                 /* TODO: We should use a switch statement, imagine how nice */
                 Message ircMessage = ircEvent.getMessage();
@@ -849,7 +842,7 @@ public class Client : Thread
                 // string messageToSend = "PONG "~pongEvent.getID();
                 Message pongMessage = new Message("", "PONG", pongEvent.getID());
                 client.sendMessage(pongMessage);
-                logger.log("Ponged back with "~pongEvent.getID());
+                DEBUG("Ponged back with "~pongEvent.getID());
             }
         }
         engine.addSignalHandler(new PongSignal(this));
@@ -1034,7 +1027,7 @@ public class Client : Thread
     {
         /* Set the state of running to false */
         running = false;
-        logger.log("disconnect() begin");
+        DEBUG("disconnect() begin");
 
         /* Shutdown the socket */
 
@@ -1048,7 +1041,7 @@ public class Client : Thread
          */
         import std.socket : SocketShutdown;
         socket.shutdown(SocketShutdown.BOTH);
-        logger.log("disconnect() socket shutdown");
+        DEBUG("disconnect() socket shutdown");
 
 
     }
@@ -1062,20 +1055,20 @@ public class Client : Thread
     {
         /* Stop the receive queue manager and wait for it to stop */
         receiver.end();
-        logger.log("doThreadCleanup() recvQueue manager stopped");
+        DEBUG("doThreadCleanup() recvQueue manager stopped");
         receiver = null;
 
         /* Stop the send queue manager and wait for it to stop */
         sender.end();
-        logger.log("doThreadCleanup() sendQueue manager stopped");
+        DEBUG("doThreadCleanup() sendQueue manager stopped");
         sender = null;
 
         /* TODO: Stop eventy (FIXME: I don't know if this is implemented in Eventy yet, do this!) */
         engine.shutdown();
-        logger.log("doThreadCleanup() eventy stopped");
+        DEBUG("doThreadCleanup() eventy stopped");
         engine = null;
 
-        logger.log("doThreadCleanup() end");
+        DEBUG("doThreadCleanup() end");
     }
 
     /** 
@@ -1088,8 +1081,8 @@ public class Client : Thread
     private void processMessage(ubyte[] message)
     {
         // import std.stdio;
-        // logger.log("Message length: "~to!(string)(message.length));
-        // logger.log("InterpAsString: "~cast(string)message);
+        // DEBUG("Message length: "~to!(string)(message.length));
+        // DEBUG("InterpAsString: "~cast(string)message);
 
         receiveQ(message);
     }

diff --git a/source/birchwood/client/receiver.d b/source/birchwood/client/receiver.d
@@ -21,6 +21,7 @@ version(unittest)
 {
     import std.stdio : writeln;
 }
+import birchwood.logging;
 
 /** 
  * Manages the receive queue and performs
@@ -150,7 +151,7 @@ public final class ReceiverThread : Thread
              */
             if(pingMessage !is null)
             {
-                logger.log("Found a ping: "~pingMessage.toString());
+                DEBUG("Found a ping: "~pingMessage.toString());
 
                 /* Extract the PING ID */
                 string pingID = pingMessage.getParams();

diff --git a/source/birchwood/logging.d b/source/birchwood/logging.d
@@ -0,0 +1,56 @@
+/**
+ * Internal logging facilities
+ */
+module birchwood.logging;
+
+import gogga;
+import gogga.extras;
+import dlog.basic : Level, FileHandler;
+import std.stdio : stdout;
+
+/** 
+ * Globally available logger
+ */
+package __gshared GoggaLogger logger;
+
+/**
+ * Initializes a logger instance
+ * globally
+ */
+__gshared static this()
+{
+    logger = new GoggaLogger();
+
+    GoggaMode mode;
+
+    // TODO: Add flag support
+    version(DBG_VERBOSE_LOGGING)
+    {
+        mode = GoggaMode.RUSTACEAN;
+    }
+    else
+    {
+        mode = GoggaMode.SIMPLE;
+    }
+
+    logger.mode(mode);
+
+    Level level = Level.DEBUG;
+
+    // TODO: Add flag support
+    // version(DBG_DEBUG_LOGGING)
+    // {
+    //     level = Level.DEBUG;
+    // }
+    // else
+    // {
+    //     level = Level.INFO;
+    // }
+
+
+    logger.setLevel(level);
+    logger.addHandler(new FileHandler(stdout));
+}
+
+// Bring in helper methods
+mixin LoggingFuncs!(logger);
diff --git a/source/birchwood/protocol/messages.d b/source/birchwood/protocol/messages.d
@@ -11,32 +11,10 @@ import birchwood.protocol.constants : ReplyType;
 
 import birchwood.client.exceptions;
 import birchwood.config.conninfo : ChecksMode;
-
-// TODO: Before release we should remove this import
-import std.stdio : writeln;
+import birchwood.logging;
 
 /* TODO: We could move these all to `package.d` */
 
-/* Static is redundant as module is always static , gshared needed */
-/* Apparebky works without gshared, that is kinda sus ngl */
-package __gshared Logger logger;
-/**
-* source/birchwood/messages.d(10,8): Error: variable `birchwood.messages.logger` is a thread-local class and cannot have a static initializer. Use `static this()` to initialize instead.
-*
-* It is complaining that it wopuld static init per thread, static this() for module is required but that would
-* do a module init per thread, so __gshared static this() is needed, we want one global init - a single logger
-* variable and also class init
-*/
-
-__gshared static this()
-{
-    logger = new DefaultLogger();
-}
-
-/**
-* Encoding/decoding primitives
-*/
-
 /** 
  * Encodes the provided message into a CRLF
  * terminated byte array
@@ -140,8 +118,8 @@ public final class Message
             }
             catch(ConvException e)
             {
-                logger.log("<<< Unsupported response code (Error below) >>>");
-                logger.log(e);
+                DEBUG("<<< Unsupported response code (Error below) >>>");
+                DEBUG(e);
             }
         }
 
@@ -286,7 +264,7 @@ public final class Message
             {
                 from = message[1..firstSpace];
 
-                // logger.log("from: "~from);
+                // DEBUG("from: "~from);
 
                 /* TODO: Find next space (what follows `from` is  `' ' { ' ' }`) */
                 ulong i = firstSpace;
@@ -306,7 +284,7 @@ public final class Message
 
                 /* Extract the command */
                 command = rem[0..idx];
-                // logger.log("command: "~command);
+                // DEBUG("command: "~command);
 
                 /* Params are everything till the end */
                 i = idx;
@@ -318,12 +296,12 @@ public final class Message
                     }
                 }
                 params = rem[i..rem.length];
-                // logger.log("params: "~params);
+                // DEBUG("params: "~params);
             }
             else
             {
                 //TODO: handle
-                logger.log("Malformed message start after :");
+                DEBUG("Malformed message start after :");
                 assert(false);
             }