janiussyafiq commented on PR #13232: URL: https://github.com/apache/apisix/pull/13232#issuecomment-4306533741
Thanks for the thorough and detailed explanation @shreemaan-abhishek, and I totally agree with your view and have already implemented the fix based on your suggestion. The changes made: - `tracer.start()` now uses `rawget(ctx, "tracing")` instead of `ctx.tracing`, bypassing the `__index` metatable chain so each per-request ctx gets its own fresh tracing table - `tracer.release()` now uses `rawset(ctx, "tracing", nil)` to clear the per-request ctx's own field directly - `tracer.release(ngx_ctx)` is now explicitly called at all exit points of `ssl_client_hello_phase` in `init.lua` to prevent the SSL-phase tracing table from leaking for the lifetime of the TLS connection As you mentioned, one consequence of this fix is that the SSL-handshake span is now its own standalone trace rather than being stitched into the HTTP request's trace. This also means `ssl_client_hello_phase` no longer appears in the OTel exported spans — since the SSL-phase tracing table is released at the end of `ssl_client_hello_phase`, the OTel plugin (which runs in the HTTP log phase) never has access to it. As a result, the existing `ssl_client_hello_phase` child span assertion in `opentelemetry6.t` TEST 6 has been removed to reflect this new behaviour. cc @nic-6443 For the tests, I have added: - `t/node/tracer.t`: consecutive HTTPS keepalive - `t/plugin/opentelemetry6.t`: concurrent HTTP/2 span isolation test using a new `verify_isolated_traces` helper in `t/lib/test_otel.lua` that verifies each stream produces its own separate trace with no duplicate spans (cross-stream contamination) -- 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]
