From 9b3ece0c01fcde96d65450ef99d6dc1a7d501609 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Enrique=20Garc=C3=ADa=20Cota?= Date: Thu, 11 Feb 2021 20:05:32 +0100 Subject: [PATCH] feat/request tags (#102) --- kong-plugin-zipkin-1.2.0-1.rockspec | 1 + kong/plugins/zipkin/handler.lua | 17 +++++- kong/plugins/zipkin/request_tags.lua | 82 ++++++++++++++++++++++++++++ kong/plugins/zipkin/schema.lua | 1 + spec/request_tags_spec.lua | 46 ++++++++++++++++ spec/zipkin_spec.lua | 7 ++- 6 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 kong/plugins/zipkin/request_tags.lua create mode 100644 spec/request_tags_spec.lua diff --git a/kong-plugin-zipkin-1.2.0-1.rockspec b/kong-plugin-zipkin-1.2.0-1.rockspec index b96341c83ae..65199362581 100644 --- a/kong-plugin-zipkin-1.2.0-1.rockspec +++ b/kong-plugin-zipkin-1.2.0-1.rockspec @@ -26,5 +26,6 @@ build = { ["kong.plugins.zipkin.span"] = "kong/plugins/zipkin/span.lua", ["kong.plugins.zipkin.tracing_headers"] = "kong/plugins/zipkin/tracing_headers.lua", ["kong.plugins.zipkin.schema"] = "kong/plugins/zipkin/schema.lua", + ["kong.plugins.zipkin.request_tags"] = "kong/plugins/zipkin/request_tags.lua", }, } diff --git a/kong/plugins/zipkin/handler.lua b/kong/plugins/zipkin/handler.lua index 91da3d8ac06..7d42f0b7063 100644 --- a/kong/plugins/zipkin/handler.lua +++ b/kong/plugins/zipkin/handler.lua @@ -2,6 +2,8 @@ local new_zipkin_reporter = require "kong.plugins.zipkin.reporter".new local new_span = require "kong.plugins.zipkin.span".new local utils = require "kong.tools.utils" local tracing_headers = require "kong.plugins.zipkin.tracing_headers" +local request_tags = require "kong.plugins.zipkin.request_tags" + local subsystem = ngx.config.subsystem local fmt = string.format @@ -102,8 +104,10 @@ if subsystem == "http" then initialize_request = function(conf, ctx) local req = kong.request + local req_headers = req.get_headers() + local header_type, trace_id, span_id, parent_id, should_sample, baggage = - tracing_headers.parse(req.get_headers()) + tracing_headers.parse(req_headers) local method = req.get_method() if should_sample == nil then @@ -139,6 +143,17 @@ if subsystem == "http" then end end + local req_tags, err = request_tags.parse(req_headers[conf.tags_header]) + if err then + -- log a warning in case there were erroneous request tags. Don't throw the tracing away & rescue with valid tags, if any + kong.log.warn(err) + end + if req_tags then + for tag_name, tag_value in pairs(req_tags) do + request_span:set_tag(tag_name, tag_value) + end + end + ctx.zipkin = { request_span = request_span, header_type = header_type, diff --git a/kong/plugins/zipkin/request_tags.lua b/kong/plugins/zipkin/request_tags.lua new file mode 100644 index 00000000000..88c94659210 --- /dev/null +++ b/kong/plugins/zipkin/request_tags.lua @@ -0,0 +1,82 @@ +-- Module for parsing Zipkin span tags introduced by requests with a special header +-- by default the header is called Zipkin-Tags +-- +-- For example, the following http request header: +-- +-- Zipkin-Tags: foo=bar; baz=qux +-- +-- Will add two tags to the request span in Zipkin + + +local split = require "kong.tools.utils".split + +local match = string.match + +local request_tags = {} + + +-- note that and errors is an output value; we do this instead of +-- a return in order to be more efficient (allocate less tables) +local function parse_tags(tags_string, dest, errors) + local items = split(tags_string, ";") + local item + + for i = 1, #items do + item = items[i] + if item ~= "" then + local name, value = match(item, "^%s*(%S+)%s*=%s*(.*%S)%s*$") + if name then + dest[name] = value + + else + errors[#errors + 1] = item + end + end + end +end + + +-- parses req_headers into extra zipkin tags +-- returns tags, err +-- note that both tags and err can be non-nil when the request could parse some tags but rejects others +-- tag can be nil when tags_header is nil. That is not an error (err will be empty) +function request_tags.parse(tags_header) + if not tags_header then + return nil, nil + end + + local t = type(tags_header) + local tags, errors = {}, {} + + -- "normal" requests are strings + if t == "string" then + parse_tags(tags_header, tags, errors) + + -- requests where the tags_header_name header is used more than once get an array + -- + -- example - a request with the headers: + -- zipkin-tags: foo=bar + -- zipkin-tags: baz=qux + -- + -- will get such array. We have to transform that into { foo=bar, baz=qux } + elseif t == "table" then + for i = 1, #tags_header do + parse_tags(tags_header[i], tags, errors) + end + + else + return nil, + string.format("unexpected tags_header type: %s (%s)", + tostring(tags_header), t) + end + + if next(errors) then + errors = "Could not parse the following Zipkin tags: " .. table.concat(errors, ", ") + else + errors = nil + end + + return tags, errors +end + +return request_tags diff --git a/kong/plugins/zipkin/schema.lua b/kong/plugins/zipkin/schema.lua index 6ba394f9084..e9c65a2b7a4 100644 --- a/kong/plugins/zipkin/schema.lua +++ b/kong/plugins/zipkin/schema.lua @@ -57,6 +57,7 @@ return { one_of = { "preserve", "b3", "b3-single", "w3c", "jaeger" } } }, { default_header_type = { type = "string", required = true, default = "b3", one_of = { "b3", "b3-single", "w3c", "jaeger" } } }, + { tags_header = { type = "string", required = true, default = "Zipkin-Tags" } }, { static_tags = { type = "array", elements = static_tag, custom_validator = validate_static_tags } }, }, diff --git a/spec/request_tags_spec.lua b/spec/request_tags_spec.lua new file mode 100644 index 00000000000..51761d0e254 --- /dev/null +++ b/spec/request_tags_spec.lua @@ -0,0 +1,46 @@ +local parse = require("kong.plugins.zipkin.request_tags").parse + +describe("request_tags", function() + describe("parse", function() + it("parses simple case, swallowing spaces", function() + assert.same({ foo = "bar" }, parse("foo=bar")) + assert.same({ foo = "bar" }, parse("foo= bar")) + assert.same({ foo = "bar" }, parse("foo =bar")) + assert.same({ foo = "bar" }, parse("foo = bar")) + assert.same({ foo = "bar" }, parse(" foo = bar")) + assert.same({ foo = "bar" }, parse("foo = bar ")) + assert.same({ foo = "bar" }, parse(" foo = bar ")) + end) + it("rejects invalid tags, keeping valid ones", function() + local tags, err = parse("foobar;foo=;=bar;keep=true;=") + assert.same(tags, {keep = "true"}) + assert.equals("Could not parse the following Zipkin tags: foobar, foo=, =bar, =", err) + end) + it("allows spaces on values, but not on keys", function() + assert.same({ foo = "bar baz" }, parse("foo=bar baz")) + local tags, err = parse("foo bar=baz") + assert.same(tags, {}) + assert.equals("Could not parse the following Zipkin tags: foo bar=baz", err) + end) + it("parses multiple tags separated by semicolons, swallowing spaces", function() + assert.same({ foo = "bar", baz = "qux" }, parse("foo=bar;baz=qux")) + assert.same({ foo = "bar", baz = "qux" }, parse("foo =bar;baz=qux")) + assert.same({ foo = "bar", baz = "qux" }, parse("foo= bar;baz=qux")) + assert.same({ foo = "bar", baz = "qux" }, parse(" foo=bar ;baz=qux")) + assert.same({ foo = "bar", baz = "qux" }, parse("foo = bar ;baz=qux")) + assert.same({ foo = "bar", baz = "qux" }, parse(" foo = bar ;baz=qux")) + assert.same({ foo = "bar", baz = "qux" }, parse(" foo = bar ;baz =qux")) + assert.same({ foo = "bar", baz = "qux" }, parse(" foo = bar ;baz = qux")) + assert.same({ foo = "bar", baz = "qux" }, parse(" foo = bar ;baz = qux")) + end) + it("swallows empty tags between semicolons silently", function() + local tags, err = parse(";;foo=bar;;;;baz=qux;;") + assert.same({ foo = "bar", baz = "qux" }, tags) + assert.is_nil(err) + end) + it("parses multiple tags separated by semicolons, in an array", function() + assert.same({ foo = "bar", baz = "qux", quux = "quuz" }, + parse({"foo=bar;baz=qux", "quux=quuz"})) + end) + end) +end) diff --git a/spec/zipkin_spec.lua b/spec/zipkin_spec.lua index 65fe0732d5a..094c39fadba 100644 --- a/spec/zipkin_spec.lua +++ b/spec/zipkin_spec.lua @@ -5,7 +5,7 @@ local to_hex = require "resty.string".to_hex local fmt = string.format -local ZIPKIN_HOST = "zipkin" +local ZIPKIN_HOST = os.getenv("ZIPKIN_HOST") or "zipkin" local ZIPKIN_PORT = 9411 local GRPCBIN_HOST = "grpcbin" local GRPCBIN_PORT = 9000 @@ -226,6 +226,7 @@ describe("http integration tests with zipkin server [#" headers = { ["x-b3-sampled"] = "1", host = "http-route", + ["zipkin-tags"] = "foo=bar; baz=qux" }, }) assert.response(r).has.status(200) @@ -245,6 +246,8 @@ describe("http integration tests with zipkin server [#" ["http.status_code"] = "200", -- found (matches server status) lc = "kong", static = "ok", + foo = "bar", + baz = "qux", }, request_tags) local consumer_port = request_span.remoteEndpoint.port assert_is_integer(consumer_port) @@ -475,6 +478,7 @@ describe("http integration tests with zipkin server [#" headers = { ["x-b3-traceid"] = trace_id, ["x-b3-sampled"] = "1", + ["zipkin-tags"] = "error = true" }, }) assert.response(r).has.status(404) @@ -495,6 +499,7 @@ describe("http integration tests with zipkin server [#" ["http.status_code"] = "404", -- note that this was "not found" lc = "kong", static = "ok", + error = "true", }, request_tags) local consumer_port = request_span.remoteEndpoint.port assert_is_integer(consumer_port)