You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/05/13 02:49:14 UTC

[GitHub] [apisix] spacewander commented on a diff in pull request #7036: feat(xrpc): register variable rpc_time & redis_cmd_line

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


##########
docs/en/latest/apisix-variable.md:
##########
@@ -39,5 +39,7 @@ List in alphabetical order:
 | route_name       | core    | name of `route`        |   |
 | service_id       | core    | id of `service`        |   |
 | service_name     | core    | name of `service`      |   |
+| rpc_time         | xRPC    | time spent at the rpc request level |   |
+| redis_cmd_line   | redis   | the type of Redis command |   |

Review Comment:
   ```suggestion
   | redis_cmd_line   | Redis   | the content of Redis command |   |
   ```



##########
docs/en/latest/xrpc.md:
##########
@@ -131,6 +131,52 @@ One specifies the `superior_id`, whose corresponding value is the ID of another
 
 For example, for the Dubbo RPC protocol, the subordinate route is matched based on the service_name and other parameters configured in the route and the actual service_name brought in the request. If the match is successful, the configuration above the subordinate route is used, otherwise the configuration of the superior route is still used. In the above example, if the match for route 2 is successful, it will be forwarded to upstream 2; otherwise, it will still be forwarded to upstream 1.
 
+### Log Reporting
+
+xRPC supports logging-related functions. You can use this feature to filter requests that require attention, such as high latency, excessive transfer content, etc.
+
+Each logger item configuration parameter will contain
+
+- name: the Logger plugin name,
+- filter: the prerequisites for the execution of the logger plugin(e.g., request processing time exceeding a given value),
+- conf: the configuration of the logger plugin itself.
+
+ The following configuration is an example:
+
+```json
+{
+    ...
+    "protocol": {
+        "name": "redis",
+        "logger": {
+            {
+                "name": "syslog",
+                "filter": [
+                    ["rpc_time", ">=", 10]
+                ],
+                "conf": {
+                    "host": "127.0.0.1",
+                    "port": 8125,
+                }
+            }
+        }
+    }
+}
+```
+
+This configuration means that when the `rpc_time` is greater than 10 ms, xPRC reports the request log to the log server via the `syslog` plugin. `conf` is the configuration of the logging server required by the `syslog` plugin.
+
+Unlike standard TCP proxies, which only execute a logger when the connection is broken, xRPC's executed logger at the end of each 'request'.

Review Comment:
   ```suggestion
   Unlike standard TCP proxies, which only execute a logger when the connection is closed, xRPC's executed logger at the end of each 'request'.
   ```



##########
apisix/stream/xrpc/runner.lua:
##########
@@ -27,6 +27,19 @@ local pcall = pcall
 local ipairs = ipairs
 local tostring = tostring
 
+
+core.ctx.register_var("rpc_time", function(ctx)
+    local session = ctx.xrpc_session
+    local curr_ctx_id = session._currr_ctx_id
+    local curr_ctx = session._ctxs[curr_ctx_id]

Review Comment:
   I think we can implement a special var meta for the xRPC ctx?
   A new version of https://github.com/apache/apisix/blob/4690feb421779f5b79e8dd990dc00f4d3f1052d0/apisix/core/ctx.lua#L216.
   
   Therefore we can avoid looking up current xRPC ctx in every vars.
   
   BTW, the `release_vars` is not called for xRPC ctx.
   



##########
apisix/stream/xrpc/runner.lua:
##########
@@ -27,6 +27,19 @@ local pcall = pcall
 local ipairs = ipairs
 local tostring = tostring
 
+
+core.ctx.register_var("rpc_time", function(ctx)
+    local session = ctx.xrpc_session
+    local curr_ctx_id = session._currr_ctx_id
+    local curr_ctx = session._ctxs[curr_ctx_id]
+
+    if not curr_ctx then
+        core.log.warn("can't find current context by id: ", curr_ctx_id)
+        return nil
+    end
+    return curr_ctx._rpc_end_time * 1000 - curr_ctx._rpc_start_time * 1000

Review Comment:
   Let's use second as the unit, like the `request_time`



##########
docs/en/latest/xrpc.md:
##########
@@ -131,6 +131,52 @@ One specifies the `superior_id`, whose corresponding value is the ID of another
 
 For example, for the Dubbo RPC protocol, the subordinate route is matched based on the service_name and other parameters configured in the route and the actual service_name brought in the request. If the match is successful, the configuration above the subordinate route is used, otherwise the configuration of the superior route is still used. In the above example, if the match for route 2 is successful, it will be forwarded to upstream 2; otherwise, it will still be forwarded to upstream 1.
 
+### Log Reporting
+
+xRPC supports logging-related functions. You can use this feature to filter requests that require attention, such as high latency, excessive transfer content, etc.
+
+Each logger item configuration parameter will contain
+
+- name: the Logger plugin name,
+- filter: the prerequisites for the execution of the logger plugin(e.g., request processing time exceeding a given value),
+- conf: the configuration of the logger plugin itself.
+
+ The following configuration is an example:
+
+```json
+{
+    ...
+    "protocol": {
+        "name": "redis",
+        "logger": {
+            {
+                "name": "syslog",
+                "filter": [
+                    ["rpc_time", ">=", 10]
+                ],
+                "conf": {
+                    "host": "127.0.0.1",
+                    "port": 8125,
+                }
+            }
+        }
+    }
+}
+```
+
+This configuration means that when the `rpc_time` is greater than 10 ms, xPRC reports the request log to the log server via the `syslog` plugin. `conf` is the configuration of the logging server required by the `syslog` plugin.
+
+Unlike standard TCP proxies, which only execute a logger when the connection is broken, xRPC's executed logger at the end of each 'request'.
+
+The protocol itself defines the granularity of the specific request, and the xRPC extension code implements the request's granularity.
+
+For example, in the Redis protocol, the execution of a command is considered a request.
+
+### Exposes variables

Review Comment:
   We can omit this section as it is written in docs/en/latest/apisix-variable.md



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