Copilot commented on code in PR #13444:
URL: https://github.com/apache/apisix/pull/13444#discussion_r3308924407


##########
apisix/plugins/toolset/src/trace.lua:
##########
@@ -0,0 +1,461 @@
+--
+-- 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 require = require
+local apisix = require("apisix")
+local core = require("apisix.core")
+local uuid = require("resty.jit-uuid")
+
+local conf_path = "apisix.plugins.toolset.config"
+
+local ngx = ngx
+local pairs = pairs
+local type = type
+local package = package
+local tostring = tostring
+local format = string.format
+local floor = math.floor
+local gsub = ngx.re.gsub
+local m_random = math.random
+local m_randomseed = math.randomseed
+local t_remove = table.remove
+local re_match = ngx.re.match
+local counter = 1
+
+local old_http_access_phase
+local old_match_route
+local old_http_log_phase
+local old_http_balancer_phase
+local old_http_header_filter_phase
+local old_http_body_filter_phase
+local old_resolve
+
+local schema = {}
+
+local PHASE_UPSTREAM = "upstream (req + response)"
+local PHASE_CLIENT = "response"
+
+local suffix = [[
++----------+---------------------------+----------+-------------------------+
+]]
+local prefix = [[
+
++----------+---------------------------+----------+-------------------------+
+| Role     | Phase                     | Timespan | Start time              |
+]] .. suffix
+
+local trace_headers = {
+  "x-request-id", -- request id header
+  "sw8",          -- skywalking
+  "traceparent",  -- opentelemetry
+  "x-b3-traceid", -- zipkin
+}
+local plugin_name = "trace"
+
+local _M = {
+  version = 0.1,
+  priority = 22901,
+  name = plugin_name,
+  schema = schema,
+  scope = "global",
+}
+
+local function nspaces(n)
+  return (" "):rep(n)
+end
+
+local function add_entry(phase, timespan, curtime)
+  core.log.info("add entry for: ", phase)
+  local role
+  local tpl = [[
+| %s| %s| %s| %s |
+]]
+  if phase == PHASE_UPSTREAM then
+    role = "Upstream "
+  elseif phase == PHASE_CLIENT then
+    role = "Client   "
+  else
+    role = "APISIX   "
+  end
+
+  -- add spaces around the text for table formatting
+  phase = phase .. nspaces(26 - #phase)
+  timespan = timespan .. nspaces(9 - #tostring(timespan))
+  ngx.ctx.trace_log = ngx.ctx.trace_log .. format(tpl, role, phase, timespan, 
curtime)
+end
+
+
+local function timespan(raw)
+  if raw == 0 then
+    return "0ms"
+  end
+  local factor = 1000 -- 1000ms in 1s
+  local unit = "ms"
+  if raw >= 1 then  -- if greater than 1s don't convert to ms
+    factor = 1
+    unit = "s"
+  end
+  return floor(raw * factor + 0.5) .. unit
+end
+
+
+local function localtime_msec(now)
+  local lt = ngx.localtime()
+  local msec = now * 1000 - floor(now) * 1000
+  if msec > 0 then
+    return lt .. "." .. msec
+  end
+  return lt .. ".000"
+end
+
+
+local function match(incoming, conf)
+  conf = gsub(conf, "\\*", ".*")
+  conf = "^" .. conf .. "$"
+  core.log.info("matching: ", incoming, " against: ", conf)
+
+  local matches = re_match(incoming, "^" .. conf .. "$", "jo")
+  if not matches then
+    return nil
+  end
+  return matches[0]
+end
+
+
+local unique_random
+do
+  local numbers = {}
+  for i = 1, 100 do
+    numbers[i] = i
+  end
+  unique_random = function()
+    m_randomseed(ngx.now())
+    while true do
+      local index = m_random(100)
+      local num = numbers[index]
+      if num then
+        t_remove(numbers, index)
+        return num
+      end
+    end
+  end
+end
+
+
+local function incr_counter()
+  counter = counter + 1
+  if counter > 99 then
+    counter = 0
+  end
+end
+
+
+local function preprocess(trace_conf, ctx)
+  if not trace_conf.rate or type(trace_conf.rate) ~= "number" then
+    ctx.trace = true -- trace all reqs if rate isn't defined
+    return
+  end
+  if trace_conf.rate == 1 then
+    ctx.trace = counter == 1 -- trace only first request
+    incr_counter()
+    return
+  end
+  core.log.info("trace_conf.rate: ", trace_conf.rate)
+  local rand = unique_random()
+  if rand <= trace_conf.rate then
+    ctx.trace = true
+  end
+  core.log.info("random number: ", rand)
+  incr_counter()
+end
+
+
+local function check(trace_conf, uri_or_host)
+  for _, val in pairs(trace_conf) do
+    if match(uri_or_host, val) == uri_or_host then
+      return true
+    end
+  end
+  return false
+end
+
+
+local function check_host(trace_conf)
+  local req_host = core.request.header(ngx.ctx, "host")
+  if (trace_conf.hosts and #trace_conf.hosts > 0) and (req_host and #req_host 
> 0) then
+    return check(trace_conf.hosts, req_host)
+  end
+  -- pass host check if hosts field is not defined in config.lua
+  return trace_conf.hosts ~= nil
+end
+
+
+local function check_uri(trace_conf)
+  if trace_conf.paths and #trace_conf.paths > 0 then
+    return check(trace_conf.paths, ngx.ctx.api_ctx.var.request_uri)
+  end
+  -- pass uri check if paths field is not defined in config.lua
+  return true
+end
+
+
+local function prepend(ctx, field, val)
+  ctx.trace_log = "\n" .. field .. ": " .. val .. ctx.trace_log
+end
+
+
+local function add_headers(ctx)
+  local count = 0
+  for _, header_field in pairs(trace_headers) do
+    local val = core.request.header(ctx, header_field)
+    if val and #val > 0 then
+      prepend(ctx, header_field, val)
+      count = count + 1
+    end
+  end
+  return count
+end
+
+
+local function add_vars(ctx, vars)
+  local count = 0
+  if vars and #vars > 0 then
+    for _, var in pairs(vars) do
+      local val = ngx.var[var]
+      if val and #val > 0 then
+        prepend(ctx, var, val)
+        count = count + 1
+      end
+    end
+  end
+  return count
+end
+
+
+function _M.init()
+  package.loaded[conf_path] = false

Review Comment:
   `package.loaded[conf_path]` is set to `false`, which prevents Lua 
`require()` from reloading and causes `require(conf_path).trace` to index a 
boolean/falsey value. Use `nil` to force reload (as done in other modules), or 
avoid manipulating `package.loaded` and instead read config from 
`plugin_attr.toolset`/local_conf.
   



##########
apisix/plugins/toolset/src/trace.lua:
##########
@@ -0,0 +1,461 @@
+--
+-- 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 require = require
+local apisix = require("apisix")
+local core = require("apisix.core")
+local uuid = require("resty.jit-uuid")
+
+local conf_path = "apisix.plugins.toolset.config"
+
+local ngx = ngx
+local pairs = pairs
+local type = type
+local package = package
+local tostring = tostring
+local format = string.format
+local floor = math.floor
+local gsub = ngx.re.gsub
+local m_random = math.random
+local m_randomseed = math.randomseed
+local t_remove = table.remove
+local re_match = ngx.re.match
+local counter = 1
+
+local old_http_access_phase
+local old_match_route
+local old_http_log_phase
+local old_http_balancer_phase
+local old_http_header_filter_phase
+local old_http_body_filter_phase
+local old_resolve
+
+local schema = {}
+
+local PHASE_UPSTREAM = "upstream (req + response)"
+local PHASE_CLIENT = "response"
+
+local suffix = [[
++----------+---------------------------+----------+-------------------------+
+]]
+local prefix = [[
+
++----------+---------------------------+----------+-------------------------+
+| Role     | Phase                     | Timespan | Start time              |
+]] .. suffix
+
+local trace_headers = {
+  "x-request-id", -- request id header
+  "sw8",          -- skywalking
+  "traceparent",  -- opentelemetry
+  "x-b3-traceid", -- zipkin
+}
+local plugin_name = "trace"
+
+local _M = {
+  version = 0.1,
+  priority = 22901,
+  name = plugin_name,
+  schema = schema,
+  scope = "global",
+}
+
+local function nspaces(n)
+  return (" "):rep(n)
+end
+
+local function add_entry(phase, timespan, curtime)
+  core.log.info("add entry for: ", phase)
+  local role
+  local tpl = [[
+| %s| %s| %s| %s |
+]]
+  if phase == PHASE_UPSTREAM then
+    role = "Upstream "
+  elseif phase == PHASE_CLIENT then
+    role = "Client   "
+  else
+    role = "APISIX   "
+  end
+
+  -- add spaces around the text for table formatting
+  phase = phase .. nspaces(26 - #phase)
+  timespan = timespan .. nspaces(9 - #tostring(timespan))
+  ngx.ctx.trace_log = ngx.ctx.trace_log .. format(tpl, role, phase, timespan, 
curtime)
+end
+
+
+local function timespan(raw)
+  if raw == 0 then
+    return "0ms"
+  end
+  local factor = 1000 -- 1000ms in 1s
+  local unit = "ms"
+  if raw >= 1 then  -- if greater than 1s don't convert to ms
+    factor = 1
+    unit = "s"
+  end
+  return floor(raw * factor + 0.5) .. unit
+end
+
+
+local function localtime_msec(now)
+  local lt = ngx.localtime()
+  local msec = now * 1000 - floor(now) * 1000
+  if msec > 0 then
+    return lt .. "." .. msec
+  end
+  return lt .. ".000"
+end
+
+
+local function match(incoming, conf)
+  conf = gsub(conf, "\\*", ".*")
+  conf = "^" .. conf .. "$"
+  core.log.info("matching: ", incoming, " against: ", conf)
+
+  local matches = re_match(incoming, "^" .. conf .. "$", "jo")
+  if not matches then
+    return nil
+  end
+  return matches[0]
+end
+
+
+local unique_random
+do
+  local numbers = {}
+  for i = 1, 100 do
+    numbers[i] = i
+  end
+  unique_random = function()
+    m_randomseed(ngx.now())
+    while true do
+      local index = m_random(100)
+      local num = numbers[index]
+      if num then
+        t_remove(numbers, index)
+        return num
+      end
+    end

Review Comment:
   `unique_random()` removes entries from a fixed 1..100 array but never 
repopulates it; after ~100 calls, all slots become `nil` and the `while true` 
loop can spin forever. Also reseeding `math.randomseed()` on every call reduces 
randomness and can repeat values across requests. Prefer a simple 
`math.random(100)` (seeded once at init) or a non-blocking PRNG.
   



##########
apisix/plugins/toolset/src/trace.lua:
##########
@@ -0,0 +1,461 @@
+--
+-- 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 require = require
+local apisix = require("apisix")
+local core = require("apisix.core")
+local uuid = require("resty.jit-uuid")
+
+local conf_path = "apisix.plugins.toolset.config"
+
+local ngx = ngx
+local pairs = pairs
+local type = type
+local package = package
+local tostring = tostring
+local format = string.format
+local floor = math.floor
+local gsub = ngx.re.gsub
+local m_random = math.random
+local m_randomseed = math.randomseed
+local t_remove = table.remove
+local re_match = ngx.re.match
+local counter = 1
+
+local old_http_access_phase
+local old_match_route
+local old_http_log_phase
+local old_http_balancer_phase
+local old_http_header_filter_phase
+local old_http_body_filter_phase
+local old_resolve
+
+local schema = {}
+
+local PHASE_UPSTREAM = "upstream (req + response)"
+local PHASE_CLIENT = "response"
+
+local suffix = [[
++----------+---------------------------+----------+-------------------------+
+]]
+local prefix = [[
+
++----------+---------------------------+----------+-------------------------+
+| Role     | Phase                     | Timespan | Start time              |
+]] .. suffix
+
+local trace_headers = {
+  "x-request-id", -- request id header
+  "sw8",          -- skywalking
+  "traceparent",  -- opentelemetry
+  "x-b3-traceid", -- zipkin
+}
+local plugin_name = "trace"
+
+local _M = {
+  version = 0.1,
+  priority = 22901,
+  name = plugin_name,
+  schema = schema,
+  scope = "global",
+}
+
+local function nspaces(n)
+  return (" "):rep(n)
+end
+
+local function add_entry(phase, timespan, curtime)
+  core.log.info("add entry for: ", phase)
+  local role
+  local tpl = [[
+| %s| %s| %s| %s |
+]]
+  if phase == PHASE_UPSTREAM then
+    role = "Upstream "
+  elseif phase == PHASE_CLIENT then
+    role = "Client   "
+  else
+    role = "APISIX   "
+  end
+
+  -- add spaces around the text for table formatting
+  phase = phase .. nspaces(26 - #phase)
+  timespan = timespan .. nspaces(9 - #tostring(timespan))
+  ngx.ctx.trace_log = ngx.ctx.trace_log .. format(tpl, role, phase, timespan, 
curtime)
+end
+
+
+local function timespan(raw)
+  if raw == 0 then
+    return "0ms"
+  end
+  local factor = 1000 -- 1000ms in 1s
+  local unit = "ms"
+  if raw >= 1 then  -- if greater than 1s don't convert to ms
+    factor = 1
+    unit = "s"
+  end
+  return floor(raw * factor + 0.5) .. unit
+end
+
+
+local function localtime_msec(now)
+  local lt = ngx.localtime()
+  local msec = now * 1000 - floor(now) * 1000
+  if msec > 0 then
+    return lt .. "." .. msec
+  end
+  return lt .. ".000"
+end
+
+
+local function match(incoming, conf)
+  conf = gsub(conf, "\\*", ".*")
+  conf = "^" .. conf .. "$"
+  core.log.info("matching: ", incoming, " against: ", conf)
+
+  local matches = re_match(incoming, "^" .. conf .. "$", "jo")
+  if not matches then
+    return nil
+  end
+  return matches[0]
+end
+
+
+local unique_random
+do
+  local numbers = {}
+  for i = 1, 100 do
+    numbers[i] = i
+  end
+  unique_random = function()
+    m_randomseed(ngx.now())
+    while true do
+      local index = m_random(100)
+      local num = numbers[index]
+      if num then
+        t_remove(numbers, index)
+        return num
+      end
+    end
+  end
+end
+
+
+local function incr_counter()
+  counter = counter + 1
+  if counter > 99 then
+    counter = 0
+  end
+end
+
+
+local function preprocess(trace_conf, ctx)
+  if not trace_conf.rate or type(trace_conf.rate) ~= "number" then
+    ctx.trace = true -- trace all reqs if rate isn't defined
+    return
+  end
+  if trace_conf.rate == 1 then
+    ctx.trace = counter == 1 -- trace only first request

Review Comment:
   Sampling counter logic is off-by-one: `incr_counter()` resets `counter` to 
`0`, but the `rate == 1` path only traces when `counter == 1`, which results in 
tracing every 101st request (sequence includes a `counter == 0` request). Use a 
consistent 0..99 or 1..100 range and reset accordingly.
   



##########
apisix/plugins/toolset/init.lua:
##########
@@ -0,0 +1,170 @@
+--
+-- 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 pairs = pairs
+local core = require("apisix.core")
+local ngx = ngx
+local cache = core.table.new(0, 32)
+local stop_timer = false
+local load, unload = "load", "unload"
+local package = package
+local pcall = pcall
+local require = require
+local string = string
+
+local _M = {
+  version = 0.1,
+  priority = 22901,
+  name = "toolset",
+  schema = {},
+  scope = "global",
+}
+
+
+local function get_plugin_config()
+  -- clear cache to reload
+  package.loaded["apisix.plugins.toolset.config"] = nil
+  local loaded, plugins_config = pcall(require, 
"apisix.plugins.toolset.config")
+  if loaded and plugins_config == true then
+    core.log.warn("empty plugin config file")
+    return nil
+  end
+  if not loaded then
+    core.log.error("failed to load plugin config: ", plugins_config)
+    return nil
+  end
+  return plugins_config
+end
+
+
+local function is_config_changed(plugin_name, plugin_config)
+  if core.table.deep_eq(cache[plugin_name], plugin_config) then
+    return false
+  end
+  return true
+end
+
+
+local function is_config_empty(plugin_config)
+  return plugin_config == nil or core.table.deep_eq(plugin_config, {})
+end
+
+
+local function perform_operation_for_plugin(plugin_name, plugin_config, 
operation)
+  if operation == load then
+    local loaded, plugin = pcall(require, "apisix.plugins.toolset.src."
+                           .. string.gsub(plugin_name, "_", "-"))
+    if not loaded then
+      core.log.warn("could not load plugin because it was not found: ", 
plugin_name)
+      return
+    end
+    core.log.warn("initializing sub plugin for toolset plugin: ", plugin_name)
+    plugin.init()
+    cache[plugin_name] = plugin_config
+  elseif operation == unload then
+    local loaded, plugin = pcall(require, "apisix.plugins.toolset.src." ..
+                                 string.gsub(plugin_name, "_", "-"))
+    if not loaded then
+      core.log.warn("could not unload plugin because it was not found: ", 
plugin_name)
+      return
+    end
+    core.log.warn("destroying sub plugin for toolset plugin: ", plugin_name)
+    plugin.destroy()
+    cache[plugin_name] = nil
+  end
+end
+
+
+local function sync()
+  if stop_timer then
+    return
+  end
+  core.log.info("syncing toolset plugin")
+  local plugin_configs = get_plugin_config()
+  local processed_plugins = {}
+  if plugin_configs then
+    for plugin_name, plugin_config in pairs(plugin_configs) do

Review Comment:
   `sync()` logs "syncing toolset plugin" at INFO every second, which can flood 
error logs when the plugin is enabled. Consider logging only on config changes, 
using a lower level (DEBUG), and/or using APISIX timers registry utilities that 
avoid per-second log noise.



##########
apisix/plugins/toolset/src/table-count/init.lua:
##########
@@ -0,0 +1,121 @@
+--
+-- 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 ngx = require("ngx")
+local process = require("ngx.process")
+
+local pairs = pairs
+local ipairs = ipairs
+local type = type
+local timer = ngx.timer
+local require = require
+local package = package
+
+local plugin_name = "table-count"
+
+local schema = {}
+local stop = false
+-- only one run of init() function should be running at a time.
+-- when init() is reloaded the run number is incremented. It also helps in 
debugging.
+local current_run = 0
+
+local _M = {
+  version = 0.1,
+  priority = 22902,
+  name = plugin_name,
+  schema = schema,
+  scope = "global",
+}
+
+local function tab_item_count(tab, cache,depth)
+  if depth == 0 then
+    core.log.warn("out of depth..skipping count")
+    return
+  end
+  depth = depth - 1
+  cache = cache or {}
+  local count = 0
+  for _, value in pairs(tab) do
+    if cache[value] then
+      core.log.warn("circular reference detected..skipping count")
+      goto continue
+    end
+    if type(value) == "table" and not cache[value] then
+      cache[value] = true
+      local tab_count = tab_item_count(value, cache,depth)
+      if tab_count then
+        count = count + tab_count + 1
+      end
+    else
+      count = count + 1
+    end
+    ::continue::
+  end
+  return count
+end
+
+function _M.init()
+  package.loaded["apisix.plugins.toolset.config"] = nil
+  local config = require("apisix.plugins.toolset.config").table_count
+  if config.lua_modules == nil or #config.lua_modules == 0 then
+    core.log.warn("no lua_modules provided for table count")
+    return
+  end
+  if not config.scopes then
+    core.log.warn("no scope provided. Running for all scopes")
+    goto continue
+  end
+  if #config.scopes ~= 0 then
+    for _,scope in ipairs(config.scopes) do
+      if process.type() == scope then
+        goto continue
+      end
+    end
+    return
+  end
+  ::continue::
+  -- Extract configuration values
+  current_run = current_run + 1
+  local interval = config.interval or 5
+  local run_count
+  run_count = function(run_no)
+    local depth = config.depth or 1
+    for _, package_name in ipairs(config.lua_modules) do
+      local package = require(package_name)
+      local count = tab_item_count(package, {},depth)
+      core.log.warn("package ", package_name, " table count is: ", count," for 
loaded: ",run_no)
+    end
+    if stop or run_no ~= current_run then
+      return
+    end
+    local ok, err = timer.at(interval, run_count,current_run)
+    if not ok then
+      core.log.error("failed to create timer for running table count ", err)
+    end
+  end
+
+  local ok, err = timer.at(0, run_count,current_run)
+  if not ok then
+    core.log.error("failed to create timer for running table count ", err)
+  end
+end
+
+function _M.destroy()
+  stop = true
+end

Review Comment:
   `stop` is set to `true` in `destroy()` but never reset in `init()`. After 
the sub-plugin is unloaded once, subsequent reloads will keep `stop == true` 
and the periodic timer will not reschedule, so the plugin effectively cannot be 
re-enabled without a full process restart. Reset `stop = false` during init (or 
clear module state on reload).



##########
t/plugin/trace.t:
##########
@@ -0,0 +1,471 @@
+#
+# 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';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+my $user_yaml_config = <<_EOC_;
+plugins:
+  - toolset
+  - serverless-post-function
+_EOC_
+    $block->set_value("extra_yaml_config", $user_yaml_config);
+
+    if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+        $block->set_value("no_error_log", "[error]");
+    }
+
+    if (!defined $block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: create route
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                ngx.HTTP_PUT,
+                [[{
+                    "uri": "/hello",
+                    "upstream": {
+                        "type": "roundrobin",
+                        "nodes": {
+                        "127.0.0.1:1980": 1
+                        }
+                    }
+                }]]
+            )
+
+            if code >= 300 then
+                ngx.status = code
+                ngx.say(body)
+                return
+            end
+            ngx.say("done")
+
+            local file, err = io.open("apisix/plugins/toolset/config.lua", 
"w+")
+            if not file then
+                ngx.status = 500
+                ngx.say("Failed test: failed to open config file")
+                return
+            end
+            local old = file:read("*all")
+            file:write([[
+return {
+  trace = {
+    rate = 1
+  }
+}
+]])
+            file:close()
+        }
+    }
+--- response_body
+done
+
+
+
+=== TEST 2: test table layout
+--- request
+GET /hello
+--- error_log
+| Role     | Phase                     | Timespan | Start time              |
+
+
+
+=== TEST 3: remove plugin and send request after plugin reload
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local http = require("resty.http")
+
+            local conf, err = io.open("t/servroot/conf/config.yaml", "w+")
+            if not conf then
+                ngx.status = 500
+                ngx.say("Failed test: failed to open config file")
+                return
+            end
+
+            -- yaml config to remove trace plugin
+            local config = "deployment:\n  role: traditional\n  
role_traditional:\n    config_provider: etcd\n  admin:\n    admin_key: 
null\napisix:\n  node_listen: 1984\n  proxy_mode: http&stream\n  
stream_proxy:\n    tcp:\n      - 9100\n  enable_resolv_search_opt: 
false\nplugins:\n  - serverless-post-function\n"
+            conf:write(config)
+
+            -- reload plugins
+            local code, _, org_body = t('/apisix/admin/plugins/reload', 
ngx.HTTP_PUT)
+            if code >= 300 then
+                ngx.status = code
+                ngx.say(message)

Review Comment:
   On error, this code prints `message`, but that variable is never defined 
(the response body is in `org_body`). This can hide the real failure cause and 
even throw another error in the test handler.
   



##########
t/plugin/trace.t:
##########
@@ -0,0 +1,471 @@
+#
+# 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';
+
+repeat_each(1);
+log_level('info');
+worker_connections(256);
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+    my ($block) = @_;
+
+my $user_yaml_config = <<_EOC_;
+plugins:
+  - toolset
+  - serverless-post-function
+_EOC_
+    $block->set_value("extra_yaml_config", $user_yaml_config);
+
+    if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+        $block->set_value("no_error_log", "[error]");
+    }
+
+    if (!defined $block->request) {
+        $block->set_value("request", "GET /t");
+    }
+
+});
+
+run_tests();
+
+__DATA__
+
+=== TEST 1: create route
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                ngx.HTTP_PUT,
+                [[{
+                    "uri": "/hello",
+                    "upstream": {
+                        "type": "roundrobin",
+                        "nodes": {
+                        "127.0.0.1:1980": 1
+                        }
+                    }
+                }]]
+            )
+
+            if code >= 300 then
+                ngx.status = code
+                ngx.say(body)
+                return
+            end
+            ngx.say("done")
+
+            local file, err = io.open("apisix/plugins/toolset/config.lua", 
"w+")
+            if not file then
+                ngx.status = 500
+                ngx.say("Failed test: failed to open config file")
+                return
+            end
+            local old = file:read("*all")
+            file:write([[
+return {
+  trace = {
+    rate = 1
+  }
+}
+]])
+            file:close()

