diff --git a/changelog/unreleased/kong/fix-opentelemetry-parent-id.yml b/changelog/unreleased/kong/fix-opentelemetry-parent-id.yml new file mode 100644 index 000000000000..5eb4c0284329 --- /dev/null +++ b/changelog/unreleased/kong/fix-opentelemetry-parent-id.yml @@ -0,0 +1,3 @@ +message: "**Opentelemetry**: fix an issue that resulted in traces with invalid parent IDs when `balancer` instrumentation was enabled" +type: bugfix +scope: Plugin diff --git a/kong/plugins/opentelemetry/handler.lua b/kong/plugins/opentelemetry/handler.lua index 96f186cdf29a..b0a4bfa67d35 100644 --- a/kong/plugins/opentelemetry/handler.lua +++ b/kong/plugins/opentelemetry/handler.lua @@ -115,16 +115,18 @@ function OpenTelemetryHandler:access(conf) -- overwrite trace id -- as we are in a chain of existing trace if trace_id then + -- to propagate the correct trace ID we have to set it here + -- before passing this span to propagation.set() injected_parent_span.trace_id = trace_id kong.ctx.plugin.trace_id = trace_id end - -- overwrite parent span's parent_id + -- overwrite root span's parent_id if span_id then - injected_parent_span.parent_id = span_id + root_span.parent_id = span_id elseif parent_id then - injected_parent_span.parent_id = parent_id + root_span.parent_id = parent_id end propagation_set(conf.header_type, header_type, injected_parent_span, "w3c") diff --git a/spec/03-plugins/37-opentelemetry/03-propagation_spec.lua b/spec/03-plugins/37-opentelemetry/03-propagation_spec.lua index 35c32a8488bf..daf0a6ee2d84 100644 --- a/spec/03-plugins/37-opentelemetry/03-propagation_spec.lua +++ b/spec/03-plugins/37-opentelemetry/03-propagation_spec.lua @@ -32,6 +32,30 @@ local function assert_has_span(name, spans) return span end +local function get_span_by_id(spans, id) + for _, span in ipairs(spans) do + if span.span_id == id then + return span + end + end +end + +local function assert_correct_trace_hierarchy(spans, incoming_span_id) + for _, span in ipairs(spans) do + if span.name == "kong" then + -- if there is an incoming span id, it should be the parent of the root span + if incoming_span_id then + assert.equals(incoming_span_id, span.parent_id) + end + + else + -- all other spans in this trace should have a local span as parent + assert.not_equals(incoming_span_id, span.parent_id) + assert.is_truthy(get_span_by_id(spans, span.parent_id)) + end + end +end + for _, strategy in helpers.each_strategy() do describe("propagation tests #" .. strategy, function() local service @@ -321,7 +345,7 @@ describe("propagation tests #" .. strategy, function() end) end) -for _, instrumentation in ipairs({ "request", "request,balancer" }) do +for _, instrumentation in ipairs({ "request", "request,balancer", "all" }) do describe("propagation tests with enabled " .. instrumentation .. " instrumentation (issue #11294) #" .. strategy, function() local service, route local proxy_client @@ -370,12 +394,12 @@ describe("propagation tests with enabled " .. instrumentation .. " instrumentati it("sets the outgoint parent span's ID correctly", function() local trace_id = gen_trace_id() - local span_id = gen_span_id() + local incoming_span_id = gen_span_id() local thread = helpers.tcp_server(TCP_PORT) local r = proxy_client:get("/", { headers = { - traceparent = fmt("00-%s-%s-01", trace_id, span_id), + traceparent = fmt("00-%s-%s-01", trace_id, incoming_span_id), host = "http-route" }, }) @@ -398,6 +422,8 @@ describe("propagation tests with enabled " .. instrumentation .. " instrumentati local json = cjson.decode(body) assert.matches("00%-" .. trace_id .. "%-" .. parent_span.span_id .. "%-01", json.headers.traceparent) + + assert_correct_trace_hierarchy(spans, incoming_span_id) end) end) end diff --git a/spec/fixtures/custom_plugins/kong/plugins/trace-propagator/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/trace-propagator/handler.lua index 13b692e44603..daf8a36c3581 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/trace-propagator/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/trace-propagator/handler.lua @@ -33,10 +33,10 @@ function _M:access(conf) end if span_id then - injected_parent_span.parent_id = span_id + root_span.parent_id = span_id elseif parent_id then - injected_parent_span.parent_id = parent_id + root_span.parent_id = parent_id end local type = header_type and "preserve" or "w3c"