Ludovic Courtès <ludo@gnu.org> ezt írta (időpont: 2018. jún. 23., Szo,
23:51):

> Hello Gábor!
>
> Gábor 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’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…?
>
>
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 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.
>
>
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’.
>