-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(debug-mode): add dynamic debug mode #5012
Conversation
|
||
```yaml | ||
http: | ||
dynamic: true # 是否动态开启高级调试模式 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need English comment in English doc
t/debug/dynamic-hook.t
Outdated
no_root_location(); | ||
no_shuffle(); | ||
|
||
sub read_file($) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move the read_file into APISIX.pm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to open another PR to do this, which is used in several test cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is already done in the master branch. Let's merge the master.
apisix/debug.lua
Outdated
@@ -30,6 +29,7 @@ local setmetatable = setmetatable | |||
local pcall = pcall | |||
local ipairs = ipairs | |||
local unpack = unpack | |||
local inspect = require "inspect" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to put all require together, and in the form require("ngx.process")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
require("apisix.debug")
in init.lua
--> require("ngx.process")
in debug.lua
, will result in error as process.lua:5: unsupported subsystem: stream
, see: https://github.com/apache/apisix/runs/3578221555
ngx.process API for stream subsystem is not implemented on openresty 1.19.3.2(but is implemented on 1.19.9.1), so I move require("ngx.process")
to function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I mean to use require("inspect")
, the require("ngx.process")
is just an example.
apisix/debug.lua
Outdated
return true | ||
end | ||
|
||
function _M.dynamic_enable(api_ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is called in the request level, but it changes a field at the module level. Look like it will affect other requests?
� Conflicts: � apisix/debug.lua � conf/debug.yaml
t/debug/dynamic-hook.t
Outdated
no_root_location(); | ||
no_shuffle(); | ||
|
||
sub read_file($) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is already done in the master branch. Let's merge the master.
apisix/debug.lua
Outdated
@@ -30,6 +29,7 @@ local setmetatable = setmetatable | |||
local pcall = pcall | |||
local ipairs = ipairs | |||
local unpack = unpack | |||
local inspect = require "inspect" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I mean to use require("inspect")
, the require("ngx.process")
is just an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a question. After debug.yaml
is changed, do we need to restart APISIX for it?
For example, enable/disable debug mode.
ignore it. |
What this PR does / why we need it:
Add dynamic debug mode. In scenarios where write file permissions are tightly controlled, advanced debugging mode can be dynamically enabled or disabled via the API to help debug production environments.
Pre-submission checklist: