Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(instrumentation): fix high cardinality span name #10577

Merged
merged 6 commits into from
Apr 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions kong/pdk/tracing.lua
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,34 @@ local function get_trace_id_based_sampler(rate)
end
end

-- @table span
-- @class span : table
--
--- Trace Context. Those IDs are all represented as bytes, and the length may vary.
-- We try best to preserve as much information as possible from the tracing context.
-- @field trace_id bytes auto generated 16 bytes ID if not designated
-- @field span_id bytes
-- @field parent_span_id bytes
--
--- Timing. All times are in nanoseconds.
-- @field start_time_ns number
-- @field end_time_ns number
--
--- Scopes and names. Defines what the span is about.
-- TODO: service should be retrieved from kong service instead of from plugin instances. It should be the same for spans from a single request.
-- service name/top level scope is defined by plugin instances.
-- @field name string type of the span. Should be of low cardinality. Good examples are "proxy", "DNS query", "database query". Approximately operation name of DataDog.
-- resource_name of Datadog is built from attirbutes.
--
--- Other fields
-- @field should_sample boolean whether the span should be sampled
-- @field kind number TODO: Should we remove this field? It's used by OTEL and zipkin. Maybe move this to impl_specific.
-- @field attributes table extra information about the span. Attribute of OTEL or meta of Datadog.
-- TODO: @field impl_specific table implementation specific fields. For example, impl_specific.datadog is used by Datadog tracer.
-- TODO: @field events table list of events.
--
--- Internal fields
-- @field tracer table
-- @field parent table
local span_mt = {}
span_mt.__index = span_mt

Expand Down Expand Up @@ -423,7 +450,7 @@ local function new_tracer(name, options)
--- Get the active span
-- Returns the root span by default
--
-- @function kong.tracing.new_span
-- @function kong.tracing.active_span
-- @phases rewrite, access, header_filter, response, body_filter, log, admin_api
-- @treturn table span
function self.active_span()
Expand All @@ -436,7 +463,7 @@ local function new_tracer(name, options)

--- Set the active span
--
-- @function kong.tracing.new_span
-- @function kong.tracing.set_active_span
-- @phases rewrite, access, header_filter, response, body_filter, log, admin_api
-- @tparam table span
function self.set_active_span(span)
Expand All @@ -453,7 +480,7 @@ local function new_tracer(name, options)

--- Create a new Span
--
-- @function kong.tracing.new_span
-- @function kong.tracing.start_span
-- @phases rewrite, access, header_filter, response, body_filter, log, admin_api
-- @tparam string name span name
-- @tparam table options TODO(mayo)
Expand Down
29 changes: 11 additions & 18 deletions kong/tracing/instrumentation.lua
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function _M.db_query(connector)
local f = connector.query

local function wrap(self, sql, ...)
local span = tracer.start_span("query")
local span = tracer.start_span("kong.database.query")
span:set_attribute("db.system", kong.db and kong.db.strategy)
span:set_attribute("db.statement", sql)
-- raw query
Expand All @@ -59,7 +59,7 @@ end

-- Record Router span
function _M.router()
return tracer.start_span("router")
return tracer.start_span("kong.router")
end


Expand Down Expand Up @@ -94,11 +94,12 @@ function _M.balancer(ctx)

