akashchamp opened a new pull request, #13985: URL: https://github.com/apache/apisix/pull/13985
### Description <!-- Please include a summary of the change and which issue is fixed. --> <!-- Please also include relevant motivation and context. --> The vendored `opentelemetry-lua` dependency (pinned `opentelemetry-lua = 0.2-6`) has a bug in `batch_span_processor.lua`'s `create_timer()`: when `ngx.timer.at()` fails because the worker is exiting — which happens near the end of every graceful shutdown/deploy — it falls back to a *synchronous* span export (`self:flush_all()`), which opens a cosocket. Since `span:finish()` (called from `apisix/plugins/opentelemetry.lua`'s `_M.log` and `create_child_span`) runs inline inside `body_filter_by_lua*`/`log_by_lua*`, that synchronous cosocket call trips APISIX's own phase guard in `apisix/patch.lua` and crashes with an uncaught Lua error: ``` 2026/09/20 09:31:05 [error] 70#70: *1277095 failed to run log_by_lua*: /usr/local/apisix/apisix/patch.lua:406: API disabled in the context of log_by_lua* ``` Because this only requires the worker to be exiting — not any specific plugin config — it fires on effectively every rolling deploy of any release with tracing enabled on any route. The real fix belongs upstream in `opentelemetry-lua` (already filed/fixed there: yangxikun/opentelemetry-lua#107, fix at yangxikun/opentelemetry-lua#106). Until APISIX bumps the pinned version to pick that up, this PR adds the defense-in-depth guard suggested in the issue: wrap `span:finish()` in `pcall` so this class of "third-party tracer code doing something disallowed in a restricted phase" can't crash the request phase, even though it can't be fully prevented from the plugin side alone. #### Which issue(s) this PR fixes: Fixes #13980 ### Checklist - [x] I have explained the need for this PR and the problem it solves - [x] I have explained the changes or the new features added to this PR - [ ] I have added tests corresponding to this change — see "How this was tested" below for why I held off on a checked-in `t/*.t` test and what I verified instead - [ ] I have updated the documentation to reflect this change — this is an internal defense-in-depth guard with no user-visible behavior change, so I didn't see a doc to update; happy to add one if reviewers want it documented - [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first) ### How this was tested - `make lint` (luacheck over `apisix`/`t/lib`, LuaJIT compile-style check via `lj-releng`, test-file style check) passes clean: `Total: 0 warnings / 0 errors in 408 files`. - This environment doesn't have a running OpenResty/`test-nginx` harness available, so I could not run `make test`'s `t/plugin/opentelemetry*.t` suite directly, and a genuine worker-shutdown race isn't something the existing `Test::Nginx` `.t` framework can trigger deterministically (there's no existing precedent for mocking `ngx.timer.at()`/`ngx.worker.exiting()` failures anywhere in `t/`). Instead, I reproduced the bug and verified the fix directly against the real vendored dependency: I fetched the actual, unmodified `opentelemetry-lua` v0.2.6 source (`batch_span_processor.lua`, `global.lua`, `metrics_reporter.lua` — the exact version pinned in `apisix-master-0.rockspec`), mocked only the `ngx.*` surface it touches (including an `ngx.socket.tcp()` that raises the exact `apisix/patch.lua` phase-guard error), and confirmed with a plain Lua script that: 1. **Before the fix** (calling `span:finish()` directly), the phase-guard error escapes uncaught — reproducing the issue exactly, including the verbatim error string from the report. 2. **After the fix** (`finish_span()`, the new `pcall` wrapper), the same error is caught and logged as a warning instead of propagating — no crash. 3. **Sanity check**: on the normal (non-shutdown) path, where nothing fails, the guard is a no-op — the exporter still runs and no warning is logged. All three checks pass (`ALL CHECKS PASSED`). I'm glad to add this as a checked-in test if there's a preferred location/pattern for it — I didn't want to guess at one given there's no existing precedent in `t/` for this kind of timer/shutdown mocking. ### Limitations - This only guards the two `span:finish()` call sites APISIX's own `opentelemetry.lua` plugin controls. It does not fix the root cause in the vendored library, which still needs the pinned `opentelemetry-lua` version bumped once yangxikun/opentelemetry-lua#106 is released. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
