This is an automated email from the ASF dual-hosted git repository. membphis pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/apisix.git
The following commit(s) were added to refs/heads/master by this push: new 7aa6c12 fix(zipkin): always delivery x-b3-sampled header (#3519) 7aa6c12 is described below commit 7aa6c123b0f06621a2bf5d6cb318bf51c69e16d6 Author: 罗泽轩 <spacewander...@gmail.com> AuthorDate: Thu Feb 4 23:39:46 2021 +0800 fix(zipkin): always delivery x-b3-sampled header (#3519) According to https://github.com/openzipkin/b3-propagation#sampling-state: > sampling applies consistently per-trace: once the sampling decision is made, the same value should be consistently sent downstream We should pass the right x-b3-sampled according to our decision. --- apisix/plugins/zipkin.lua | 7 +- apisix/plugins/zipkin/codec.lua | 12 ++-- t/lib/server.lua | 10 +++ t/plugin/zipkin.t | 144 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 9 deletions(-) diff --git a/apisix/plugins/zipkin.lua b/apisix/plugins/zipkin.lua index 01234ba..fddcd6c 100644 --- a/apisix/plugins/zipkin.lua +++ b/apisix/plugins/zipkin.lua @@ -67,7 +67,7 @@ local function create_tracer(conf,ctx) local headers = core.request.headers(ctx) --- X-B3-Sampled: if an upstream decided to sample this request, we do too. + -- X-B3-Sampled: if the client decided to sample this request, we do too. local sample = headers["x-b3-sampled"] if sample == "1" or sample == "true" then conf.sample_ratio = 1 @@ -75,8 +75,8 @@ local function create_tracer(conf,ctx) conf.sample_ratio = 0 end --- X-B3-Flags: if it equals '1' then it overrides sampling policy --- We still want to warn on invalid sample header, so do this after the above + -- X-B3-Flags: if it equals '1' then it overrides sampling policy + -- We still want to warn on invalid sample header, so do this after the above local debug = headers["x-b3-flags"] if debug == "1" then conf.sample_ratio = 1 @@ -105,6 +105,7 @@ function _M.rewrite(plugin_conf, ctx) ctx.opentracing_sample = tracer.sampler:sample() if not ctx.opentracing_sample then + core.request.set_header("x-b3-sampled", "0") return end diff --git a/apisix/plugins/zipkin/codec.lua b/apisix/plugins/zipkin/codec.lua index f1dad87..7ad4743 100644 --- a/apisix/plugins/zipkin/codec.lua +++ b/apisix/plugins/zipkin/codec.lua @@ -38,6 +38,9 @@ local function new_extractor() local had_invalid_id = false local trace_id = headers["x-b3-traceid"] + local parent_span_id = headers["x-b3-parentspanid"] + local request_span_id = headers["x-b3-spanid"] + -- Validate trace id if trace_id and ((#trace_id ~= 16 and #trace_id ~= 32) or trace_id:match("%X")) then @@ -45,7 +48,6 @@ local function new_extractor() had_invalid_id = true end - local parent_span_id = headers["x-b3-parentspanid"] -- Validate parent_span_id if parent_span_id and (#parent_span_id ~= 16 or parent_span_id:match("%X")) then @@ -53,7 +55,6 @@ local function new_extractor() had_invalid_id = true end - local request_span_id = headers["x-b3-spanid"] -- Validate request_span_id if request_span_id and (#request_span_id ~= 16 or request_span_id:match("%X")) then @@ -79,7 +80,7 @@ local function new_extractor() request_span_id = from_hex(request_span_id) return new_span_context(trace_id, request_span_id, parent_span_id, - baggage) + baggage) end end @@ -90,9 +91,8 @@ local function new_injector() headers["x-b3-parentspanid"] = span_context.parent_id and to_hex(span_context.parent_id) or nil headers["x-b3-spanid"] = to_hex(span_context.span_id) - local flags = core.request.header(nil, "x-b3-flags") - headers["x-b3-flags"] = flags - headers["x-b3-sampled"] = (not flags) + -- when we call this function, we already start to sample + headers["x-b3-sampled"] = "1" for key, value in span_context:each_baggage_item() do -- XXX: https://github.com/opentracing/specification/issues/117 headers["uberctx-"..key] = ngx.escape_uri(value) diff --git a/t/lib/server.lua b/t/lib/server.lua index b9eef33..924163d 100644 --- a/t/lib/server.lua +++ b/t/lib/server.lua @@ -340,6 +340,16 @@ function _M.headers() end +function _M.echo() + ngx.req.read_body() + local hdrs = ngx.req.get_headers() + for k, v in pairs(hdrs) do + ngx.header[k] = v + end + ngx.say(ngx.req.get_body_data() or "") +end + + function _M.log() ngx.req.read_body() local body = ngx.req.get_body_data() diff --git a/t/plugin/zipkin.t b/t/plugin/zipkin.t index 1964a82..720412b 100644 --- a/t/plugin/zipkin.t +++ b/t/plugin/zipkin.t @@ -415,3 +415,147 @@ property "server_addr" validation failed: failed to match pattern "^[0-9]{1,3}.[ [200, 200, 200, 403] --- no_error_log [error] + + + +=== TEST 14: check zipkin headers +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "zipkin": { + "endpoint": "http://127.0.0.1:9999/mock_zipkin", + "sample_ratio": 1 + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/echo" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 15: set x-b3-sampled if sampled +--- request +GET /echo +--- response_headers +x-b3-sampled: 1 + + + +=== TEST 16: don't sample if disabled +--- request +GET /echo +--- more_headers +x-b3-sampled: 0 +--- response_headers +x-b3-sampled: 0 + + + +=== TEST 17: don't sample if disabled (old way) +--- request +GET /echo +--- more_headers +x-b3-sampled: false +--- response_headers +x-b3-sampled: 0 + + + +=== TEST 18: sample according to the header +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "zipkin": { + "endpoint": "http://127.0.0.1:9999/mock_zipkin", + "sample_ratio": 0.00001 + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/echo" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 19: don't sample by default +--- request +GET /echo +--- response_headers +x-b3-sampled: 0 + + + +=== TEST 20: sample if needed +--- request +GET /echo +--- more_headers +x-b3-sampled: 1 +--- response_headers +x-b3-sampled: 1 + + + +=== TEST 21: sample if debug +--- request +GET /echo +--- more_headers +x-b3-flags: 1 +--- response_headers +x-b3-sampled: 1 + + + +=== TEST 22: sample if needed (old way) +--- request +GET /echo +--- more_headers +x-b3-sampled: true +--- response_headers +x-b3-sampled: 1