From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48414) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fXMvr-0005ur-E2 for guix-patches@gnu.org; Mon, 25 Jun 2018 04:33:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fXMvm-0006PV-BU for guix-patches@gnu.org; Mon, 25 Jun 2018 04:33:07 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:54583) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fXMvm-0006PM-7v for guix-patches@gnu.org; Mon, 25 Jun 2018 04:33:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fXMvl-0000Fx-VG for guix-patches@gnu.org; Mon, 25 Jun 2018 04:33:01 -0400 Subject: [bug#31911] [PATCH] services: Add prometheus-node-exporter-service-type. Resent-Message-ID: MIME-Version: 1.0 References: <20180620125946.7264-1-boskovits@gmail.com> <87po0hw4r4.fsf@gnu.org> In-Reply-To: <87po0hw4r4.fsf@gnu.org> From: =?UTF-8?Q?G=C3=A1bor?= Boskovits Date: Mon, 25 Jun 2018 10:31:45 +0200 Message-ID: Content-Type: multipart/alternative; boundary="000000000000eae8e5056f7336df" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 31911@debbugs.gnu.org --000000000000eae8e5056f7336df Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s ezt =C3=ADrta (id=C5=91pont: 2018. j=C3= =BAn. 23., Szo, 23:51): > Hello G=C3=A1bor! > > G=C3=A1bor Boskovits skribis: > > > * gnu/services/monitoring.scm (prometheus-node-exporter-service-type): > > New variable. > > (): New record type. > > (prometheus-node-exporter-shepherd-service): New procedure. > > * gnu/doc/guix.texi (Monitoring Services): Document it. > > That=E2=80=99s very useful! Hopefully we can start using it on our build= farm, > though we=E2=80=99ll need Grafena (?) or something to visualize those sta= ts, > right? OTOH apparently it already provides a web interface, so=E2=80=A6? > > This is actually only one half of the solution, this provides an endpoint for the prometheus server to scrape. I've not yet packaged the server part. The server has the needed visualization capabilities. > 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? > > Ok, will do. > 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 a= nd > > +operating system statistics provided by *NIX kernels available for the > > The first sentence looks like a tautology. :-) > I=E2=80=99m also unconvinced by the =E2=80=9C*NIX=E2=80=9D notation. > > What about: > > The Prometheus ``node exporter'' makes hardware and operating system > statistics available for the=E2=80=A6 > > 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. > > Most of these were copied almost verbatim form darkstat service. Maybe these modification should be also applied there. Will send an updated patch. Should I contact Sou Bunnbu regadring the darkstat documentation, or should I fix it? > With these changes and ideally a simple test, this is ready to go IMO! > > Thanks, > Ludo=E2=80=99. > --000000000000eae8e5056f7336df Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Ludovic Court= =C3=A8s <ludo@gnu.org> ezt =C3=AD= rta (id=C5=91pont: 2018. j=C3=BAn. 23., Szo, 23:51):
Hello G=C3=A1bor!

G=C3=A1bor Boskovits <boskovits@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=E2=80=99s very useful!=C2=A0 Hopefully we can start using it on our bu= ild farm,
though we=E2=80=99ll need Grafena (?) or something to visualize those stats= ,
right?=C2=A0 OTOH apparently it already provides a web interface, so=E2=80= =A6?


This is actually only one half of the = solution, this provides an endpoint for
the prometheus server to = scrape. I've not yet packaged the server part.
The server has= the needed visualization capabilities.
=C2=A0
It would be nice to add a system test for this service.=C2=A0 It could ensu= re
that representative URLs return 200 or 404, for instance (see the
hpcguix-web test in (gnu tests web) as an example.)

WDYT?


Ok, will do.
=C2=A0
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 th= e

The first sentence looks like a tautology.=C2=A0 :-)
I=E2=80=99m also unconvinced by the =E2=80=9C*NIX=E2=80=9D notation.

What about:

=C2=A0 The Prometheus ``node exporter'' makes hardware and operatin= g system
=C2=A0 statistics available for the=E2=80=A6

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}.<= br> > +
> +@table @asis
> +@item @code{package} (default: @code{go-github-com-prometheus-node-ex= porter})
> +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.


Most of these were copied almost verba= tim form darkstat service. Maybe these modification should be also applied = there.
Will send an updated patch. Should I contact Sou Bunnbu re= gadring the darkstat documentation, or should I fix it?
=C2=A0
With these changes and ideally a simple test, this is ready to go IMO!

Thanks,
Ludo=E2=80=99.
--000000000000eae8e5056f7336df--