From 502b06328e1377c1c9881c7865779b2737ef62e3 Mon Sep 17 00:00:00 2001 From: Jonne Nauha Date: Sat, 19 Nov 2016 02:49:14 +0200 Subject: [PATCH] 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 | 56 ++++++++++++++++++++++++++++++++++++------------- varnish_test.go | 50 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) diff --git a/prometheus.go b/prometheus.go index c5071dc..d6f72fc 100644 --- a/prometheus.go +++ b/prometheus.go @@ -1,6 +1,7 @@ package main import ( + "regexp" "strings" "sync" "time" @@ -164,6 +165,35 @@ var ( } ) +var ( + // (prefix:). + 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})(.*)`) + // (,(),) + 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 { - // . - 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 { - // (,,) - 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 { diff --git a/varnish_test.go b/varnish_test.go index 019b17a..24a3a73 100644 --- a/varnish_test.go +++ b/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