From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp29.i.mail.ru (smtp29.i.mail.ru [94.100.177.89]) (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 B88D74765E0 for ; Tue, 29 Dec 2020 19:14:02 +0300 (MSK) Date: Tue, 29 Dec 2020 19:13:59 +0300 From: "Alexander V. Tikhonov" Message-ID: <20201229161359.GA41108@hpalx> References: <20201228181729.640270-1-gorcunov@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201228181729.640270-1-gorcunov@gmail.com> Subject: Re: [Tarantool-patches] [PATCH] crash: extend report with instance data List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tarantool-patches@dev.tarantool.org Hi Cyrill, thanks for the patch, as I see no new degradation found in gitlab-ci testing commit criteria pipeline [1], patch LGTM. [1] - https://gitlab.com/tarantool/tarantool/-/pipelines/235161990 On Mon, Dec 28, 2020 at 09:17:29PM +0300, Cyrill Gorcunov via Tarantool-patches wrote: > Product team would prefer to have more data to > be included into a crash report. > > So we add "instance" key with appropriate values > (just like regular feedback entry has). For example > > | { > | "crashdump": { > | "version": "1", > | "data": { > | "uname": { > | "sysname": "Linux", > | "release": "5.9.14-100.fc32.x86_64", > | "version": "#1 SMP Fri Dec 11 14:30:38 UTC 2020", > | "machine": "x86_64" > | }, > | "instance": { > | "server_id": "336bfbfd-9e71-4728-91e3-ba84aec4d7ea", > | "cluster_id": "176f3669-488f-46a5-a744-1be0b8a31029", > | "uptime": "3" > | }, > | "build": { > | "version": "2.7.0-183-g02970b402", > | "cmake_type": "Linux-x86_64-Debug" > | }, > | "signal": { > | "signo": 11, > | "si_code": 0, > | "si_addr": "0x3e800095fb9", > | "backtrace": "#0 0x6317ab in crash_collect+bf...", > | "timestamp": "2020-12-28 21:09:29 MSK" > | } > | } > | } > | } > > Closes #5668 > > Signed-off-by: Cyrill Gorcunov > --- > issue https://github.com/tarantool/tarantool/issues/5668 > branch gorcunov/gh-5668-crash-extend > > src/lib/core/crash.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 45 insertions(+) > > diff --git a/src/lib/core/crash.c b/src/lib/core/crash.c > index 8397603bc..9f86f7a24 100644 > --- a/src/lib/core/crash.c > +++ b/src/lib/core/crash.c > @@ -14,7 +14,9 @@ > > #include "small/static.h" > #include "trivia/util.h" > +#include "uuid/tt_uuid.h" > > +#include "box/replication.h" > #include "backtrace.h" > #include "crash.h" > #include "say.h" > @@ -110,6 +112,7 @@ static struct crash_info { > static char tarantool_path[PATH_MAX]; > static char feedback_host[CRASH_FEEDBACK_HOST_MAX]; > static bool send_crashinfo = false; > +static uint64_t timestamp_mono = 0; > > static inline uint64_t > timespec_to_ns(struct timespec *ts) > @@ -143,6 +146,19 @@ crash_init(const char *tarantool_bin) > strlcpy_a(tarantool_path, tarantool_bin); > if (strlen(tarantool_path) < strlen(tarantool_bin)) > pr_panic("executable path is trimmed"); > + > + /* > + * We need to keep clock data locally to > + * report uptime without binding to libev > + * and etc. Because we're reporting information > + * at the moment when crash happens and we are to > + * be independent as much as we can. > + */ > + struct timespec ts; > + if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) > + timestamp_mono = timespec_to_ns(&ts); > + else > + pr_syserr("Can't fetch monotonic clock, ignore"); > } > > void > @@ -287,6 +303,35 @@ crash_report_feedback_daemon(struct crash_info *cinfo) > > /* Extend size, because now uname_ptr is not needed. */ > size = e - p; > + > + /* > + * Instance block requires uuid encoding so take it > + * from the tail of the buffer. > + */ > + snprintf_safe("\"instance\":{"); > + char *uuid_buf = &tail[-(UUID_STR_LEN+1)]; > + if (p >= uuid_buf) > + return -1; > + size = uuid_buf - p; > + > + tt_uuid_to_string(&INSTANCE_UUID, uuid_buf); > + snprintf_safe("\"server_id\":\"%s\",", uuid_buf); > + tt_uuid_to_string(&REPLICASET_UUID, uuid_buf); > + snprintf_safe("\"cluster_id\":\"%s\",", uuid_buf); > + > + /* No need for uuid_buf anymore. */ > + size = e - p; > + > + struct timespec ts; > + uint64_t uptime_ns; > + if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) > + uptime_ns = timespec_to_ns(&ts) - timestamp_mono; > + else > + uptime_ns = 0; > + snprintf_safe("\"uptime\":\"%llu\"", > + (unsigned long long)uptime_ns / 1000000000); > + snprintf_safe("},"); > + > snprintf_safe("\"build\":{"); > snprintf_safe("\"version\":\"%s\",", PACKAGE_VERSION); > snprintf_safe("\"cmake_type\":\"%s\"", BUILD_INFO); > -- > 2.26.2 >