Re: [PR] fix: Adding request-id header in case of empty header value in request [apisix]
Baoyuantop merged PR #12837: URL: https://github.com/apache/apisix/pull/12837 -- 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]
Re: [PR] fix: Adding request-id header in case of empty header value in request [apisix]
Baoyuantop commented on code in PR #12837: URL: https://github.com/apache/apisix/pull/12837#discussion_r2734545638 ## t/plugin/request-id.t: ## @@ -505,3 +505,29 @@ X-Request-ID: 123 --- wait: 5 --- response_body true + Review Comment: ```suggestion ``` -- 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]
Re: [PR] fix: Adding request-id header in case of empty header value in request [apisix]
Baoyuantop commented on code in PR #12837: URL: https://github.com/apache/apisix/pull/12837#discussion_r2734502760 ## apisix/plugins/request-id.lua: ## @@ -102,7 +102,7 @@ end function _M.rewrite(conf, ctx) local headers = ngx.req.get_headers() local uuid_val -if not headers[conf.header_name] then +if not headers[conf.header_name] or headers[conf.header_name] == "" then Review Comment: ```suggestion local header_req_id = headers[conf.header_name] if not header_req_id or header_req_id == "" then ``` ## apisix/plugins/request-id.lua: ## @@ -120,7 +120,7 @@ function _M.header_filter(conf, ctx) end local headers = ngx.resp.get_headers() -if not headers[conf.header_name] then +if not headers[conf.header_name] or headers[conf.header_name] == "" then Review Comment: ```suggestion local header_req_id = headers[conf.header_name] if not header_req_id or header_req_id == "" then ``` -- 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]
Re: [PR] fix: Adding request-id header in case of empty header value in request [apisix]
Baoyuantop commented on code in PR #12837: URL: https://github.com/apache/apisix/pull/12837#discussion_r2719431085 ## apisix/plugins/request-id.lua: ## @@ -102,7 +102,7 @@ end function _M.rewrite(conf, ctx) local headers = ngx.req.get_headers() local uuid_val -if not headers[conf.header_name] then +if not headers[conf.header_name] or headers[conf.header_name] == "" then Review Comment: Hi @shubhammxv, there's only one minor code style suggestion; the rest is LGTM. -- 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]
Re: [PR] fix: Adding request-id header in case of empty header value in request [apisix]
Baoyuantop commented on code in PR #12837: URL: https://github.com/apache/apisix/pull/12837#discussion_r2719426988 ## apisix/plugins/request-id.lua: ## @@ -102,7 +102,7 @@ end function _M.rewrite(conf, ctx) local headers = ngx.req.get_headers() local uuid_val -if not headers[conf.header_name] then +if not headers[conf.header_name] or headers[conf.header_name] == "" then Review Comment: We can switch to a more readable style: ``` local header_req_id = headers[conf.header_name] if not header_req_id or header_req_id == "" then ... end ``` -- 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]
Re: [PR] fix: Adding request-id header in case of empty header value in request [apisix]
shubhammxv commented on PR #12837: URL: https://github.com/apache/apisix/pull/12837#issuecomment-3689245229 Hi @Baoyuantop In all the scenarios, we return `x-request-id` in response headers when `include_in_response` is true except when it comes as empty. When it is passed as empty, then no header comes back in response (empty value header gets omitted) and it gives perception as if plugin is not working. Also, the plugin's main functionality is to track the requests, But empty value will not really help track it further. -- 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]
Re: [PR] fix: Adding request-id header in case of empty header value in request [apisix]
Baoyuantop commented on PR #12837: URL: https://github.com/apache/apisix/pull/12837#issuecomment-3688452186 Hi @shubhammxv, could you explain in detail why this modification was necessary? What is your specific scenario? -- 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]
