Properly handle empty job and label values

An empty job name was always an error, but it was previously only
detected when pushing to the PGW and receiving an error. With this
commit, the error is detected before pushing.

An empty label value should have been OK but was encoded in a way that
couldn't be pushed to the
PGW. Cf. https://github.com/prometheus/pushgateway/issues/344 . This
commit changes the creation of the path in the URL so that it works
with empty label values.

Signed-off-by: beorn7 <beorn@grafana.com>

prometheus/push/push.go

@@ -37,6 +37,7 @@ package push
 import (
 	"bytes"
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -56,6 +57,8 @@ const (
 	base64Suffix = "@base64"
 )
 
+var errJobEmpty = errors.New("job name is empty")
+
 // HTTPDoer is an interface for the one method of http.Client that is used by Pusher
 type HTTPDoer interface {
 	Do(*http.Request) (*http.Response, error)
@@ -80,14 +83,17 @@ type Pusher struct {
 }
 
 // New creates a new Pusher to push to the provided URL with the provided job
-// name. You can use just host:port or ip:port as url, in which case “http://”
+// name (which must not be empty). You can use just host:port or ip:port as url,
-// is added automatically. Alternatively, include the schema in the
+// in which case “http://” is added automatically. Alternatively, include the
-// URL. However, do not include the “/metrics/jobs/…” part.
+// schema in the URL. However, do not include the “/metrics/jobs/…” part.
 func New(url, job string) *Pusher {
 	var (
 		reg = prometheus.NewRegistry()
 		err error
 	)
+	if job == "" {
+		err = errJobEmpty
+	}
 	if !strings.Contains(url, "://") {
 		url = "http://" + url
 	}
@@ -267,7 +273,7 @@ func (p *Pusher) push(method string) error {
 		return err
 	}
 	defer resp.Body.Close()
-	// Pushgateway 0.10+ responds with StatusOK, earlier versions with StatusAccepted.
+	// Depending on version and configuration of the PGW, StatusOK or StatusAccepted may be returned.
 	if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusAccepted {
 		body, _ := ioutil.ReadAll(resp.Body) // Ignore any further error as this is for an error message only.
 		return fmt.Errorf("unexpected status code %d while pushing to %s: %s", resp.StatusCode, p.fullURL(), body)
@@ -278,9 +284,11 @@ func (p *Pusher) push(method string) error {
 // fullURL assembles the URL used to push/delete metrics and returns it as a
 // string. The job name and any grouping label values containing a '/' will
 // trigger a base64 encoding of the affected component and proper suffixing of
-// the preceding component. If the component does not contain a '/' but other
+// the preceding component. Similarly, an empty grouping label value will be
-// special character, the usual url.QueryEscape is used for compatibility with
+// encoded as base64 just with a single `=` padding character (to avoid an empty
-// older versions of the Pushgateway and for better readability.
+// path component). If the component does not contain a '/' but other special
+// characters, the usual url.QueryEscape is used for compatibility with older
+// versions of the Pushgateway and for better readability.
 func (p *Pusher) fullURL() string {
 	urlComponents := []string{}
 	if encodedJob, base64 := encodeComponent(p.job); base64 {
@@ -299,9 +307,12 @@ func (p *Pusher) fullURL() string {
 }
 
 // encodeComponent encodes the provided string with base64.RawURLEncoding in
-// case it contains '/'. If not, it uses url.QueryEscape instead. It returns
+// case it contains '/' and as "=" in case it is empty. If neither is the case,
-// true in the former case.
+// it uses url.QueryEscape instead. It returns true in the former two cases.
 func encodeComponent(s string) (string, bool) {
+	if s == "" {
+		return "=", true
+	}
 	if strings.Contains(s, "/") {
 		return base64.RawURLEncoding.EncodeToString([]byte(s)), true
 	}

prometheus/push/push_test.go

@@ -176,6 +176,36 @@ func TestPush(t *testing.T) {
 		t.Error("unexpected path:", lastPath)
 	}
 
+	// Empty label value triggers special base64 encoding.
+	if err := New(pgwOK.URL, "testjob").
+		Grouping("empty", "").
+		Collector(metric1).
+		Collector(metric2).
+		Push(); err != nil {
+		t.Fatal(err)
+	}
+	if lastMethod != http.MethodPut {
+		t.Errorf("got method %q for Push, want %q", lastMethod, http.MethodPut)
+	}
+	if !bytes.Equal(lastBody, wantBody) {
+		t.Errorf("got body %v, want %v", lastBody, wantBody)
+	}
+	if lastPath != "/metrics/job/testjob/empty@base64/=" {
+		t.Error("unexpected path:", lastPath)
+	}
+
+	// Empty job name results in error.
+	if err := New(pgwErr.URL, "").
+		Collector(metric1).
+		Collector(metric2).
+		Push(); err == nil {
+		t.Error("push with empty job succeded")
+	} else {
+		if got, want := err, errJobEmpty; got != want {
+			t.Errorf("got error %q, want %q", got, want)
+		}
+	}
+
 	// Push some Collectors with a broken PGW.
 	if err := New(pgwErr.URL, "testjob").
 		Collector(metric1).
@@ -251,5 +281,4 @@ func TestPush(t *testing.T) {
 	if lastPath != "/metrics/job/testjob/a/x/b/y" && lastPath != "/metrics/job/testjob/b/y/a/x" {
 		t.Error("unexpected path:", lastPath)
 	}
-
 }