GNU bug report logs - #41688
[PATCH] Add rsyslog

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 41688 in the body.
You can then email your comments to 41688 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: Add rsyslog
Date: Wed, 3 Jun 2020 09:29:57 -0500

[Message part 1 (text/plain, inline)]

[0001-gnu-Add-libestr.patch (text/x-patch, attachment)]

[0005-gnu-Add-rsyslog.patch (text/x-patch, attachment)]

[0004-gnu-Add-liblognorm.patch (text/x-patch, attachment)]

[0002-gnu-Add-libfastjson.patch (text/x-patch, attachment)]

[0003-gnu-Add-liblogging.patch (text/x-patch, attachment)]

Message #10 received at 41688 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
Cc: 41688 <at> debbugs.gnu.org
Subject: Re: [bug#41688] Add rsyslog
Date: Fri, 12 Jun 2020 18:45:26 +0200

[Message part 1 (text/plain, inline)]

Hi Katherine,

Thanks for this long patch series!  I’ve applied half of it.  Here are
some comments:

Katherine Cox-Buday <cox.katherine.e <at> gmail.com> skribis:

> From 8d8d97441b397a48ae26761c2d826f35ae5d56e9 Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
> Date: Tue, 2 Jun 2020 14:55:23 -0500
> Subject: [PATCH 1/5] gnu: Add libestr.
>
> * gnu/packages/c.scm (libestr): New variable.

Applied with the changes below.

[Message part 2 (text/x-patch, inline)]

diff --git a/gnu/packages/c.scm b/gnu/packages/c.scm
index d3820c88b2..12a9b56c51 100644
--- a/gnu/packages/c.scm
+++ b/gnu/packages/c.scm
@@ -370,6 +370,6 @@ releases.")
     (home-page "https://github.com/rsyslog/libestr")
     (synopsis "Helper functions for handling strings")
     (description
-     "A library which contains some essential string manipulation
-functions and more, like escaping special characters.")
-    (license license:lgpl2.1)))
+     "This C library contains some essential string manipulation functions and
+more, like escaping special characters.")
+    (license license:lgpl2.1+)))

[Message part 3 (text/plain, inline)]

> From 0e0e9cc550ff002d7bebb4f93b573c5ba527ac4d Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
> Date: Tue, 2 Jun 2020 15:47:41 -0500
> Subject: [PATCH 5/5] gnu: Add rsyslog.
>
> * gnu/packages/logging.scm (rsyslog): New variable.

[...]

