Skip to content

Commit

Permalink
fix(tracing): handle error when DNS query fails (#11935)
Browse files Browse the repository at this point in the history
  • Loading branch information
StarlightIbuki authored Nov 14, 2023
1 parent 2b8c69e commit 4d1fbba
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: "**tracing:** Fixed an issue where a DNS query failure would cause a tracing failure."
type: bugfix
scope: Core
7 changes: 6 additions & 1 deletion kong/tracing/instrumentation.lua
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,12 @@ do
if span then
span:set_attribute("dns.record.domain", host)
span:set_attribute("dns.record.port", port)
span:set_attribute("dns.record.ip", ip_addr)
if ip_addr then
span:set_attribute("dns.record.ip", ip_addr)
else
span:record_error(res_port)
span:set_status(2)
end
span:finish()
end

Expand Down
61 changes: 60 additions & 1 deletion spec/02-integration/14-tracing/01-instrumentations_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ for _, strategy in helpers.each_strategy() do

describe("tracing instrumentations spec #" .. strategy, function()

local function setup_instrumentations(types, custom_spans)
local function setup_instrumentations(types, custom_spans, post_func)
local bp, _ = assert(helpers.get_db_utils(strategy, {
"services",
"routes",
Expand Down Expand Up @@ -96,6 +96,10 @@ for _, strategy in helpers.each_strategy() do
}
})

if post_func then
post_func(bp)
end

assert(helpers.start_kong {
database = strategy,
nginx_conf = "spec/fixtures/custom_nginx.template",
Expand Down Expand Up @@ -512,5 +516,60 @@ for _, strategy in helpers.each_strategy() do
assert_has_spans("kong", spans, 1)
end)
end)

describe("#regression", function ()
describe("nil attribute for dns_query when fail to query", function ()
lazy_setup(function()
setup_instrumentations("dns_query", true, function(bp)
-- intentionally trigger a DNS query error
local service = bp.services:insert({
name = "inexist-host-service",
host = "really-inexist-host",
port = 80,
})

bp.routes:insert({
service = service,
protocols = { "http" },
paths = { "/test" },
})
end)
end)

lazy_teardown(function()
helpers.stop_kong()
end)

it("contains the expected kong.dns span", function ()
local thread = helpers.tcp_server(TCP_PORT)
local r = assert(proxy_client:send {
method = "GET",
path = "/test",
})
assert.res_status(503, r)

-- Getting back the TCP server input
local ok, res = thread:join()
assert.True(ok)
assert.is_string(res)

local spans = cjson.decode(res)
assert_has_spans("kong", spans)
local dns_spans = assert_has_spans("kong.dns", spans)
local upstream_dns
for _, dns_span in ipairs(dns_spans) do
if dns_span.attributes["dns.record.domain"] == "really-inexist-host" then
upstream_dns = dns_span
break
end
end

assert.is_not_nil(upstream_dns)
assert.is_nil(upstream_dns.attributes["dns.record.ip"])
-- has error reported
assert.is_not_nil(upstream_dns.events)
end)
end)
end)
end)
end

1 comment on commit 4d1fbba

@khcp-gha-bot
Copy link

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:4d1fbbad21e5c04526f776886d702de8bc997332
Artifacts available https://github.com/Kong/kong/actions/runs/6866824294

Please sign in to comment.