From 164807a74b41e318bea2f0c2cdfc6ddaccc8be70 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 2 Dec 2024 13:19:58 +0530 Subject: [PATCH 01/11] first commit --- src/ngx_http_apisix_log_handler.c | 16 ++++++++++++++++ src/ngx_http_apisix_module.c | 15 +++++++++++---- src/ngx_http_apisix_module.h | 3 +++ 3 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 src/ngx_http_apisix_log_handler.c diff --git a/src/ngx_http_apisix_log_handler.c b/src/ngx_http_apisix_log_handler.c new file mode 100644 index 0000000..6936ee8 --- /dev/null +++ b/src/ngx_http_apisix_log_handler.c @@ -0,0 +1,16 @@ +#include "ngx_http_apisix_module.h" + +char * +ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) +{ + ngx_str_t *value; + ngx_http_apisix_loc_conf_t *loc_conf = conf; + + value = cf->args->elts; + if (value[1].data[0] != '$') { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid variable name \"%V\"", &value[1]); + return NGX_CONF_ERROR; + } + + value[1].len--; +} diff --git a/src/ngx_http_apisix_module.c b/src/ngx_http_apisix_module.c index 024e5ea..8dfa583 100644 --- a/src/ngx_http_apisix_module.c +++ b/src/ngx_http_apisix_module.c @@ -1,9 +1,9 @@ #include #include #include -#include +#include #include "ngx_http_apisix_module.h" - +#include "ngx_http_apisix_log_handler.c" #define NGX_HTTP_APISIX_SSL_ENC 1 #define NGX_HTTP_APISIX_SSL_SIGN 2 @@ -33,7 +33,13 @@ static ngx_command_t ngx_http_apisix_cmds[] = { NGX_HTTP_LOC_CONF_OFFSET, offsetof(ngx_http_apisix_loc_conf_t, delay_client_max_body_check), NULL }, - + { ngx_string("apisix_error_log_request_id"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, + ngx_http_apisix_error_log_request_id, //TODO: Implement this setup function + NGX_HTTP_LOC_CONF_OFFSET, + offsetof(ngx_http_apisix_loc_conf_t, request_id_var_index), + NULL + }, ngx_null_command }; @@ -100,7 +106,7 @@ ngx_http_apisix_create_loc_conf(ngx_conf_t *cf) } conf->delay_client_max_body_check = NGX_CONF_UNSET; - + conf->request_id_var_index = NGX_CONF_UNSET; return conf; } @@ -111,6 +117,7 @@ ngx_http_apisix_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) ngx_http_apisix_loc_conf_t *prev = parent; ngx_http_apisix_loc_conf_t *conf = child; + ngx_conf_merge_value(conf->request_id_var_index, prev->request_id_var_index, NGX_CONF_UNSET); ngx_conf_merge_value(conf->delay_client_max_body_check, prev->delay_client_max_body_check, 0); diff --git a/src/ngx_http_apisix_module.h b/src/ngx_http_apisix_module.h index a363e8f..5d42941 100644 --- a/src/ngx_http_apisix_module.h +++ b/src/ngx_http_apisix_module.h @@ -7,6 +7,8 @@ typedef struct { ngx_flag_t delay_client_max_body_check; + ngx_int_t request_id_var_index; + } ngx_http_apisix_loc_conf_t; @@ -57,5 +59,6 @@ ngx_int_t ngx_http_apisix_is_body_filter_by_lua_skipped(ngx_http_request_t *r); ngx_flag_t ngx_http_apisix_is_ntls_enabled(ngx_http_conf_ctx_t *conf_ctx); +char * ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); #endif /* _NGX_HTTP_APISIX_H_INCLUDED_ */ From e6198517eb8724ebc3ea2b8e72db1147db14a7b0 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 2 Dec 2024 15:37:15 +0530 Subject: [PATCH 02/11] add log handler module --- config | 1 + src/ngx_http_apisix_log_handler.c | 114 ++++++++++++++++++++++++++++++ src/ngx_http_apisix_module.c | 26 ++++--- src/ngx_http_apisix_module.h | 6 +- t/request-id-err-log.t | 24 +++++++ 5 files changed, 158 insertions(+), 13 deletions(-) create mode 100644 t/request-id-err-log.t diff --git a/config b/config index 08e5616..0a2e547 100644 --- a/config +++ b/config @@ -1,6 +1,7 @@ ngx_module_type=HTTP ngx_module_name=ngx_http_apisix_module ngx_module_srcs="$ngx_addon_dir/src/ngx_http_apisix_module.c" +ngx_module_srcs="$ngx_addon_dir/src/ngx_http_apisix_log_handler.c" ngx_module_deps=$ngx_addon_dir/src/ngx_http_apisix_module.h ngx_module_incs="$ngx_addon_dir/src" diff --git a/src/ngx_http_apisix_log_handler.c b/src/ngx_http_apisix_log_handler.c index 6936ee8..e4885d7 100644 --- a/src/ngx_http_apisix_log_handler.c +++ b/src/ngx_http_apisix_log_handler.c @@ -1,5 +1,111 @@ #include "ngx_http_apisix_module.h" +#include +#include +#include +/* + * This function contains the logic to append the Request ID to + * the error log line when being called. + * Get the location configuration from helper function. Find indexed variable with the loc_conf->request_id_var_index. and add that to buffer. + */ +static u_char* +ngx_http_apisix_error_log_handler(ngx_http_request_t *r, u_char *buf, size_t len) +{ + ngx_http_variable_value_t *request_id_var; + ngx_http_apisix_loc_conf_t *loc_conf; + + loc_conf = ngx_http_get_module_loc_conf(r, ngx_http_apisix_module); + if (loc_conf->request_id_var_index == NGX_CONF_UNSET) { + return buf; + } + + request_id_var = ngx_http_get_indexed_variable(r, loc_conf->request_id_var_index); + if (request_id_var == NULL || request_id_var->not_found) { + return buf; + } + buf = ngx_snprintf(buf, len, ", request_id: \"%v\"", request_id_var); + return buf; +} + + +/* + * This function replaces the original HTTP error + * log handler (r->log_handler). It executes the original logic + * and then our error log handler: ngx_http_apisix_error_log_handler + * This function returns the final message. + */ +static u_char* +ngx_http_apisix_combined_error_log_handler(ngx_http_request_t *r, ngx_http_request_t *sr, u_char *buf, size_t len) +{ + u_char *p; + ngx_http_apisix_ctx_t *ctx; + + ctx = ngx_http_apisix_get_module_ctx(r); + if (ctx == NULL || ctx->orig_log_handler == NULL) { + return buf; + } + + //Get the original log message + p = ctx->orig_log_handler(r, sr, buf, len); + //p - buf calculates the number of bytes written by the original log handler into the buffer. + //len -= (p - buf) reduces the remaining buffer length by the amount already used. + len -= p-buf; + + //Apisix log handler + buf = ngx_http_apisix_error_log_handler(r, buf, len); + return buf; +} + + +//It replaces the r->log_handler which is the log handler of the request with the combined log handler. +// Creates the apisix context we need from the request to act on it. +static ngx_int_t +ngx_http_apisix_replace_error_log_handler(ngx_http_request_t *r) +{ + ngx_http_apisix_ctx_t *ctx; + ctx = ngx_http_apisix_get_module_ctx(r); + if (ctx == NULL) { + return NGX_OK; + } + + if (r->log_handler == NULL){ + return NGX_DECLINED; + } + + /* + * Store the original log handler in ctx->orig_log_handler, replace + * it with the combined log handler, which will execute the original + * handler's logic in addition to our own. + */ + ctx->orig_log_handler = r->log_handler; + r->log_handler = ngx_http_apisix_combined_error_log_handler; + + return NGX_DECLINED; +} + +//This function is part of postconfiguration passed to module context and will override the post_read_phase with custom log handler. +// It extracts the pointer to log handler from the post read phase handlers and then override that with new function address. +char * +ngx_http_apisix_error_log_init(ngx_conf_t *cf) +{ + ngx_http_handler_pt *h; + ngx_http_core_main_conf_t *cmcf; + + cmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_core_module); + h = ngx_array_push(&cmcf->phases[NGX_HTTP_POST_READ_PHASE].handlers); + if (h == NULL) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "failed setting error log handler"); + return NGX_CONF_ERROR; + } + + *h = ngx_http_apisix_replace_error_log_handler; + + return NGX_CONF_OK; +} + +// This function does the translation of the configuration file to the internal representation. +// So this will just set the value in loc_conf that it gets by reference. char * ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { @@ -13,4 +119,12 @@ ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *c } value[1].len--; + value[1].data++; + + loc_conf->request_id_var_index = ngx_http_get_variable_index(cf, &value[1]); + if (loc_conf->request_id_var_index == NGX_ERROR) { + return NGX_CONF_ERROR; + } + + return NGX_CONF_OK; } diff --git a/src/ngx_http_apisix_module.c b/src/ngx_http_apisix_module.c index 8dfa583..211d47a 100644 --- a/src/ngx_http_apisix_module.c +++ b/src/ngx_http_apisix_module.c @@ -1,4 +1,4 @@ -#include + #include #include #include @@ -19,12 +19,20 @@ static ngx_str_t remote_port = ngx_string("remote_port"); static ngx_str_t realip_remote_addr = ngx_string("realip_remote_addr"); static ngx_str_t realip_remote_port = ngx_string("realip_remote_port"); - +static ngx_int_t ngx_http_apisix_init(ngx_conf_t *cf); static void *ngx_http_apisix_create_main_conf(ngx_conf_t *cf); static void *ngx_http_apisix_create_loc_conf(ngx_conf_t *cf); static char *ngx_http_apisix_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child); +static ngx_int_t +ngx_http_apisix_init(ngx_conf_t *cf) +{ + if (ngx_http_apisix_error_log_init(cf) != NGX_CONF_OK) { + return NGX_ERROR; + } + return NGX_OK; +} static ngx_command_t ngx_http_apisix_cmds[] = { { ngx_string("apisix_delay_client_max_body_check"), @@ -33,20 +41,19 @@ static ngx_command_t ngx_http_apisix_cmds[] = { NGX_HTTP_LOC_CONF_OFFSET, offsetof(ngx_http_apisix_loc_conf_t, delay_client_max_body_check), NULL }, - { ngx_string("apisix_error_log_request_id"), + { + ngx_string("apisix_request_id_var"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, - ngx_http_apisix_error_log_request_id, //TODO: Implement this setup function + ngx_http_apisix_error_log_request_id, NGX_HTTP_LOC_CONF_OFFSET, offsetof(ngx_http_apisix_loc_conf_t, request_id_var_index), NULL - }, + }, ngx_null_command }; - - static ngx_http_module_t ngx_http_apisix_module_ctx = { NULL, /* preconfiguration */ - NULL, /* postconfiguration */ + ngx_http_apisix_init, /* postconfiguration */ ngx_http_apisix_create_main_conf, /* create main configuration */ NULL, /* init main configuration */ @@ -74,7 +81,6 @@ ngx_module_t ngx_http_apisix_module = { NGX_MODULE_V1_PADDING }; - static void * ngx_http_apisix_create_main_conf(ngx_conf_t *cf) { @@ -94,7 +100,7 @@ ngx_http_apisix_create_main_conf(ngx_conf_t *cf) return acf; } - +//These below functions are responsible for translating genertic configuration into our specific struct. static void * ngx_http_apisix_create_loc_conf(ngx_conf_t *cf) { diff --git a/src/ngx_http_apisix_module.h b/src/ngx_http_apisix_module.h index 5d42941..dce2f11 100644 --- a/src/ngx_http_apisix_module.h +++ b/src/ngx_http_apisix_module.h @@ -4,7 +4,6 @@ #include - typedef struct { ngx_flag_t delay_client_max_body_check; ngx_int_t request_id_var_index; @@ -26,7 +25,7 @@ typedef struct { off_t client_max_body_size; ngx_http_apisix_gzip_t *gzip; - + ngx_http_log_handler_pt orig_log_handler; unsigned client_max_body_size_set:1; unsigned mirror_enabled:1; unsigned request_buffering:1; @@ -60,5 +59,6 @@ ngx_int_t ngx_http_apisix_is_body_filter_by_lua_skipped(ngx_http_request_t *r); ngx_flag_t ngx_http_apisix_is_ntls_enabled(ngx_http_conf_ctx_t *conf_ctx); char * ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); - +char * ngx_http_apisix_error_log_init(ngx_conf_t *cf); +char * ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); #endif /* _NGX_HTTP_APISIX_H_INCLUDED_ */ diff --git a/t/request-id-err-log.t b/t/request-id-err-log.t new file mode 100644 index 0000000..50de0b0 --- /dev/null +++ b/t/request-id-err-log.t @@ -0,0 +1,24 @@ +use t::APISIX_NGINX 'no_plan'; + +run_tests; + +__DATA__ + +=== TEST 1: global client_max_body_size set +--- config + location /t { + set $request_id 1234; + apisix_request_id_var $request_id; + content_by_lua_block { + ngx.log(ngx.INFO, "log_msg") + ngx.exit(200) + } + } +--- request +GET /test +--- error_log eval +qr/log_msg.*request_id: "1234"$/ +--- no_error_log +[error] +[crit] +[alert] From 41582ef6f7de355199cc5ff8bf10cd0d74d11945 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 2 Dec 2024 15:41:55 +0530 Subject: [PATCH 03/11] fix test name --- t/request-id-err-log.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/request-id-err-log.t b/t/request-id-err-log.t index 50de0b0..206e083 100644 --- a/t/request-id-err-log.t +++ b/t/request-id-err-log.t @@ -4,7 +4,7 @@ run_tests; __DATA__ -=== TEST 1: global client_max_body_size set +=== TEST 1: request_id in error log set --- config location /t { set $request_id 1234; From a1f7c3abeca7a0da40c7703131a9237f4e092bed Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 2 Dec 2024 17:24:50 +0530 Subject: [PATCH 04/11] final --- config | 1 - src/ngx_http_apisix_log_handler.c | 130 ----------------------------- src/ngx_http_apisix_module.c | 134 +++++++++++++++++++++++++++++- 3 files changed, 132 insertions(+), 133 deletions(-) delete mode 100644 src/ngx_http_apisix_log_handler.c diff --git a/config b/config index 0a2e547..08e5616 100644 --- a/config +++ b/config @@ -1,7 +1,6 @@ ngx_module_type=HTTP ngx_module_name=ngx_http_apisix_module ngx_module_srcs="$ngx_addon_dir/src/ngx_http_apisix_module.c" -ngx_module_srcs="$ngx_addon_dir/src/ngx_http_apisix_log_handler.c" ngx_module_deps=$ngx_addon_dir/src/ngx_http_apisix_module.h ngx_module_incs="$ngx_addon_dir/src" diff --git a/src/ngx_http_apisix_log_handler.c b/src/ngx_http_apisix_log_handler.c deleted file mode 100644 index e4885d7..0000000 --- a/src/ngx_http_apisix_log_handler.c +++ /dev/null @@ -1,130 +0,0 @@ -#include "ngx_http_apisix_module.h" -#include -#include -#include -/* - * This function contains the logic to append the Request ID to - * the error log line when being called. - * Get the location configuration from helper function. Find indexed variable with the loc_conf->request_id_var_index. and add that to buffer. - */ -static u_char* -ngx_http_apisix_error_log_handler(ngx_http_request_t *r, u_char *buf, size_t len) -{ - ngx_http_variable_value_t *request_id_var; - ngx_http_apisix_loc_conf_t *loc_conf; - - loc_conf = ngx_http_get_module_loc_conf(r, ngx_http_apisix_module); - if (loc_conf->request_id_var_index == NGX_CONF_UNSET) { - return buf; - } - - request_id_var = ngx_http_get_indexed_variable(r, loc_conf->request_id_var_index); - if (request_id_var == NULL || request_id_var->not_found) { - return buf; - } - buf = ngx_snprintf(buf, len, ", request_id: \"%v\"", request_id_var); - return buf; -} - - -/* - * This function replaces the original HTTP error - * log handler (r->log_handler). It executes the original logic - * and then our error log handler: ngx_http_apisix_error_log_handler - * This function returns the final message. - */ -static u_char* -ngx_http_apisix_combined_error_log_handler(ngx_http_request_t *r, ngx_http_request_t *sr, u_char *buf, size_t len) -{ - u_char *p; - ngx_http_apisix_ctx_t *ctx; - - ctx = ngx_http_apisix_get_module_ctx(r); - if (ctx == NULL || ctx->orig_log_handler == NULL) { - return buf; - } - - //Get the original log message - p = ctx->orig_log_handler(r, sr, buf, len); - //p - buf calculates the number of bytes written by the original log handler into the buffer. - //len -= (p - buf) reduces the remaining buffer length by the amount already used. - len -= p-buf; - - //Apisix log handler - buf = ngx_http_apisix_error_log_handler(r, buf, len); - return buf; -} - - -//It replaces the r->log_handler which is the log handler of the request with the combined log handler. -// Creates the apisix context we need from the request to act on it. -static ngx_int_t -ngx_http_apisix_replace_error_log_handler(ngx_http_request_t *r) -{ - ngx_http_apisix_ctx_t *ctx; - - ctx = ngx_http_apisix_get_module_ctx(r); - if (ctx == NULL) { - return NGX_OK; - } - - if (r->log_handler == NULL){ - return NGX_DECLINED; - } - - /* - * Store the original log handler in ctx->orig_log_handler, replace - * it with the combined log handler, which will execute the original - * handler's logic in addition to our own. - */ - ctx->orig_log_handler = r->log_handler; - r->log_handler = ngx_http_apisix_combined_error_log_handler; - - return NGX_DECLINED; -} - -//This function is part of postconfiguration passed to module context and will override the post_read_phase with custom log handler. -// It extracts the pointer to log handler from the post read phase handlers and then override that with new function address. -char * -ngx_http_apisix_error_log_init(ngx_conf_t *cf) -{ - ngx_http_handler_pt *h; - ngx_http_core_main_conf_t *cmcf; - - cmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_core_module); - h = ngx_array_push(&cmcf->phases[NGX_HTTP_POST_READ_PHASE].handlers); - if (h == NULL) { - ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, - "failed setting error log handler"); - return NGX_CONF_ERROR; - } - - *h = ngx_http_apisix_replace_error_log_handler; - - return NGX_CONF_OK; -} - -// This function does the translation of the configuration file to the internal representation. -// So this will just set the value in loc_conf that it gets by reference. -char * -ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) -{ - ngx_str_t *value; - ngx_http_apisix_loc_conf_t *loc_conf = conf; - - value = cf->args->elts; - if (value[1].data[0] != '$') { - ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid variable name \"%V\"", &value[1]); - return NGX_CONF_ERROR; - } - - value[1].len--; - value[1].data++; - - loc_conf->request_id_var_index = ngx_http_get_variable_index(cf, &value[1]); - if (loc_conf->request_id_var_index == NGX_ERROR) { - return NGX_CONF_ERROR; - } - - return NGX_CONF_OK; -} diff --git a/src/ngx_http_apisix_module.c b/src/ngx_http_apisix_module.c index 211d47a..c5d78d2 100644 --- a/src/ngx_http_apisix_module.c +++ b/src/ngx_http_apisix_module.c @@ -1,9 +1,9 @@ +#include #include #include -#include +#include #include "ngx_http_apisix_module.h" -#include "ngx_http_apisix_log_handler.c" #define NGX_HTTP_APISIX_SSL_ENC 1 #define NGX_HTTP_APISIX_SSL_SIGN 2 @@ -51,6 +51,7 @@ static ngx_command_t ngx_http_apisix_cmds[] = { }, ngx_null_command }; + static ngx_http_module_t ngx_http_apisix_module_ctx = { NULL, /* preconfiguration */ ngx_http_apisix_init, /* postconfiguration */ @@ -838,3 +839,132 @@ ngx_http_apisix_is_ntls_enabled(ngx_http_conf_ctx_t *conf_ctx) acf = ngx_http_get_module_main_conf(conf_ctx, ngx_http_apisix_module); return acf->enable_ntls; } + +/*******************Log handler***************** */ +/* + * This function contains the logic to append the Request ID to + * the error log line when being called. + * Get the location configuration from helper function. Find indexed variable with the loc_conf->request_id_var_index. and add that to buffer. + */ +static u_char* +ngx_http_apisix_error_log_handler(ngx_http_request_t *r, u_char *buf, size_t len) +{ + ngx_http_variable_value_t *request_id_var; + ngx_http_apisix_loc_conf_t *loc_conf; + + loc_conf = ngx_http_get_module_loc_conf(r, ngx_http_apisix_module); + if (loc_conf->request_id_var_index == NGX_CONF_UNSET) { + return buf; + } + + request_id_var = ngx_http_get_indexed_variable(r, loc_conf->request_id_var_index); + if (request_id_var == NULL || request_id_var->not_found) { + return buf; + } + buf = ngx_snprintf(buf, len, ", request_id: \"%v\"", request_id_var); + return buf; +} + + +/* + * This function replaces the original HTTP error + * log handler (r->log_handler). It executes the original logic + * and then our error log handler: ngx_http_apisix_error_log_handler + * This function returns the final message. + */ +static u_char* +ngx_http_apisix_combined_error_log_handler(ngx_http_request_t *r, ngx_http_request_t *sr, u_char *buf, size_t len) +{ + u_char *p; + ngx_http_apisix_ctx_t *ctx; + + ctx = ngx_http_apisix_get_module_ctx(r); + if (ctx == NULL || ctx->orig_log_handler == NULL) { + return buf; + } + + //Get the original log message + p = ctx->orig_log_handler(r, sr, buf, len); + //p - buf calculates the number of bytes written by the original log handler into the buffer. + //len -= (p - buf) reduces the remaining buffer length by the amount already used. + len -= p-buf; + + //Apisix log handler + buf = ngx_http_apisix_error_log_handler(r, buf, len); + return buf; +} + + +//It replaces the r->log_handler which is the log handler of the request with the combined log handler. +// Creates the apisix context we need from the request to act on it. +static ngx_int_t +ngx_http_apisix_replace_error_log_handler(ngx_http_request_t *r) +{ + ngx_http_apisix_ctx_t *ctx; + + ctx = ngx_http_apisix_get_module_ctx(r); + if (ctx == NULL) { + return NGX_OK; + } + + if (r->log_handler == NULL){ + return NGX_DECLINED; + } + + /* + * Store the original log handler in ctx->orig_log_handler, replace + * it with the combined log handler, which will execute the original + * handler's logic in addition to our own. + */ + ctx->orig_log_handler = r->log_handler; + r->log_handler = ngx_http_apisix_combined_error_log_handler; + + return NGX_DECLINED; +} + +//This function is part of postconfiguration passed to module context and will override the post_read_phase with custom log handler. +// It extracts the pointer to log handler from the post read phase handlers and then override that with new function address. +char * +ngx_http_apisix_error_log_init(ngx_conf_t *cf) +{ + ngx_http_handler_pt *h; + ngx_http_core_main_conf_t *cmcf; + + cmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_core_module); + h = ngx_array_push(&cmcf->phases[NGX_HTTP_POST_READ_PHASE].handlers); + if (h == NULL) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "failed setting error log handler"); + return NGX_CONF_ERROR; + } + + *h = ngx_http_apisix_replace_error_log_handler; + + return NGX_CONF_OK; +} + +// This function does the translation of the configuration file to the internal representation. +// So this will just set the value in loc_conf that it gets by reference. +char * +ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) +{ + ngx_str_t *value; + ngx_http_apisix_loc_conf_t *loc_conf = conf; + + value = cf->args->elts; + if (value[1].data[0] != '$') { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid variable name \"%V\"", &value[1]); + return NGX_CONF_ERROR; + } + + value[1].len--; + value[1].data++; + + loc_conf->request_id_var_index = ngx_http_get_variable_index(cf, &value[1]); + if (loc_conf->request_id_var_index == NGX_ERROR) { + return NGX_CONF_ERROR; + } + + return NGX_CONF_OK; +} + From a86cdf6d886ea3289278db4626eeaa4969eeefa0 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 2 Dec 2024 17:29:21 +0530 Subject: [PATCH 05/11] fix test --- t/request-id-err-log.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/request-id-err-log.t b/t/request-id-err-log.t index 206e083..ad68e67 100644 --- a/t/request-id-err-log.t +++ b/t/request-id-err-log.t @@ -7,8 +7,8 @@ __DATA__ === TEST 1: request_id in error log set --- config location /t { - set $request_id 1234; - apisix_request_id_var $request_id; + set $request_id_new 1234; + apisix_request_id_var $request_id_new; content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200) From 86b3ffb2bef296a3e1ffaa7ebe84a22e4b3fcfa3 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 2 Dec 2024 17:53:11 +0530 Subject: [PATCH 06/11] add tests --- src/ngx_http_apisix_module.c | 112 ++++++++++++------------ t/request-id-err-log.t | 162 ++++++++++++++++++++++++++++++++++- 2 files changed, 217 insertions(+), 57 deletions(-) diff --git a/src/ngx_http_apisix_module.c b/src/ngx_http_apisix_module.c index c5d78d2..88ca7ac 100644 --- a/src/ngx_http_apisix_module.c +++ b/src/ngx_http_apisix_module.c @@ -854,12 +854,12 @@ ngx_http_apisix_error_log_handler(ngx_http_request_t *r, u_char *buf, size_t len loc_conf = ngx_http_get_module_loc_conf(r, ngx_http_apisix_module); if (loc_conf->request_id_var_index == NGX_CONF_UNSET) { - return buf; + return buf; } request_id_var = ngx_http_get_indexed_variable(r, loc_conf->request_id_var_index); if (request_id_var == NULL || request_id_var->not_found) { - return buf; + return buf; } buf = ngx_snprintf(buf, len, ", request_id: \"%v\"", request_id_var); return buf; @@ -875,23 +875,23 @@ ngx_http_apisix_error_log_handler(ngx_http_request_t *r, u_char *buf, size_t len static u_char* ngx_http_apisix_combined_error_log_handler(ngx_http_request_t *r, ngx_http_request_t *sr, u_char *buf, size_t len) { - u_char *p; - ngx_http_apisix_ctx_t *ctx; + u_char *p; + ngx_http_apisix_ctx_t *ctx; - ctx = ngx_http_apisix_get_module_ctx(r); - if (ctx == NULL || ctx->orig_log_handler == NULL) { - return buf; - } + ctx = ngx_http_apisix_get_module_ctx(r); + if (ctx == NULL || ctx->orig_log_handler == NULL) { + return buf; + } - //Get the original log message - p = ctx->orig_log_handler(r, sr, buf, len); - //p - buf calculates the number of bytes written by the original log handler into the buffer. - //len -= (p - buf) reduces the remaining buffer length by the amount already used. - len -= p-buf; + //Get the original log message + p = ctx->orig_log_handler(r, sr, buf, len); + //p - buf calculates the number of bytes written by the original log handler into the buffer. + //len -= (p - buf) reduces the remaining buffer length by the amount already used. + len -= p-buf; - //Apisix log handler - buf = ngx_http_apisix_error_log_handler(r, buf, len); - return buf; + //Apisix log handler + buf = ngx_http_apisix_error_log_handler(r, buf, len); + return buf; } @@ -900,26 +900,26 @@ ngx_http_apisix_combined_error_log_handler(ngx_http_request_t *r, ngx_http_reque static ngx_int_t ngx_http_apisix_replace_error_log_handler(ngx_http_request_t *r) { - ngx_http_apisix_ctx_t *ctx; + ngx_http_apisix_ctx_t *ctx; - ctx = ngx_http_apisix_get_module_ctx(r); - if (ctx == NULL) { - return NGX_OK; - } + ctx = ngx_http_apisix_get_module_ctx(r); + if (ctx == NULL) { + return NGX_OK; + } - if (r->log_handler == NULL){ - return NGX_DECLINED; - } + if (r->log_handler == NULL){ + return NGX_DECLINED; + } - /* - * Store the original log handler in ctx->orig_log_handler, replace - * it with the combined log handler, which will execute the original - * handler's logic in addition to our own. - */ - ctx->orig_log_handler = r->log_handler; - r->log_handler = ngx_http_apisix_combined_error_log_handler; +/* + * Store the original log handler in ctx->orig_log_handler, replace + * it with the combined log handler, which will execute the original + * handler's logic in addition to our own. + */ + ctx->orig_log_handler = r->log_handler; + r->log_handler = ngx_http_apisix_combined_error_log_handler; - return NGX_DECLINED; + return NGX_DECLINED; } //This function is part of postconfiguration passed to module context and will override the post_read_phase with custom log handler. @@ -927,20 +927,20 @@ ngx_http_apisix_replace_error_log_handler(ngx_http_request_t *r) char * ngx_http_apisix_error_log_init(ngx_conf_t *cf) { - ngx_http_handler_pt *h; - ngx_http_core_main_conf_t *cmcf; + ngx_http_handler_pt *h; + ngx_http_core_main_conf_t *cmcf; - cmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_core_module); - h = ngx_array_push(&cmcf->phases[NGX_HTTP_POST_READ_PHASE].handlers); - if (h == NULL) { - ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, - "failed setting error log handler"); - return NGX_CONF_ERROR; - } + cmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_core_module); + h = ngx_array_push(&cmcf->phases[NGX_HTTP_POST_READ_PHASE].handlers); + if (h == NULL) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "failed setting error log handler"); + return NGX_CONF_ERROR; +} - *h = ngx_http_apisix_replace_error_log_handler; + *h = ngx_http_apisix_replace_error_log_handler; - return NGX_CONF_OK; + return NGX_CONF_OK; } // This function does the translation of the configuration file to the internal representation. @@ -949,22 +949,22 @@ char * ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { ngx_str_t *value; - ngx_http_apisix_loc_conf_t *loc_conf = conf; + ngx_http_apisix_loc_conf_t *loc_conf = conf; - value = cf->args->elts; - if (value[1].data[0] != '$') { - ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid variable name \"%V\"", &value[1]); - return NGX_CONF_ERROR; - } + value = cf->args->elts; + if (value[1].data[0] != '$') { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid variable name \"%V\"", &value[1]); + return NGX_CONF_ERROR; + } - value[1].len--; - value[1].data++; + value[1].len--; + value[1].data++; - loc_conf->request_id_var_index = ngx_http_get_variable_index(cf, &value[1]); - if (loc_conf->request_id_var_index == NGX_ERROR) { - return NGX_CONF_ERROR; - } + loc_conf->request_id_var_index = ngx_http_get_variable_index(cf, &value[1]); + if (loc_conf->request_id_var_index == NGX_ERROR) { + return NGX_CONF_ERROR; + } - return NGX_CONF_OK; + return NGX_CONF_OK; } diff --git a/t/request-id-err-log.t b/t/request-id-err-log.t index ad68e67..69b6960 100644 --- a/t/request-id-err-log.t +++ b/t/request-id-err-log.t @@ -15,10 +15,170 @@ __DATA__ } } --- request -GET /test +GET /t --- error_log eval qr/log_msg.*request_id: "1234"$/ --- no_error_log [error] [crit] [alert] + + + +=== TEST 2: request_id in error log set when a runtime error occurs +--- config + location /t { + set $request_id_new 1234; + apisix_request_id_var $request_id_new; + content_by_lua_block { + error("error_message") + } + } +--- request +GET /t +--- error_code: 500 +--- error_log eval +qr/.*request_id: "1234".*$/ + + + +=== TEST 3: scoping: value is appended correctly to error logs +based on the location where the directive is defined +--- config + location /append_req_id { + set $req_id_a 123456; + apisix_request_id_var $req_id_a; + content_by_lua_block { + ngx.log(ngx.INFO, "log_msg") + ngx.exit(200) + } + } + location /append_method { + set $req_id_b 654321; + apisix_request_id_var $req_id_b; + + content_by_lua_block { + ngx.log(ngx.INFO, "log_msg") + ngx.exit(200) + } + } +--- pipelined_requests eval +["GET /append_req_id", "GET /append_method"] +--- error_code eval +[200, 200, 200] +--- error_log eval +[ 'request_id: "123456"', 'request_id: "654321"' ] +--- no_error_log +[error] +[crit] +[alert] + + + +=== TEST 4: scoping: value is NOT appended to error logs for the location where the directive is NOT defined +--- config + location /append { + set $req_id 123456; + apisix_request_id_var $req_id; + content_by_lua_block { + ngx.log(ngx.ERR, "log_msg") + ngx.exit(200) + } + } + location /no_append { + content_by_lua_block { + ngx.log(ngx.INFO, "log_msg") + ngx.exit(200) + } + } +--- request +GET /no_append +--- error_code: 200 +--- no_error_log eval +qr/log_msg.*request_id/ + + + +=== TEST 5: scoping: value is appended correctly to error logs when the directive is in the main configuration +--- http_config + apisix_request_id_var $req_id; +--- config + set $req_id 123456; + location = /test { + content_by_lua_block { + ngx.log(ngx.INFO, "log_msg") + ngx.exit(200) + } + } +--- request +GET /test +--- error_code: 200 +--- error_log eval +qr/log_msg.*request_id: "123456"$/ +--- no_error_log +[error] +[crit] +[alert] + + + +=== TEST 6: scoping: value is appended correctly to error logs and the local directive overrides the global one +--- http_config + apisix_request_id_var $req_id_global; +--- config + set $req_id_global global; + set $req_id_local local; + + location = /test { + apisix_request_id_var $req_id_local; + content_by_lua_block { + ngx.log(ngx.INFO, "log_msg") + ngx.exit(200) + } + } +--- request +GET /test +--- error_code: 200 +--- error_log eval +qr/log_msg.*request_id: "local"$/ +--- no_error_log eval +qr/log_msg.*request_id: "global"$/ + + + +=== TEST 7: Request ID variable changes are applied to the error log output +--- config + location = /test { + set $my_var ""; + apisix_request_id_var $my_var; + rewrite_by_lua_block { + ngx.log(ngx.INFO, "rewrite_0") + ngx.var.my_var = "changed_in_rewrite" + ngx.log(ngx.INFO, "rewrite_1") + ngx.var.my_var = "changed_in_rewrite_2" + ngx.log(ngx.INFO, "rewrite_2") + } + access_by_lua_block { + ngx.log(ngx.INFO, "access_0") + ngx.var.my_var = "changed_in_access" + ngx.log(ngx.INFO, "access_1") + ngx.var.my_var = "changed_in_access_2" + ngx.log(ngx.INFO, "access_2") + ngx.exit(200) + } + } +--- request +GET /test +--- error_log eval +[ + qr/rewrite_0.*request_id: ""$/, + qr/rewrite_1.*request_id: "changed_in_rewrite"$/, + qr/rewrite_2.*request_id: "changed_in_rewrite_2"$/, + qr/access_0.*request_id: "changed_in_rewrite_2"$/, + qr/access_1.*request_id: "changed_in_access"$/, + qr/access_2.*request_id: "changed_in_access_2"$/, +] +--- no_error_log +[error] +[crit] +[alert] From 3e53a25b78f9749ed14aaededd216ec8642fd27e Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 2 Dec 2024 18:36:46 +0530 Subject: [PATCH 07/11] fix tests --- src/ngx_http_apisix_module.c | 30 ++++++++++++------------ t/request-id-err-log.t | 45 +++++++++++++++++++++--------------- 2 files changed, 42 insertions(+), 33 deletions(-) diff --git a/src/ngx_http_apisix_module.c b/src/ngx_http_apisix_module.c index 88ca7ac..8dfc86e 100644 --- a/src/ngx_http_apisix_module.c +++ b/src/ngx_http_apisix_module.c @@ -849,20 +849,20 @@ ngx_http_apisix_is_ntls_enabled(ngx_http_conf_ctx_t *conf_ctx) static u_char* ngx_http_apisix_error_log_handler(ngx_http_request_t *r, u_char *buf, size_t len) { - ngx_http_variable_value_t *request_id_var; - ngx_http_apisix_loc_conf_t *loc_conf; + ngx_http_variable_value_t *request_id_var; + ngx_http_apisix_loc_conf_t *loc_conf; - loc_conf = ngx_http_get_module_loc_conf(r, ngx_http_apisix_module); - if (loc_conf->request_id_var_index == NGX_CONF_UNSET) { - return buf; - } + loc_conf = ngx_http_get_module_loc_conf(r, ngx_http_apisix_module); + if (loc_conf->request_id_var_index == NGX_CONF_UNSET) { + return buf; + } - request_id_var = ngx_http_get_indexed_variable(r, loc_conf->request_id_var_index); - if (request_id_var == NULL || request_id_var->not_found) { - return buf; - } - buf = ngx_snprintf(buf, len, ", request_id: \"%v\"", request_id_var); - return buf; + request_id_var = ngx_http_get_indexed_variable(r, loc_conf->request_id_var_index); + if (request_id_var == NULL || request_id_var->not_found) { + return buf; + } + buf = ngx_snprintf(buf, len, ", request_id: \"%v\"", request_id_var); + return buf; } @@ -904,7 +904,7 @@ ngx_http_apisix_replace_error_log_handler(ngx_http_request_t *r) ctx = ngx_http_apisix_get_module_ctx(r); if (ctx == NULL) { - return NGX_OK; + return NGX_ERROR; } if (r->log_handler == NULL){ @@ -935,8 +935,8 @@ ngx_http_apisix_error_log_init(ngx_conf_t *cf) if (h == NULL) { ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "failed setting error log handler"); - return NGX_CONF_ERROR; -} + return NGX_CONF_ERROR; + } *h = ngx_http_apisix_replace_error_log_handler; diff --git a/t/request-id-err-log.t b/t/request-id-err-log.t index 69b6960..19a69f5 100644 --- a/t/request-id-err-log.t +++ b/t/request-id-err-log.t @@ -42,32 +42,41 @@ qr/.*request_id: "1234".*$/ -=== TEST 3: scoping: value is appended correctly to error logs -based on the location where the directive is defined +=== TEST 3: scoping: value is appended correctly to error logs based on the location where the directive is defined --- config - location /append_req_id { - set $req_id_a 123456; - apisix_request_id_var $req_id_a; + location = /append_method { + set $req_id_b 654321; + apisix_request_id_var $req_id_b; + content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200) } } - location /append_method { - set $req_id_b 654321; - apisix_request_id_var $req_id_b; - + location = /append_req_id { + set $req_id_a 123456; + apisix_request_id_var $req_id_a; content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200) } } ---- pipelined_requests eval -["GET /append_req_id", "GET /append_method"] ---- error_code eval -[200, 200, 200] +--- request +GET /append_method --- error_log eval -[ 'request_id: "123456"', 'request_id: "654321"' ] +qr/log_msg.*request_id: "654321"$/ +--- no_error_log +[error] +[crit] +[alert] + + + +=== TEST 4: Send request to different location +--- request +GET /append_req_id +--- error_log eval +qr/log_msg.*request_id: "123456"$/ --- no_error_log [error] [crit] @@ -75,7 +84,7 @@ based on the location where the directive is defined -=== TEST 4: scoping: value is NOT appended to error logs for the location where the directive is NOT defined +=== TEST 5: scoping: value is NOT appended to error logs for the location where the directive is NOT defined --- config location /append { set $req_id 123456; @@ -99,7 +108,7 @@ qr/log_msg.*request_id/ -=== TEST 5: scoping: value is appended correctly to error logs when the directive is in the main configuration +=== TEST 6: scoping: value is appended correctly to error logs when the directive is in the main configuration --- http_config apisix_request_id_var $req_id; --- config @@ -122,7 +131,7 @@ qr/log_msg.*request_id: "123456"$/ -=== TEST 6: scoping: value is appended correctly to error logs and the local directive overrides the global one +=== TEST 7: scoping: value is appended correctly to error logs and the local directive overrides the global one --- http_config apisix_request_id_var $req_id_global; --- config @@ -146,7 +155,7 @@ qr/log_msg.*request_id: "global"$/ -=== TEST 7: Request ID variable changes are applied to the error log output +=== TEST 8: Request ID variable changes are applied to the error log output --- config location = /test { set $my_var ""; From 28bf785ad964acc77ede568e76c3fdee1d0296ff Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 2 Dec 2024 18:39:03 +0530 Subject: [PATCH 08/11] remove unnecessary blank line --- src/ngx_http_apisix_module.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ngx_http_apisix_module.c b/src/ngx_http_apisix_module.c index 8dfc86e..bf484d6 100644 --- a/src/ngx_http_apisix_module.c +++ b/src/ngx_http_apisix_module.c @@ -1,4 +1,3 @@ - #include #include #include From 9590efc182c11b94cd2959ff3fcbf15604fb1792 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Mon, 2 Dec 2024 21:24:20 +0530 Subject: [PATCH 09/11] remove unwanted comments --- src/ngx_http_apisix_module.c | 24 +++++------------------- src/ngx_http_apisix_module.h | 1 + 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/src/ngx_http_apisix_module.c b/src/ngx_http_apisix_module.c index bf484d6..77524cd 100644 --- a/src/ngx_http_apisix_module.c +++ b/src/ngx_http_apisix_module.c @@ -4,6 +4,7 @@ #include #include "ngx_http_apisix_module.h" + #define NGX_HTTP_APISIX_SSL_ENC 1 #define NGX_HTTP_APISIX_SSL_SIGN 2 @@ -53,7 +54,7 @@ static ngx_command_t ngx_http_apisix_cmds[] = { static ngx_http_module_t ngx_http_apisix_module_ctx = { NULL, /* preconfiguration */ - ngx_http_apisix_init, /* postconfiguration */ + ngx_http_apisix_init, /* postconfiguration */ ngx_http_apisix_create_main_conf, /* create main configuration */ NULL, /* init main configuration */ @@ -81,6 +82,7 @@ ngx_module_t ngx_http_apisix_module = { NGX_MODULE_V1_PADDING }; + static void * ngx_http_apisix_create_main_conf(ngx_conf_t *cf) { @@ -100,7 +102,6 @@ ngx_http_apisix_create_main_conf(ngx_conf_t *cf) return acf; } -//These below functions are responsible for translating genertic configuration into our specific struct. static void * ngx_http_apisix_create_loc_conf(ngx_conf_t *cf) { @@ -840,11 +841,6 @@ ngx_http_apisix_is_ntls_enabled(ngx_http_conf_ctx_t *conf_ctx) } /*******************Log handler***************** */ -/* - * This function contains the logic to append the Request ID to - * the error log line when being called. - * Get the location configuration from helper function. Find indexed variable with the loc_conf->request_id_var_index. and add that to buffer. - */ static u_char* ngx_http_apisix_error_log_handler(ngx_http_request_t *r, u_char *buf, size_t len) { @@ -865,12 +861,6 @@ ngx_http_apisix_error_log_handler(ngx_http_request_t *r, u_char *buf, size_t len } -/* - * This function replaces the original HTTP error - * log handler (r->log_handler). It executes the original logic - * and then our error log handler: ngx_http_apisix_error_log_handler - * This function returns the final message. - */ static u_char* ngx_http_apisix_combined_error_log_handler(ngx_http_request_t *r, ngx_http_request_t *sr, u_char *buf, size_t len) { @@ -894,8 +884,6 @@ ngx_http_apisix_combined_error_log_handler(ngx_http_request_t *r, ngx_http_reque } -//It replaces the r->log_handler which is the log handler of the request with the combined log handler. -// Creates the apisix context we need from the request to act on it. static ngx_int_t ngx_http_apisix_replace_error_log_handler(ngx_http_request_t *r) { @@ -921,8 +909,7 @@ ngx_http_apisix_replace_error_log_handler(ngx_http_request_t *r) return NGX_DECLINED; } -//This function is part of postconfiguration passed to module context and will override the post_read_phase with custom log handler. -// It extracts the pointer to log handler from the post read phase handlers and then override that with new function address. + char * ngx_http_apisix_error_log_init(ngx_conf_t *cf) { @@ -942,8 +929,7 @@ ngx_http_apisix_error_log_init(ngx_conf_t *cf) return NGX_CONF_OK; } -// This function does the translation of the configuration file to the internal representation. -// So this will just set the value in loc_conf that it gets by reference. + char * ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { diff --git a/src/ngx_http_apisix_module.h b/src/ngx_http_apisix_module.h index dce2f11..8e122bf 100644 --- a/src/ngx_http_apisix_module.h +++ b/src/ngx_http_apisix_module.h @@ -4,6 +4,7 @@ #include + typedef struct { ngx_flag_t delay_client_max_body_check; ngx_int_t request_id_var_index; From 2b025d967bf38d1ad9a5c6c53ea51f0f75ec70f2 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Tue, 3 Dec 2024 12:29:03 +0530 Subject: [PATCH 10/11] apply suggestions --- src/ngx_http_apisix_module.c | 18 +++++++++--------- src/ngx_http_apisix_module.h | 2 +- t/request-id-err-log.t | 23 +++++++++++------------ 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/ngx_http_apisix_module.c b/src/ngx_http_apisix_module.c index bf484d6..c0c48a0 100644 --- a/src/ngx_http_apisix_module.c +++ b/src/ngx_http_apisix_module.c @@ -41,11 +41,11 @@ static ngx_command_t ngx_http_apisix_cmds[] = { offsetof(ngx_http_apisix_loc_conf_t, delay_client_max_body_check), NULL }, { - ngx_string("apisix_request_id_var"), + ngx_string("lua_error_log_request_id"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, ngx_http_apisix_error_log_request_id, NGX_HTTP_LOC_CONF_OFFSET, - offsetof(ngx_http_apisix_loc_conf_t, request_id_var_index), + offsetof(ngx_http_apisix_loc_conf_t, apisix_request_id_var_index), NULL }, ngx_null_command @@ -112,7 +112,7 @@ ngx_http_apisix_create_loc_conf(ngx_conf_t *cf) } conf->delay_client_max_body_check = NGX_CONF_UNSET; - conf->request_id_var_index = NGX_CONF_UNSET; + conf->apisix_request_id_var_index = NGX_CONF_UNSET; return conf; } @@ -123,7 +123,7 @@ ngx_http_apisix_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) ngx_http_apisix_loc_conf_t *prev = parent; ngx_http_apisix_loc_conf_t *conf = child; - ngx_conf_merge_value(conf->request_id_var_index, prev->request_id_var_index, NGX_CONF_UNSET); + ngx_conf_merge_value(conf->apisix_request_id_var_index, prev->apisix_request_id_var_index, NGX_CONF_UNSET); ngx_conf_merge_value(conf->delay_client_max_body_check, prev->delay_client_max_body_check, 0); @@ -843,7 +843,7 @@ ngx_http_apisix_is_ntls_enabled(ngx_http_conf_ctx_t *conf_ctx) /* * This function contains the logic to append the Request ID to * the error log line when being called. - * Get the location configuration from helper function. Find indexed variable with the loc_conf->request_id_var_index. and add that to buffer. + * Get the location configuration from helper function. Find indexed variable with the loc_conf->apisix_request_id_var_index. and add that to buffer. */ static u_char* ngx_http_apisix_error_log_handler(ngx_http_request_t *r, u_char *buf, size_t len) @@ -852,11 +852,11 @@ ngx_http_apisix_error_log_handler(ngx_http_request_t *r, u_char *buf, size_t len ngx_http_apisix_loc_conf_t *loc_conf; loc_conf = ngx_http_get_module_loc_conf(r, ngx_http_apisix_module); - if (loc_conf->request_id_var_index == NGX_CONF_UNSET) { + if (loc_conf->apisix_request_id_var_index == NGX_CONF_UNSET) { return buf; } - request_id_var = ngx_http_get_indexed_variable(r, loc_conf->request_id_var_index); + request_id_var = ngx_http_get_indexed_variable(r, loc_conf->apisix_request_id_var_index); if (request_id_var == NULL || request_id_var->not_found) { return buf; } @@ -959,8 +959,8 @@ ngx_http_apisix_error_log_request_id(ngx_conf_t *cf, ngx_command_t *cmd, void *c value[1].len--; value[1].data++; - loc_conf->request_id_var_index = ngx_http_get_variable_index(cf, &value[1]); - if (loc_conf->request_id_var_index == NGX_ERROR) { + loc_conf->apisix_request_id_var_index = ngx_http_get_variable_index(cf, &value[1]); + if (loc_conf->apisix_request_id_var_index == NGX_ERROR) { return NGX_CONF_ERROR; } diff --git a/src/ngx_http_apisix_module.h b/src/ngx_http_apisix_module.h index dce2f11..3a4ffc2 100644 --- a/src/ngx_http_apisix_module.h +++ b/src/ngx_http_apisix_module.h @@ -6,7 +6,7 @@ typedef struct { ngx_flag_t delay_client_max_body_check; - ngx_int_t request_id_var_index; + ngx_int_t apisix_request_id_var_index; } ngx_http_apisix_loc_conf_t; diff --git a/t/request-id-err-log.t b/t/request-id-err-log.t index 19a69f5..b66baac 100644 --- a/t/request-id-err-log.t +++ b/t/request-id-err-log.t @@ -7,8 +7,8 @@ __DATA__ === TEST 1: request_id in error log set --- config location /t { - set $request_id_new 1234; - apisix_request_id_var $request_id_new; + set $request_id_new 1234; + lua_error_log_request_id $request_id_new; content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200) @@ -28,8 +28,8 @@ qr/log_msg.*request_id: "1234"$/ === TEST 2: request_id in error log set when a runtime error occurs --- config location /t { - set $request_id_new 1234; - apisix_request_id_var $request_id_new; + set $request_id_new 1234; + lua_error_log_request_id $request_id_new; content_by_lua_block { error("error_message") } @@ -46,8 +46,7 @@ qr/.*request_id: "1234".*$/ --- config location = /append_method { set $req_id_b 654321; - apisix_request_id_var $req_id_b; - + lua_error_log_request_id $req_id_b; content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200) @@ -55,7 +54,7 @@ qr/.*request_id: "1234".*$/ } location = /append_req_id { set $req_id_a 123456; - apisix_request_id_var $req_id_a; + lua_error_log_request_id $req_id_a; content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200) @@ -88,7 +87,7 @@ qr/log_msg.*request_id: "123456"$/ --- config location /append { set $req_id 123456; - apisix_request_id_var $req_id; + lua_error_log_request_id $req_id; content_by_lua_block { ngx.log(ngx.ERR, "log_msg") ngx.exit(200) @@ -110,7 +109,7 @@ qr/log_msg.*request_id/ === TEST 6: scoping: value is appended correctly to error logs when the directive is in the main configuration --- http_config - apisix_request_id_var $req_id; + lua_error_log_request_id $req_id; --- config set $req_id 123456; location = /test { @@ -133,13 +132,13 @@ qr/log_msg.*request_id: "123456"$/ === TEST 7: scoping: value is appended correctly to error logs and the local directive overrides the global one --- http_config - apisix_request_id_var $req_id_global; + lua_error_log_request_id $req_id_global; --- config set $req_id_global global; set $req_id_local local; location = /test { - apisix_request_id_var $req_id_local; + lua_error_log_request_id $req_id_local; content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200) @@ -159,7 +158,7 @@ qr/log_msg.*request_id: "global"$/ --- config location = /test { set $my_var ""; - apisix_request_id_var $my_var; + lua_error_log_request_id $my_var; rewrite_by_lua_block { ngx.log(ngx.INFO, "rewrite_0") ngx.var.my_var = "changed_in_rewrite" From 93203d1ce21045a5e0b9d90024b009922340a553 Mon Sep 17 00:00:00 2001 From: Ashish Tiwari Date: Tue, 3 Dec 2024 14:15:41 +0530 Subject: [PATCH 11/11] apply suggestion --- t/request-id-err-log.t | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/t/request-id-err-log.t b/t/request-id-err-log.t index b66baac..6818749 100644 --- a/t/request-id-err-log.t +++ b/t/request-id-err-log.t @@ -4,11 +4,11 @@ run_tests; __DATA__ -=== TEST 1: request_id in error log set +=== TEST 1: apisix_request_id in error log set --- config location /t { - set $request_id_new 1234; - lua_error_log_request_id $request_id_new; + set $apisix_request_id 1234; + lua_error_log_request_id $apisix_request_id; content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200) @@ -25,11 +25,11 @@ qr/log_msg.*request_id: "1234"$/ -=== TEST 2: request_id in error log set when a runtime error occurs +=== TEST 2: apisix_request_id in error log set when a runtime error occurs --- config location /t { - set $request_id_new 1234; - lua_error_log_request_id $request_id_new; + set $apisix_request_id 1234; + lua_error_log_request_id $apisix_request_id; content_by_lua_block { error("error_message") } @@ -45,16 +45,16 @@ qr/.*request_id: "1234".*$/ === TEST 3: scoping: value is appended correctly to error logs based on the location where the directive is defined --- config location = /append_method { - set $req_id_b 654321; - lua_error_log_request_id $req_id_b; + set $apisix_request_id_b 654321; + lua_error_log_request_id $apisix_request_id_b; content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200) } } location = /append_req_id { - set $req_id_a 123456; - lua_error_log_request_id $req_id_a; + set $apisix_request_id_a 123456; + lua_error_log_request_id $apisix_request_id_a; content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200) @@ -86,8 +86,8 @@ qr/log_msg.*request_id: "123456"$/ === TEST 5: scoping: value is NOT appended to error logs for the location where the directive is NOT defined --- config location /append { - set $req_id 123456; - lua_error_log_request_id $req_id; + set $apisix_request_id 123456; + lua_error_log_request_id $apisix_request_id; content_by_lua_block { ngx.log(ngx.ERR, "log_msg") ngx.exit(200) @@ -109,9 +109,9 @@ qr/log_msg.*request_id/ === TEST 6: scoping: value is appended correctly to error logs when the directive is in the main configuration --- http_config - lua_error_log_request_id $req_id; + lua_error_log_request_id $apisix_request_id; --- config - set $req_id 123456; + set $apisix_request_id 123456; location = /test { content_by_lua_block { ngx.log(ngx.INFO, "log_msg") @@ -132,13 +132,13 @@ qr/log_msg.*request_id: "123456"$/ === TEST 7: scoping: value is appended correctly to error logs and the local directive overrides the global one --- http_config - lua_error_log_request_id $req_id_global; + lua_error_log_request_id $apisix_request_id_global; --- config - set $req_id_global global; - set $req_id_local local; + set $apisix_request_id_global global; + set $apisix_request_id_local local; location = /test { - lua_error_log_request_id $req_id_local; + lua_error_log_request_id $apisix_request_id_local; content_by_lua_block { ngx.log(ngx.INFO, "log_msg") ngx.exit(200)