Tarantool development patches archive
 help / color / mirror / Atom feed
From: Serge Petrenko <sergepetrenko@tarantool.org>
To: Cyrill Gorcunov <gorcunov@gmail.com>,
	tml <tarantool-patches@dev.tarantool.org>
Cc: Mons Anderson <v.perepelitsa@corp.mail.ru>,
	Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v4 4/4] crash: report crash data to the feedback server
Date: Fri, 11 Dec 2020 15:57:20 +0300	[thread overview]
Message-ID: <738d0eec-6ce6-1f24-5d25-e143e96bdd68@tarantool.org> (raw)
In-Reply-To: <20201210161832.729439-5-gorcunov@gmail.com>


10.12.2020 19:18, Cyrill Gorcunov пишет:
> We have a feedback server which gathers information about a running instnace.
> While general info is enough for now we may loose a precious information about
> crashes (such as call backtrace which caused the issue, type of build and etc).
>
> In the commit we add support of sending this kind of infomation to the feedback
> server. Internally we gather the reason of failure, pack it into base64 form
> and then run another tarantool instance which sends it out.
>
> A typical report might look like
>
>   | {
>   |   "crashdump": {
>   |     "version": "1",
>   |     "data": "eyJ1bmFtZSI6eyJzeXNuYW1lIjoiTGludXgiLCJyZWxlYXNlIjoiNS"
>   |             "45LjExLTEwMC5mYzMyLng4Nl82NCIsInZlcnNpb24iOiIjMSBTTVAg"
>   |             "VHVlIE5vdiAyNCAxOToxNjo1MyBVVEMgMjAyMCIsIm1hY2hpbmUiOi"
>   |             "J4ODZfNjQifSwiYnVpbGQiOnsidmVyc2lvbiI6IjIuNy4wLTgzLWc5"
>   |             "ZTg1MDM4ZmIiLCJjbWFrZV90eXBlIjoiTGludXgteDg2XzY0LURlYn"
>   |             "VnIn0sInNpZ25hbCI6eyJzaWdubyI6MTEsInNpX2NvZGUiOjAsInNp"
>   |             "X2FkZHIiOiIweDNlODAwMDVjOGE5IiwiYmFja3RyYWNlIjoiSXpBZ0"
>   |             "lEQjROak14WkRBeklHbHVJR055WVhOb1gyTnZiR3hsWTNRclltWUtJ"
>   |             "ekVnSURCNE5qTXlaV1JoSUdsdUlHTnlZWE5vWDNOcFoyNWhiRjlqWW"
>   |             "lzMU1Rb2pNaUFnTUhnM1pqRmtORGt5TXpsaE9UQWdhVzRnWm5WdWJH"
>   |             "OWphMlpwYkdVck5qQUtJek1nSURCNE4yWXhaRFE0WkdReFl6VmxJR2"
>   |             "x1SUdWd2IyeHNYM2RoYVhRck5XVUtJelFnSURCNE9ETTNOekV6SUds"
>   |             "dUlHVndiMnhzWDNCdmJHd3JPVGtLSXpVZ0lEQjRPRE5oWkRRNElHbH"
>   |             "VJR1YyWDNKMWJpczBPRGNLSXpZZ0lEQjROakJtTUdGbElHbHVJSFJo"
>   |             "Y21GdWRHOXZiRjlzZFdGZmNuVnVYM05qY21sd2RDc3hNemdLSXpjZ0"
>   |             "lEQjRORFEyTldZMUlHbHVJRzFoYVc0ck5XRmpDaU00SUNBd2VEZG1N"
>   |             "V1EwT0dObU56QTBNaUJwYmlCZlgyeHBZbU5mYzNSaGNuUmZiV0ZwYm"
>   |             "l0bU1nb2pPU0FnTUhnME5EUmhNR1VnYVc0Z1gzTjBZWEowS3pKbENn"
>   |             "PT0iLCJ0aW1lc3RhbXAiOiIyMDIwLTEyLTEwIDE3OjA4OjQyIE1TSyJ9fQ=="
>   |   }
>   | }
>
> The `data` value is a single string I wrapped it for commit message only.
> When `data` is decoded it consists of
>
>   | {
>   |   "uname": {
>   |     "sysname": "Linux",
>   |     "release": "5.9.11-100.fc32.x86_64",
>   |     "version": "#1 SMP Tue Nov 24 19:16:53 UTC 2020",
>   |     "machine": "x86_64"
>   |   },
>   |   "build": {
>   |     "version": "2.7.0-83-g9e85038fb",
>   |     "cmake_type": "Linux-x86_64-Debug"
>   |   },
>   |   "signal": {
>   |     "signo": 11,
>   |     "si_code": 0,
>   |     "si_addr": "0x3e80005c8a9",
>   |     "backtrace": "IzAgIDB4NjMxZDAzIGluIGNyYXNoX2NvbGxlY3QrYmYKIzEgID"
>   |                  "B4NjMyZWRhIGluIGNyYXNoX3NpZ25hbF9jYis1MQojMiAgMHg3ZjFkNDkyMzlhO"
>   |                  "TAgaW4gZnVubG9ja2ZpbGUrNjAKIzMgIDB4N2YxZDQ4ZGQxYzVlIGluIGVwb2xs"
>   |                  "X3dhaXQrNWUKIzQgIDB4ODM3NzEzIGluIGVwb2xsX3BvbGwrOTkKIzUgIDB4ODN"
>   |                  "hZDQ4IGluIGV2X3J1bis0ODcKIzYgIDB4NjBmMGFlIGluIHRhcmFudG9vbF9sdW"
>   |                  "FfcnVuX3NjcmlwdCsxMzgKIzcgIDB4NDQ2NWY1IGluIG1haW4rNWFjCiM4ICAwe"
>   |                  "DdmMWQ0OGNmNzA0MiBpbiBfX2xpYmNfc3RhcnRfbWFpbitmMgojOSAgMHg0NDRh"
>   |                  "MGUgaW4gX3N0YXJ0KzJlCg==",
>   |     "timestamp": "2020-12-10 17:08:42 MSK"
>   |   }
>   | }


