From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256
 bits)) (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id D66B945C305
 for <tarantool-patches@dev.tarantool.org>;
 Sat,  5 Dec 2020 21:34:21 +0300 (MSK)
References: <20201204153003.175555-1-gorcunov@gmail.com>
 <20201204153003.175555-5-gorcunov@gmail.com>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <90d006bf-99ca-17bd-85cb-d5575dbe875f@tarantool.org>
Date: Sat, 5 Dec 2020 19:34:20 +0100
MIME-Version: 1.0
In-Reply-To: <20201204153003.175555-5-gorcunov@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH v3 4/4] cfg: configure crash report
 sending
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Cyrill Gorcunov <gorcunov@gmail.com>, tml <tarantool-patches@dev.tarantool.org>
Cc: Mons Anderson <v.perepelitsa@corp.mail.ru>

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.