> +(define-public rsyslog
> +  (let ((modules (list "fmhash" "fmhttp" "imbatchreport"
> +                       "imczmq" "imdiag" "imdocker"
> +                       "imfile" "imgssapi" "imkafka"
> +                       "imklog" "imkmsg" "immark"
> +                       "improg" "impstats" "imptcp" "imtcp"
> +                       "imtuxedoulog" "imudp" "imuxsock"
> +                       "mmanon" "mmaudit" "mmcount"
> +                       "mmdblookup" "mmexternal"
> +                       "mmfields" "mmjsonparse"
> +                       "mmkubernetes" "mmnormalize"
> +                       "mmpstrucdata" "mmrfc5424addhmac"
> +                       "mmrm1stspace" "mmsequence"
> +                       "mmsnmptrapd" "mmtaghostname"
> +                       "mmutf8fix" "omclickhouse"
> +                       "omczmq" "omelasticsearch"
> +                       "omfile-hardened" "omgssapi"
> +                       "omhttpfs" "omhttp" "omkafka"
> +                       "omlibdbi" "ommail" "ommysql" "ompgsql"
> +                       "omprog" "omruleset" "omsnmp"
> +                       "omstdout" "omtcl" "omtesting"
> +                       "omudpspoof" "omuxsock"
> +                       "pmaixforwardedfrom" "pmciscoios"
> +                       "pmcisconames" "pmdb2diag"
> +                       "pmlastmsg" "pmnormalize"
> +                       "pmnull" "pmpanngfw" "pmsnare")))

Could you add a comment explaining where this list comes from?
Alternatively, do you think it could be generated at build time?

> +         #:configure-flags
> +         (list
> +          "--enable-largefile"
> +          "--enable-inet"
> +          "--enable-regexp"
> +          "--enable-rsyslogrt"
> +          "--enable-rsyslogd"

That’s also a very long list.  :-)

Can we rely on defaults?  Or, alternatively, could you add a comment
explaining how to come up with the list?

> +      (outputs `("out" ,@modules))

Oh.  Is it a good idea to have one output per module?  We’ve never done
such a thing before.  Usually, extra outputs are justified if not having
them would lead to a big closure (as reported by ‘guix size’).  The
solution is usually to have two or three outputs, but not 20ish.  :-)

WDYT?

> +      (home-page "https://www.rsyslog.com/")
> +      (synopsis "RSYSLOG is the rocket-fast system for log processing")
> +      (description
> +       "It offers high-performance, great security features and a modular

s/It/Rsyslog/

> +      (license (list license:gpl3
> +                     license:asl2.0
> +                     license:lgpl3)))))

Please add a comment above explaining if it’s triple-licensing or a
combination, and possibly add missing ‘+’ signs.

> From 3667290c9d419ce5404d6a0631bf20ddbcf1c286 Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
> Date: Tue, 2 Jun 2020 15:34:19 -0500
> Subject: [PATCH 4/5] gnu: Add liblognorm.
>
> * gnu/packages/c.scm (liblognorm): New variable.

[...]

> +    (arguments
> +     ;; Bash scripts interact with the filesystem
> +     `(#:tests? #f

Could you clarify what this means with regards to running tests?

> +       #:configure-flags
> +       (list (string-append "--includedir="
> +                            (assoc-ref %outputs "dev")
> +                            "/include"))
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'install 'fix-circular-dependency
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (write (string-append "KT: " (assoc-ref outputs "lib")
> +                                   "/lib/pkgconfig"))
> +             (write (string-append "KT: " (assoc-ref outputs "dev")
> +                                   "/lib/pkgconfig"))

Leftovers.  :-)

> +             (let ((pkgconfig (string-append (assoc-ref outputs "dev")
> +                                             "/lib/pkgconfig")))
> +               (mkdir-p pkgconfig)
> +               (rename-file (string-append (assoc-ref outputs "lib")
> +                                           "/lib/pkgconfig")
> +                            pkgconfig)))))))

Missing #t return value.

> +    (outputs '("out" "lib" "dev"))

“dev” is not part of the output names conventionally used.  However,
there’s “include”, which is automatically recognized by gnu-build-system
and turned into a ‘--includedir’ flag.  Perhaps you could try this?

Or you can just have “out” and “lib”: so far, unless headers are very
large, we just keep them alongside the library.

> +    (home-page "https://www.liblognorm.com")
> +    (synopsis
> +     "Fast samples-based log normalization library")
> +    (description
> +     "Liblognorm normalizes event data into well-defined name-value
> +pairs and a set of tags describing the message.")
> +    (license license:lgpl2.1)))

Could it be lgpl2.1+?  (I haven’t checked.)

> From db4bcfc18e95fa39851c72414261b19b25c49db0 Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
> Date: Tue, 2 Jun 2020 15:24:59 -0500
> Subject: [PATCH 2/5] gnu: Add libfastjson.
>
> * gnu/packages/c.scm (libfastjson): New variable.

Applied with minor changes.

[Message part 4 (text/x-patch, inline)]

diff --git a/gnu/packages/c.scm b/gnu/packages/c.scm
index e935042572..2e62111d44 100644
--- a/gnu/packages/c.scm
+++ b/gnu/packages/c.scm
@@ -394,14 +394,9 @@ more, like escaping special characters.")
        ("automake" ,automake)
        ("libtool" ,libtool)))
     (home-page "https://github.com/rsyslog/libfastjson")
-    (synopsis "Fast json library for C ")
+    (synopsis "Fast JSON library for C")
     (description
-     "libfastjson is a fork from json-c, and is currently under development.
-The aim of this project is not to provide a slightly modified clone of json-c.
-It's aim is to provide: a small library with essential json handling
-functions, sufficiently good json support (not 100% standards compliant), and
-be very fast in processing.")
-    (license
-     (license:non-copyleft
-      "https://github.com/rsyslog/libfastjson/blob/master/COPYING"
-      "It is a MIT license."))))
+     "libfastjson is a fork from json-c aiming to provide: a small library
+with essential JSON handling functions, sufficiently good JSON support (not
+100% standards compliant), and very fast processing.")
+    (license license:expat)))

