Skip to content

Commit cbd264a

Browse files
prasunsrivastav123-langprasunsrivastav123-lang
authored
fix(opentelemetry): validate x-request-id before using it as trace_id (#12990)
1 parent 100be4c commit cbd264a

2 files changed

Lines changed: 289 additions & 2 deletions

File tree

‎apisix/plugins/opentelemetry.lua‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ local pairs = pairs
4848
local ipairs = ipairs
4949
local unpack = unpack
5050
local string_format = string.format
51+
local string_lower = string.lower
5152
local update_time = ngx.update_time
5253
local tostring = tostring
5354

@@ -57,6 +58,29 @@ local lrucache = core.lrucache.new({
5758

5859
local asterisk = string.byte("*", 1)
5960

61+
-- capture the library's default (random) generator once, so wrapping it below
62+
-- stays idempotent even when create_tracer_obj re-runs after cache expiry
63+
local original_new_ids = id_generator.new_ids
64+
65+
66+
-- expects an already-lowercased string
67+
local function is_valid_trace_id(trace_id)
68+
if not trace_id or #trace_id ~= 32 then
69+
return false
70+
end
71+
72+
if not trace_id:match("^[0-9a-f]+$") then
73+
return false
74+
end
75+
76+
-- W3C Trace Context: all-zero trace_id is invalid
77+
if trace_id == "00000000000000000000000000000000" then
78+
return false
79+
end
80+
81+
return true
82+
end
83+
6084
local metadata_schema = {
6185
type = "object",
6286
properties = {
@@ -231,10 +255,28 @@ end
231255

232256

233257
local function create_tracer_obj(conf, plugin_info)
258+
-- id_generator is a shared module, so restore the default before applying
259+
-- the override: without this, switching trace_id_source back to "random"
260+
-- would keep honoring X-Request-Id until the worker restarts
261+
id_generator.new_ids = original_new_ids
262+
234263
if plugin_info.trace_id_source == "x-request-id" then
235264
id_generator.new_ids = function()
236-
local trace_id = core.request.headers()["x-request-id"] or ngx_var.request_id
237-
return trace_id, id_generator.new_span_id()
265+
local trace_id = core.request.headers()["x-request-id"]
266+
or ngx_var.request_id
267+
268+
-- a duplicated X-Request-Id makes get_headers() return a table;
269+
-- only a plain, valid 32-hex string can be used as a trace_id,
270+
-- anything else (UUID, table, empty, ...) falls back to the
271+
-- default generator instead of crashing the otlp encoder
272+
if type(trace_id) == "string" then
273+
trace_id = string_lower(trace_id)
274+
if is_valid_trace_id(trace_id) then
275+
return trace_id, id_generator.new_span_id()
276+
end
277+
end
278+
279+
return original_new_ids()
238280
end
239281
end
240282
-- create exporter

‎t/plugin/opentelemetry.t‎

Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,3 +734,248 @@ plugins:
734734
grep apisix.phase.access ci/pod/otelcol-contrib/data-otlp.json | tail -n 1
735735
--- response_body eval
736736
qr/otel-meta-change-second/
737+
738+
739+
740+
=== TEST 31: reset metadata trace_id_source = x-request-id
741+
--- config
742+
location /t {
743+
content_by_lua_block {
744+
local t = require("lib.test_admin").test
745+
local code, body = t('/apisix/admin/plugin_metadata/opentelemetry',
746+
ngx.HTTP_PUT,
747+
[[{
748+
"batch_span_processor": {
749+
"max_export_batch_size": 1,
750+
"inactive_timeout": 0.5
751+
},
752+
"trace_id_source": "x-request-id",
753+
"resource": {
754+
"service.name": "APISIX"
755+
},
756+
"collector": {
757+
"address": "127.0.0.1:4318",
758+
"request_timeout": 3
759+
}
760+
}]]
761+
)
762+
if code >= 300 then
763+
ngx.status = code
764+
end
765+
ngx.say(body)
766+
}
767+
}
768+
769+
770+
771+
=== TEST 32: reset route for x-request-id validation
772+
--- config
773+
location /t {
774+
content_by_lua_block {
775+
local t = require("lib.test_admin").test
776+
local code, body = t('/apisix/admin/routes/1',
777+
ngx.HTTP_PUT,
778+
[[{
779+
"name": "route_name",
780+
"plugins": {
781+
"opentelemetry": {
782+
"sampler": {
783+
"name": "always_on"
784+
}
785+
}
786+
},
787+
"upstream": {
788+
"nodes": {
789+
"127.0.0.1:1980": 1
790+
},
791+
"type": "roundrobin"
792+
},
793+
"uri": "/opentracing"
794+
}]]
795+
)
796+
if code >= 300 then
797+
ngx.status = code
798+
end
799+
ngx.say(body)
800+
}
801+
}
802+
803+
804+
805+
=== TEST 33: invalid (UUID) x-request-id should not crash
806+
--- request
807+
GET /opentracing
808+
--- more_headers
809+
X-Request-Id: 550e8400-e29b-41d4-a716-446655440000
810+
--- wait: 2
811+
--- response_body
812+
opentracing
813+
--- no_error_log
814+
[error]
815+
816+
817+
818+
=== TEST 34: invalid x-request-id still exports a valid random trace id
819+
--- exec
820+
tail -n 1 ci/pod/otelcol-contrib/data-otlp.json
821+
--- response_body eval
822+
qr/"traceId"\s*:\s*"[0-9a-f]{32}"/
823+
824+
825+
826+
=== TEST 35: all-zero x-request-id must not be used as trace id
827+
--- request
828+
GET /opentracing
829+
--- more_headers
830+
X-Request-Id: 00000000000000000000000000000000
831+
--- wait: 2
832+
--- response_body
833+
opentracing
834+
835+
836+
837+
=== TEST 36: all-zero id is replaced by a non-zero random trace id
838+
--- exec
839+
tail -n 1 ci/pod/otelcol-contrib/data-otlp.json
840+
--- response_body eval
841+
qr/"traceId"\s*:\s*"(?!0{32})[0-9a-f]{32}"/
842+
843+
844+
845+
=== TEST 37: uppercase 32-hex x-request-id is used
846+
--- request
847+
GET /opentracing
848+
--- more_headers
849+
X-Request-Id: 550E8400E29B41D4A716446655440000
850+
--- wait: 2
851+
--- response_body
852+
opentracing
853+
854+
855+
856+
=== TEST 38: uppercase 32-hex is lowercased and used as trace id
857+
--- exec
858+
tail -n 1 ci/pod/otelcol-contrib/data-otlp.json
859+
--- response_body eval
860+
qr/"traceId"\s*:\s*"550e8400e29b41d4a716446655440000"/
861+
862+
863+
864+
=== TEST 39: duplicated x-request-id header should not crash
865+
--- request
866+
GET /opentracing
867+
--- more_headers
868+
X-Request-Id: 550e8400e29b41d4a716446655440000
869+
X-Request-Id: aabbccddeeff00112233445566778899
870+
--- wait: 2
871+
--- response_body
872+
opentracing
873+
--- no_error_log
874+
[error]
875+
876+
877+
878+
=== TEST 40: missing x-request-id falls back to default generator
879+
--- request
880+
GET /opentracing
881+
--- wait: 2
882+
--- response_body
883+
opentracing
884+
885+
886+
887+
=== TEST 41: missing x-request-id still exports a valid trace id
888+
--- exec
889+
tail -n 1 ci/pod/otelcol-contrib/data-otlp.json
890+
--- response_body eval
891+
qr/"traceId"\s*:\s*"[0-9a-f]{32}"/
892+
893+
894+
895+
=== TEST 42: non-hex x-request-id falls back to default generator
896+
--- request
897+
GET /opentracing
898+
--- more_headers
899+
X-Request-Id: zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz
900+
--- wait: 2
901+
--- response_body
902+
opentracing
903+
904+
905+
906+
=== TEST 43: non-hex x-request-id still exports a valid trace id
907+
--- exec
908+
tail -n 1 ci/pod/otelcol-contrib/data-otlp.json
909+
--- response_body eval
910+
qr/"traceId"\s*:\s*"[0-9a-f]{32}"/
911+
912+
913+
914+
=== TEST 44: empty x-request-id falls back to default generator
915+
--- request
916+
GET /opentracing
917+
--- more_headers
918+
X-Request-Id:
919+
--- wait: 2
920+
--- response_body
921+
opentracing
922+
--- no_error_log
923+
[error]
924+
925+
926+
927+
=== TEST 45: empty x-request-id still exports a valid trace id
928+
--- exec
929+
tail -n 1 ci/pod/otelcol-contrib/data-otlp.json
930+
--- response_body eval
931+
qr/"traceId"\s*:\s*"[0-9a-f]{32}"/
932+
933+
934+
935+
=== TEST 46: switch metadata trace_id_source back to random
936+
--- config
937+
location /t {
938+
content_by_lua_block {
939+
local t = require("lib.test_admin").test
940+
local code, body = t('/apisix/admin/plugin_metadata/opentelemetry',
941+
ngx.HTTP_PUT,
942+
[[{
943+
"batch_span_processor": {
944+
"max_export_batch_size": 1,
945+
"inactive_timeout": 0.5
946+
},
947+
"trace_id_source": "random",
948+
"resource": {
949+
"service.name": "APISIX"
950+
},
951+
"collector": {
952+
"address": "127.0.0.1:4318",
953+
"request_timeout": 3
954+
}
955+
}]]
956+
)
957+
if code >= 300 then
958+
ngx.status = code
959+
end
960+
ngx.say(body)
961+
}
962+
}
963+
964+
965+
966+
=== TEST 47: after switching to random, send a valid 32-hex x-request-id
967+
--- request
968+
GET /opentracing
969+
--- more_headers
970+
X-Request-Id: 550e8400e29b41d4a716446655440000
971+
--- wait: 2
972+
--- response_body
973+
opentracing
974+
975+
976+
977+
=== TEST 48: x-request-id must be ignored once trace_id_source is random
978+
--- exec
979+
tail -n 1 ci/pod/otelcol-contrib/data-otlp.json
980+
--- response_body eval
981+
qr/"traceId"\s*:\s*"(?!550e8400e29b41d4a716446655440000)[0-9a-f]{32}"/

0 commit comments

Comments
 (0)