[GitHub] [apisix] spacewander commented on a diff in pull request #6960: feat(xRPC): support log filter

[GitBox <gi...@apache.org>]

spacewander commented on code in PR #6960:
URL: https://github.com/apache/apisix/pull/6960#discussion_r866669511


##########
apisix/stream/xrpc/runner.lua:
##########
@@ -70,10 +74,54 @@ local function put_req_ctx(session, ctx)
 end
 
 
+local function log_filter(session, ctx)
+    local logger = session._route.protocol.logger
+    if not logger or not logger.filter or #logger.filter == 0 then
+        return false
+    end
+
+    local expr, err = expr.new(logger.filter)
+    if err then
+        core.log.error("failed to validate the 'filter' expression: ", err)
+        return false
+    end
+    return expr:eval(ctx)
+end
+
+
+local function run_log_plugin(session, ctx)
+    local logger = session._route.protocol.logger
+
+    local pkg_name = "apisix.stream.plugins." .. logger.name
+    local ok, plugin = pcall(require, pkg_name)
+
+    core.ctx.set_vars_meta(ctx)
+    ctx.conf_id = tostring(logger.conf)
+    ctx.conf_type = "xrpc-logger"
+
+    if not ok then

Review Comment:
   Better to check ok just after pcall



##########
apisix/schema_def.lua:
##########
@@ -807,6 +807,26 @@ local xrpc_protocol_schema = {
             description = "protocol-specific configuration",
             type = "object",
         },
+        logger = {

Review Comment:
   Better to use array so that we can add multiple logger in the future



##########
apisix/stream/xrpc/runner.lua:
##########
@@ -70,10 +74,54 @@ local function put_req_ctx(session, ctx)
 end
 
 
+local function log_filter(session, ctx)
+    local logger = session._route.protocol.logger
+    if not logger or not logger.filter or #logger.filter == 0 then
+        return false
+    end
+
+    local expr, err = expr.new(logger.filter)
+    if err then
+        core.log.error("failed to validate the 'filter' expression: ", err)
+        return false
+    end
+    return expr:eval(ctx)
+end
+
+
+local function run_log_plugin(session, ctx)
+    local logger = session._route.protocol.logger
+
+    local pkg_name = "apisix.stream.plugins." .. logger.name
+    local ok, plugin = pcall(require, pkg_name)
+
+    core.ctx.set_vars_meta(ctx)
+    ctx.conf_id = tostring(logger.conf)
+    ctx.conf_type = "xrpc-logger"
+
+    if not ok then
+        core.log.error("failed to load plugin [", plugin, "] err: ", plugin)
+        return
+    end
+
+    local log_func = plugin.log
+    if log_func then
+        log_func(logger.conf, ctx)
+    end
+end
+
+
 local function finish_req(protocol, session, ctx)
     ctx._rpc_end_time = ngx_now()
 
-    protocol.log(session, ctx)
+    local matched = log_filter(session, ctx)
+    core.log.info("log filter result: ", matched)
+
+    if matched then
+        protocol.log(session, ctx)

Review Comment:
   We need to run `protocol.log` whether the logger is matched or not, otherwise the reuse logic in 
   https://github.com/apache/apisix/blob/2800a06f00ff9010d893eb8c4b9c94359cc3a96b/apisix/stream/xrpc/protocols/redis/init.lua#L287 won't work.



##########
apisix/stream/xrpc/runner.lua:
##########
@@ -70,10 +74,54 @@ local function put_req_ctx(session, ctx)
 end
 
 
+local function log_filter(session, ctx)
+    local logger = session._route.protocol.logger
+    if not logger or not logger.filter or #logger.filter == 0 then
+        return false
+    end
+
+    local expr, err = expr.new(logger.filter)
+    if err then
+        core.log.error("failed to validate the 'filter' expression: ", err)
+        return false
+    end
+    return expr:eval(ctx)
+end
+
+
+local function run_log_plugin(session, ctx)
+    local logger = session._route.protocol.logger
+
+    local pkg_name = "apisix.stream.plugins." .. logger.name
+    local ok, plugin = pcall(require, pkg_name)
+
+    core.ctx.set_vars_meta(ctx)
+    ctx.conf_id = tostring(logger.conf)
+    ctx.conf_type = "xrpc-logger"
+
+    if not ok then
+        core.log.error("failed to load plugin [", plugin, "] err: ", plugin)

Review Comment:
   ```suggestion
           core.log.error("failed to load plugin [", logger.name, "] err: ", plugin)
   ```



##########
apisix/stream/xrpc/runner.lua:
##########
@@ -70,10 +74,54 @@ local function put_req_ctx(session, ctx)
 end
 
 
+local function log_filter(session, ctx)

Review Comment:
   ```suggestion
   local function filter_logger(session, ctx)
   ```
   would be better?



-- 
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