Thanks for the patch!

Please find one comment below.


> @@ -126,6 +214,179 @@ crash_collect(int signo, siginfo_t *siginfo, void *ucontext)
>   	return cinfo;
>   }
>   
> +/**
> + * Report crash information to the feedback daemon
> + * (ie send it to feedback daemon).
> + */
> +static void
> +crash_report_feedback_daemon(struct crash_info *cinfo)
> +{
> +	/*
> +	 * Update to a new number if format get changed.
> +	 */
> +	static const int crashinfo_version = 1;
> +
> +	char *p = static_alloc(SMALL_STATIC_SIZE);
> +	char *e = &p[SMALL_STATIC_SIZE - 1];
> +	char *head = p;
> +	char *tail = &p[SMALL_STATIC_SIZE - 8];
> +
> +	/*
> +	 * Note that while we encode the report we
> +	 * intensively use a tail of the allocated
> +	 * buffer as a temporary store.
> +	 */
> +
> +#define snprintf_safe(fmt, ...)					\
> +	do {							\
> +		p += snprintf(p, e - p, fmt, ##__VA_ARGS__);	\
> +		if (p >= e)					\
> +			goto out;				\
> +	} while (0)
> +
> +	/*
> +	 * On linux there is new_utsname structure which
> +	 * encodes each field to __NEW_UTS_LEN + 1 => 64 + 1 = 65.
> +	 *
> +	 * So lets just reserve more data in advance: 5 fields
> +	 * 128 bytes each => 640 bytes.
> +	 */
> +	static_assert(sizeof(struct utsname) <= 640,
> +		      "utsname is bigger than expected");


This static assert fails on my mac.
Looks like `struct utsname` is 1280 bytes in size there.


> +
> +	/*
> +	 * Lets reuse tail of the buffer as a temp space.
> +	 */
> +	struct utsname *uname_ptr = (void *)&tail[-640];
> +	if (p > (char *)(void *)uname_ptr)
> +		goto out;
> +
> +	if (uname(uname_ptr) != 0) {
> +		pr_syserr("uname call failed, ignore");
> +		memset(uname_ptr, 0, 640);
> +	}
> +

-- 
Serge Petrenko

  reply	other threads:[~2020-12-11 12:57 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-10 16:18 [Tarantool-patches] [PATCH v4 0/4] crash: implement sending feedback Cyrill Gorcunov
2020-12-10 16:18 ` [Tarantool-patches] [PATCH v4 1/4] util: introduce strlcpy helper Cyrill Gorcunov
2020-12-11  7:34   ` Serge Petrenko
2020-12-11  7:58     ` Serge Petrenko
2020-12-11 10:04       ` Cyrill Gorcunov
2020-12-11 11:07         ` Serge Petrenko
2020-12-11 11:38           ` Cyrill Gorcunov
2020-12-14 22:54             ` Vladislav Shpilevoy
2020-12-14 22:54   ` Vladislav Shpilevoy
2020-12-15  8:47     ` Cyrill Gorcunov
2020-12-10 16:18 ` [Tarantool-patches] [PATCH v4 2/4] backtrace: allow to specify destination buffer Cyrill Gorcunov
2020-12-11  7:50   ` Serge Petrenko
2020-12-10 16:18 ` [Tarantool-patches] [PATCH v4 3/4] crash: move fatal signal handling in Cyrill Gorcunov
2020-12-11  9:31   ` Serge Petrenko
2020-12-11 10:38     ` Cyrill Gorcunov
2020-12-11 11:12       ` Serge Petrenko
2020-12-14 22:54   ` Vladislav Shpilevoy
2020-12-15  8:16     ` Cyrill Gorcunov
2020-12-20 14:48       ` Vladislav Shpilevoy
2020-12-20 15:49         ` Cyrill Gorcunov
2020-12-20 16:07           ` Vladislav Shpilevoy
2020-12-20 16:58             ` Cyrill Gorcunov
2020-12-20 15:45   ` Vladislav Shpilevoy
2020-12-10 16:18 ` [Tarantool-patches] [PATCH v4 4/4] crash: report crash data to the feedback server Cyrill Gorcunov
2020-12-11 12:57   ` Serge Petrenko [this message]
2020-12-12 16:19     ` Cyrill Gorcunov
2020-12-12 17:07       ` Cyrill Gorcunov
2020-12-14  9:41         ` Serge Petrenko
2020-12-14 22:54   ` Vladislav Shpilevoy
2020-12-16 11:16     ` Cyrill Gorcunov
2020-12-16 20:31       ` Cyrill Gorcunov
2020-12-20 15:16         ` Vladislav Shpilevoy
2020-12-20 18:26           ` Cyrill Gorcunov
2020-12-20 14:48       ` Vladislav Shpilevoy
2020-12-20 18:21         ` Cyrill Gorcunov
2020-12-20 18:41           ` Vladislav Shpilevoy
2020-12-20 19:16             ` Cyrill Gorcunov
2020-12-21 17:01               ` Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=738d0eec-6ce6-1f24-5d25-e143e96bdd68@tarantool.org \
    --to=sergepetrenko@tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.perepelitsa@corp.mail.ru \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v4 4/4] crash: report crash data to the feedback server' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox