[GitHub] [apisix] leslie-tsang commented on a diff in pull request #9425: fix: syslog plugin doesn't work

["leslie-tsang (via GitHub)" <gi...@apache.org>]

leslie-tsang commented on code in PR #9425:
URL: https://github.com/apache/apisix/pull/9425#discussion_r1189624206


##########
apisix/plugins/sls-logger.lua:
##########
@@ -21,7 +21,7 @@ local bp_manager_mod = require("apisix.utils.batch-processor-manager")
 
 local plugin_name = "sls-logger"
 local ngx = ngx
-local rf5424 = require("apisix.plugins.slslog.rfc5424")
+local rf5424 = require("apisix.syslog.rfc5424")

Review Comment:
   Please put the `syslog` library in the `plugins` folder. :)



##########
apisix/plugins/syslog/init.lua:
##########
@@ -75,28 +79,32 @@ end
 
 -- called in log phase of APISIX
 function _M.push_entry(conf, ctx, entry)
-    if batch_processor_manager:add_entry(conf, entry) then
+    local json_str, err = core.json.encode(entry)
+    if not json_str then
+        core.log.error('error occurred while encoding the data: ', err)
+        return
+    end
+
+    local rfc5424_data = rfc5424.encode("SYSLOG", "INFO", ctx.var.host,
+                                "apisix", ctx.var.pid, json_str)
+
+    if batch_processor_manager:add_entry(conf, rfc5424_data) then
         return
     end
 
     -- Generate a function to be executed by the batch processor
     local cp_ctx = core.table.clone(ctx)
-    local func = function(entries, batch_max_size)
-        local data, err
-        if batch_max_size == 1 then
-            data, err = core.json.encode(entries[1]) -- encode as single {}
-        else
-            data, err = core.json.encode(entries) -- encode as array [{}]
-        end
-
-        if not data then
-            return false, 'error occurred while encoding the data: ' .. err
+    local func = function(entries)
+        local items = {}
+        for _, e in ipairs(entries) do
+            table.insert(items, e)

Review Comment:
   ```suggestion
               core.table.insert(items, e)
   ```
   Would be better to `local` the `core.table.insert`.



##########
apisix/plugins/sls-logger.lua:
##########
@@ -138,9 +138,14 @@ function _M.log(conf, ctx)
         return
     end
 
-    local rf5424_data = rf5424.encode("SYSLOG", "INFO", ctx.var.host,"apisix",
-                                      ctx.var.pid, conf.project, conf.logstore,
-                                      conf.access_key_id, conf.access_key_secret, json_str)
+    local structured_data = {
+        {name = "project", value = conf.project},
+        {name = "logstore", value = conf.logstore},
+        {name = "access-key-id", value = conf.access_key_id},
+        {name = "access-key-secret", value = conf.access_key_secret}

Review Comment:
   ```suggestion
           {name = "access-key-secret", value = conf.access_key_secret},
   ```



##########
apisix/plugins/syslog/init.lua:
##########
@@ -18,6 +18,9 @@
 local core = require("apisix.core")
 local bp_manager_mod = require("apisix.utils.batch-processor-manager")
 local logger_socket = require("resty.logger.socket")
+local rfc5424 = require("apisix.syslog.rfc5424")
+local ipairs = ipairs
+local table = table

Review Comment:
   please use the `core.table.*` instead. :) 



##########
apisix/syslog/rfc5424.lua:
##########
@@ -95,10 +95,20 @@ function _M.encode(facility, severity, hostname, appname, pid, project,
         appname = "-"
     end
 
-    return "<" .. pri .. ">1 " .. t .. " " .. hostname .. " " .. appname .. " " .. pid
-           .. " - [logservice project=\"" .. project .. "\" logstore=\"" .. logstore
-           .. "\" access-key-id=\"" .. access_key_id .. "\" access-key-secret=\""
-           .. access_key_secret .. "\"] " .. msg .. "\n"
+    local structured_data_str = "-"
+
+    if structured_data then
+        structured_data_str = "[logservice"
+        for _, sd_param in ipairs(structured_data) do
+            structured_data_str = structured_data_str .. " " .. sd_param.name
+                                  .. "=\"" .. sd_param.value .. "\""
+        end
+        structured_data_str = structured_data_str .. "]"
+    end
+
+    return "<" .. pri .. ">1 " .. t .. " " .. hostname .. " "
+           .. appname .. " " .. pid .. " - " .. structured_data_str .. " "
+           .. msg .. "\n"

Review Comment:
   please use `core.string.format` to format the output.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org