Fix bug in VBE.xxx metrics. The label detection was bad and failed to…

… parse certain UUID based identifiers (prefixed ones).

This caused to fallback to 'id' label. If the scrape contained both 'backend' + 'server' labeled and 'id' labeled metrics, the varnish server reports errors. For a metric name, all need to have the same amount and named labels.
Now if server label cannot be parsed it will be set to 'unkown' and the full ident set as 'backend'.

Fixes #5 #8

prometheus.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"regexp"
 	"strings"
 	"sync"
 	"time"
@@ -164,6 +165,35 @@ var (
 	}
 )
 
+var (
+	// (prefix:)<uuid>.<name>
+	regexBackendUUID = regexp.MustCompile(`([[0-9A-Za-z]{8}-[0-9A-Za-z]{4}-[0-9A-Za-z]{4}-[89ABab][0-9A-Za-z]{3}-[0-9A-Za-z]{12})(.*)`)
+	// <name>(<ip>,(<something>),<port>)
+	regexBackendParen = regexp.MustCompile(`(.*)\((.*)\)`)
+)
+
+func findLabelValue(name string, keys, values []string) string {
+	for i, key := range keys {
+		if key == name {
+			if i < len(values) {
+				return values[i]
+			}
+			return ""
+		}
+	}
+	return ""
+}
+
+func cleanBackendName(name string) string {
+	name = strings.Trim(name, ".")
+	for _, prefix := range []string{"boot.", "root:"} {
+		if startsWith(name, prefix, caseInsensitive) {
+			name = name[len(prefix):]
+		}
+	}
+	return name
+}
+
 // https://prometheus.io/docs/practices/naming/
 func computePrometheusInfo(vName, vGroup, vIdentifier, vDescription string) (name, description string, labelKeys, labelValues []string) {
 	// name and description
@@ -186,20 +216,18 @@ func computePrometheusInfo(vName, vGroup, vIdentifier, vDescription string) (nam
 	{
 		if len(vIdentifier) > 0 {
 			if isVBE := startsWith(vName, "VBE.", caseSensitive); isVBE {
-				// @todo this is quick and dirty, do regexp?
-				if VarnishVersion.Major == 4 && VarnishVersion.Minor >= 1 {
-					// <uuid>.<name>
-					if len(vIdentifier) > 37 && vIdentifier[8] == '-' && vIdentifier[36] == '.' {
-						labelKeys, labelValues = append(labelKeys, "server"), append(labelValues, vIdentifier[0:36])
-						labelKeys, labelValues = append(labelKeys, "backend"), append(labelValues, vIdentifier[37:])
-					}
-				} else {
-					// <name>(<ip>,<something>,<port>)
-					iStart, iEnd := strings.Index(vIdentifier, "("), strings.Index(vIdentifier, ")")
-					if iStart > 0 && iEnd > 1 && iStart < iEnd {
-						labelKeys, labelValues = append(labelKeys, "server"), append(labelValues, vIdentifier[iStart+1:iEnd])
-						labelKeys, labelValues = append(labelKeys, "backend"), append(labelValues, vIdentifier[0:iStart])
-					}
+				if hits := regexBackendUUID.FindAllStringSubmatch(vIdentifier, -1); len(hits) > 0 && len(hits[0]) >= 3 {
+					labelKeys, labelValues = append(labelKeys, "backend"), append(labelValues, cleanBackendName(hits[0][2]))
+					labelKeys, labelValues = append(labelKeys, "server"), append(labelValues, hits[0][1])
+				} else if hits := regexBackendParen.FindAllStringSubmatch(vIdentifier, -1); len(hits) > 0 && len(hits[0]) >= 3 {
+					labelKeys, labelValues = append(labelKeys, "backend"), append(labelValues, cleanBackendName(hits[0][1]))
+					labelKeys, labelValues = append(labelKeys, "server"), append(labelValues, strings.Replace(hits[0][2], ",,", ":", 1))
+				}
+				// We must be consistent with the number of labels and their names inside this scrape and between scrapes, or we will get this error:
+				// https://github.com/prometheus/client_golang/blob/3fb8ace93bc4ccddea55af62320c2fd109252880/prometheus/registry.go#L704-L707
+				if len(labelKeys) == 0 {
+					labelKeys, labelValues = append(labelKeys, "backend"), append(labelValues, cleanBackendName(vIdentifier))
+					labelKeys, labelValues = append(labelKeys, "server"), append(labelValues, "unknown")
 				}
 			}
 			if len(labelKeys) == 0 {

varnish_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"fmt"
 	"runtime"
 	"testing"
 
@@ -42,6 +43,55 @@ func Test_VarnishVersion(t *testing.T) {
 	}
 }
 
+func dummyBackendValue(backend string) (string, map[string]interface{}) {
+	return fmt.Sprintf("VBE.%s.happy", backend), map[string]interface{}{
+		"description": "Happy health probes",
+		"type":        "VBE",
+		"ident":       backend,
+		"flag":        "b",
+		"format":      "b",
+		"value":       0,
+	}
+}
+
+func Test_VarnishBackendNames(t *testing.T) {
+	for _, backend := range []string{
+		"eu1_x.y-z:w(192.52.0.192,,8085)", // 4.0.3
+		"root:eu2_x.y-z:w",                // 4.1
+		"def0e7f7-a676-4eed-9d8b-78ef7ce21e93.us1_x.y-z:w",
+		"root:29813cbb-7329-4eb8-8969-26be2ef58c88.us2_x.y-z:w", // ??
+		"boot.default",
+		"ce19737f-72b5-4f4b-9d39-3d8c2d28240b.default",
+	} {
+		vName, data := dummyBackendValue(backend)
+		var (
+			vGroup       = prometheusGroup(vName)
+			vDescription string
+			vIdentifier  string
+			vErr         error
+		)
+		if value, ok := data["description"]; ok && vErr == nil {
+			if vDescription, ok = value.(string); !ok {
+				vErr = fmt.Errorf("%s description it not a string", vName)
+			}
+		}
+		if value, ok := data["ident"]; ok && vErr == nil {
+			if vIdentifier, ok = value.(string); !ok {
+				vErr = fmt.Errorf("%s ident it not a string", vName)
+			}
+		}
+		if vErr != nil {
+			t.Error(vErr)
+			return
+		}
+		name, _, labelKeys, labelValues := computePrometheusInfo(vName, vGroup, vIdentifier, vDescription)
+		t.Logf("%s > %s\n", backend, name)
+		t.Logf("  ident   : %s\n", vIdentifier)
+		t.Logf("  backend : %s\n", findLabelValue("backend", labelKeys, labelValues))
+		t.Logf("  server  : %s\n", findLabelValue("server", labelKeys, labelValues))
+	}
+}
+
 func Test_VarnishMetrics(t *testing.T) {
 	// @todo This is kind of pointless. The idea was to test against
 	// JSON output of different versions of Varnish. Enable back at some point