spacewander commented on a change in pull request #2097: URL: https://github.com/apache/apisix/pull/2097#discussion_r477029630
########## File path: apisix/plugins/log-rotate.lua ########## @@ -0,0 +1,243 @@ +-- +-- 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 process = require("ngx.process") +local signal = require("resty.signal") +local ngx = ngx +local lfs = require("lfs") +local io = io +local os = os +local table = table +local select = select +local type = type +local string = string +local local_conf + + +local timer +local plugin_name = "log-rotate" +local INTERVAL = 60 * 60 -- rotate interval (unit: second) +local MAX_KEPT = 24 * 7 -- max number of log files will be kept +local schema = { + type = "object", + properties = {}, + additionalProperties = false, +} + + +local _M = { + version = 0.1, + priority = 100, + name = plugin_name, + schema = schema, +} + + +local function file_exists(path) + local file = io.open(path, "r") + if file then + file:close() + end + return file ~= nil +end + + +local function get_last_index(str, key) + local rev = string.reverse(str) + local _, idx = string.find(rev, key) + local n + if idx then + n = string.len(rev) - idx + 1 + end + + return n +end + + +local function get_log_path_info(file_type) + local_conf = core.config.local_conf() + local conf_path + if file_type == "error.log" then + conf_path = local_conf and local_conf.nginx_config and + local_conf.nginx_config.error_log + else + conf_path = local_conf and local_conf.nginx_config and + local_conf.nginx_config.http and + local_conf.nginx_config.http.access_log + end + + local prefix = ngx.config.prefix() + + if conf_path then + local root = string.sub(conf_path, 1, 1) Review comment: Better to use `string.byte(conf_path, 1, 1)`, and compare it with ascii number. ########## File path: apisix/plugins/log-rotate.lua ########## @@ -0,0 +1,243 @@ +-- +-- 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 process = require("ngx.process") +local signal = require("resty.signal") +local ngx = ngx +local lfs = require("lfs") +local io = io +local os = os +local table = table +local select = select +local type = type +local string = string +local local_conf + + +local timer +local plugin_name = "log-rotate" +local INTERVAL = 60 * 60 -- rotate interval (unit: second) +local MAX_KEPT = 24 * 7 -- max number of log files will be kept +local schema = { + type = "object", + properties = {}, + additionalProperties = false, +} + + +local _M = { + version = 0.1, + priority = 100, + name = plugin_name, + schema = schema, +} + + +local function file_exists(path) + local file = io.open(path, "r") + if file then + file:close() + end + return file ~= nil +end + + +local function get_last_index(str, key) + local rev = string.reverse(str) + local _, idx = string.find(rev, key) Review comment: Plain string find required. And better to put this function as utility function too. ########## File path: apisix/plugins/log-rotate.lua ########## @@ -0,0 +1,243 @@ +-- +-- 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 process = require("ngx.process") +local signal = require("resty.signal") +local ngx = ngx +local lfs = require("lfs") +local io = io +local os = os +local table = table +local select = select +local type = type +local string = string +local local_conf + + +local timer +local plugin_name = "log-rotate" +local INTERVAL = 60 * 60 -- rotate interval (unit: second) +local MAX_KEPT = 24 * 7 -- max number of log files will be kept +local schema = { + type = "object", + properties = {}, + additionalProperties = false, +} + + +local _M = { + version = 0.1, + priority = 100, + name = plugin_name, + schema = schema, +} + + +local function file_exists(path) + local file = io.open(path, "r") + if file then + file:close() + end + return file ~= nil +end + + +local function get_last_index(str, key) + local rev = string.reverse(str) + local _, idx = string.find(rev, key) + local n + if idx then + n = string.len(rev) - idx + 1 + end + + return n +end + + +local function get_log_path_info(file_type) + local_conf = core.config.local_conf() + local conf_path + if file_type == "error.log" then + conf_path = local_conf and local_conf.nginx_config and + local_conf.nginx_config.error_log + else + conf_path = local_conf and local_conf.nginx_config and + local_conf.nginx_config.http and + local_conf.nginx_config.http.access_log + end + + local prefix = ngx.config.prefix() + + if conf_path then + local root = string.sub(conf_path, 1, 1) + -- relative path + if root ~= "/" then + conf_path = prefix .. conf_path + end + local n = get_last_index(conf_path, "/") + if n ~= nil then + local dir = string.sub(conf_path, 1, n) + local name = string.sub(conf_path, n + 1) + return dir, name + end Review comment: Better to move the stuff above into two function: `is_abs` and `split_path` in a utility file so that other people can reuse it. ########## File path: apisix/plugins/log-rotate.lua ########## @@ -0,0 +1,243 @@ +-- +-- 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 process = require("ngx.process") +local signal = require("resty.signal") +local ngx = ngx +local lfs = require("lfs") +local io = io +local os = os +local table = table +local select = select +local type = type +local string = string +local local_conf + + +local timer +local plugin_name = "log-rotate" +local INTERVAL = 60 * 60 -- rotate interval (unit: second) +local MAX_KEPT = 24 * 7 -- max number of log files will be kept +local schema = { + type = "object", + properties = {}, + additionalProperties = false, +} + + +local _M = { + version = 0.1, + priority = 100, + name = plugin_name, + schema = schema, +} + + +local function file_exists(path) + local file = io.open(path, "r") + if file then + file:close() + end + return file ~= nil +end + + +local function get_last_index(str, key) + local rev = string.reverse(str) + local _, idx = string.find(rev, key) + local n + if idx then + n = string.len(rev) - idx + 1 + end + + return n +end + + +local function get_log_path_info(file_type) + local_conf = core.config.local_conf() + local conf_path + if file_type == "error.log" then + conf_path = local_conf and local_conf.nginx_config and + local_conf.nginx_config.error_log + else + conf_path = local_conf and local_conf.nginx_config and + local_conf.nginx_config.http and + local_conf.nginx_config.http.access_log + end + + local prefix = ngx.config.prefix() + + if conf_path then + local root = string.sub(conf_path, 1, 1) + -- relative path + if root ~= "/" then + conf_path = prefix .. conf_path + end + local n = get_last_index(conf_path, "/") + if n ~= nil then + local dir = string.sub(conf_path, 1, n) + local name = string.sub(conf_path, n + 1) Review comment: if n == #conf_path, the filename is empty. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
