Please help review and approve one change

[Maxim Dounin]

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/