-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set opentracing context for a request in an earlier phase #85
Comments
I think that would be ok. Can you try making the change and seeing if it breaks anything? |
I'll give it a try, but not sure when. |
Subrequests begin their lives in the server-rewrite phase, so this module is still loaded for subrequests.
I moved it to the SERVER_REWRITE phase, because when I moved it earlier than the SERVER_REWRITE phase, the module doesn't get loaded for subrequests. This seems to work fine for me. [Edit: except apparently I'm not passing the tests. I'll take a look.] |
@terrynsun SERVER_REWRITE is probably good enough, maybe open a PR so that the maintainer can help with tests. |
Subrequests begin their lives in the server-rewrite phase, so this module is still loaded for subrequests.
…ib#85) (opentracing-contrib#98)" This reverts commit 1e1e992.
…ng-contrib#85) (opentracing-contrib#98)"" This reverts commit 3017074.
Done in #98 |
I'm working on kubernetes/ingress-nginx#3783 to enable tracing of each Nginx phase we run Lua code. I've noticed that
ngx.var.opentracing_binary_context
is not set atrewrite
phase and when I try to access it in Lua I get:Looking at the source code, I noticed that opentracing runs after rewrite phase:
nginx-opentracing/opentracing/src/ngx_http_opentracing_module.cpp
Line 172 in 9cde691
Can we inject it at
NGX_HTTP_POST_READ_PHASE
orNGX_HTTP_SERVER_REWRITE_PHASE
? According to http://nginx.org/en/docs/dev/development_guide.html#http_phasesNGX_HTTP_POST_READ_PHASE
is the earliest phase.The text was updated successfully, but these errors were encountered: