From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 38890272BB for ; Thu, 29 Mar 2018 04:11:40 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id egvA6gcNsbAG for ; Thu, 29 Mar 2018 04:11:40 -0400 (EDT) Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 6309A272A8 for ; Thu, 29 Mar 2018 04:11:39 -0400 (EDT) From: Georgy Kirichenko Subject: [tarantool-patches] Re: [log 1/1] log: Fix syslog logger Date: Thu, 29 Mar 2018 11:11:32 +0300 Message-ID: <3209994.qyiTjyFyQt@home.lan> In-Reply-To: <1522224245.228428557@f507.i.mail.ru> References: <1522224245.228428557@f507.i.mail.ru> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart3700058.yZA4quE7fM"; micalg="pgp-sha256"; protocol="application/pgp-signature" Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Ilya Markov --nextPart3700058.yZA4quE7fM Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Please check and fix travis for the branch first Note: please include a branch url (not a branch name) under the `---' line On Wednesday, March 28, 2018 11:04:05 AM MSK Ilya Markov wrote: > * Remove rewriting format of default logger in case of syslog option. > * Add facility option parsing and use parsed results in format message > according to RFC3164. Possible values and default value of syslog > facility are taken from nginx( https://nginx.ru/en/docs/syslog.html ) > > Closes #3244 > > branch: gh-3244-syslog-facility > --- > src/say.c | 74 > +++++++++++++++++++++++++++++++++++++++++++--------- src/say.h | > 31 +++++++++++++++++++++- > test/unit/say.c | 26 +++++++++++++++--- > test/unit/say.result | 33 +++++++++++++++-------- > 4 files changed, 135 insertions(+), 29 deletions(-) > > diff --git a/src/say.c b/src/say.c > index b92514d..68e3b09 100644 > --- a/src/say.c > +++ b/src/say.c > @@ -191,16 +191,17 @@ say_set_log_level(int new_level) > void > say_set_log_format(enum say_format format) > { > - > + /* > + * For syslog, default or boot log type the log format can > + * not be changed. > + */ > + bool allowed_to_change = log_default->type == SAY_LOGGER_STDERR || > + log_default->type == SAY_LOGGER_PIPE || > + log_default->type == SAY_LOGGER_FILE; > switch (format) { > case SF_JSON: > - /* > - * For syslog, default or boot log type the log format can > - * not be changed. > - */ > - if (log_default->type != SAY_LOGGER_STDERR && > - log_default->type != SAY_LOGGER_PIPE && > - log_default->type != SAY_LOGGER_FILE) { > + > + if (!allowed_to_change) { > say_error("json log format is not supported when output is '%s'", > say_logger_type_strs[log_default->type]); > return; > @@ -208,6 +209,9 @@ say_set_log_format(enum say_format format) > log_set_format(log_default, say_format_json); > break; > case SF_PLAIN: > + if (!allowed_to_change) { > + return; > + } > log_set_format(log_default, say_format_plain); > break; > default: > @@ -477,6 +481,11 @@ log_syslog_init(struct log *log, const char *init_str) > log->syslog_ident = strdup("tarantool"); > else > log->syslog_ident = strdup(opts.identity); > + > + if (opts.facility == syslog_facility_MAX) > + log->syslog_facility = SYSLOG_LOCAL7; > + else > + log->syslog_facility = opts.facility; > say_free_syslog_opts(&opts); > log->fd = log_syslog_connect(log); > if (log->fd < 0) { > @@ -814,14 +823,14 @@ say_format_syslog(struct log *log, char *buf, int len, > int level, const char *fi > /* Format syslog header according to RFC */ > int prio = level_to_syslog_priority(level); > - SNPRINT(total, snprintf, buf, len, "<%d>", LOG_MAKEPRI(1, prio)); > + SNPRINT(total, snprintf, buf, len, "<%d>", > + LOG_MAKEPRI(8 * log->syslog_facility, prio)); > SNPRINT(total, strftime, buf, len, "%h %e %T ", &tm); > SNPRINT(total, snprintf, buf, len, "%s[%d]:", log->syslog_ident, > getpid()); > /* Format message */ > SNPRINT(total, say_format_plain_tail, buf, len, level, filename, line, > error, format, ap); > - > return total; > } > > @@ -963,11 +972,45 @@ say_parse_logger_type(const char **str, enum > say_logger_type *type) return 0; > } > > +static const char *syslog_facility_strs[] = { > + [SYSLOG_KERN] = "kern", > + [SYSLOG_USER] = "user", > + [SYSLOG_MAIL] = "mail", > + [SYSLOG_DAEMON] = "daemon", > + [SYSLOG_AUTH] = "auth", > + [SYSLOG_INTERN] = "intern", > + [SYSLOG_LPR] = "lpr", > + [SYSLOG_NEWS] = "news", > + [SYSLOG_UUCP] = "uucp", > + [SYSLOG_CLOCK] = "clock", > + [SYSLOG_AUTHPRIV] = "authpriv", > + [SYSLOG_FTP] = "ftp", > + [SYSLOG_NTP] = "ntp", > + [SYSLOG_AUDIT] = "audit", > + [SYSLOG_ALERT] = "alert", > + [SYSLOG_CRON] = "cron", > + [SYSLOG_LOCAL0] = "local0", > + [SYSLOG_LOCAL1] = "local1", > + [SYSLOG_LOCAL2] = "local2", > + [SYSLOG_LOCAL3] = "local3", > + [SYSLOG_LOCAL4] = "local4", > + [SYSLOG_LOCAL5] = "local5", > + [SYSLOG_LOCAL6] = "local6", > + [SYSLOG_LOCAL7] = "local7", > + [syslog_facility_MAX] = "unknown", > +}; > + > +enum syslog_facility > +say_syslog_facility_by_name(const char *facility) > +{ > + return STR2ENUM(syslog_facility, facility); > +} > + > int > say_parse_syslog_opts(const char *init_str, struct say_syslog_opts *opts) > { > opts->identity = NULL; > - opts->facility = NULL; > + opts->facility = syslog_facility_MAX; > opts->copy = strdup(init_str); > if (opts->copy == NULL) { > diag_set(OutOfMemory, strlen(init_str), "malloc", "opts->copy"); > @@ -987,9 +1030,14 @@ say_parse_syslog_opts(const char *init_str, struct > say_syslog_opts *opts) goto duplicate; > opts->identity = value; > } else if (say_parse_prefix(&value, "facility=")) { > - if (opts->facility != NULL) > + if (opts->facility != syslog_facility_MAX) > goto duplicate; > - opts->facility = value; > + opts->facility = say_syslog_facility_by_name(value); > + if (opts->facility == syslog_facility_MAX) { > + diag_set(IllegalParams, "bad syslog facility option '%s'", > + value); > + goto error; > + } > } else { > diag_set(IllegalParams, "bad option '%s'", option); > goto error; > diff --git a/src/say.h b/src/say.h > index 46e6976..0703c6a 100644 > --- a/src/say.h > +++ b/src/say.h > @@ -97,6 +97,34 @@ enum say_logger_type { > SAY_LOGGER_SYSLOG > }; > > +enum syslog_facility { > + SYSLOG_KERN = 0, > + SYSLOG_USER, > + SYSLOG_MAIL, > + SYSLOG_DAEMON, > + SYSLOG_AUTH, > + SYSLOG_INTERN, > + SYSLOG_LPR, > + SYSLOG_NEWS, > + SYSLOG_UUCP, > + SYSLOG_CLOCK, > + SYSLOG_AUTHPRIV, > + SYSLOG_FTP, > + SYSLOG_NTP, > + SYSLOG_AUDIT, > + SYSLOG_ALERT, > + SYSLOG_CRON, > + SYSLOG_LOCAL0, > + SYSLOG_LOCAL1, > + SYSLOG_LOCAL2, > + SYSLOG_LOCAL3, > + SYSLOG_LOCAL4, > + SYSLOG_LOCAL5, > + SYSLOG_LOCAL6, > + SYSLOG_LOCAL7, > + syslog_facility_MAX, > +}; > + > struct log; > > typedef int (*log_format_func_t)(struct log *log, char *buf, int len, int > level, @@ -119,6 +147,7 @@ struct log { > pid_t pid; > /* Application identifier used to group syslog messages. */ > char *syslog_ident; > + enum syslog_facility syslog_facility; > struct rlist in_log_list; > }; > > @@ -349,7 +378,7 @@ say_parse_logger_type(const char **str, enum > say_logger_type *type); /** Syslog logger initialization params */ > struct say_syslog_opts { > const char *identity; > - const char *facility; > + enum syslog_facility facility; > /* Input copy (content unspecified). */ > char *copy; > }; > diff --git a/test/unit/say.c b/test/unit/say.c > index 8c996ba..4e5de75 100644 > --- a/test/unit/say.c > +++ b/test/unit/say.c > @@ -39,7 +39,7 @@ parse_syslog_opts(const char *input) > if (opts.identity) > note("identity: %s", opts.identity); > if (opts.facility) > - note("facility: %s", opts.facility); > + note("facility: %i", opts.facility); > say_free_syslog_opts(&opts); > return 0; > } > @@ -124,7 +124,7 @@ int main() > fiber_init(fiber_c_invoke); > say_logger_init("/dev/null", S_INFO, 0, "plain", 0); > > - plan(23); > + plan(29); > > #define PARSE_LOGGER_TYPE(input, rc) \ > ok(parse_logger_type(input) == rc, "%s", input) > @@ -149,7 +149,10 @@ int main() > PARSE_SYSLOG_OPTS("identity=tarantool", 0); > PARSE_SYSLOG_OPTS("facility=user", 0); > PARSE_SYSLOG_OPTS("identity=xtarantoolx,facility=local1", 0); > - PARSE_SYSLOG_OPTS("facility=foo,identity=bar", 0); > + PARSE_SYSLOG_OPTS("identity=xtarantoolx,facility=kern", 0); > + PARSE_SYSLOG_OPTS("identity=xtarantoolx,facility=uucp", 0); > + PARSE_SYSLOG_OPTS("identity=xtarantoolx,facility=foo", -1); > + PARSE_SYSLOG_OPTS("facility=authpriv,identity=bar", 0); > PARSE_SYSLOG_OPTS("invalid=", -1); > PARSE_SYSLOG_OPTS("facility=local1,facility=local2", -1); > PARSE_SYSLOG_OPTS("identity=foo,identity=bar", -1); > @@ -170,7 +173,7 @@ int main() > log_set_format(&test_log, format_func_custom); > log_say(&test_log, 0, NULL, 0, NULL, "hello %s", "user"); > > - FILE* fd = fopen(tmp_filename, "r"); > + FILE* fd = fopen(tmp_filename, "r+"); > const size_t len = 4096; > char line[len]; > > @@ -187,6 +190,21 @@ int main() > } > log_destroy(&test_log); > > + log_create(&test_log, "syslog:identity=tarantool,facility=local0", false); > + test_log.fd = fileno(fd); > + /* redirect stderr to /dev/null in order to filter > + * it out from result file. > + */ > + freopen("/dev/null", "w", stderr); > + ok(strncmp(test_log.syslog_ident, "tarantool", 9) == 0, "parsed > identity"); + ok(test_log.syslog_facility == SYSLOG_LOCAL0, "parsed > facility"); + long before = ftell(fd); > + log_say(&test_log, 0, NULL, 0, NULL, "hello %s", "user"); > + fseek(fd, before, SEEK_SET); > + > + if (fgets(line, len, fd) != NULL) { > + ok(strstr(line, "<131>") != NULL, "syslog line"); > + } > // test on log_rotate signal handling > struct ev_signal ev_sig; > ev_signal_init(&ev_sig, say_logrotate, SIGHUP); > diff --git a/test/unit/say.result b/test/unit/say.result > index dfd3fc0..db7befb 100644 > --- a/test/unit/say.result > +++ b/test/unit/say.result > @@ -1,4 +1,4 @@ > -1..23 > +1..29 > # type: file > # next: > ok 1 - > @@ -33,20 +33,31 @@ ok 10 - syslog:identity= > ok 11 - unknown: > # next: unknown:example.org > ok 12 - unknown:example.org > +# facility: 24 > ok 13 - > # identity: tarantool > +# facility: 24 > ok 14 - identity=tarantool > -# facility: user > +# facility: 1 > ok 15 - facility=user > # identity: xtarantoolx > -# facility: local1 > +# facility: 17 > ok 16 - identity=xtarantoolx,facility=local1 > +# identity: xtarantoolx > +ok 17 - identity=xtarantoolx,facility=kern > +# identity: xtarantoolx > +# facility: 8 > +ok 18 - identity=xtarantoolx,facility=uucp > +ok 19 - identity=xtarantoolx,facility=foo > # identity: bar > -# facility: foo > -ok 17 - facility=foo,identity=bar > -ok 18 - invalid= > -ok 19 - facility=local1,facility=local2 > -ok 20 - identity=foo,identity=bar > -ok 21 - plain > -ok 22 - json > -ok 23 - custom > +# facility: 10 > +ok 20 - facility=authpriv,identity=bar > +ok 21 - invalid= > +ok 22 - facility=local1,facility=local2 > +ok 23 - identity=foo,identity=bar > +ok 24 - plain > +ok 25 - json > +ok 26 - custom > +ok 27 - parsed identity > +ok 28 - parsed facility > +ok 29 - syslog line --nextPart3700058.yZA4quE7fM Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part. Content-Transfer-Encoding: 7Bit -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEBJFDbU76LsBbgHBsvKOmCX79zb4FAlq8n7QACgkQvKOmCX79 zb7oNQgAjoLuPHyVz5rdDkD1jCyZbkkRAkHXdRda8p5KIJDJ4DjKmUYgtR2qEKSc /xrBP8ZSwlVOvJLQtO4bBmbrV+eOgNwDyA6wXiRWuodTwAsELxknd+ELLzi5Xl31 vS2ToLn0fySCM1JI9iUQggE7Juxhq33ysg4M11kQcvLsAYh7pvHMKPkIxATGY4PP ySpJOh5YIPHK1svhrgLr3+4vLCBY/m32VmDC+40SGPkRrEy4Gn6LoXkg8E/IOTm4 eN8ylj283/YSRAWeN4BkMxzB+sIuFsFCMgI3j+uTPfxToMPYX3dGHOMiFkRfKqTz +XEyZMjn85KTyE6/2KWUbYR8BeefSg== =M9xz -----END PGP SIGNATURE----- --nextPart3700058.yZA4quE7fM--