Re: [PR] fix(opentelemetry): validate x-request-id before using it as trace_id [apisix]

["AlinsRan (via GitHub)" <gi...@apache.org>]

AlinsRan commented on code in PR #12990:
URL: https://github.com/apache/apisix/pull/12990#discussion_r3025359907


##########
apisix/plugins/opentelemetry.lua:
##########
@@ -231,9 +249,17 @@ end
 
 local function create_tracer_obj(conf, plugin_info)
     if plugin_info.trace_id_source == "x-request-id" then
+        local _original_new_ids = id_generator.new_ids
+
         id_generator.new_ids = function()
-            local trace_id = core.request.headers()["x-request-id"] or ngx_var.request_id
-            return trace_id, id_generator.new_span_id()
+            local trace_id = core.request.headers()["x-request-id"]
+                            or ngx_var.request_id
+
+            if is_valid_trace_id(trace_id) then
+                return trace_id, id_generator.new_span_id()
+            end
+
+            return _original_new_ids()

Review Comment:
   The lrucache has a 24-hour TTL (line 54). After expiry, create_tracer_obj is called again. At that point id_generator.new_ids is already the closure from the previous call, so _original_new_ids captures the already-overridden function. Each 24h expiry adds another wrapper layer — unbounded nesting over days/weeks in production.
   The pre-PR code was idempotent (it overwrote the same closure). This PR makes it strictly worse.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org

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