Please help review and approve one change

[Xiaohua Wang]

Hi Maxim,
Thanks for your code review and good comments.
The basic idea for http-call-trace is to provide a mechanism for nginx to
have a sync-like call stack trace to enhance trouble shooting for load test.
Since nginx is event driven and asynchronous based software,
it's good for performance, but not easy and natural for debugging and
trouble shooting like synchronous based, specially for load test.

DTrace certainly can trace the request in one worker process,
but only can trace entry/existence of one function.
But in nginx source code, there are some functions which has a lot of lines
and "goto",
sometime need to trace the call flow at the middle of functions.
And DTrace depends on particular OS.

Why not use "--with-debug" or by default, there are two reasons:
1) want to has http-call-trace for release build means even no "--with-debug",
then developer can decide it by options.
2) currently, http-call-trace use http request nginx internal private
header to store call stack, so don't want to use by default.

This http-call-trace optional feature (like tool/utility functions) is
mainly used to help troubleshooting for load test.
It do need to add trace code in nginx functions.

Regarding the code style and code quality, they can be refined accordingly.

Could you consider a bit more on http-call-trace feature? Especially from
perspective of customization development based on nginx.

Thanks a lot.

On Mon, Dec 25, 2017 at 10:12 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:

> Hello!
>
> On Mon, Dec 25, 2017 at 11:24:51AM +0800, Xiaohua Wang wrote:
>
> > Hi Maxim,
> > Thanks for you reply.
> > Actually we already tried to use the dtrace method to do real-time
> tracing
> > as you mentioned.
> > dtrace is helpful in some scenarios.
> > But there are several issues we met while using dtrace,
> > 1) the drace usage is not so friendly, as reference guide mentioned, need
> > to define a lot of structure in dtrace script.
> > 2) in performance/load test, our purpose is to trace HTTP requests call
> in
> > nginx, instead of tracing some methods in one process.
>
> DTrace certainly can be used for tracing particular requests,
> though you have to write appropriate D code to do this.
>
> > And what I want to add http-call-trace in nginx source code,
> > 1) it is a separated module, can be controlled by configure options
> > (--with-http-call-trace), no impact any existence source code.
> > 2) it's very easy to use and support alll OS platforms which nginx can
> > support, no dependency on Dtrace tool.
>
> And here are downsides, in no particular order:
>
> - As in the patch suggested, the code literally does nothing.  To
>   do something meaningful it additionally requires introducing
>   trace calls in various functions.
>
> - The code is far from being in nginx coding style.
>
> - The code uses hacks to store its data.
>
> - The code is http-only for some reason (well, actually the reason
>   is clear: because it uses a http-only hack).
>
> Note well that it's not a module as you claim, but rather an
> optional set of functions for call tracing, an optional feature
> similar to --with-debug.
>
> It is also not clear why it is made optional - rather, it should
> be made a part of --with-debug instead, or even the default, as it
> neither depends on any external libraries nor introduces
> additional calls by itself.  The only reason for being optional
> seems to be the quality of the code.
>
> > Could you help think it again?
>
> Sorry, but the answer is still no.
>
> --
> Maxim Dounin
> http://mdounin.ru/
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20171226/5c51298a/attachment.html>