WriteToTextfile function for node exporter textfiles

[sevagh]

@beorn7

This is my attempt to add (similar, and implementation copied from, client_python) WriteToTextfile for a Registry.

This is so we can write Go tools that emit Node Exporter textfiles.

My strategy is to create the same output from the promhttp HTML website.

[sevagh]

Usage looks like:

...
import "github.com/prometheus/client_golang/prometheus"

var (
        fakeGauge = prometheus.NewGaugeVec(
                prometheus.GaugeOpts{
                        Name: "fake_metric",
                        Help: "this is so fake",
                },
                []string{"name", "foo", "bar"},
        )
)

        registry := prometheus.NewRegistry()
        registry.MustRegister(fakeGauge)
        fakeGauge.With(prometheus.Labels{"name": "hello", "foo": "xyz", "bar": "abc"}).Set(65.99)
        registry.WriteToTextfile("./helloworld.txt")

[sevagh]

In my test, the output I compare to (master...sevagh:master#diff-7c4dacb8aec4877f9e6a1fd1768ce969R878) is copy-pasted from the promhttp output, since I figure the best test is to see that the HTTP metrics page and text file output are the same.

I also had to manually create a "+Inf" bucket (master...sevagh:master#diff-6223e8ae8180a02a4024a006b8e5f2f8R597) in my output string. Not sure if it's expected for Histogram.GetBucket() -> []*Bucket to not return the +Inf bucket.

[sevagh]

I tried to think of ways of making the code less ugly, but it's hard, given that the functions for GetHistogram().GetBucket() and GetSummary().GetQuantile(), and GetGauge/Counter/Untyped().GetValue() are similar but there are no interfaces or abstractions.

[beorn7]

This is indeed a cool feature. So cool that it already exists, kind of…

This is the version of your initial example program, using the existing functionality:

package main

import (
	"fmt"
	"io/ioutil"
	"os"
	"path/filepath"

	"github.com/prometheus/client_golang/prometheus"
	"github.com/prometheus/common/expfmt"
)

var (
	fakeGauge = prometheus.NewGaugeVec(
		prometheus.GaugeOpts{
			Name: "fake_metric",
			Help: "this is so fake",
		},
		[]string{"name", "foo", "bar"},
	)
)

func WriteToTextfile(g prometheus.Gatherer, filename string) error {
	tmp, err := ioutil.TempFile(filepath.Dir(filename), filepath.Base(filename))
	defer os.Remove(tmp.Name())

	mfs, err := g.Gather()
	if err != nil {
		return err
	}
	for _, mf := range mfs {
		if _, err := expfmt.MetricFamilyToText(tmp, mf); err != nil {
			return err
		}
	}
	return os.Rename(tmp.Name(), filename)
}

func main() {
	registry := prometheus.NewRegistry()
	registry.MustRegister(fakeGauge)
	fakeGauge.With(prometheus.Labels{"name": "hello", "foo": "xyz", "bar": "abc"}).Set(65.99)

	if err := WriteToTextfile(registry, "test.prom"); err != nil {
		fmt.Println("Textfile creation failed:", err)
	}
}

As you can see, it's only nine lines of code. However, this is probably providing enough convenience and encodes the operational concern of first writing into a temp file that it warrants the creation of an own function in the client library. It should, however, not be a method on the Registry type but a top level function so that it works on any implementation of Gatherer.

In other words: We could just add my WriteToTextfile as above and use your test from this PR to test it. What do you think? Would you like to change your PR accordingly?

(I think there is no need to replicate the PID way of naming the temp file as we have ioutil.TempFile in the Go standard library.)

[beorn7]

Improved version of my suggested function:

func WriteToTextfile(filename string, g prometheus.Gatherer) error {
	tmp, err := ioutil.TempFile(filepath.Dir(filename), filepath.Base(filename))
	if err != nil {
		return err
	}
	defer os.Remove(tmp.Name())

	mfs, err := g.Gather()
	if err != nil {
		return err
	}
	for _, mf := range mfs {
		if _, err := expfmt.MetricFamilyToText(tmp, mf); err != nil {
			return err
		}
	}
	if err := tmp.Close(); err != nil {
		return err
	}
	return os.Rename(tmp.Name(), filename)
}

• Same order of arguments as in the Pythos client library.
• Proper error handling.
• Actually closing the file.

[sevagh]

Sounds good to me - I used your code, and adapted the test. I also wrote a little doc string for the new function:

// WriteToTextfile formats the metrics of the provided Gatherer interface and
// emits them in text format to the path. Intended for use with the node_exporter
// textfile collector

[beorn7]

Thank you. Almost perfect, just a bit of nitpicking about the doc comment.

[beorn7]

Almost perfect. I'd add the fact that the node exporter wants the filename to look like *.prom.

Suggested modified text (with a few more tweaks):

WriteToTextfile calls Gather on the provided Gatherer, encodes the result in the Prometheus text format, and writes it to a temporary file. Upon success, the temporary file is renamed to the provided filename. This is intended for use with the textfile collector of the node exporter. Note that the node exporter expects the filename to be suffixed with ".prom".

[sevagh]

Not sure if this is related to how Python and Go write/create files differently, I'm running into this:

shanssian@systest-8:/usr/share/ansible$ stat -c '%a %n' /opt/node-exporter-sb/textfiles/smart.prom
644 /opt/node-exporter-sb/textfiles/smart.prom
shanssian@systest-8:/usr/share/ansible$ stat -c '%a %n' /opt/node-exporter-sb/textfiles/disks.prom
600 /opt/node-exporter-sb/textfiles/disks.prom

smart.prom is generated using Python write_to_textfile (in a cronjob running as root), disks.prom is using Go code that uses prometheus.WriteToTextfile from this pull request.

node_exporter can't collect it:

node-exporter-sb/textfiles/disks.prom\": open /opt/node-exporter-sb/textfiles/disks.prom: permission denied","source":"textfile.go:204","time":"2018-11-02T09:42:34-07:00"}

Perhaps I should manually fix up the permissions in the code?

[sevagh]

I think it's from the Python code manually creating a tempfile, while Go ioutil uses OS tempfile specifics with tighter permissions.

[beorn7]

I guess 644 makes sense in this context. Users could still tweak the permissions of the directory if they want to lock things down.

[beorn7]

Thanks for the new feature. :o)