[GitHub] [apisix] tokers commented on a change in pull request #3551: feat(zipkin): support b3 req header

[GitBox <gi...@apache.org>]

tokers commented on a change in pull request #3551:
URL: https://github.com/apache/apisix/pull/3551#discussion_r579647322



##########
File path: apisix/plugins/zipkin.lua
##########
@@ -88,29 +110,58 @@ function _M.rewrite(plugin_conf, ctx)
     local headers = core.request.headers(ctx)
     local per_req_sample_ratio
 
-    -- 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
-        per_req_sample_ratio = 1
-    elseif sample == "0" or sample == "false" then
-        per_req_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
+    -- We still want to warn on invalid sampled header, so do this after the above
     local debug = headers["x-b3-flags"]
     if debug == "1" then
         per_req_sample_ratio = 1
     end
 
+    local trace_id, request_span_id, sampled, parent_span_id
+    local b3 = headers["b3"]
+    if b3 then
+        -- don't pass b3 header by default
+        core.request.set_header(ctx, "b3", nil)
+
+        local err
+        err, trace_id, request_span_id, sampled, parent_span_id = parse_b3(b3)
+
+        if err then
+            core.log.warn("invalid b3 header: ", b3, ", ignored: ", err)

Review comment:
       I think `400` is better, since an invalid header is always caused by client.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org