fix(log-rotate): skip access log when enable_access_log is set to false #11310

flearc · 2024-05-31T10:26:26Z

Description

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

…to false

apisix/plugins/log-rotate.lua

github-actions · 2024-09-17T10:04:58Z

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.

flearc · 2024-09-17T12:50:57Z

@shreemaan-abhishek PTAL

apisix/plugins/log-rotate.lua

shreemaan-abhishek · 2024-09-25T12:24:29Z

apisix/plugins/log-rotate.lua

@@ -299,9 +303,9 @@ local function rotate()
    elseif max_size > 0 then
        local access_log_file_size = file_size(default_logs[DEFAULT_ACCESS_LOG_FILENAME].file)
        local error_log_file_size = file_size(default_logs[DEFAULT_ERROR_LOG_FILENAME].file)
-        local files = core.table.new(2, 0)
+        local files = {}


why make this change? It seems unrelated to me. If it really is, you should remove it.

Same that no need to pre-allocate. It only needs to store one filename when enable_access_log is false.

shreemaan-abhishek · 2024-09-25T12:24:47Z

apisix/plugins/log-rotate.lua

@@ -210,7 +210,7 @@ local function rotate_file(files, now_time, max_kept, timeout)
        return
    end

-    local new_files = core.table.new(2, 0)
+    local new_files = core.table.new(#files, 0)


seems like unrelated change.

new_files stores the name of access.log and error.log. If enable_access_log is set to false, nginx will not generate access.log file, no need to pre-allocate an array of length 2.

shreemaan-abhishek · 2024-09-25T12:25:07Z

apisix/plugins/log-rotate.lua

@@ -76,7 +77,6 @@ end


 local function get_log_path_info(file_type)
-    local_conf = core.config.local_conf()


why remove this line?

local_conf is initialized in the init function now.

flearc · 2024-09-29T03:02:31Z

@shreemaan-abhishek Thanks for your advice! I have updated my PR and I would like to explain it again that since we can disable access.log by setting enable_access_log to false, the length of files and new_files is not guaranteed to be 2. So I adjusted the logic here.

flearc · 2024-10-08T01:56:30Z

@shreemaan-abhishek The lint CI has been fixed. The other two test cases' failures seems unrelated. Please review.

github-actions · 2024-12-16T10:05:50Z

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@apisix.apache.org list. Thank you for your contributions.

github-actions · 2025-01-24T10:05:32Z

This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

flearc · 2025-05-16T10:58:13Z

@Baoyuantop I can't reopen it, maybe need a new PR.

flearc · 2025-05-20T08:37:48Z

@Baoyuantop Two checks are failing, but they don’t appear to be related to this PR.

Baoyuantop · 2025-05-20T08:48:39Z

Please merge master branch.

flearc · 2025-05-20T09:06:17Z

Please merge master branch.

done, please review @Baoyuantop

flearc · 2025-05-26T01:49:26Z

@Baoyuantop The failing CI test is not related to the changes in this PR, and I've verified that the same failure exists in the latest several commits on master, it needs to be addressed in a separate PR.

Baoyuantop · 2025-06-09T05:55:15Z

Hi @flearc, can you merge the latest master branch?

flearc · 2025-06-09T06:03:43Z

Hi @flearc, can you merge the latest master branch?

@Baoyuantop done

flearc · 2025-06-13T09:45:22Z

@Baoyuantop
The t/plugin/request-id.t test failed in CI, but I couldn’t reproduce it locally. I believe the failure is not related to the changes in this PR. Could you please help check what might have caused it?

flearc · 2025-06-17T06:52:24Z

Hi @Baoyuantop, the tests have passed. Could you please assign additional reviewers for further review and merge?

fix: log-rotate should skip access log when enable_access_log is set …

6a7a5ff

…to false

flearc changed the title ~~fix(log-rotate): skip access log when enable_access_log is set to false~~ WIP: fix(log-rotate): skip access log when enable_access_log is set to false May 31, 2024

flearc and others added 2 commits June 4, 2024 10:17

Merge branch 'apache:master' into feature/log-rotate

d3f88a7

fix test case

6c07b96

flearc changed the title ~~WIP: fix(log-rotate): skip access log when enable_access_log is set to false~~ fix(log-rotate): skip access log when enable_access_log is set to false Jun 7, 2024

fix lint

3afcb7f

shreemaan-abhishek reviewed Jun 10, 2024

View reviewed changes

apisix/plugins/log-rotate.lua Outdated Show resolved Hide resolved

Merge branch 'apache:master' into feature/log-rotate

2f63bb2

flearc requested a review from shreemaan-abhishek July 12, 2024 02:23

Merge remote-tracking branch 'origin/master' into feature/log-rotate

7bb61cb

github-actions bot added the stale label Sep 17, 2024

github-actions bot removed the stale label Sep 18, 2024

shreemaan-abhishek reviewed Sep 25, 2024

View reviewed changes

fix: core.table.try_read_attr

f34647c

dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 29, 2024

flearc requested a review from shreemaan-abhishek September 29, 2024 02:52

shreemaan-abhishek previously approved these changes Oct 3, 2024

View reviewed changes

fix lint ci

fd04395

flearc dismissed shreemaan-abhishek’s stale review via fd04395 October 8, 2024 01:53

flearc requested a review from shreemaan-abhishek October 17, 2024 05:31

github-actions bot added the stale label Dec 16, 2024

github-actions bot closed this Jan 24, 2025

Baoyuantop reopened this May 19, 2025

github-actions bot removed the stale label May 19, 2025

Merge branch 'apache:master' into feature/log-rotate

bbf0bd8

Merge branch 'apache:master' into feature/log-rotate

bd7548f

Merge branch 'apache:master' into feature/log-rotate

e2271da

Baoyuantop added this to ⚡️ Apache APISIX Roadmap Jun 13, 2025

Baoyuantop removed this from ⚡️ Apache APISIX Roadmap Jun 13, 2025

Baoyuantop assigned flearc Jun 13, 2025

Baoyuantop added this to ⚡️ Apache APISIX Roadmap Jun 13, 2025

Baoyuantop moved this to 🏗 In progress in ⚡️ Apache APISIX Roadmap Jun 13, 2025

Baoyuantop moved this from 🏗 In progress to 👀 In review in ⚡️ Apache APISIX Roadmap Jun 13, 2025

Baoyuantop self-requested a review June 17, 2025 08:40

Baoyuantop approved these changes Jul 3, 2025

View reviewed changes

Baoyuantop requested review from membphis, bzp2010, nic-6443, Revolyssup and AlinsRan July 3, 2025 02:08

		@@ -76,7 +77,6 @@ end


		local function get_log_path_info(file_type)
		local_conf = core.config.local_conf()

fix(log-rotate): skip access log when enable_access_log is set to false #11310

Are you sure you want to change the base?

fix(log-rotate): skip access log when enable_access_log is set to false #11310

Conversation

flearc commented May 31, 2024

Description

Checklist

Uh oh!

Uh oh!

github-actions bot commented Sep 17, 2024

Uh oh!

flearc commented Sep 17, 2024

Uh oh!

Uh oh!

shreemaan-abhishek Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

flearc Sep 29, 2024

Choose a reason for hiding this comment

Uh oh!

shreemaan-abhishek Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

flearc Sep 29, 2024

Choose a reason for hiding this comment

Uh oh!

shreemaan-abhishek Sep 25, 2024

Choose a reason for hiding this comment

Uh oh!

flearc Sep 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flearc commented Sep 29, 2024

Uh oh!

flearc commented Oct 8, 2024

Uh oh!

github-actions bot commented Dec 16, 2024

Uh oh!

github-actions bot commented Jan 24, 2025

Uh oh!

flearc commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flearc commented May 20, 2025

Uh oh!

Baoyuantop commented May 20, 2025

Uh oh!

flearc commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flearc commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Baoyuantop commented Jun 9, 2025

Uh oh!

flearc commented Jun 9, 2025

Uh oh!

flearc commented Jun 13, 2025

Uh oh!

flearc commented Jun 17, 2025

Uh oh!

Uh oh!

flearc Sep 29, 2024 •

edited

Loading

flearc commented May 16, 2025 •

edited

Loading

flearc commented May 20, 2025 •

edited

Loading

flearc commented May 26, 2025 •

edited

Loading