Skip to content

Commit

Permalink
perf(tracing): do not create spans in timer phase
Browse files Browse the repository at this point in the history
Before this change timers would generate spans, which means DB and DNS
spans in recurring timers would be continuously generated and
garbage-collected.

This commit checks the exact ngx phase and runs it against a whitelist
to ensure `timer` phase does not generate spans.
  • Loading branch information
samugi committed Nov 23, 2023
1 parent cc6f139 commit aa7074f
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 6 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/kong/perf-tracing-from-timers.yml
@@ -0,0 +1,3 @@
message: "Performance optimization to avoid unnecessary creations and garbage-collections of spans"
type: "performance"
scope: "PDK"
16 changes: 12 additions & 4 deletions kong/pdk/tracing.lua
Expand Up @@ -9,7 +9,6 @@ local require = require
local ffi = require "ffi"
local tablepool = require "tablepool"
local new_tab = require "table.new"
local base = require "resty.core.base"
local utils = require "kong.tools.utils"
local phase_checker = require "kong.pdk.private.phases"

Expand Down Expand Up @@ -421,6 +420,15 @@ noop_tracer.set_active_span = NOOP
noop_tracer.process_span = NOOP
noop_tracer.set_should_sample = NOOP

local VALID_TRACING_PHASES = {
rewrite = true,
access = true,
header_filter = true,
body_filter = true,
log = true,
content = true,
}

--- New Tracer
local function new_tracer(name, options)
name = name or "default"
Expand Down Expand Up @@ -450,7 +458,7 @@ local function new_tracer(name, options)
-- @phases rewrite, access, header_filter, response, body_filter, log, admin_api
-- @treturn table span
function self.active_span()
if not base.get_request() then
if not VALID_TRACING_PHASES[ngx.get_phase()] then
return
end

Expand All @@ -463,7 +471,7 @@ local function new_tracer(name, options)
-- @phases rewrite, access, header_filter, response, body_filter, log, admin_api
-- @tparam table span
function self.set_active_span(span)
if not base.get_request() then
if not VALID_TRACING_PHASES[ngx.get_phase()] then
return
end

Expand All @@ -482,7 +490,7 @@ local function new_tracer(name, options)
-- @tparam table options TODO(mayo)
-- @treturn table span
function self.start_span(...)
if not base.get_request() then
if not VALID_TRACING_PHASES[ngx.get_phase()] then
return noop_span
end

Expand Down
10 changes: 8 additions & 2 deletions spec/01-unit/26-tracing/01-tracer_pdk_spec.lua
Expand Up @@ -49,17 +49,23 @@ end
local unhook_log_spy = debug.sethook

describe("Tracer PDK", function()
local ok, err, _
local ok, err, old_ngx_get_phase, _
local log_spy

lazy_setup(function()
local kong_global = require "kong.global"
_G.kong = kong_global.new()
kong_global.init_pdk(kong)
log_spy = hook_log_spy()
old_ngx_get_phase = ngx.get_phase
-- trick the pdk into thinking we are not in the timer context
_G.ngx.get_phase = function() return "access" end -- luacheck: ignore
end)

lazy_teardown(unhook_log_spy)
lazy_teardown(function()
unhook_log_spy()
_G.ngx.get_phase = old_ngx_get_phase -- luacheck: ignore
end)

describe("initialize tracer", function()

Expand Down
6 changes: 6 additions & 0 deletions spec/03-plugins/37-opentelemetry/01-otlp_spec.lua
Expand Up @@ -44,16 +44,22 @@ local pb_decode_span = function(data)
end

describe("Plugin: opentelemetry (otlp)", function()
local old_ngx_get_phase

lazy_setup(function ()
-- overwrite for testing
pb.option("enum_as_value")
pb.option("auto_default_values")
old_ngx_get_phase = ngx.get_phase
-- trick the pdk into thinking we are not in the timer context
_G.ngx.get_phase = function() return "access" end -- luacheck: ignore
end)

lazy_teardown(function()
-- revert it back
pb.option("enum_as_name")
pb.option("no_default_values")
_G.ngx.get_phase = old_ngx_get_phase -- luacheck: ignore
end)

after_each(function ()
Expand Down

0 comments on commit aa7074f

Please sign in to comment.