Skip to content
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

Merged
merged 27 commits into from Sep 22, 2021
Merged

Conversation

tzssangglass
Copy link
Member

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:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@tzssangglass tzssangglass marked this pull request as draft September 8, 2021 03:52
@tzssangglass tzssangglass marked this pull request as ready for review September 12, 2021 11:17

```yaml
http:
dynamic: true # 是否动态开启高级调试模式
Copy link
Member

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

no_root_location();
no_shuffle();

sub read_file($) {
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

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.

docs/en/latest/architecture-design/debug-mode.md Outdated Show resolved Hide resolved
docs/en/latest/architecture-design/debug-mode.md Outdated Show resolved Hide resolved
docs/en/latest/architecture-design/debug-mode.md Outdated Show resolved Hide resolved
conf/debug.yaml Outdated Show resolved Hide resolved
apisix/debug.lua Outdated
@@ -30,6 +29,7 @@ local setmetatable = setmetatable
local pcall = pcall
local ipairs = ipairs
local unpack = unpack
local inspect = require "inspect"
Copy link
Member

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

Copy link
Member Author

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.

Copy link
Member

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)
Copy link
Member

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?

apisix/debug.lua Outdated Show resolved Hide resolved
apisix/init.lua Outdated Show resolved Hide resolved
docs/en/latest/architecture-design/debug-mode.md Outdated Show resolved Hide resolved
docs/en/latest/architecture-design/debug-mode.md Outdated Show resolved Hide resolved
docs/en/latest/architecture-design/debug-mode.md Outdated Show resolved Hide resolved
no_root_location();
no_shuffle();

sub read_file($) {
Copy link
Member

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"
Copy link
Member

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.

docs/zh/latest/architecture-design/debug-mode.md Outdated Show resolved Hide resolved
apisix/debug.lua Outdated Show resolved Hide resolved
apisix/debug.lua Outdated Show resolved Hide resolved
apisix/debug.lua Outdated Show resolved Hide resolved
Copy link
Member

@nic-chen nic-chen left a 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.

@nic-chen
Copy link
Member

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.

@spacewander spacewander merged commit c4c9b1f into apache:master Sep 22, 2021
@tzssangglass tzssangglass deleted the edbh branch October 26, 2021 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants