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

Reply via email to