Skip to content

Commit

Permalink
perf(request-id): use $kong_request_id for $request_id for better…
Browse files Browse the repository at this point in the history
… performance (#11725)

This has better performance than `$request_id`.

KAG-2734
  • Loading branch information
chronolaw authored and samugi committed Nov 20, 2023
1 parent ef7ec33 commit 6ef29b7
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 19 deletions.
2 changes: 1 addition & 1 deletion changelog/unreleased/kong/lua_kong_nginx_module_bump.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
message: Bump lua-kong-nginx-module from 0.6.0 to 0.7.1
message: Bump lua-kong-nginx-module from 0.6.0 to 0.8.0
type: dependency
scope: Core
5 changes: 3 additions & 2 deletions kong/plugins/aws-lambda/request-util.lua
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ local ngx_decode_base64 = ngx.decode_base64
local cjson = require "cjson.safe"

local pl_stringx = require("pl.stringx")
local date = require "date"
local date = require("date")
local get_request_id = require("kong.tracing.request_id").get

local EMPTY = {}

Expand Down Expand Up @@ -221,7 +222,7 @@ local function aws_serializer(ctx, config)
sourceIp = var.realip_remote_addr or var.remote_addr,
userAgent = headers["user-agent"],
}
local requestId = var.request_id
local requestId = get_request_id()
local start_time = ngx_req_start_time()
-- The CLF-formatted request time (dd/MMM/yyyy:HH:mm:ss +-hhmm).
local requestTime = date(start_time):fmt("%d/%b/%Y:%H:%M:%S %z")
Expand Down
4 changes: 2 additions & 2 deletions kong/templates/nginx_kong.lua
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ exit_worker_by_lua_block {
log_format kong_log_format '$remote_addr - $remote_user [$time_local] '
'"$request" $status $body_bytes_sent '
'"$http_referer" "$http_user_agent" '
'kong_request_id: "$request_id"';
'kong_request_id: "$kong_request_id"';
# Load variable indexes
lua_kong_load_var_index default;
Expand Down Expand Up @@ -79,7 +79,7 @@ server {
# Append the kong request id to the error log
# https://github.com/Kong/lua-kong-nginx-module#lua_kong_error_log_request_id
lua_kong_error_log_request_id $request_id;
lua_kong_error_log_request_id $kong_request_id;
access_log ${{PROXY_ACCESS_LOG}} kong_log_format;
error_log ${{PROXY_ERROR_LOG}} ${{LOG_LEVEL}};
Expand Down
8 changes: 5 additions & 3 deletions kong/tracing/request_id.lua
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
local ngx = ngx
local var = ngx.var
local get_phase = ngx.get_phase

local NGX_VAR_PHASES = {
set = true,
Expand All @@ -21,14 +23,14 @@ local function get()
local rid = get_ctx_request_id()

if not rid then
local phase = ngx.get_phase()
local phase = get_phase()
if not NGX_VAR_PHASES[phase] then
return nil, "cannot access ngx.var in " .. phase .. " phase"
end

-- first access to the request id for this request:
-- initialize with the value of $request_id
rid = ngx.var.request_id
-- initialize with the value of $kong_request_id
rid = var.kong_request_id
ngx.ctx.request_id = rid
end

Expand Down
2 changes: 1 addition & 1 deletion spec/01-unit/10-log_serializer_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe("kong.log.serialize", function()
},
},
var = {
request_id = "1234",
kong_request_id = "1234",
request_uri = "/request_uri",
upstream_uri = "/upstream_uri",
scheme = "http",
Expand Down
13 changes: 11 additions & 2 deletions spec/01-unit/26-tracing/03-request-id_spec.lua
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
local request_id = require "kong.tracing.request_id"
local function reload_module(name)
package.loaded[name] = nil
return require(name)
end


local function reset_globals(id)
_G.ngx.ctx = {}
_G.ngx.var = {
request_id = id,
kong_request_id = id,
}
_G.ngx.get_phase = function() -- luacheck: ignore
return "access"
Expand Down Expand Up @@ -43,6 +47,9 @@ describe("Request ID unit tests", function()
end)

it("returns the expected Request ID and caches it in ctx", function()

local request_id = reload_module("kong.tracing.request_id")

local id, err = request_id.get()
assert.is_nil(err)
assert.equal(ngx_var_request_id, id)
Expand All @@ -54,6 +61,8 @@ describe("Request ID unit tests", function()
it("fails if accessed from phase that cannot read ngx.var", function()
_G.ngx.get_phase = function() return "init" end

local request_id = reload_module("kong.tracing.request_id")

local id, err = request_id.get()
assert.is_nil(id)
assert.equal("cannot access ngx.var in init phase", err)
Expand Down
29 changes: 21 additions & 8 deletions spec/03-plugins/27-aws-lambda/05-aws-serializer_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ describe("[AWS Lambda] aws-gateway input", function()
local old_ngx
local aws_serialize


local function reload_module()
-- make sure to reload the module
package.loaded["kong.tracing.request_id"] = nil
package.loaded["kong.plugins.aws-lambda.request-util"] = nil
aws_serialize = require "kong.plugins.aws-lambda.request-util".aws_serializer
end


setup(function()
old_ngx = ngx
local body_data
Expand All @@ -20,18 +29,16 @@ describe("[AWS Lambda] aws-gateway input", function()
start_time = function() return mock_request.start_time end,
},
log = function() end,
get_phase = function() -- luacheck: ignore
return "access"
end,
encode_base64 = old_ngx.encode_base64
}, {
-- look up any unknown key in the mock request, eg. .var and .ctx tables
__index = function(self, key)
return mock_request and mock_request[key]
end,
})


-- make sure to reload the module
package.loaded["kong.plugins.aws-lambda.request-util"] = nil
aws_serialize = require "kong.plugins.aws-lambda.request-util".aws_serializer
end)

teardown(function()
Expand Down Expand Up @@ -60,7 +67,7 @@ describe("[AWS Lambda] aws-gateway input", function()
var = {
request_method = "GET",
upstream_uri = "/123/strip/more?boolean=;multi-query=first;single-query=hello%20world;multi-query=second",
request_id = "1234567890",
kong_request_id = "1234567890",
host = "abc.myhost.com",
remote_addr = "123.123.123.123"
},
Expand All @@ -76,6 +83,8 @@ describe("[AWS Lambda] aws-gateway input", function()
},
}

reload_module()

local out = aws_serialize()

assert.same({
Expand Down Expand Up @@ -140,7 +149,7 @@ describe("[AWS Lambda] aws-gateway input", function()
var = {
request_method = "GET",
upstream_uri = "/plain/strip/more?boolean=;multi-query=first;single-query=hello%20world;multi-query=second",
request_id = "1234567890",
kong_request_id = "1234567890",
host = "def.myhost.com",
remote_addr = "123.123.123.123"
},
Expand All @@ -151,6 +160,8 @@ describe("[AWS Lambda] aws-gateway input", function()
},
}

reload_module()

local out = aws_serialize()

assert.same({
Expand Down Expand Up @@ -235,7 +246,7 @@ describe("[AWS Lambda] aws-gateway input", function()
request_method = "GET",
upstream_uri = "/plain/strip/more",
http_content_type = tdata.ct,
request_id = "1234567890",
kong_request_id = "1234567890",
host = "def.myhost.com",
remote_addr = "123.123.123.123"
},
Expand All @@ -246,6 +257,8 @@ describe("[AWS Lambda] aws-gateway input", function()
},
}

reload_module()

local out = aws_serialize()

assert.same({
Expand Down

0 comments on commit 6ef29b7

Please sign in to comment.