for i = 1, try_count do
local try = balancer_tries[i]
local span_name = "balancer try #" .. i
local span_name = "kong.balancer"
local span_options = {
span_kind = 3, -- client
start_time_ns = try.balancer_start_ns,
attributes = {
["try_count"] = i,
["net.peer.ip"] = try.ip,
["net.peer.port"] = try.port,
}
Expand Down Expand Up @@ -144,7 +145,7 @@ local function plugin_callback(phase)
local plugin_name = plugin.name
local name = name_memo[plugin_name]
if not name then
name = phase .. " phase: " .. plugin_name
name = "kong." .. phase .. ".plugin." .. plugin_name
name_memo[plugin_name] = name
end

Expand Down Expand Up @@ -179,8 +180,8 @@ function _M.http_client()
attributes["http.proxy"] = http_proxy
end

local span = tracer.start_span("HTTP " .. method .. " " .. uri, {
span_kind = 3,
local span = tracer.start_span("kong.internal.request", {
samugi marked this conversation as resolved.
Show resolved Hide resolved
span_kind = 3, -- client
attributes = attributes,
})

Expand Down Expand Up @@ -209,12 +210,9 @@ _M.available_types = available_types

-- Record inbound request
function _M.request(ctx)
local req = kong.request
local client = kong.client

local method = get_method()
local path = req.get_path()
local span_name = method .. " " .. path
local scheme = ctx.scheme or var.scheme
local host = var.host
-- passing full URI to http.url attribute
Expand All @@ -229,7 +227,7 @@ function _M.request(ctx)
http_flavor = string.format("%.1f", http_flavor)
end

local active_span = tracer.start_span(span_name, {
local active_span = tracer.start_span("kong", {
samugi marked this conversation as resolved.
Show resolved Hide resolved
span_kind = 2, -- server
start_time_ns = start_time,
attributes = {
Expand All @@ -250,16 +248,11 @@ local patch_dns_query
do
local raw_func
local patch_callback
local name_memo = {}

local function wrap(host, port)
local name = name_memo[host]
if not name then
name = "DNS: " .. host
name_memo[host] = name
end

local span = tracer.start_span(name)
local span = tracer.start_span("kong.dns", {
span_kind = 3, -- client
})
local ip_addr, res_port, try_list = raw_func(host, port)
if span then
span:set_attribute("dns.record.domain", host)
Expand Down
14 changes: 7 additions & 7 deletions spec/02-integration/14-tracing/01-instrumentations_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ for _, strategy in helpers.each_strategy() do
expected_span_num = 4
end
assert.is_same(expected_span_num, #spans, res)
assert.is_same("query", spans[2].name)
assert.is_same("kong.database.query", spans[2].name)
end)
end)

Expand Down Expand Up @@ -131,7 +131,7 @@ for _, strategy in helpers.each_strategy() do
-- Making sure it's alright
local spans = cjson.decode(res)
assert.is_same(2, #spans, res)
assert.is_same("router", spans[2].name)
assert.is_same("kong.router", spans[2].name)
end)
end)

Expand Down Expand Up @@ -160,7 +160,7 @@ for _, strategy in helpers.each_strategy() do
-- Making sure it's alright
local spans = cjson.decode(res)
assert.is_same(5, #spans, res)
assert.matches("HTTP GET", spans[3].name)
assert.matches("kong.internal.request", spans[3].name)
end)
end)

Expand Down Expand Up @@ -189,7 +189,7 @@ for _, strategy in helpers.each_strategy() do
-- Making sure it's alright
local spans = cjson.decode(res)
assert.is_same(2, #spans, res)
assert.is_same("balancer try #1", spans[2].name)
assert.is_same("kong.balancer", spans[2].name)
end)
end)

Expand Down Expand Up @@ -218,7 +218,7 @@ for _, strategy in helpers.each_strategy() do
-- Making sure it's alright
local spans = cjson.decode(res)
assert.is_same(2, #spans, res)
assert.is_same("rewrite phase: " .. tcp_trace_plugin_name, spans[2].name)
assert.is_same("kong.rewrite.plugin." .. tcp_trace_plugin_name, spans[2].name)
end)
end)

Expand Down Expand Up @@ -247,7 +247,7 @@ for _, strategy in helpers.each_strategy() do
-- Making sure it's alright
local spans = cjson.decode(res)
assert.is_same(2, #spans, res)
assert.is_same("header_filter phase: " .. tcp_trace_plugin_name, spans[2].name)
assert.is_same("kong.header_filter.plugin." .. tcp_trace_plugin_name, spans[2].name)
end)
end)

Expand Down Expand Up @@ -281,7 +281,7 @@ for _, strategy in helpers.each_strategy() do

local found
for _, span in ipairs(spans) do
if span.name == "DNS: konghq.com" then
if span.name == "kong.dns" then
found = true
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/02-integration/14-tracing/02-propagation_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ for _, strategy in helpers.each_strategy() do
local spans = cjson.decode(res)
assert.is_same(2, #spans, res)
local balancer_span = spans[2]
assert.is_same("balancer try #1", balancer_span.name)
assert.is_same("kong.balancer", balancer_span.name)

local traceparent = assert(body.headers.traceparent)
local trace_id = balancer_span.trace_id
Expand Down