Merge pull request #577 from grafana/owilliams/durations

Make duration parsing more flexible in storage schema parsing.

go-whisper/whisper.go

@@ -5,9 +5,9 @@ package whisper
 import (
 	"fmt"
 	"math"
-	"regexp"
-	"strconv"
 	"strings"
+
+	"github.com/raintank/dur"
 )
 
 const (
@@ -21,43 +21,6 @@ const (
 	Years   = 86400 * 365
 )
 
-func unitMultiplier(s string) (int, error) {
-	switch {
-	case strings.HasPrefix(s, "s"):
-		return Seconds, nil
-	case strings.HasPrefix(s, "m"):
-		return Minutes, nil
-	case strings.HasPrefix(s, "h"):
-		return Hours, nil
-	case strings.HasPrefix(s, "d"):
-		return Days, nil
-	case strings.HasPrefix(s, "w"):
-		return Weeks, nil
-	case strings.HasPrefix(s, "y"):
-		return Years, nil
-	}
-	return 0, fmt.Errorf("Invalid unit multiplier [%v]", s)
-}
-
-var retentionRegexp *regexp.Regexp = regexp.MustCompile("^(\\d+)([smhdwy]+)$")
-
-func parseRetentionPart(retentionPart string) (int, error) {
-	part, err := strconv.ParseInt(retentionPart, 10, 32)
-	if err == nil {
-		return int(part), nil
-	}
-	if !retentionRegexp.MatchString(retentionPart) {
-		return 0, fmt.Errorf("%v", retentionPart)
-	}
-	matches := retentionRegexp.FindStringSubmatch(retentionPart)
-	value, err := strconv.ParseInt(matches[1], 10, 32)
-	if err != nil {
-		panic(fmt.Sprintf("Regex on %v is borked, %v cannot be parsed as int", retentionPart, matches[1]))
-	}
-	multiplier, err := unitMultiplier(matches[2])
-	return multiplier * int(value), err
-}
-
 /*
 Parse a retention definition as you would find in the storage-schemas.conf of a Carbon install.
 Note that this only parses a single retention definition, if you have multiple definitions (separated by a comma)
@@ -72,18 +35,20 @@ func ParseRetentionDef(retentionDef string) (*Retention, error) {
 	if len(parts) != 2 {
 		return nil, fmt.Errorf("Not enough parts in retentionDef [%v]", retentionDef)
 	}
-	precision, err := parseRetentionPart(parts[0])
+	// raintank ParseDuration is a more flexible superset of graphite's
+	// specification since many files use other units.
+	precision, err := dur.ParseDuration(parts[0])
 	if err != nil {
 		return nil, fmt.Errorf("Failed to parse precision: %v", err)
 	}
 
-	points, err := parseRetentionPart(parts[1])
+	points, err := dur.ParseDuration(parts[1])
 	if err != nil {
 		return nil, fmt.Errorf("Failed to parse points: %v", err)
 	}
 	points /= precision
 
-	return &Retention{precision, points}, err
+	return &Retention{int(precision), int(points)}, err
 }
 
 func ParseRetentionDefs(retentionDefs string) (Retentions, error) {

go-whisper/whisper_test.go

@@ -29,11 +29,12 @@ func testParseRetentionDef(t *testing.T, retentionDef string, expectedPrecision,
 }
 
 func TestParseRetentionDef(t *testing.T) {
-	testParseRetentionDef(t, "1s:5m", 1, 300, false)
+	testParseRetentionDef(t, "1s:5min", 1, 300, false)
+	testParseRetentionDef(t, "1s:5minaf", 1, 300, true)
 	testParseRetentionDef(t, "1m:30m", 60, 30, false)
 	testParseRetentionDef(t, "1m", 0, 0, true)
 	testParseRetentionDef(t, "1m:30m:20s", 0, 0, true)
-	testParseRetentionDef(t, "1f:30s", 0, 0, true)
+	testParseRetentionDef(t, "1f:30seconds", 0, 0, true)
 	testParseRetentionDef(t, "1m:30f", 0, 0, true)
 }
 

go.mod

@@ -23,6 +23,7 @@ require (
 	github.com/kisielk/og-rek v1.2.0
 	github.com/metrics20/go-metrics20 v0.0.0-20180821133656-717ed3a27bf9
 	github.com/prometheus/procfs v0.15.1
+	github.com/raintank/dur v0.0.0-20220106223125-d7c6a541ef3a
 	github.com/sirupsen/logrus v1.9.3
 	github.com/streadway/amqp v1.1.0
 	github.com/stretchr/testify v1.10.0

go.sum

@@ -142,6 +142,8 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
 github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc=
 github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk=
+github.com/raintank/dur v0.0.0-20220106223125-d7c6a541ef3a h1:ic0R3TWw1y25d48s4aS1cVNarVq58zkzVna+ezzaa9o=
+github.com/raintank/dur v0.0.0-20220106223125-d7c6a541ef3a/go.mod h1:7BB8LeqBvE3vEKv3ZbgA324vRXhUVO9pT9UrKnYDnx8=
 github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a h1:9ZKAASQSHhDYGoxY8uLVpewe1GDZ2vu2Tr/vTdVAkFQ=
 github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
 github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=

route/grafananet_test.go

@@ -12,7 +12,7 @@ func TestNewGrafanaNetConfig(t *testing.T) {
 	// note: the goal of this test is not to strictly test the correctness of the schemas reading
 	// we have separate tests for that
 
-	schemasFile := test.TempFdOrFatal("carbon-relay-ng-TestNewGrafanaNetConfig-schemasFile-valid", "[default]\npattern = .*\nretentions = 10s:1d", t)
+	schemasFile := test.TempFdOrFatal("carbon-relay-ng-TestNewGrafanaNetConfig-schemasFile-valid", "[default]\npattern = .*\nretentions = 10min:1m", t)
 	defer os.Remove(schemasFile.Name())
 
 	otherFile := test.TempFdOrFatal("carbon-relay-ng-TestNewGrafanaNetConfig-otherFile", "this is not a schemas or aggregation file", t)