Skip to content

Commit

Permalink
fix(pdk): fix log serialize upstream_status entry nil bug in subrequ…
Browse files Browse the repository at this point in the history
…est case (#12953)

FTI-5844
  • Loading branch information
oowl authored Jun 5, 2024
1 parent c39b654 commit 33765ab
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
message: |
**PDK**: Fixed a bug that log serializer will log `upstream_status` as nil in the requests that contains subrequest
type: bugfix
scope: PDK
5 changes: 1 addition & 4 deletions kong/pdk/log.lua
Original file line number Diff line number Diff line change
Expand Up @@ -814,10 +814,7 @@ do
end
end

-- The value of upstream_status is a string, and status codes may be
-- seperated by comma or grouped by colon, according to
-- the nginx doc: http://nginx.org/en/docs/http/ngx_http_upstream_module.html#upstream_status
local upstream_status = var.upstream_status or ""
local upstream_status = var.upstream_status or ctx.buffered_status or ""

local response_source = okong.response.get_source(ongx.ctx)
local response_source_name = TYPE_NAMES[response_source]
Expand Down
52 changes: 52 additions & 0 deletions spec/03-plugins/04-file-log/01-log_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,36 @@ for _, strategy in helpers.each_strategy() do
},
}

local route10 = bp.routes:insert {
hosts = { "file_logging10.test" },
response_buffering = true,
}

bp.plugins:insert({
name = "pre-function",
route = { id = route10.id },
config = {
access = {
[[
kong.service.request.enable_buffering()
]],
},
}
})

bp.plugins:insert {
route = { id = route10.id },
name = "file-log",
config = {
path = FILE_LOG_PATH,
reopen = true,
custom_fields_by_lua = {
new_field = "return 123",
route = "return nil", -- unset route field
},
},
}

assert(helpers.start_kong({
database = strategy,
nginx_conf = "spec/fixtures/custom_nginx.template",
Expand Down Expand Up @@ -337,6 +367,28 @@ for _, strategy in helpers.each_strategy() do
assert.is_number(log_message.response.size)
assert.same(nil, log_message.route)
end)
it("correct upstream status when we use response phase", function()
local uuid = random_string()

-- Making the request
local res = assert(proxy_client:send({
method = "GET",
path = "/status/200",
headers = {
["file-log-uuid"] = uuid,
["Host"] = "file_logging10.test"
}
}))
assert.res_status(200, res)

local log_message = wait_for_json_log_entry()
assert.same("127.0.0.1", log_message.client_ip)
assert.same(uuid, log_message.request.headers["file-log-uuid"])
assert.is_number(log_message.request.size)
assert.is_number(log_message.response.size)
assert.same(nil, log_message.route)
assert.same(200, log_message.upstream_status)
end)
end)

it("logs to file #grpc", function()
Expand Down
7 changes: 6 additions & 1 deletion t/01-pdk/02-log/00-phase_checks.t
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,12 @@ qq{
},
response = {
get_source = function() return "service" end,
},
},
service = {
response = {
get_status = function() return 200 end,
},
},
}
}
},
Expand Down

1 comment on commit 33765ab

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bazel Build

Docker image available kong/kong:33765abea234442ac29db4fd13474c38c2ddc3f0
Artifacts available https://github.com/Kong/kong/actions/runs/9378716744

Please sign in to comment.