Re: [Tarantool-patches] [PATCH v3 4/4] cfg: configure crash report sending

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

See 4 comments below.

On 04.12.2020 16:30, Cyrill Gorcunov via Tarantool-patches wrote:
> Introcude a new option to box.cfg{} to control sending

1. Introcude -> Introduce.

> of a crash report.
> 
> There is no simple way to test all this because it involves
> subsequent runs of tarantool. I've been using one terminal
> with tarantool running inside as
> 
>> box.cfg{feedback_host="127.0.0.1:1500"}
> 
> another terminal to listen for incoming data
> 
>> while true ; do nc -l -p 1500 -c 'echo -e "HTTP/1.1 200 OK\n\n $(date)"'; done
> 
> and one another to kill main tarantool instance
> 
>> kill -11 `pidof tarantool`
> 
> If we run as
> 
>> box.cfg{feedback_host="127.0.0.1:1500", feedback_no_crashinfo=true}
> 
> then the crash report will not be sent.
> 
> Closes #5261
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> 
> @TarantoolBot document
> Title: Configuration reference

2. Please, be more specific. I guess about 80% of all
doc tickets are about configuration update.

> Header: Feedback

3. What is 'Header:'? Docbot does not parse anything except
Title.

> For better analisys of program crashes the information associated with
> the crash such as
> 
>  - utsname (similar to `uname -a` output except the network name)
>  - build information
>  - reason for a crash
>  - call backtrace
> 
> is sent to the feedback server. To disable it set `feedback_no_crashinfo`
> to `false`.
> ---
>  src/box/lua/feedback_daemon.lua | 7 +++++++
>  src/box/lua/load_cfg.lua        | 3 +++
>  src/lib/core/errstat.c          | 2 +-
>  test/box/admin.result           | 2 ++
>  test/box/cfg.result             | 4 ++++
>  5 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/src/box/lua/feedback_daemon.lua b/src/box/lua/feedback_daemon.lua
> index 1d39012ed..613d2864c 100644
> --- a/src/box/lua/feedback_daemon.lua
> +++ b/src/box/lua/feedback_daemon.lua
> @@ -360,12 +361,18 @@ local function reload(self)
>      self:start()
>  end
>  
> +ffi.cdef[[
> +void
> +cfg_errstat_set_params(void);

4. You must add this function to exports.h,
otherwise it may be deleted by some optimization, like
it happened to other functions and entire modules before.