Merge pull request #345 from mtrop-godaddy/fix/traceparent-otel-gating

Gate W3C trace support to request phases with ngx.var access

Author: sriemer

.github/workflows/test.yml

@@ -6,11 +6,11 @@ jobs:
   luacheck:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
-    - uses: leafo/gh-actions-lua@v8
+    - uses: actions/checkout@v6
+    - uses: leafo/gh-actions-lua@v13
       with:
         luaVersion: "luajit-openresty"
-    - uses: leafo/gh-actions-luarocks@v5
+    - uses: leafo/gh-actions-luarocks@v6
       with:
         luarocksVersion: "3.12.2"
     - run: luarocks install luacheck
@@ -20,7 +20,6 @@ jobs:
     strategy:
       matrix:
         openresty_version:
-          - 1.19.9.1
           - 1.21.4.4
           - 1.25.3.2
           - 1.27.1.2
@@ -33,7 +32,7 @@ jobs:
       options: --init
 
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v6
     - name: Install deps
       run: |
         apk add --no-cache curl perl bash wget git perl-dev libarchive-tools nodejs build-base
@@ -43,7 +42,7 @@ jobs:
       run: curl -s -L https://cpanmin.us | perl - App::cpanminus > /bin/cpanm && chmod +x /bin/cpanm
 
     - name: Cache
-      uses: actions/cache@v3
+      uses: actions/cache@v5
       with:
         path: |
           ~/.cpan

lib/resty/http.lua

@@ -62,6 +62,22 @@ local EXPECTING_BODY = {
 }
 
 
+-- ngx.var is only readable in request-bearing phases. Reading it from a
+-- request-less phase (init, init_worker, timer, etc.) raises an error, so we
+-- gate any ngx.var access on being in one of these phases.
+local NGX_VAR_PHASES = {
+    set            = true,
+    rewrite        = true,
+    server_rewrite = true,
+    access         = true,
+    content        = true,
+    header_filter  = true,
+    body_filter    = true,
+    log            = true,
+    precontent     = true,
+}
+
+
 -- Reimplemented coroutine.wrap, returning "nil, err" if the coroutine cannot
 -- be resumed. This protects user code from infinite loops when doing things like
 -- repeat
@@ -739,8 +755,11 @@ function _M.send_request(self, params)
     if params.version == 1.0 and not headers["Connection"] then
         headers["Connection"] = "Keep-Alive"
     end
-    -- W3C trace context support with NGINX tracer
-    if not headers["traceparent"] and ngx.var.http_traceparent then
+    -- W3C trace context support with NGINX tracer.
+    -- Only read ngx.var in request-bearing phases; doing so during
+    -- init/init_worker/timer (config preload, background jobs) raises an error.
+    if NGX_VAR_PHASES[ngx.get_phase()]
+        and not headers["traceparent"] and ngx.var.http_traceparent then
       headers["traceparent"] = ngx.var.http_traceparent
     end
 

t/21-traceparent-header.t

@@ -22,6 +22,46 @@ our $HttpConfig = qq{
     }
 };
 
+# Same as $HttpConfig, but fires a request from a request-less phase
+# (an init_worker timer) where ngx.var is disabled.
+our $HttpConfigInitWorker = qq{
+    lua_package_path "$pwd/lib/?.lua;/usr/local/share/lua/5.1/?.lua;;";
+    error_log logs/error.log debug;
+    resolver 8.8.8.8 ipv6=off;
+
+    init_by_lua_block {
+        if $ENV{TEST_COVERAGE} == 1 then
+            jit.off()
+            require("luacov.runner").init()
+        end
+
+        require("resty.http").debug(true)
+    }
+
+    init_worker_by_lua_block {
+        local function make_request(premature)
+            if premature then
+                return
+            end
+
+            local http = require "resty.http"
+            local httpc = http.new()
+            local res, err = httpc:request_uri("http://www.google.com")
+            if err then
+                ngx.log(ngx.ERR, "init_worker request failed: ", err)
+            end
+            ngx.log(ngx.INFO, "init_worker request completed")
+        end
+
+        -- cosockets are unavailable directly in init_worker, so defer to a
+        -- timer. The timer phase is also request-less (ngx.var disabled).
+        local ok, err = ngx.timer.at(0, make_request)
+        if not ok then
+            ngx.log(ngx.ERR, "failed to create timer: ", err)
+        end
+    }
+};
+
 no_long_string();
 #no_diff();
 
@@ -93,3 +133,24 @@ GET /lua
 traceparent: 00-000000000000000019f4e02c82857913-11488c6e00d1d248-01
 --- error_log
 traceparent: 00-00000000000000006633c2d00527dd33-1af98f7e6ecd16ff-01
+
+
+=== TEST 4: A request from a request-less phase does not error reading ngx.var
+--- http_config eval: $::HttpConfigInitWorker
+--- config
+    location /lua {
+        content_by_lua_block {
+            -- give the init_worker timer time to fire and finish
+            ngx.sleep(0.5)
+            ngx.say("ok")
+        }
+    }
+--- request
+GET /lua
+--- response_body
+ok
+--- no_error_log
+[error]
+API disabled in the context of ngx.timer
+--- error_log
+init_worker request completed