GNU bug report logs - #31911
[PATCH] services: Add prometheus-node-exporter-service-type.

Previous Next

Package: guix-patches;

Reported by: Gábor Boskovits <boskovits <at> gmail.com>

Date: Wed, 20 Jun 2018 13:01:01 UTC

Severity: normal

Tags: patch

Done: Gábor Boskovits <boskovits <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


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

From: ludo <at> gnu.org (Ludovic Courtès)
To: Gábor Boskovits <boskovits <at> gmail.com>
Cc: 31911 <at> debbugs.gnu.org
Subject: Re: [bug#31911] [PATCH] services: Add
 prometheus-node-exporter-service-type.
Date: Sat, 23 Jun 2018 23:51:43 +0200
Hello Gábor!

Gábor Boskovits <boskovits <at> gmail.com> skribis:

> * gnu/services/monitoring.scm (prometheus-node-exporter-service-type):
> New variable.
> (<prometheus-node-exporter-configuration>): New record type.
> (prometheus-node-exporter-shepherd-service): New procedure.
> * gnu/doc/guix.texi (Monitoring Services): Document it.

That’s very useful!  Hopefully we can start using it on our build farm,
though we’ll need Grafena (?) or something to visualize those stats,
right?  OTOH apparently it already provides a web interface, so…?

It would be nice to add a system test for this service.  It could ensure
that representative URLs return 200 or 404, for instance (see the
hpcguix-web test in (gnu tests web) as an example.)

WDYT?

Minor comments:

> +@subsubheading Prometheus Node Exporter Service

Leave a newline here.

> +@cindex prometheus-node-exporter
> +Prometheus node exporter is a Prometheus exporter. It makes hardware and
> +operating system statistics provided by *NIX kernels available for the

The first sentence looks like a tautology.  :-)
I’m also unconvinced by the “*NIX” notation.

What about:

  The Prometheus ``node exporter'' makes hardware and operating system
  statistics available for the…

Please leave two spaces after end-of-sentence periods.

> +@defvar {Scheme variable} prometheus-node-exporter-service-type

Should be @defvr instead of @defvar.

> +@deftp {Data Type} prometheus-node-exporter-configuration
> +Data type representing the configuration of @command{node_exporter}.
> +
> +@table @asis
> +@item @code{package} (default: @code{go-github-com-prometheus-node-exporter})
> +The prometheus-node-exporter package to use.
> +@item @code{web-listen-address} (default: @code{":9100"})
> +Bind the web interface to the specified address.

Please add a newline before the second @item.

With these changes and ideally a simple test, this is ready to go IMO!

Thanks,
Ludo’.




This bug report was last modified 6 years and 318 days ago.

Previous Next


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