[Message part 5 (text/plain, inline)]

> From 9e0c24cc6e0666581b11b45f831ab49486adbdf8 Mon Sep 17 00:00:00 2001
> From: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
> Date: Tue, 2 Jun 2020 15:29:08 -0500
> Subject: [PATCH 3/5] gnu: Add liblogging.
>
> * gnu/packages/c.scm (liblogging): New variable.

Applied.

Thank you!

Ludo’.

Message #15 received at 41688-done <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 41688-done <at> debbugs.gnu.org
Cc: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
Subject: [PATCH] Add rsyslog
Date: Fri, 31 Dec 2021 10:22:11 +0100

Hi,

I’ve followed Ludo’s suggestions for the remaining package definitions
and pushed the result to the “master” branch.

Among the changes:

- fixed and clarified licenses
- removed per-module outputs
- removed default configure flags and added a comment about enabling
  optional features
- enabled tests
- removed “dev” output
- used new input style

Now I guess we’ll need an rsyslog service :)

Thank you, Katherine, for the patches and your patience!

-- 
Ricardo

Message #18 received at 41688-done <at> debbugs.gnu.org (full text, mbox):

From: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 41688-done <at> debbugs.gnu.org
Subject: Re: [PATCH] Add rsyslog
Date: Wed, 05 Jan 2022 14:04:46 -0600

Ricardo Wurmus <rekado <at> elephly.net> writes:

> I’ve followed Ludo’s suggestions for the remaining package definitions
> and pushed the result to the “master” branch.

Thank you so much, Ricardo!

It is my fault for letting this sit, but I think we should unbundle the outputs once more -- even if they must become separate packages.

As you know, rsyslog is meant to forward (egress) and receive (ingress) syslog logs, and it does so through these plug-ins. For ingress, I don't think this package is necessarily a problem, but systems that want to forward logs may have very limited resources (e.g. I have a wifi repeater that forwards logs). I didn't want to require things like PostgreSQL and MySQL, dependencies required by ingress, on egress.

A secondary concern is that a package like rsyslog becomes quite sticky since there are lots and lots of modules, and I imagine the Guix closure becomes quite big.

> Among the changes:
>
> - fixed and clarified licenses
> - removed per-module outputs
> - removed default configure flags and added a comment about enabling
>   optional features
> - enabled tests
> - removed “dev” output
> - used new input style
>
> Now I guess we’ll need an rsyslog service :)

I'm working[1] on it! Although it is frustrating at how little time I have to dedicate to things like this :(

> Thank you, Katherine, for the patches and your patience!

Likewise, thank you for your contributions and getting this landed. Cheers!

[1] - https://github.com/kat-co/guix-channels/blob/upstream-staging/upstream/services/rsyslog.scm

-- 
Katherine

Message #21 received at 41688-done <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Katherine Cox-Buday <cox.katherine.e <at> gmail.com>
Cc: 41688-done <at> debbugs.gnu.org
Subject: Re: [PATCH] Add rsyslog
Date: Wed, 05 Jan 2022 21:51:49 +0100

Katherine Cox-Buday <cox.katherine.e <at> gmail.com> writes:

> I think we should unbundle the outputs once more -- even if they must
> become separate packages.

If we can build the plugins separately (and arrange for rsyslog to find
them via some search path) I’d be happy to see them moved to their own
packages.  Having them as separate outputs would only help a little as
you’d still need to have *all* the inputs ready to build the thing, even
if you just want to use one of the many plugins.

>> Now I guess we’ll need an rsyslog service :)
>
> I'm working[1] on it! Although it is frustrating at how little time I have to dedicate to things like this :(

Excellent, I’m looking forward to a service!  Perhaps we could also use
the existing syslog service with it if that service allows us to pass a
plain configuration file.  It should already let us swap out the syslog
package.

-- 
Ricardo

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.