Use enum for logging level

Author: khamilowicz

c_src/unifex/unifex/logger.c

@@ -54,13 +54,22 @@ static void free_tags_copy(char **tags, unsigned int tags_length) {
   free(tags);
 }
 
-static void free_pending_message(char *level, char *message, char **tags,
+static void free_pending_message(char *message, char **tags,
                                  unsigned int tags_length) {
-  free((void *)level);
   free((void *)message);
   free_tags_copy(tags, tags_length);
 }
 
+const char *unifex_log_level_to_string(UnifexLogLevel level) {
+  switch (level) {
+    case UNIFEX_LOG_LEVEL_DEBUG: return "debug";
+    case UNIFEX_LOG_LEVEL_INFO:  return "info";
+    case UNIFEX_LOG_LEVEL_WARN:  return "warning";
+    case UNIFEX_LOG_LEVEL_ERROR: return "error";
+    default: return "unknown";
+  }
+}
+
 const char *unifex_logger_get_target() { return target_pid_name; }
 
 uint64_t unifex_logger_get_timestamp() {
@@ -113,7 +122,7 @@ void unifex_logger_cleanup() {
   pthread_cond_destroy(&queue.cond);
 }
 
-bool unifex_log(const char *level, const char *message, const char **tags,
+bool unifex_log(UnifexLogLevel level, const char *message, const char **tags,
                 unsigned int tags_length) {
   if (!message) {
     return false;
@@ -131,7 +140,6 @@ bool unifex_log(const char *level, const char *message, const char **tags,
 
   uint64_t timestamp = unifex_logger_get_timestamp();
 
-  char *level_copy = strdup(level);
   char *message_copy = strdup(message);
 
   char **tags_copy = NULL;
@@ -150,8 +158,8 @@ bool unifex_log(const char *level, const char *message, const char **tags,
     }
   }
 
-  if (!level_copy || !message_copy || tags_alloc_failed) {
-    free_pending_message(level_copy, message_copy, tags_copy, tags_length);
+  if (!message_copy || tags_alloc_failed) {
+    free_pending_message(message_copy, tags_copy, tags_length);
     return false;
   }
 
@@ -160,11 +168,11 @@ bool unifex_log(const char *level, const char *message, const char **tags,
   if (queue.count >= UNIFEX_LOGGER_MAX_QUEUE_SIZE) {
     queue.dropped_count++;
     pthread_mutex_unlock(&queue.mutex);
-    free_pending_message(level_copy, message_copy, tags_copy, tags_length);
+    free_pending_message(message_copy, tags_copy, tags_length);
     return false;
   }
 
-  queue.messages[queue.tail].level = level_copy;
+  queue.messages[queue.tail].level = level;
   queue.messages[queue.tail].message = message_copy;
   queue.messages[queue.tail].timestamp = timestamp;
   queue.messages[queue.tail].tags = tags_copy;
@@ -213,7 +221,7 @@ void *unifex_logger_worker(void *arg) {
         send_log_func(global_env, batch[i].level, batch[i].message,
                       batch[i].timestamp, batch[i].tags, batch[i].tags_length);
       }
-      free_pending_message(batch[i].level, batch[i].message, batch[i].tags,
+      free_pending_message(batch[i].message, batch[i].tags,
                            batch[i].tags_length);
     }
 

c_src/unifex/unifex/logger.h

@@ -23,14 +23,19 @@ extern "C" {
 #define UNIFEX_LOGGER_BACKEND_CTOR_PRIORITY 1000
 #define UNIFEX_LOGGER_QUEUE_CTOR_PRIORITY 2000
 
-// Log level constants
-#define UNIFEX_LOG_LEVEL_DEBUG "debug"
-#define UNIFEX_LOG_LEVEL_INFO "info"
-#define UNIFEX_LOG_LEVEL_WARN "warning"
-#define UNIFEX_LOG_LEVEL_ERROR "error"
+// Log level enum for compile-time type safety
+typedef enum {
+  UNIFEX_LOG_LEVEL_DEBUG,
+  UNIFEX_LOG_LEVEL_INFO,
+  UNIFEX_LOG_LEVEL_WARN,
+  UNIFEX_LOG_LEVEL_ERROR,
+} UnifexLogLevel;
+
+// Convert log level enum to string representation (for internal use by backends)
+const char *unifex_log_level_to_string(UnifexLogLevel level);
 
 typedef struct {
-  char *level;
+  UnifexLogLevel level;
   char *message;
   uint64_t timestamp;
   char **tags;
@@ -49,15 +54,15 @@ typedef struct {
   pthread_t worker_thread;
 } UnifexLoggerQueue;
 
-typedef int (*UnifexLoggerSendFunc)(void *env, const char *level,
+typedef int (*UnifexLoggerSendFunc)(void *env, UnifexLogLevel level,
                                     const char *message, uint64_t timestamp,
                                     char **tags, unsigned int tags_length);
 
 void unifex_logger_init();
 
 void unifex_logger_cleanup();
 
-bool unifex_log(const char *level, const char *message, const char **tags,
+bool unifex_log(UnifexLogLevel level, const char *message, const char **tags,
                 unsigned int tags_length);
 
 void unifex_logger_register_send_func(UnifexLoggerSendFunc func);

c_src/unifex/unifex/logger_backend.h

@@ -6,15 +6,17 @@
  * Include the appropriate backend implementation in your project.
  */
 
+#include "logger.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-int unifex_logger_nif_send(void *env, const char *level, const char *message,
+int unifex_logger_nif_send(void *env, UnifexLogLevel level, const char *message,
                            uint64_t timestamp, char **tags,
                            unsigned int tags_length);
 
-int unifex_logger_cnode_send(void *env, const char *level, const char *message,
+int unifex_logger_cnode_send(void *env, UnifexLogLevel level, const char *message,
                              uint64_t timestamp, char **tags,
                              unsigned int tags_length);
 

c_src/unifex/unifex/logger_cnode.c

@@ -10,7 +10,7 @@
 #include "logger.h"
 #include "logger_backend.h"
 
-int unifex_logger_cnode_send(void *env_ptr, const char *level,
+int unifex_logger_cnode_send(void *env_ptr, UnifexLogLevel level,
                              const char *message, uint64_t timestamp,
                              char **tags, unsigned int tags_length) {
   UnifexEnv *env = (UnifexEnv *)env_ptr;
@@ -25,7 +25,7 @@ int unifex_logger_cnode_send(void *env_ptr, const char *level,
 
   ei_x_encode_atom(&out_buff, "unifex_logger");
 
-  ei_x_encode_atom(&out_buff, level);
+  ei_x_encode_atom(&out_buff, unifex_log_level_to_string(level));
 
   ei_x_encode_binary(&out_buff, message, strlen(message));
 

c_src/unifex/unifex/logger_nif.c

@@ -11,7 +11,7 @@
 
 static UnifexEnv *fallback_env = NULL;
 
-int unifex_logger_nif_send(void *env_ptr, const char *level,
+int unifex_logger_nif_send(void *env_ptr, UnifexLogLevel level,
                            const char *message, uint64_t timestamp, char **tags,
                            unsigned int tags_length) {
   UnifexEnv *env = (UnifexEnv *)env_ptr;
@@ -37,7 +37,7 @@ int unifex_logger_nif_send(void *env_ptr, const char *level,
     return 0;
   }
 
-  ERL_NIF_TERM level_atom = enif_make_atom(send_env, level);
+  ERL_NIF_TERM level_atom = enif_make_atom(send_env, unifex_log_level_to_string(level));
   ERL_NIF_TERM message_term = unifex_string_to_term(send_env, message);
   ERL_NIF_TERM timestamp_term = enif_make_uint64(send_env, timestamp);
 

lib/unifex/logger.ex

@@ -4,8 +4,6 @@ defmodule Unifex.Logger do
   use GenServer
   require Logger
 
-  @valid_levels ~w(emergency alert critical error warning notice info debug)a
-
   @spec start_link(any()) :: GenServer.on_start()
   def start_link(_opts) do
     GenServer.start_link(__MODULE__, [], name: __MODULE__)
@@ -17,13 +15,16 @@ defmodule Unifex.Logger do
   end
 
   @impl true
-  @spec handle_info({:unifex_logger, atom(), String.t(), integer(), list(atom())}, map()) ::
+  @spec handle_info(
+          {:unifex_logger, atom() | String.t(), String.t(), integer(), list(String.t())},
+          map()
+        ) ::
           {:noreply, map()}
   def handle_info({:unifex_logger, level, message, timestamp, tags}, state) do
     metadata = [tags: tags, unifex_nif: true, timestamp: timestamp]
 
     Logger.log(
-      normalize_level(level),
+      level,
       fn -> format_message(message, tags, timestamp) end,
       metadata
     )
@@ -38,21 +39,7 @@ defmodule Unifex.Logger do
   end
 
   @doc false
-  @spec normalize_level(atom()) :: atom()
-  def normalize_level(level) when level in @valid_levels, do: level
-
-  @doc false
-  @spec normalize_level(any()) :: atom()
-  def normalize_level(level) do
-    Logger.warning(
-      "Unifex.Logger received unknown log level #{inspect(level)}, defaulting to :info"
-    )
-
-    :info
-  end
-
-  @doc false
-  @spec format_message(String.t(), list(atom()), integer()) :: String.t()
+  @spec format_message(String.t(), list(String.t()), integer()) :: String.t()
   def format_message(message, tags, timestamp) do
     tag_parts = Enum.map(tags, &"[#{&1}]")
     Enum.join(["[#{format_timestamp(timestamp)}]"] ++ tag_parts ++ [message], " ")

pages/logger.md

@@ -50,13 +50,13 @@ UNIFEX_TERM process(UnifexEnv *env, int num) {
 ### `unifex_log`
 
 ```c
-bool unifex_log(const char *level, const char *message, const char **tags,
+bool unifex_log(UnifexLogLevel level, const char *message, const char **tags,
                 unsigned int tags_length);
 ```
 
-* `level` - one of the `UNIFEX_LOG_LEVEL_DEBUG` / `_INFO` / `_WARN` / `_ERROR` constants
-  (or any other string; it's copied, so any level your `Unifex.Logger` handler
-  understands works).
+* `level` - one of the `UNIFEX_LOG_LEVEL_DEBUG`, `UNIFEX_LOG_LEVEL_INFO`,
+  `UNIFEX_LOG_LEVEL_WARN`, or `UNIFEX_LOG_LEVEL_ERROR` enum values. This is a
+  typed enum, so the compiler will catch typos or invalid values at compile time.
 * `message` - the log message. Must not be `NULL`.
 * `tags` / `tags_length` - an optional array of extra tag strings attached to the
   message (pass `NULL, 0` for none).

test/unifex/logger_test.exs

@@ -15,25 +15,6 @@ defmodule Unifex.LoggerTest do
       end
     end
 
-    test "normalizes valid log levels" do
-      assert Unifex.Logger.normalize_level(:debug) == :debug
-      assert Unifex.Logger.normalize_level(:info) == :info
-      assert Unifex.Logger.normalize_level(:warning) == :warning
-      assert Unifex.Logger.normalize_level(:error) == :error
-      assert Unifex.Logger.normalize_level(:critical) == :critical
-      assert Unifex.Logger.normalize_level(:alert) == :alert
-      assert Unifex.Logger.normalize_level(:emergency) == :emergency
-      assert Unifex.Logger.normalize_level(:notice) == :notice
-    end
-
-    test "normalizes invalid log levels to :info with warning" do
-      # This test verifies that unknown levels are normalized to :info
-      # The warning is a side effect that we can't easily capture here
-      assert Unifex.Logger.normalize_level(:unknown_level) == :info
-      assert Unifex.Logger.normalize_level("string_level") == :info
-      assert Unifex.Logger.normalize_level(123) == :info
-    end
-
     test "formats message without tags" do
       # microseconds since epoch
       timestamp = 1_700_000_000_000_000