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/11/25 03:42:22 UTC

[GitHub] [apisix] kingluo opened a new pull request, #8400: feat: add inspect plugin

kingluo opened a new pull request, #8400:
URL: https://github.com/apache/apisix/pull/8400

   ### Description
   
   <!-- Please include a summary of the change and which issue is fixed. -->
   <!-- Please also include relevant motivation and context. -->
   
   Fixes # (issue)
   
   ### Checklist
   
   - [ ] I have explained the need for this PR and the problem it solves
   - [ ] I have explained the changes or the new features added to this PR
   - [ ] I have added tests corresponding to this change
   - [ ] I have updated the documentation to reflect this change
   - [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
   
   <!--
   
   Note
   
   1. Mark the PR as draft until it's ready to be reviewed.
   2. Always add/update tests for any changes unless you have a good reason.
   3. Always update the documentation to reflect the changes made in the PR.
   4. Make a new commit to resolve conversations instead of `push -f`.
   5. To resolve merge conflicts, merge master instead of rebasing.
   6. Use "request review" to notify the reviewer after making changes.
   7. Only a reviewer can mark a conversation as resolved.
   
   -->
   


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


[GitHub] [apisix] moonming commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
moonming commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1032126686


##########
t/plugin/inspect.t:
##########
@@ -0,0 +1,98 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('warn');
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!defined $block->request) {
+        $block->set_value("request", "GET /t");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: simple hook in route

Review Comment:
   only one test case?



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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1049348420


##########
docs/en/latest/plugins/inspect.md:
##########
@@ -0,0 +1,171 @@
+---
+title: inspect
+keywords:
+  - APISIX
+  - Plugin
+  - Inspect
+  - Dynamic Lua Debugging
+description: This document contains information about the Apache APISIX inspect Plugin.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+## Description
+
+It's useful to set arbitrary breakpoint in any Lua file to inspect the context information,
+e.g. print local variables if some condition satisfied.
+
+In this way, you don't need to modify the source code of your project, and just get diagnose information
+on demand, i.e. dynamic logging.
+
+This plugin supports setting breakpoints within both interpretd function and jit compiled function.
+The breakpoint could be at any position within the function. The function could be global/local/module/ananymous.
+
+## Features
+
+* Set breakpoint at any position
+* Dynamic breakpoint
+* customized breakpoint handler
+* You could define one-shot breakpoint
+* Work for jit compiled function
+* If function reference specified, then performance impact is only bound to that function (JIT compiled code will not trigger debug hook, so they would run fast even if hook is enabled)
+* If all breakpoints deleted, jit could recover
+
+## Operation Graph
+
+![Operation Graph](https://raw.githubusercontent.com/apache/apisix/master/docs/assets/images/plugin/inspect.png)
+
+## API to define hook in hooks file
+
+### require("resty.inspect.dbg").set_hook(file, line, func, filter_func)
+
+The breakpoint is specified by `file` (full qualified or short file name) and the `line` number.
+
+The `func` specified the scope (which function or global) of jit cache to flush:
+
+* If the breakpoint is related to a module function or
+global function, you should set it that function reference, then only the jit cache of that function would
+be flushed, and it would not affect other caches to avoid slowing down other parts of the program.
+
+* If the breakpointis related to local function or anonymous function,
+then you have to set it to `nil` (because no way to get function reference), which would flush the whole jit cache of Lua vm.
+
+You attach a `filter_func` function of the breakpoint, the function takes the `info` as argument and returns
+true of false to determine whether the breakpoint would be removed. You could setup one-shot breakpoint
+at ease.
+
+The `info` is a hash table which contains below keys:
+
+* `finfo`: `debug.getinfo(level, "nSlf")`
+* `uv`: upvalues hash table
+* `vals`: local variables hash table
+
+## Attributes
+
+| Name               | Type    | Required | Default | Description                                                                                    |
+|--------------------|---------|----------|---------|------------------------------------------------------------------------------------------------|
+| delay           | integer | False     | 3 | Time in seconds specifying how often to check the hooks file.                                       |
+| hooks_file           | string | False     | "/var/run/apisix_inspect_hooks.lua"  | Lua file to define hooks, which could be a link file. |
+
+## Enabling the Plugin
+
+Plugin is enabled by default (`conf/config-default.yaml`):
+
+```yaml title="conf/config-default.yaml"
+plugins:
+    - inspect
+
+plugin_attr:
+  inspect:
+    delay: 3
+    hooks_file: "/var/run/apisix_inspect_hooks.lua"
+```
+
+## Example usage
+
+```bash
+# create test route
+curl http://127.0.0.1:9180/apisix/admin/routes/test_limit_req -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "methods": ["GET"],
+    "uri": "/get",
+    "plugins": {
+        "limit-req": {
+            "rate": 100,
+            "burst": 0,
+            "rejected_code": 503,
+            "key_type": "var",
+            "key": "remote_addr"
+        }
+    },
+    "upstream": {
+        "type": "roundrobin",
+        "nodes": {
+            "httpbin.org": 1
+        }
+    }
+}'
+
+# create a hooks file to set a test breakpoint
+# Note that the breakpoint is associated with the line number,
+# so if the Lua code changes, you need to adjust the line number in the hooks file
+cat <<EOF >/tmp/hooks.lua

Review Comment:
   ok, I would change it.



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


[GitHub] [apisix] membphis commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
membphis commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1049135524


##########
docs/en/latest/plugins/inspect.md:
##########
@@ -0,0 +1,171 @@
+---
+title: inspect
+keywords:
+  - APISIX
+  - Plugin
+  - inspect
+  - inspect

Review Comment:
   we should more this line



##########
docs/en/latest/plugins/inspect.md:
##########
@@ -0,0 +1,171 @@
+---
+title: inspect
+keywords:
+  - APISIX
+  - Plugin
+  - inspect
+  - inspect
+description: This document contains information about the Apache APISIX inspect Plugin.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+## Description
+
+It's useful to set arbitrary breakpoint in any lua file to inspect the context information,

Review Comment:
   `lua` -> `Lua`



##########
docs/en/latest/plugins/inspect.md:
##########
@@ -0,0 +1,171 @@
+---
+title: inspect
+keywords:
+  - APISIX
+  - Plugin
+  - inspect
+  - inspect
+description: This document contains information about the Apache APISIX inspect Plugin.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+## Description
+
+It's useful to set arbitrary breakpoint in any lua file to inspect the context information,
+e.g. print local variables if some condition satisfied.
+
+In this way, you don't need to modify the source code of your project, and just get diagnose information
+on demand, i.e. dynamic logging.
+
+This plugin supports setting breakpoints within both interpretd function and jit compiled function.
+The breakpoint could be at any position within the function. The function could be global/local/module/ananymous.
+
+## Features
+
+* set breakpoint at any position
+* dynamic breakpoint

Review Comment:
   ditto



##########
docs/en/latest/plugins/inspect.md:
##########
@@ -0,0 +1,171 @@
+---
+title: inspect
+keywords:
+  - APISIX
+  - Plugin
+  - inspect
+  - inspect
+description: This document contains information about the Apache APISIX inspect Plugin.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+## Description
+
+It's useful to set arbitrary breakpoint in any lua file to inspect the context information,
+e.g. print local variables if some condition satisfied.
+
+In this way, you don't need to modify the source code of your project, and just get diagnose information
+on demand, i.e. dynamic logging.
+
+This plugin supports setting breakpoints within both interpretd function and jit compiled function.
+The breakpoint could be at any position within the function. The function could be global/local/module/ananymous.
+
+## Features
+
+* set breakpoint at any position

Review Comment:
   Capitalized?



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


[GitHub] [apisix] spacewander commented on pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
spacewander commented on PR #8400:
URL: https://github.com/apache/apisix/pull/8400#issuecomment-1347818767

   > @spacewander Add nil case handling already. GitHub failed to add an outdated flag here.
   > 
   > [kingluo/apisix@`f4fe75f`/apisix/inspect/init.lua#L46-L50](https://github.com/kingluo/apisix/blob/f4fe75f0f3855883b46a61d078018a4396639f13/apisix/inspect/init.lua#L46-L50)
   
   I see. Thanks for your clarification.


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


[GitHub] [apisix] spacewander commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1044124774


##########
apisix/inspect/dbg.lua:
##########
@@ -0,0 +1,153 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local string_format = string.format
+local debug = debug
+local ipairs = ipairs
+local pcall = pcall
+local table_insert = table.insert
+local jit = jit
+
+local _M = {}
+
+local hooks = {}
+
+function _M.getname(n)
+    if n.what == "C" then
+        return n.name
+    end
+    local lc = string_format("%s:%d", n.short_src, n.currentline)
+    if n.what ~= "main" and n.namewhat ~= "" then
+        return string_format("%s (%s)", lc, n.name)
+    else
+        return lc
+    end
+end
+
+local function hook(evt, arg)
+    local level = 2
+    local finfo = debug.getinfo(level, "nSlf")
+    local key = finfo.source .. "#" .. arg
+
+    local hooks2 = {}
+    for _, hook in ipairs(hooks) do
+        if key:sub(-#hook.key) == hook.key then
+            local filter_func = hook.filter_func
+            local info = {finfo = finfo, uv = {}, vals = {}}
+
+            -- upvalues
+            local i = 1
+            while true do
+                local name, value = debug.getupvalue(finfo.func, i)
+                if name == nil then break end
+                if name:sub(1, 1) ~= "(" then
+                    info.uv[name] = value
+                end
+                i = i + 1
+            end
+
+            -- local values
+            local i = 1
+            while true do
+                local name, value = debug.getlocal(level, i)
+                if not name then break end
+                if name:sub(1, 1) ~= "(" then
+                    info.vals[name] = value
+                end
+                i = i + 1
+            end
+
+            local r1, r2_or_err = pcall(filter_func, info)
+            if not r1 then
+                core.log.error("inspect: pcall filter_func:", r2_or_err)

Review Comment:
   We can use `continue` to goto next loop, so we can skip the `r1 and ` in the next block when `not r1` is true.



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


[GitHub] [apisix] spacewander commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1045448865


##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,134 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")
+local lfs = require("lfs")
+local cjson = require("cjson")
+local io = io
+local table_insert = table.insert
+local pcall = pcall
+local ipairs = ipairs
+local os = os
+local ngx = ngx
+local loadstring = loadstring
+
+local _M = {}
+
+local last_modified = 0
+
+local stop = false
+
+local running = false
+
+local last_report_time = 0
+
+local REPORT_INTERVAL = 30 -- secs
+
+local function is_file_exists(file)
+   local f = io.open(file, "r")
+   if f then
+       io.close(f)
+       return true
+   else
+       return false
+   end
+end
+
+local function run_lua_file(file)
+    local f, err = io.open(file, "rb")
+    if not f then
+        return false, err
+    end
+    local code = f:read("*all")

Review Comment:
   What about your opinion on this?



##########
Makefile:
##########
@@ -315,6 +315,9 @@ install: runtime
 	$(ENV_INSTALL) -d $(ENV_INST_LUADIR)/apisix/plugins/ip-restriction
 	$(ENV_INSTALL) apisix/plugins/ip-restriction/*.lua $(ENV_INST_LUADIR)/apisix/plugins/ip-restriction/
 
+	$(ENV_INSTALL) -d $(ENV_INST_LUADIR)/apisix/inspect
+	$(ENV_INSTALL) apisix/inspect/*.lua $(ENV_INST_LUADIR)/apisix/inspect/

Review Comment:
   Should be moved under `$(ENV_INSTALL) apisix/include/`



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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1043239482


##########
apisix/plugins/inspect.lua:
##########
@@ -0,0 +1,62 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local plugin = require("apisix.plugin")
+local inspect = require("apisix.inspect")
+local ngx = ngx
+
+
+local plugin_name = "inspect"
+
+
+local schema = {
+    type = "object",
+    properties = {},
+}
+
+
+local _M = {
+    version = 0.1,
+    priority = 200,
+    name = plugin_name,
+    schema = schema,
+}
+
+
+function _M.check_schema(conf, schema_type)
+    return core.schema.check(schema, conf)
+end
+
+
+function _M.init()
+    local attr = plugin.plugin_attr(plugin_name)

Review Comment:
   This plugin is special: it's global and not associated with routes. It needs to load the config and run in the `init()` method, which is called when the plugin first loaded.
   Loading plugins happens before plugin_metadata initialization, so I think it's not suitable to use metadata for this plugin.
   
   https://github.com/apache/apisix/blob/4694685c60f1c4db57c9d3920b90567ae9e32e5c/apisix/plugin.lua#L746-L757



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


[GitHub] [apisix] kingluo commented on pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on PR #8400:
URL: https://github.com/apache/apisix/pull/8400#issuecomment-1352720938

   @moonming `/var/run/apisix_inspect_hooks.lua` is only writeable for the root user. So I think no risk here. But it's up to you. Do you want to change it to `/usr/local/openresty/apisix_inspect_hooks.lua`?


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


[GitHub] [apisix] moonming commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
moonming commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1049149692


##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,128 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")

Review Comment:
   What‘s `dbg` means?



##########
conf/config-default.yaml:
##########
@@ -555,6 +556,9 @@ plugin_attr:
       send: 60s
 #  redirect:
 #    https_port: 8443   # the default port for use by HTTP redirects to HTTPS
+  inspect:
+    delay: 3

Review Comment:
   3s or 3 mins?



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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1049331347


##########
conf/config-default.yaml:
##########
@@ -555,6 +556,9 @@ plugin_attr:
       send: 60s
 #  redirect:
 #    https_port: 8443   # the default port for use by HTTP redirects to HTTPS
+  inspect:
+    delay: 3            # in seconds
+    hooks_file: "/var/run/apisix_inspect_hooks.lua"

Review Comment:
   yes, this hooks file is used to define breakpoints, which itself is lua file. When the administrator needs to set breakpoints to inspect something, he would edit this file, or link another file to this path.



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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1032169399


##########
t/plugin/inspect.t:
##########
@@ -0,0 +1,98 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+log_level('warn');
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+    if (!defined $block->request) {
+        $block->set_value("request", "GET /t");
+    }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: simple hook in route

Review Comment:
   Not finished yet.



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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1043239482


##########
apisix/plugins/inspect.lua:
##########
@@ -0,0 +1,62 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local plugin = require("apisix.plugin")
+local inspect = require("apisix.inspect")
+local ngx = ngx
+
+
+local plugin_name = "inspect"
+
+
+local schema = {
+    type = "object",
+    properties = {},
+}
+
+
+local _M = {
+    version = 0.1,
+    priority = 200,
+    name = plugin_name,
+    schema = schema,
+}
+
+
+function _M.check_schema(conf, schema_type)
+    return core.schema.check(schema, conf)
+end
+
+
+function _M.init()
+    local attr = plugin.plugin_attr(plugin_name)

Review Comment:
   This plugin is special: it's global and not associated with routes. It needs to load the config and run in the `init()` method, which is called when the plugin first loaded.
   Loading plugins happens before plugin_metadata initialization, so I think it's not suitable to use metadata for this plugin. And also, this plugin does not need to change the config at runtime.
   
   https://github.com/apache/apisix/blob/4694685c60f1c4db57c9d3920b90567ae9e32e5c/apisix/plugin.lua#L746-L757



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


[GitHub] [apisix] tzssangglass commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1045643280


##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,127 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")
+local lfs = require("lfs")
+local pl_path = require("pl.path")
+local io = io
+local table_insert = table.insert
+local pcall = pcall
+local ipairs = ipairs
+local os = os
+local ngx = ngx
+local loadstring = loadstring
+
+local _M = {}
+
+local last_modified = 0
+
+local stop = false
+
+local running = false
+
+local last_report_time = 0
+
+local REPORT_INTERVAL = 30 -- secs
+
+local function run_lua_file(file)
+    local f, err = io.open(file, "rb")
+    if not f then
+        return false, err
+    end
+    local code = f:read("*all")
+    f:close()
+    if code == nil then
+        return false, "cannot read hooks file"
+    end
+    local func, err = loadstring(code)
+    if not func then
+        return false, err
+    end
+    func()

Review Comment:
   do we need use `pcall(func())` here?



##########
apisix/inspect/dbg.lua:
##########
@@ -0,0 +1,151 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local string_format = string.format
+local debug = debug
+local ipairs = ipairs
+local pcall = pcall
+local table_insert = table.insert
+local jit = jit
+
+local _M = {}
+
+local hooks = {}
+
+function _M.getname(n)
+    if n.what == "C" then
+        return n.name
+    end
+    local lc = string_format("%s:%d", n.short_src, n.currentline)
+    if n.what ~= "main" and n.namewhat ~= "" then
+        return string_format("%s (%s)", lc, n.name)
+    else
+        return lc
+    end
+end
+
+local function hook(evt, arg)

Review Comment:
   `evt` unused in `hook` function?



##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,127 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")
+local lfs = require("lfs")
+local pl_path = require("pl.path")
+local io = io
+local table_insert = table.insert
+local pcall = pcall
+local ipairs = ipairs
+local os = os
+local ngx = ngx
+local loadstring = loadstring
+
+local _M = {}
+
+local last_modified = 0
+
+local stop = false
+
+local running = false
+
+local last_report_time = 0
+
+local REPORT_INTERVAL = 30 -- secs
+
+local function run_lua_file(file)
+    local f, err = io.open(file, "rb")
+    if not f then
+        return false, err
+    end
+    local code = f:read("*all")
+    f:close()
+    if code == nil then
+        return false, "cannot read hooks file"
+    end
+    local func, err = loadstring(code)
+    if not func then
+        return false, err
+    end
+    func()

Review Comment:
   do we need to use `pcall(func())` here?



##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,127 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")
+local lfs = require("lfs")
+local pl_path = require("pl.path")
+local io = io
+local table_insert = table.insert
+local pcall = pcall
+local ipairs = ipairs
+local os = os
+local ngx = ngx
+local loadstring = loadstring
+
+local _M = {}
+
+local last_modified = 0
+
+local stop = false
+
+local running = false
+
+local last_report_time = 0
+
+local REPORT_INTERVAL = 30 -- secs
+
+local function run_lua_file(file)
+    local f, err = io.open(file, "rb")
+    if not f then
+        return false, err
+    end
+    local code = f:read("*all")
+    f:close()
+    if code == nil then
+        return false, "cannot read hooks file"
+    end
+    local func, err = loadstring(code)
+    if not func then
+        return false, err
+    end
+    func()
+    return true
+end
+
+local function setup_hooks(file)
+    if pl_path.exists(file) then
+        dbg.unset_all()
+        local _, err = pcall(run_lua_file, file)
+        local hooks = {}
+        for _, hook in ipairs(dbg.hooks()) do
+            table_insert(hooks, hook.key)
+        end
+        core.log.info("set hooks: err=", err, ", hooks=", core.json.encode(hooks))

Review Comment:
   ```suggestion
           core.log.info("set hooks: err: ", err, ", hooks: ", core.json.delay_encode(hooks))
   ```
   
   is better?



##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,127 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")
+local lfs = require("lfs")
+local pl_path = require("pl.path")
+local io = io
+local table_insert = table.insert
+local pcall = pcall
+local ipairs = ipairs
+local os = os
+local ngx = ngx
+local loadstring = loadstring
+
+local _M = {}
+
+local last_modified = 0
+
+local stop = false
+
+local running = false
+
+local last_report_time = 0
+
+local REPORT_INTERVAL = 30 -- secs
+
+local function run_lua_file(file)
+    local f, err = io.open(file, "rb")
+    if not f then
+        return false, err
+    end
+    local code = f:read("*all")
+    f:close()
+    if code == nil then
+        return false, "cannot read hooks file"
+    end
+    local func, err = loadstring(code)
+    if not func then
+        return false, err
+    end
+    func()
+    return true
+end
+
+local function setup_hooks(file)
+    if pl_path.exists(file) then
+        dbg.unset_all()
+        local _, err = pcall(run_lua_file, file)
+        local hooks = {}
+        for _, hook in ipairs(dbg.hooks()) do
+            table_insert(hooks, hook.key)
+        end
+        core.log.info("set hooks: err=", err, ", hooks=", core.json.encode(hooks))

Review Comment:
   ```suggestion
           core.log.info("set hooks: err: ", err, ", hooks: ", core.json.delay_encode(hooks))
   ```
   
   is 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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1049331347


##########
conf/config-default.yaml:
##########
@@ -555,6 +556,9 @@ plugin_attr:
       send: 60s
 #  redirect:
 #    https_port: 8443   # the default port for use by HTTP redirects to HTTPS
+  inspect:
+    delay: 3            # in seconds
+    hooks_file: "/var/run/apisix_inspect_hooks.lua"

Review Comment:
   @moonming yes, this hooks file is used to define breakpoints, which itself is lua file. When the administrator needs to set breakpoints to inspect something, he would edit this file, or link another file to this path.



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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1046770907


##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,127 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")
+local lfs = require("lfs")
+local pl_path = require("pl.path")
+local io = io
+local table_insert = table.insert
+local pcall = pcall
+local ipairs = ipairs
+local os = os
+local ngx = ngx
+local loadstring = loadstring
+
+local _M = {}
+
+local last_modified = 0
+
+local stop = false
+
+local running = false
+
+local last_report_time = 0
+
+local REPORT_INTERVAL = 30 -- secs
+
+local function run_lua_file(file)
+    local f, err = io.open(file, "rb")
+    if not f then
+        return false, err
+    end
+    local code = f:read("*all")
+    f:close()
+    if code == nil then
+        return false, "cannot read hooks file"
+    end
+    local func, err = loadstring(code)
+    if not func then
+        return false, err
+    end
+    func()

Review Comment:
   Already did so in the outer function.



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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1049152166


##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,128 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")

Review Comment:
   Abbreviation for "debug". Also to avoid conflict with the built-in Lua debug module.



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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1044168450


##########
apisix/inspect/dbg.lua:
##########
@@ -0,0 +1,153 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local string_format = string.format
+local debug = debug
+local ipairs = ipairs
+local pcall = pcall
+local table_insert = table.insert
+local jit = jit
+
+local _M = {}
+
+local hooks = {}
+
+function _M.getname(n)
+    if n.what == "C" then
+        return n.name
+    end
+    local lc = string_format("%s:%d", n.short_src, n.currentline)
+    if n.what ~= "main" and n.namewhat ~= "" then
+        return string_format("%s (%s)", lc, n.name)
+    else
+        return lc
+    end
+end
+
+local function hook(evt, arg)
+    local level = 2
+    local finfo = debug.getinfo(level, "nSlf")
+    local key = finfo.source .. "#" .. arg
+
+    local hooks2 = {}
+    for _, hook in ipairs(hooks) do
+        if key:sub(-#hook.key) == hook.key then
+            local filter_func = hook.filter_func
+            local info = {finfo = finfo, uv = {}, vals = {}}
+
+            -- upvalues
+            local i = 1
+            while true do
+                local name, value = debug.getupvalue(finfo.func, i)
+                if name == nil then break end
+                if name:sub(1, 1) ~= "(" then
+                    info.uv[name] = value
+                end
+                i = i + 1
+            end
+
+            -- local values
+            local i = 1
+            while true do
+                local name, value = debug.getlocal(level, i)
+                if not name then break end
+                if name:sub(1, 1) ~= "(" then
+                    info.vals[name] = value
+                end
+                i = i + 1
+            end
+
+            local r1, r2_or_err = pcall(filter_func, info)
+            if not r1 then
+                core.log.error("inspect: pcall filter_func:", r2_or_err)

Review Comment:
   I replace the next statement with `elseif`.



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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1043062831


##########
apisix/inspect/dbg.lua:
##########
@@ -0,0 +1,153 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local string_format = string.format
+local debug = debug
+local ipairs = ipairs
+local pcall = pcall
+local table_insert = table.insert
+local jit = jit
+
+local _M = {}
+
+local hooks = {}
+
+function _M.getname(n)
+    if n.what == "C" then
+        return n.name
+    end
+    local lc = string_format("%s:%d", n.short_src, n.currentline)
+    if n.what ~= "main" and n.namewhat ~= "" then
+        return string_format("%s (%s)", lc, n.name)
+    else
+        return lc
+    end
+end
+
+local function hook(evt, arg)
+    local level = 2
+    local finfo = debug.getinfo(level, "nSlf")
+    local key = finfo.source .. "#" .. arg
+
+    local hooks2 = {}
+    for _, hook in ipairs(hooks) do
+        if key:sub(-#hook.key) == hook.key then
+            local filter_func = hook.filter_func
+            local info = {finfo = finfo, uv = {}, vals = {}}
+
+            -- upvalues
+            local i = 1
+            while true do
+                local name, value = debug.getupvalue(finfo.func, i)
+                if name == nil then break end
+                if name:sub(1, 1) ~= "(" then
+                    info.uv[name] = value
+                end
+                i = i + 1
+            end
+
+            -- local values
+            local i = 1
+            while true do
+                local name, value = debug.getlocal(level, i)
+                if not name then break end
+                if name:sub(1, 1) ~= "(" then
+                    info.vals[name] = value
+                end
+                i = i + 1
+            end
+
+            local r1, r2_or_err = pcall(filter_func, info)
+            if not r1 then
+                core.log.error("inspect: pcall filter_func:", r2_or_err)

Review Comment:
   What do you mean? Here is just error logging, after that, it needs to process the following statements.



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


[GitHub] [apisix] spacewander commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1042950949


##########
Makefile:
##########
@@ -294,6 +294,9 @@ install: runtime
 	$(ENV_INSTALL) -d $(ENV_INST_LUADIR)/apisix/http
 	$(ENV_INSTALL) apisix/http/*.lua $(ENV_INST_LUADIR)/apisix/http/
 
+	$(ENV_INSTALL) -d $(ENV_INST_LUADIR)/apisix/inspect
+	$(ENV_INSTALL) apisix/inspect/*.lua $(ENV_INST_LUADIR)/apisix/inspect/

Review Comment:
   Please add the directory in alphabetical order



##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,134 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")
+local lfs = require("lfs")
+local cjson = require("cjson")
+local io = io
+local table_insert = table.insert
+local pcall = pcall
+local ipairs = ipairs
+local os = os
+local ngx = ngx
+local loadstring = loadstring
+
+local _M = {}
+
+local last_modified = 0
+
+local stop = false
+
+local running = false
+
+local last_report_time = 0
+
+local REPORT_INTERVAL = 30 -- secs
+
+local function is_file_exists(file)
+   local f = io.open(file, "r")
+   if f then
+       io.close(f)
+       return true
+   else
+       return false
+   end
+end
+
+local function run_lua_file(file)
+    local f, err = io.open(file, "rb")
+    if not f then
+        return false, err
+    end
+    local code = f:read("*all")
+    f:close()
+    local func, err = loadstring(code)
+    if not func then
+        return false, err
+    end
+    func()
+    return true
+end
+
+local function setup_hooks(file)
+    if is_file_exists(file) then
+        dbg.unset_all()
+        local _, err = pcall(run_lua_file, file)
+        local hooks = {}
+        for _, hook in ipairs(dbg.hooks()) do
+            table_insert(hooks, hook.key)
+        end
+        core.log.info("set hooks: err=", err, ", hooks=", cjson.encode(hooks))
+    end
+end
+
+local function reload_hooks(premature, delay, file)
+    if premature or stop then
+        stop = false
+        running = false
+        return
+    end
+
+    local time, err = lfs.attributes(file, 'modification')
+    if err then
+        if last_modified ~= 0 then
+            core.log.info(err, ", disable all hooks")
+            dbg.unset_all()
+            last_modified = 0
+        end
+    elseif time ~= last_modified then
+        setup_hooks(file)
+        last_modified = time
+    else
+        local ts = os.time()
+        if ts - last_report_time >= REPORT_INTERVAL then
+            local hooks = {}
+            for _, hook in ipairs(dbg.hooks()) do
+                table_insert(hooks, hook.key)
+            end
+            core.log.info("alive hooks: ", cjson.encode(hooks))

Review Comment:
   We can use core.json?



##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,134 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")
+local lfs = require("lfs")
+local cjson = require("cjson")
+local io = io
+local table_insert = table.insert
+local pcall = pcall
+local ipairs = ipairs
+local os = os
+local ngx = ngx
+local loadstring = loadstring
+
+local _M = {}
+
+local last_modified = 0
+
+local stop = false
+
+local running = false
+
+local last_report_time = 0
+
+local REPORT_INTERVAL = 30 -- secs
+
+local function is_file_exists(file)
+   local f = io.open(file, "r")
+   if f then
+       io.close(f)
+       return true
+   else
+       return false
+   end
+end
+
+local function run_lua_file(file)
+    local f, err = io.open(file, "rb")
+    if not f then
+        return false, err
+    end
+    local code = f:read("*all")

Review Comment:
   Let's do some err check?



##########
apisix/inspect/dbg.lua:
##########
@@ -0,0 +1,153 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local string_format = string.format
+local debug = debug
+local ipairs = ipairs
+local pcall = pcall
+local table_insert = table.insert
+local jit = jit
+
+local _M = {}
+
+local hooks = {}
+
+function _M.getname(n)
+    if n.what == "C" then
+        return n.name
+    end
+    local lc = string_format("%s:%d", n.short_src, n.currentline)
+    if n.what ~= "main" and n.namewhat ~= "" then
+        return string_format("%s (%s)", lc, n.name)
+    else
+        return lc
+    end
+end
+
+local function hook(evt, arg)
+    local level = 2
+    local finfo = debug.getinfo(level, "nSlf")
+    local key = finfo.source .. "#" .. arg
+
+    local hooks2 = {}
+    for _, hook in ipairs(hooks) do
+        if key:sub(-#hook.key) == hook.key then
+            local filter_func = hook.filter_func
+            local info = {finfo = finfo, uv = {}, vals = {}}
+
+            -- upvalues
+            local i = 1
+            while true do
+                local name, value = debug.getupvalue(finfo.func, i)
+                if name == nil then break end
+                if name:sub(1, 1) ~= "(" then
+                    info.uv[name] = value
+                end
+                i = i + 1
+            end
+
+            -- local values
+            local i = 1
+            while true do
+                local name, value = debug.getlocal(level, i)
+                if not name then break end
+                if name:sub(1, 1) ~= "(" then
+                    info.vals[name] = value
+                end
+                i = i + 1
+            end
+
+            local r1, r2_or_err = pcall(filter_func, info)
+            if not r1 then
+                core.log.error("inspect: pcall filter_func:", r2_or_err)

Review Comment:
   We can put a goto continue here?



##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,134 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")
+local lfs = require("lfs")
+local cjson = require("cjson")
+local io = io
+local table_insert = table.insert
+local pcall = pcall
+local ipairs = ipairs
+local os = os
+local ngx = ngx
+local loadstring = loadstring
+
+local _M = {}
+
+local last_modified = 0
+
+local stop = false
+
+local running = false
+
+local last_report_time = 0
+
+local REPORT_INTERVAL = 30 -- secs
+
+local function is_file_exists(file)

Review Comment:
   We can use common library, like https://github.com/apache/apisix/blob/5822eca9a28ff7b92dc7fc0431281eb62dd60ba8/apisix/cli/ops.lua#L474



##########
apisix/plugins/inspect.lua:
##########
@@ -0,0 +1,62 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local plugin = require("apisix.plugin")
+local inspect = require("apisix.inspect")
+local ngx = ngx
+
+
+local plugin_name = "inspect"
+
+
+local schema = {
+    type = "object",
+    properties = {},
+}
+
+
+local _M = {
+    version = 0.1,
+    priority = 200,
+    name = plugin_name,
+    schema = schema,
+}
+
+
+function _M.check_schema(conf, schema_type)
+    return core.schema.check(schema, conf)
+end
+
+
+function _M.init()
+    local attr = plugin.plugin_attr(plugin_name)

Review Comment:
   As @tokers asked, could you use plugin metadata instead?



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


[GitHub] [apisix] moonming commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
moonming commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1049324883


##########
conf/config-default.yaml:
##########
@@ -555,6 +556,9 @@ plugin_attr:
       send: 60s
 #  redirect:
 #    https_port: 8443   # the default port for use by HTTP redirects to HTTPS
+  inspect:
+    delay: 3            # in seconds
+    hooks_file: "/var/run/apisix_inspect_hooks.lua"

Review Comment:
   What is this file for? Has Lua code in it?



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


[GitHub] [apisix] moonming commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
moonming commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1049341238


##########
docs/en/latest/plugins/inspect.md:
##########
@@ -0,0 +1,171 @@
+---
+title: inspect
+keywords:
+  - APISIX
+  - Plugin
+  - Inspect
+  - Dynamic Lua Debugging
+description: This document contains information about the Apache APISIX inspect Plugin.
+---
+
+<!--
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+-->
+
+## Description
+
+It's useful to set arbitrary breakpoint in any Lua file to inspect the context information,
+e.g. print local variables if some condition satisfied.
+
+In this way, you don't need to modify the source code of your project, and just get diagnose information
+on demand, i.e. dynamic logging.
+
+This plugin supports setting breakpoints within both interpretd function and jit compiled function.
+The breakpoint could be at any position within the function. The function could be global/local/module/ananymous.
+
+## Features
+
+* Set breakpoint at any position
+* Dynamic breakpoint
+* customized breakpoint handler
+* You could define one-shot breakpoint
+* Work for jit compiled function
+* If function reference specified, then performance impact is only bound to that function (JIT compiled code will not trigger debug hook, so they would run fast even if hook is enabled)
+* If all breakpoints deleted, jit could recover
+
+## Operation Graph
+
+![Operation Graph](https://raw.githubusercontent.com/apache/apisix/master/docs/assets/images/plugin/inspect.png)
+
+## API to define hook in hooks file
+
+### require("resty.inspect.dbg").set_hook(file, line, func, filter_func)
+
+The breakpoint is specified by `file` (full qualified or short file name) and the `line` number.
+
+The `func` specified the scope (which function or global) of jit cache to flush:
+
+* If the breakpoint is related to a module function or
+global function, you should set it that function reference, then only the jit cache of that function would
+be flushed, and it would not affect other caches to avoid slowing down other parts of the program.
+
+* If the breakpointis related to local function or anonymous function,
+then you have to set it to `nil` (because no way to get function reference), which would flush the whole jit cache of Lua vm.
+
+You attach a `filter_func` function of the breakpoint, the function takes the `info` as argument and returns
+true of false to determine whether the breakpoint would be removed. You could setup one-shot breakpoint
+at ease.
+
+The `info` is a hash table which contains below keys:
+
+* `finfo`: `debug.getinfo(level, "nSlf")`
+* `uv`: upvalues hash table
+* `vals`: local variables hash table
+
+## Attributes
+
+| Name               | Type    | Required | Default | Description                                                                                    |
+|--------------------|---------|----------|---------|------------------------------------------------------------------------------------------------|
+| delay           | integer | False     | 3 | Time in seconds specifying how often to check the hooks file.                                       |
+| hooks_file           | string | False     | "/var/run/apisix_inspect_hooks.lua"  | Lua file to define hooks, which could be a link file. |
+
+## Enabling the Plugin
+
+Plugin is enabled by default (`conf/config-default.yaml`):
+
+```yaml title="conf/config-default.yaml"
+plugins:
+    - inspect
+
+plugin_attr:
+  inspect:
+    delay: 3
+    hooks_file: "/var/run/apisix_inspect_hooks.lua"
+```
+
+## Example usage
+
+```bash
+# create test route
+curl http://127.0.0.1:9180/apisix/admin/routes/test_limit_req -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
+{
+    "methods": ["GET"],
+    "uri": "/get",
+    "plugins": {
+        "limit-req": {
+            "rate": 100,
+            "burst": 0,
+            "rejected_code": 503,
+            "key_type": "var",
+            "key": "remote_addr"
+        }
+    },
+    "upstream": {
+        "type": "roundrobin",
+        "nodes": {
+            "httpbin.org": 1
+        }
+    }
+}'
+
+# create a hooks file to set a test breakpoint
+# Note that the breakpoint is associated with the line number,
+# so if the Lua code changes, you need to adjust the line number in the hooks file
+cat <<EOF >/tmp/hooks.lua

Review Comment:
   Do not use the /tmp directory, especially for documentation



##########
conf/config-default.yaml:
##########
@@ -555,6 +556,9 @@ plugin_attr:
       send: 60s
 #  redirect:
 #    https_port: 8443   # the default port for use by HTTP redirects to HTTPS
+  inspect:
+    delay: 3            # in seconds
+    hooks_file: "/var/run/apisix_inspect_hooks.lua"

Review Comment:
   There are security risks.
   This Lua code file must be in the APISIX directory, and users who do not have write permissions to the APISIX directory cannot write or modify this file.



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


[GitHub] [apisix] kingluo commented on pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on PR #8400:
URL: https://github.com/apache/apisix/pull/8400#issuecomment-1346045097

   @spacewander Add nil case handling already. GitHub failed to add an outdated flag here.
   
   https://github.com/kingluo/apisix/blob/f4fe75f0f3855883b46a61d078018a4396639f13/apisix/inspect/init.lua#L46-L50


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


[GitHub] [apisix] soulbird commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
soulbird commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1043010044


##########
apisix/plugins/inspect.lua:
##########
@@ -0,0 +1,62 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local plugin = require("apisix.plugin")
+local inspect = require("apisix.inspect")
+local ngx = ngx
+
+
+local plugin_name = "inspect"
+
+
+local schema = {
+    type = "object",
+    properties = {},
+}
+
+
+local _M = {
+    version = 0.1,
+    priority = 200,
+    name = plugin_name,
+    schema = schema,
+}
+
+
+function _M.check_schema(conf, schema_type)
+    return core.schema.check(schema, conf)
+end
+
+
+function _M.init()
+    local attr = plugin.plugin_attr(plugin_name)
+    local delay
+    local hooks_file
+    if attr then
+        delay = attr.delay
+        hooks_file = attr.hooks_file
+    end
+    ngx.log(ngx.INFO, "delay=", delay, ", hooks_file=", hooks_file)

Review Comment:
   core.log.info



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


[GitHub] [apisix] spacewander commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1046712586


##########
apisix/inspect/init.lua:
##########
@@ -0,0 +1,127 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local dbg = require("apisix.inspect.dbg")
+local lfs = require("lfs")
+local pl_path = require("pl.path")
+local io = io
+local table_insert = table.insert
+local pcall = pcall
+local ipairs = ipairs
+local os = os
+local ngx = ngx
+local loadstring = loadstring
+
+local _M = {}
+
+local last_modified = 0
+
+local stop = false
+
+local running = false
+
+local last_report_time = 0
+
+local REPORT_INTERVAL = 30 -- secs
+
+local function run_lua_file(file)
+    local f, err = io.open(file, "rb")
+    if not f then
+        return false, err
+    end
+    local code = f:read("*all")
+    f:close()
+    if code == nil then
+        return false, "cannot read hooks file"

Review Comment:
   We can improve the error message here by adding the `err` returned from the `f:read` above.
   
   https://github.com/LuaJIT/LuaJIT/blob/8625eee71f16a3a780ec92bc303c17456efc7fb3/src/lib_aux.c#L39
   The second returned value is a string describing the err.



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


[GitHub] [apisix] tzssangglass merged pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
tzssangglass merged PR #8400:
URL: https://github.com/apache/apisix/pull/8400


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


[GitHub] [apisix] kingluo commented on pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on PR #8400:
URL: https://github.com/apache/apisix/pull/8400#issuecomment-1327207246

   > 
   
   In my opinion, `Inspect` means we could set breakpoints to get context information from any line of the code, while `diagnostic` means the action you handle the issue.
   After `inspect` gathers all information you need, you could `diagnose` where could be the source of the issue.
   The plugin could not determine the reason but it could help you to locate the reason.
   So I think `inspect` is a more accurate name for this plugin.
   
   And, the naming comes from the design document, and the document was publicized for more than 3 weeks.


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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1049152724


##########
conf/config-default.yaml:
##########
@@ -555,6 +556,9 @@ plugin_attr:
       send: 60s
 #  redirect:
 #    https_port: 8443   # the default port for use by HTTP redirects to HTTPS
+  inspect:
+    delay: 3

Review Comment:
   3 seconds, I would add a comment for this line.



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


[GitHub] [apisix] kingluo commented on a diff in pull request #8400: feat: add inspect plugin

Posted by GitBox <gi...@apache.org>.
kingluo commented on code in PR #8400:
URL: https://github.com/apache/apisix/pull/8400#discussion_r1049334826


##########
conf/config-default.yaml:
##########
@@ -555,6 +556,9 @@ plugin_attr:
       send: 60s
 #  redirect:
 #    https_port: 8443   # the default port for use by HTTP redirects to HTTPS
+  inspect:
+    delay: 3            # in seconds
+    hooks_file: "/var/run/apisix_inspect_hooks.lua"

Review Comment:
   Normally this file is non-exist. It's only available when you need to set breakpoints.



##########
conf/config-default.yaml:
##########
@@ -555,6 +556,9 @@ plugin_attr:
       send: 60s
 #  redirect:
 #    https_port: 8443   # the default port for use by HTTP redirects to HTTPS
+  inspect:
+    delay: 3            # in seconds
+    hooks_file: "/var/run/apisix_inspect_hooks.lua"

Review Comment:
   Normally this file is non-exist. It's only available when you need to set breakpoints. Please refer to the plugin doc.



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