Review Comment:
   This block uses `io.open(..., "w+")` (which truncates the file) and then 
attempts `file:read("*all")`, so `old` will always be empty and the original 
config is never restored. These tests modify a source file under 
`apisix/plugins/...` and can leak state into later tests; read the file before 
truncation and restore it in a cleanup step (or, preferably, configure via 
`extra_yaml_config` / `plugin_attr.toolset`).



##########
apisix/plugins/toolset/src/trace.lua:
##########
@@ -0,0 +1,461 @@
+--
+-- 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 require = require
+local apisix = require("apisix")
+local core = require("apisix.core")
+local uuid = require("resty.jit-uuid")
+
+local conf_path = "apisix.plugins.toolset.config"
+
+local ngx = ngx
+local pairs = pairs
+local type = type
+local package = package
+local tostring = tostring
+local format = string.format
+local floor = math.floor
+local gsub = ngx.re.gsub
+local m_random = math.random
+local m_randomseed = math.randomseed
+local t_remove = table.remove
+local re_match = ngx.re.match
+local counter = 1
+
+local old_http_access_phase
+local old_match_route
+local old_http_log_phase
+local old_http_balancer_phase
+local old_http_header_filter_phase
+local old_http_body_filter_phase
+local old_resolve
+
+local schema = {}
+
+local PHASE_UPSTREAM = "upstream (req + response)"
+local PHASE_CLIENT = "response"
+
+local suffix = [[
++----------+---------------------------+----------+-------------------------+
+]]
+local prefix = [[
+
++----------+---------------------------+----------+-------------------------+
+| Role     | Phase                     | Timespan | Start time              |
+]] .. suffix
+
+local trace_headers = {
+  "x-request-id", -- request id header
+  "sw8",          -- skywalking
+  "traceparent",  -- opentelemetry
+  "x-b3-traceid", -- zipkin
+}
+local plugin_name = "trace"
+
+local _M = {
+  version = 0.1,
+  priority = 22901,
+  name = plugin_name,
+  schema = schema,
+  scope = "global",
+}
+
+local function nspaces(n)
+  return (" "):rep(n)
+end
+
+local function add_entry(phase, timespan, curtime)
+  core.log.info("add entry for: ", phase)
+  local role
+  local tpl = [[
+| %s| %s| %s| %s |
+]]
+  if phase == PHASE_UPSTREAM then
+    role = "Upstream "
+  elseif phase == PHASE_CLIENT then
+    role = "Client   "
+  else
+    role = "APISIX   "
+  end
+
+  -- add spaces around the text for table formatting
+  phase = phase .. nspaces(26 - #phase)
+  timespan = timespan .. nspaces(9 - #tostring(timespan))
+  ngx.ctx.trace_log = ngx.ctx.trace_log .. format(tpl, role, phase, timespan, 
curtime)
+end
+
+
+local function timespan(raw)
+  if raw == 0 then
+    return "0ms"
+  end
+  local factor = 1000 -- 1000ms in 1s
+  local unit = "ms"
+  if raw >= 1 then  -- if greater than 1s don't convert to ms
+    factor = 1
+    unit = "s"
+  end
+  return floor(raw * factor + 0.5) .. unit
+end
+
+
+local function localtime_msec(now)
+  local lt = ngx.localtime()
+  local msec = now * 1000 - floor(now) * 1000
+  if msec > 0 then
+    return lt .. "." .. msec
+  end
+  return lt .. ".000"
+end
+
+
+local function match(incoming, conf)
+  conf = gsub(conf, "\\*", ".*")
+  conf = "^" .. conf .. "$"
+  core.log.info("matching: ", incoming, " against: ", conf)
+
+  local matches = re_match(incoming, "^" .. conf .. "$", "jo")
+  if not matches then
+    return nil
+  end
+  return matches[0]
+end
+
+
+local unique_random
+do
+  local numbers = {}
+  for i = 1, 100 do
+    numbers[i] = i
+  end
+  unique_random = function()
+    m_randomseed(ngx.now())
+    while true do
+      local index = m_random(100)
+      local num = numbers[index]
+      if num then
+        t_remove(numbers, index)
+        return num
+      end
+    end
+  end
+end
+
+
+local function incr_counter()
+  counter = counter + 1
+  if counter > 99 then
+    counter = 0
+  end
+end
+
+
+local function preprocess(trace_conf, ctx)
+  if not trace_conf.rate or type(trace_conf.rate) ~= "number" then
+    ctx.trace = true -- trace all reqs if rate isn't defined
+    return
+  end
+  if trace_conf.rate == 1 then
+    ctx.trace = counter == 1 -- trace only first request
+    incr_counter()
+    return
+  end
+  core.log.info("trace_conf.rate: ", trace_conf.rate)
+  local rand = unique_random()
+  if rand <= trace_conf.rate then
+    ctx.trace = true
+  end
+  core.log.info("random number: ", rand)
+  incr_counter()
+end
+
+
+local function check(trace_conf, uri_or_host)
+  for _, val in pairs(trace_conf) do
+    if match(uri_or_host, val) == uri_or_host then
+      return true
+    end
+  end
+  return false
+end
+
+
+local function check_host(trace_conf)
+  local req_host = core.request.header(ngx.ctx, "host")
+  if (trace_conf.hosts and #trace_conf.hosts > 0) and (req_host and #req_host 
> 0) then
+    return check(trace_conf.hosts, req_host)
+  end
+  -- pass host check if hosts field is not defined in config.lua
+  return trace_conf.hosts ~= nil
+end
+
+
+local function check_uri(trace_conf)
+  if trace_conf.paths and #trace_conf.paths > 0 then
+    return check(trace_conf.paths, ngx.ctx.api_ctx.var.request_uri)
+  end
+  -- pass uri check if paths field is not defined in config.lua
+  return true
+end
+
+
+local function prepend(ctx, field, val)
+  ctx.trace_log = "\n" .. field .. ": " .. val .. ctx.trace_log
+end
+
+
+local function add_headers(ctx)
+  local count = 0
+  for _, header_field in pairs(trace_headers) do
+    local val = core.request.header(ctx, header_field)
+    if val and #val > 0 then
+      prepend(ctx, header_field, val)
+      count = count + 1
+    end
+  end
+  return count
+end
+
+
+local function add_vars(ctx, vars)
+  local count = 0
+  if vars and #vars > 0 then
+    for _, var in pairs(vars) do
+      local val = ngx.var[var]
+      if val and #val > 0 then
+        prepend(ctx, var, val)
+        count = count + 1
+      end
+    end
+  end
+  return count
+end
+
+
+function _M.init()
+  package.loaded[conf_path] = false
+  local trace_conf = require(conf_path).trace
+  core.log.info("trace_conf: ", core.json.encode(trace_conf))
+
+  local conf = core.config.local_conf()
+  local router_name = "radixtree_uri"
+  if conf and conf.apisix and conf.apisix.router then
+    router_name = conf.apisix.router.http or router_name
+  end
+
+  local dns = require("apisix.core.dns.client")
+  if dns then
+    if not old_resolve then
+      old_resolve = dns.resolve
+    end
+
+    dns.resolve = function (...)
+      local match_start = ngx.now()
+      ngx.ctx.dns_lt = localtime_msec(match_start)
+      local ret = old_resolve(...)
+      ngx.update_time()
+
+      ngx.ctx.dns_resolve_timespan = ngx.now() - match_start
+      return ret
+    end
+  end
+
+  local router = require("apisix.http.router." .. router_name)
+  if not old_match_route then
+    old_match_route = router.match
+  end
+  router.match = function(...)
+    local match_start = ngx.now()
+    ngx.ctx.match_lt = localtime_msec(match_start)
+
+    old_match_route(...)
+    ngx.update_time()
+
+    ngx.ctx.match_timespan = ngx.now() - match_start

Review Comment:
   The router `match` wrapper drops the return value from 
`old_match_route(...)`. While the current HTTP path may not use it, other call 
sites and future changes may rely on the boolean result; return the result to 
preserve router semantics.
   



##########
apisix/plugins/toolset/src/trace.lua:
##########
@@ -0,0 +1,461 @@
+--
+-- 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 require = require
+local apisix = require("apisix")
+local core = require("apisix.core")
+local uuid = require("resty.jit-uuid")
+
+local conf_path = "apisix.plugins.toolset.config"
+
+local ngx = ngx
+local pairs = pairs
+local type = type
+local package = package
+local tostring = tostring
+local format = string.format
+local floor = math.floor
+local gsub = ngx.re.gsub
+local m_random = math.random
+local m_randomseed = math.randomseed
+local t_remove = table.remove
+local re_match = ngx.re.match
+local counter = 1
+
+local old_http_access_phase
+local old_match_route
+local old_http_log_phase
+local old_http_balancer_phase
+local old_http_header_filter_phase
+local old_http_body_filter_phase
+local old_resolve
+
+local schema = {}
+
+local PHASE_UPSTREAM = "upstream (req + response)"
+local PHASE_CLIENT = "response"
+
+local suffix = [[
++----------+---------------------------+----------+-------------------------+
+]]
+local prefix = [[
+
++----------+---------------------------+----------+-------------------------+
+| Role     | Phase                     | Timespan | Start time              |
+]] .. suffix
+
+local trace_headers = {
+  "x-request-id", -- request id header
+  "sw8",          -- skywalking
+  "traceparent",  -- opentelemetry
+  "x-b3-traceid", -- zipkin
+}
+local plugin_name = "trace"
+
+local _M = {
+  version = 0.1,
+  priority = 22901,
+  name = plugin_name,
+  schema = schema,
+  scope = "global",
+}
+
+local function nspaces(n)
+  return (" "):rep(n)
+end
+
+local function add_entry(phase, timespan, curtime)
+  core.log.info("add entry for: ", phase)
+  local role
+  local tpl = [[
+| %s| %s| %s| %s |
+]]
+  if phase == PHASE_UPSTREAM then
+    role = "Upstream "
+  elseif phase == PHASE_CLIENT then
+    role = "Client   "
+  else
+    role = "APISIX   "
+  end
+
+  -- add spaces around the text for table formatting
+  phase = phase .. nspaces(26 - #phase)
+  timespan = timespan .. nspaces(9 - #tostring(timespan))
+  ngx.ctx.trace_log = ngx.ctx.trace_log .. format(tpl, role, phase, timespan, 
curtime)
+end
+
+
+local function timespan(raw)
+  if raw == 0 then
+    return "0ms"
+  end
+  local factor = 1000 -- 1000ms in 1s
+  local unit = "ms"
+  if raw >= 1 then  -- if greater than 1s don't convert to ms
+    factor = 1
+    unit = "s"
+  end
+  return floor(raw * factor + 0.5) .. unit
+end
+
+
+local function localtime_msec(now)
+  local lt = ngx.localtime()
+  local msec = now * 1000 - floor(now) * 1000
+  if msec > 0 then
+    return lt .. "." .. msec
+  end
+  return lt .. ".000"
+end
+
+
+local function match(incoming, conf)
+  conf = gsub(conf, "\\*", ".*")
+  conf = "^" .. conf .. "$"
+  core.log.info("matching: ", incoming, " against: ", conf)
+
+  local matches = re_match(incoming, "^" .. conf .. "$", "jo")
+  if not matches then
+    return nil
+  end
+  return matches[0]
+end
+
+
+local unique_random
+do
+  local numbers = {}
+  for i = 1, 100 do
+    numbers[i] = i
+  end
+  unique_random = function()
+    m_randomseed(ngx.now())
+    while true do
+      local index = m_random(100)
+      local num = numbers[index]
+      if num then
+        t_remove(numbers, index)
+        return num
+      end
+    end
+  end
+end
+
+
+local function incr_counter()
+  counter = counter + 1
+  if counter > 99 then
+    counter = 0
+  end
+end
+
+
+local function preprocess(trace_conf, ctx)
+  if not trace_conf.rate or type(trace_conf.rate) ~= "number" then
+    ctx.trace = true -- trace all reqs if rate isn't defined
+    return
+  end
+  if trace_conf.rate == 1 then
+    ctx.trace = counter == 1 -- trace only first request
+    incr_counter()
+    return
+  end
+  core.log.info("trace_conf.rate: ", trace_conf.rate)
+  local rand = unique_random()
+  if rand <= trace_conf.rate then
+    ctx.trace = true
+  end
+  core.log.info("random number: ", rand)
+  incr_counter()
+end
+
+
+local function check(trace_conf, uri_or_host)
+  for _, val in pairs(trace_conf) do
+    if match(uri_or_host, val) == uri_or_host then
+      return true
+    end
+  end
+  return false
+end
+
+
+local function check_host(trace_conf)
+  local req_host = core.request.header(ngx.ctx, "host")
+  if (trace_conf.hosts and #trace_conf.hosts > 0) and (req_host and #req_host 
> 0) then
+    return check(trace_conf.hosts, req_host)
+  end
+  -- pass host check if hosts field is not defined in config.lua
+  return trace_conf.hosts ~= nil
+end
+
+
+local function check_uri(trace_conf)
+  if trace_conf.paths and #trace_conf.paths > 0 then
+    return check(trace_conf.paths, ngx.ctx.api_ctx.var.request_uri)
+  end
+  -- pass uri check if paths field is not defined in config.lua
+  return true
+end
+
+
+local function prepend(ctx, field, val)
+  ctx.trace_log = "\n" .. field .. ": " .. val .. ctx.trace_log
+end
+
+
+local function add_headers(ctx)
+  local count = 0
+  for _, header_field in pairs(trace_headers) do
+    local val = core.request.header(ctx, header_field)
+    if val and #val > 0 then
+      prepend(ctx, header_field, val)
+      count = count + 1
+    end
+  end
+  return count
+end
+
+
+local function add_vars(ctx, vars)
+  local count = 0
+  if vars and #vars > 0 then
+    for _, var in pairs(vars) do
+      local val = ngx.var[var]
+      if val and #val > 0 then
+        prepend(ctx, var, val)
+        count = count + 1
+      end
+    end
+  end
+  return count
+end
+
+
+function _M.init()
+  package.loaded[conf_path] = false
+  local trace_conf = require(conf_path).trace
+  core.log.info("trace_conf: ", core.json.encode(trace_conf))
+
+  local conf = core.config.local_conf()
+  local router_name = "radixtree_uri"
+  if conf and conf.apisix and conf.apisix.router then
+    router_name = conf.apisix.router.http or router_name
+  end
+
+  local dns = require("apisix.core.dns.client")
+  if dns then
+    if not old_resolve then
+      old_resolve = dns.resolve
+    end
+
+    dns.resolve = function (...)
+      local match_start = ngx.now()
+      ngx.ctx.dns_lt = localtime_msec(match_start)
+      local ret = old_resolve(...)
+      ngx.update_time()
+
+      ngx.ctx.dns_resolve_timespan = ngx.now() - match_start
+      return ret
+    end
+  end

Review Comment:
   `dns.resolve` is monkey-patched in `init()` (capturing `old_resolve`) but 
never restored in `destroy()`. Unloading/reloading the sub-plugin will leave 
DNS resolution instrumentation enabled globally and can stack behavior across 
reloads. Restore `dns.resolve = old_resolve` during destroy (and consider 
guarding against double-patching).



##########
apisix/plugins/toolset/src/table-count/init.lua:
##########
@@ -0,0 +1,121 @@
+--
+-- 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 ngx = require("ngx")
+local process = require("ngx.process")
+
+local pairs = pairs
+local ipairs = ipairs
+local type = type
+local timer = ngx.timer
+local require = require
+local package = package
+
+local plugin_name = "table-count"
+
+local schema = {}
+local stop = false
+-- only one run of init() function should be running at a time.
+-- when init() is reloaded the run number is incremented. It also helps in 
debugging.
+local current_run = 0
+
+local _M = {
+  version = 0.1,
+  priority = 22902,
+  name = plugin_name,
+  schema = schema,
+  scope = "global",
+}
+
+local function tab_item_count(tab, cache,depth)
+  if depth == 0 then
+    core.log.warn("out of depth..skipping count")
+    return
+  end
+  depth = depth - 1
+  cache = cache or {}
+  local count = 0
+  for _, value in pairs(tab) do
+    if cache[value] then
+      core.log.warn("circular reference detected..skipping count")
+      goto continue
+    end
+    if type(value) == "table" and not cache[value] then
+      cache[value] = true
+      local tab_count = tab_item_count(value, cache,depth)
+      if tab_count then
+        count = count + tab_count + 1
+      end
+    else
+      count = count + 1
+    end
+    ::continue::
+  end
+  return count
+end
+
+function _M.init()
+  package.loaded["apisix.plugins.toolset.config"] = nil
+  local config = require("apisix.plugins.toolset.config").table_count
+  if config.lua_modules == nil or #config.lua_modules == 0 then
+    core.log.warn("no lua_modules provided for table count")
+    return
+  end
+  if not config.scopes then
+    core.log.warn("no scope provided. Running for all scopes")
+    goto continue
+  end
+  if #config.scopes ~= 0 then
+    for _,scope in ipairs(config.scopes) do
+      if process.type() == scope then
+        goto continue
+      end
+    end
+    return
+  end
+  ::continue::
+  -- Extract configuration values
+  current_run = current_run + 1
+  local interval = config.interval or 5
+  local run_count
+  run_count = function(run_no)
+    local depth = config.depth or 1
+    for _, package_name in ipairs(config.lua_modules) do
+      local package = require(package_name)
+      local count = tab_item_count(package, {},depth)
+      core.log.warn("package ", package_name, " table count is: ", count," for 
loaded: ",run_no)

Review Comment:
   In `run_count()`, depth defaults to `1` when `config.depth` is absent 
(`local depth = config.depth or 1`), but docs/config defaults for 
`table_count.depth` are `10`. Align defaults across code, 
`apisix/plugins/toolset/config.lua`, and docs so omitting `depth` behaves as 
documented.



##########
apisix/plugins/toolset/init.lua:
##########
@@ -0,0 +1,170 @@
+--
+-- 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 pairs = pairs
+local core = require("apisix.core")
+local ngx = ngx
+local cache = core.table.new(0, 32)
+local stop_timer = false
+local load, unload = "load", "unload"
+local package = package
+local pcall = pcall
+local require = require
+local string = string
+
+local _M = {
+  version = 0.1,
+  priority = 22901,
+  name = "toolset",
+  schema = {},
+  scope = "global",
+}
+
+
+local function get_plugin_config()
+  -- clear cache to reload
+  package.loaded["apisix.plugins.toolset.config"] = nil
+  local loaded, plugins_config = pcall(require, 
"apisix.plugins.toolset.config")
+  if loaded and plugins_config == true then
+    core.log.warn("empty plugin config file")
+    return nil
+  end
+  if not loaded then
+    core.log.error("failed to load plugin config: ", plugins_config)
+    return nil
+  end
+  return plugins_config
+end

Review Comment:
   `get_plugin_config()` loads configuration from a Lua module file 
(`apisix.plugins.toolset.config`). This conflicts with the PR 
description/docs/config.yaml.example which say configuration lives under 
`plugin_attr.toolset` in `config.yaml`. Consider reading config via 
`apisix.plugin.plugin_attr("toolset")` (or 
`core.config.local_conf().plugin_attr.toolset`) and merging with defaults, 
rather than requiring a writable Lua file at runtime.



##########
docs/zh/latest/plugins/toolset.md:
##########
@@ -0,0 +1,159 @@
+---
+title: toolset
+keywords:
+  - Apache APISIX
+  - API 网关
+  - Plugin
+  - Toolset
+  - toolset
+  - trace
+  - table-count
+description: 本文介绍了关于 Apache APISIX `toolset` 插件的基本信息及使用方法。
+---
+
+<!--
+#
+# 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.
+#
+-->
+
+## 描述
+
+`toolset` 插件是一个诊断与可观测性框架,用于托管多个轻量级子插件。每个子插件通过 `config.yaml` 中的 
`plugin_attr.toolset` 进行配置,并在运行时动态加载或卸载,无需重启 APISIX。`toolset` 插件本身没有路由级别的 
schema,始终在全局范围内运行。
+

Review Comment:
   该插件文档缺少 `<head>` 中的 canonical 链接块;仓库内其他插件文档通常在顶部包含 canonical link(例如 
`docs/zh/latest/plugins/key-auth.md`)。建议补充以保持文档结构一致。



##########
apisix/plugins/toolset/src/trace.lua:
##########
@@ -0,0 +1,461 @@
+--
+-- 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 require = require
+local apisix = require("apisix")
+local core = require("apisix.core")
+local uuid = require("resty.jit-uuid")
+
+local conf_path = "apisix.plugins.toolset.config"
+
+local ngx = ngx
+local pairs = pairs
+local type = type
+local package = package
+local tostring = tostring
+local format = string.format
+local floor = math.floor
+local gsub = ngx.re.gsub
+local m_random = math.random
+local m_randomseed = math.randomseed
+local t_remove = table.remove
+local re_match = ngx.re.match
+local counter = 1
+
+local old_http_access_phase
+local old_match_route
+local old_http_log_phase
+local old_http_balancer_phase
+local old_http_header_filter_phase
+local old_http_body_filter_phase
+local old_resolve
+
+local schema = {}
+
+local PHASE_UPSTREAM = "upstream (req + response)"
+local PHASE_CLIENT = "response"
+
+local suffix = [[
++----------+---------------------------+----------+-------------------------+
+]]
+local prefix = [[
+
++----------+---------------------------+----------+-------------------------+
+| Role     | Phase                     | Timespan | Start time              |
+]] .. suffix
+
+local trace_headers = {
+  "x-request-id", -- request id header
+  "sw8",          -- skywalking
+  "traceparent",  -- opentelemetry
+  "x-b3-traceid", -- zipkin
+}
+local plugin_name = "trace"
+
+local _M = {
+  version = 0.1,
+  priority = 22901,
+  name = plugin_name,
+  schema = schema,
+  scope = "global",
+}
+
+local function nspaces(n)
+  return (" "):rep(n)
+end
+
+local function add_entry(phase, timespan, curtime)
+  core.log.info("add entry for: ", phase)
+  local role
+  local tpl = [[
+| %s| %s| %s| %s |
+]]
+  if phase == PHASE_UPSTREAM then
+    role = "Upstream "
+  elseif phase == PHASE_CLIENT then
+    role = "Client   "
+  else
+    role = "APISIX   "
+  end
+
+  -- add spaces around the text for table formatting
+  phase = phase .. nspaces(26 - #phase)
+  timespan = timespan .. nspaces(9 - #tostring(timespan))
+  ngx.ctx.trace_log = ngx.ctx.trace_log .. format(tpl, role, phase, timespan, 
curtime)
+end
+
+
+local function timespan(raw)
+  if raw == 0 then
+    return "0ms"
+  end
+  local factor = 1000 -- 1000ms in 1s
+  local unit = "ms"
+  if raw >= 1 then  -- if greater than 1s don't convert to ms
+    factor = 1
+    unit = "s"
+  end
+  return floor(raw * factor + 0.5) .. unit
+end
+
+
+local function localtime_msec(now)
+  local lt = ngx.localtime()
+  local msec = now * 1000 - floor(now) * 1000
+  if msec > 0 then
+    return lt .. "." .. msec
+  end
+  return lt .. ".000"
+end
+
+
+local function match(incoming, conf)
+  conf = gsub(conf, "\\*", ".*")
+  conf = "^" .. conf .. "$"
+  core.log.info("matching: ", incoming, " against: ", conf)
+
+  local matches = re_match(incoming, "^" .. conf .. "$", "jo")
+  if not matches then
+    return nil
+  end
+  return matches[0]
+end

Review Comment:
   `match()` converts globs by only replacing `*` with `.*` but does not escape 
other regex metacharacters (like `.`, `+`, `?`, `[`), so patterns such as 
`*.com` can over-match (e.g., treating `.` as “any char”). Implement proper 
glob-to-regex escaping before substitution to make allowlists behave as 
documented.



##########
docs/en/latest/plugins/toolset.md:
##########
@@ -0,0 +1,159 @@
+---
+title: toolset
+keywords:
+  - Apache APISIX
+  - API Gateway
+  - Plugin
+  - Toolset
+  - toolset
+  - trace
+  - table-count
+description: This document contains information about the Apache APISIX 
toolset 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
+
+The `toolset` Plugin is a diagnostics and observability framework that hosts 
multiple lightweight sub-plugins. Each sub-plugin is configured in 
`config.yaml` under the `plugin_attr.toolset` key and is dynamically loaded or 
unloaded at runtime without restarting APISIX. The `toolset` plugin itself has 
no per-route schema and always operates at the global scope.
+

Review Comment:
   The toolset plugin docs are missing the `<head>` block with a canonical 
link, which is present in other plugin docs (e.g., 
`docs/en/latest/plugins/key-auth.md`). Add the canonical link block near the 
top to follow the established documentation structure.



##########
t/table-count-example.lua:
##########
@@ -0,0 +1,71 @@
+--
+-- 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 _M = {}
+
+
+local function test_depth_more_than_10()
+  _M.a = {
+    a = {
+      a = {
+        a = {
+          a = {
+            a = {
+              a = {
+                a = {
+                  a = {
+                    a = {
+                      "should not be counted"
+                    }
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+end
+
+--- test function creates adds 1.

Review Comment:
   Comment typo/grammar: "test function creates adds 1." is ungrammatical and 
confusing. Please reword to a clear description of what the function 
adds/creates.
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to