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

fix(zipkin): do not reuse propagated_span by default #10983

Merged
merged 3 commits into from
Jun 1, 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@
- **rate-limiting**: Fixed an issue that impact the accuracy with the `redis` policy.
Thanks [@giovanibrioni](https://github.com/giovanibrioni) for contributing this change.
[#10559](https://github.com/Kong/kong/pull/10559)
- **Zipkin**: Fixed an issue that traces not being generated correctly when instrumentations are enabled.
[#10983](https://github.com/Kong/kong/pull/10983)

#### PDK

Expand Down
6 changes: 4 additions & 2 deletions kong/tracing/propagation.lua
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ end


-- set outgoing propagation headers
--
--
-- @tparam string conf_header_type type of tracing header to use
-- @tparam string found_header_type type of tracing header found in request
-- @tparam table proxy_span span to be propagated
Expand All @@ -466,7 +466,9 @@ local function set(conf_header_type, found_header_type, proxy_span, conf_default
kong.log.debug("skipping propagation of noop span")
return
end
set_propagated(proxy_span)
if reuse then
set_propagated(proxy_span)
end

local set_header = kong.service.request.set_header

Expand Down
66 changes: 66 additions & 0 deletions spec/03-plugins/34-zipkin/zipkin_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1474,3 +1474,69 @@ for _, strategy in helpers.each_strategy() do

end)
end

for _, strategy in helpers.each_strategy() do
describe("Integration tests with instrumentations enabled", function()
local proxy_client, zipkin_client, service

setup(function()
local bp = helpers.get_db_utils(strategy, { "services", "routes", "plugins" })

service = bp.services:insert {
name = string.lower("http-" .. utils.random_string()),
}

-- kong (http) mock upstream
bp.routes:insert({
name = string.lower("route-" .. utils.random_string()),
service = service,
hosts = { "http-route" },
preserve_host = true,
})

-- enable zipkin plugin globally, with sample_ratio = 0
bp.plugins:insert({
name = "zipkin",
config = {
sample_ratio = 0,
http_endpoint = fmt("http://%s:%d/api/v2/spans", ZIPKIN_HOST, ZIPKIN_PORT),
default_header_type = "b3-single",
}
})

helpers.start_kong({
database = strategy,
nginx_conf = "spec/fixtures/custom_nginx.template",
tracing_instrumentations = "all",
tracing_sampling_rate = 1,
})

proxy_client = helpers.proxy_client()
zipkin_client = helpers.http_client(ZIPKIN_HOST, ZIPKIN_PORT)
end)

teardown(function()
helpers.stop_kong()
end)

it("generates spans for regular requests", function()
local start_s = ngx.now()

local r = proxy_client:get("/", {
headers = {
["x-b3-sampled"] = "1",
host = "http-route",
["zipkin-tags"] = "foo=bar; baz=qux"
},
})
assert.response(r).has.status(200)

local spans = wait_for_spans(zipkin_client, 3, service.name)
local request_span = assert(get_span("get", spans), "request span missing")
local proxy_span = assert(get_span("get (proxy)", spans), "proxy span missing")

-- common assertions for request_span and proxy_span
assert_span_invariants(request_span, proxy_span, 16 * 2, start_s, "kong")
end)
end)
end