From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (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 1FA1F45C304 for ; Fri, 11 Dec 2020 15:57:23 +0300 (MSK) References: <20201210161832.729439-1-gorcunov@gmail.com> <20201210161832.729439-5-gorcunov@gmail.com> From: Serge Petrenko Message-ID: <738d0eec-6ce6-1f24-5d25-e143e96bdd68@tarantool.org> Date: Fri, 11 Dec 2020 15:57:20 +0300 MIME-Version: 1.0 In-Reply-To: <20201210161832.729439-5-gorcunov@gmail.com> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH v4 4/4] crash: report crash data to the feedback server List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov , tml Cc: Mons Anderson , Vladislav Shpilevoy 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