From bfffe95d07bbb69c5913f1091273a0301d5a540f Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Thu, 11 Apr 2019 12:59:53 +0300 Subject: [PATCH] refactored and added tests --- .travis.yml | 20 +++++++ README.md | 8 +++ counter.go | 105 ++++++++++++++++++++++++++++++++++ counter_test.go | 76 +++++++++++++++++++++++++ gauge.go | 47 ++++++++++++++++ gauge_test.go | 55 ++++++++++++++++++ metrics.go | 139 ++++++---------------------------------------- metrics_test.go | 119 +++++++++++++++++++++++++++++++++++++++ summary.go | 18 +++++- summary_test.go | 101 +++++++++++++++++++++++++++++++++ validator.go | 17 +++--- validator_test.go | 2 +- 12 files changed, 575 insertions(+), 132 deletions(-) create mode 100644 .travis.yml create mode 100644 counter.go create mode 100644 counter_test.go create mode 100644 gauge.go create mode 100644 gauge_test.go create mode 100644 metrics_test.go create mode 100644 summary_test.go diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..39d07e6 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,20 @@ +language: go + +go: + - 1.12.x + +script: + # build test for supported platforms + - GOOS=linux go build + - GOOS=darwin go build + - GOOS=freebsd go build + - GOOS=windows go build + - GOARCH=386 go build + + # run tests on a standard platform + - go test -v ./... -coverprofile=coverage.txt -covermode=atomic + - go test -v ./... -race + +after_success: + # Upload coverage results to codecov.io + - bash <(curl -s https://codecov.io/bash) diff --git a/README.md b/README.md index 21ca0fb..0aab32a 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,7 @@ +[![Build Status](https://travis-ci.org/VictoriaMetrics/metrics.svg)](https://travis-ci.org/VictoriaMetrics/metrics) [![GoDoc](https://godoc.org/github.com/VictoriaMetrics/metrics?status.svg)](http://godoc.org/github.com/VictoriaMetrics/metrics) [![Go Report](https://goreportcard.com/badge/github.com/VictoriaMetrics/metrics)](https://goreportcard.com/report/github.com/VictoriaMetrics/metrics) +[![codecov](https://codecov.io/gh/VictoriaMetrics/metrics/branch/master/graph/badge.svg)](https://codecov.io/gh/VictoriaMetrics/metrics) # metrics - lightweight package for exporting metrics in Prometheus format @@ -76,3 +78,9 @@ Because the `github.com/prometheus/client_golang` is too complex and is hard to Because this documentation is ignored by Prometheus. The documentation is for users. Just add comments in the source code or in other suitable place explaining each metric exposed from your application. + + +#### How to implement [CounterVec](https://godoc.org/github.com/prometheus/client_golang/prometheus#CounterVec) in `metrics`? + +Just use [GetOrCreateCounter](http://godoc.org/github.com/VictoriaMetrics/metrics#GetOrCreateCounter) +instead of `CounterVec.With`. diff --git a/counter.go b/counter.go new file mode 100644 index 0000000..e3d7f6b --- /dev/null +++ b/counter.go @@ -0,0 +1,105 @@ +package metrics + +import ( + "fmt" + "io" + "sync/atomic" +) + +// NewCounter registers and returns new counter with the given name. +// +// name must be valid Prometheus-compatible metric with possible lables. +// For instance, +// +// * foo +// * foo{bar="baz"} +// * foo{bar="baz",aaa="b"} +// +// The returned counter is safe to use from concurrent goroutines. +func NewCounter(name string) *Counter { + c := &Counter{} + registerMetric(name, c) + return c +} + +// Counter is a counter. +// +// It may be used as a gauge if Dec and Set are called. +type Counter struct { + n uint64 +} + +// Inc increments c. +func (c *Counter) Inc() { + atomic.AddUint64(&c.n, 1) +} + +// Dec decrements c. +func (c *Counter) Dec() { + atomic.AddUint64(&c.n, ^uint64(0)) +} + +// Add adds n to c. +func (c *Counter) Add(n int) { + atomic.AddUint64(&c.n, uint64(n)) +} + +// Get returns the current value for c. +func (c *Counter) Get() uint64 { + return atomic.LoadUint64(&c.n) +} + +// Set sets c value to n. +func (c *Counter) Set(n uint64) { + atomic.StoreUint64(&c.n, n) +} + +// marshalTo marshals c with the given prefix to w. +func (c *Counter) marshalTo(prefix string, w io.Writer) { + v := c.Get() + fmt.Fprintf(w, "%s %d\n", prefix, v) +} + +// GetOrCreateCounter returns registered counter with the given name +// or creates new counter if the registry doesn't contain counter with +// the given name. +// +// name must be valid Prometheus-compatible metric with possible lables. +// For instance, +// +// * foo +// * foo{bar="baz"} +// * foo{bar="baz",aaa="b"} +// +// The returned counter is safe to use from concurrent goroutines. +// +// Performance tip: prefer NewCounter instead of GetOrCreateCounter. +func GetOrCreateCounter(name string) *Counter { + metricsMapLock.Lock() + nm := metricsMap[name] + metricsMapLock.Unlock() + if nm == nil { + // Slow path - create and register missing counter. + if err := validateMetric(name); err != nil { + panic(fmt.Errorf("BUG: invalid metric name %q: %s", name, err)) + } + nmNew := &namedMetric{ + name: name, + metric: &Counter{}, + } + metricsMapLock.Lock() + nm = metricsMap[name] + if nm == nil { + nm = nmNew + metricsMap[name] = nm + metricsList = append(metricsList, nm) + } + metricsMapLock.Unlock() + } + + c, ok := nm.metric.(*Counter) + if !ok { + panic(fmt.Errorf("BUG: metric %q isn't a Counter. It is %T", name, nm.metric)) + } + return c +} diff --git a/counter_test.go b/counter_test.go new file mode 100644 index 0000000..27c9264 --- /dev/null +++ b/counter_test.go @@ -0,0 +1,76 @@ +package metrics + +import ( + "fmt" + "testing" +) + +func TestCounterSerial(t *testing.T) { + name := "CounterSerial" + c := NewCounter(name) + c.Inc() + if n := c.Get(); n != 1 { + t.Fatalf("unexpected counter value; got %d; want 1", n) + } + c.Set(123) + if n := c.Get(); n != 123 { + t.Fatalf("unexpected counter value; got %d; want 123", n) + } + c.Dec() + if n := c.Get(); n != 122 { + t.Fatalf("unexpected counter value; got %d; want 122", n) + } + c.Add(3) + if n := c.Get(); n != 125 { + t.Fatalf("unexpected counter value; got %d; want 125", n) + } + + // Verify MarshalTo + testMarshalTo(t, c, "foobar", "foobar 125\n") +} + +func TestCounterConcurrent(t *testing.T) { + name := "CounterConcurrent" + c := NewCounter(name) + err := testConcurrent(func() error { + nPrev := c.Get() + for i := 0; i < 10; i++ { + c.Inc() + if n := c.Get(); n <= nPrev { + return fmt.Errorf("counter value must be greater than %d; got %d", nPrev, n) + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } +} + +func TestGetOrCreateCounterSerial(t *testing.T) { + name := "GetOrCreateCounterSerial" + if err := testGetOrCreateCounter(name); err != nil { + t.Fatal(err) + } +} + +func TestGetOrCreateCounterConcurrent(t *testing.T) { + name := "GetOrCreateCounterConcurrent" + err := testConcurrent(func() error { + return testGetOrCreateCounter(name) + }) + if err != nil { + t.Fatal(err) + } +} + +func testGetOrCreateCounter(name string) error { + c1 := GetOrCreateCounter(name) + for i := 0; i < 10; i++ { + c2 := GetOrCreateCounter(name) + if c1 != c2 { + return fmt.Errorf("unexpected counter returned; got %p; want %p", c2, c1) + } + } + return nil +} diff --git a/gauge.go b/gauge.go new file mode 100644 index 0000000..6d89f18 --- /dev/null +++ b/gauge.go @@ -0,0 +1,47 @@ +package metrics + +import ( + "fmt" + "io" +) + +// NewGauge registers and returns gauge with the given name, which calls f +// to obtain gauge value. +// +// name must be valid Prometheus-compatible metric with possible labels. +// For instance, +// +// * foo +// * foo{bar="baz"} +// * foo{bar="baz",aaa="b"} +// +// f must be safe for concurrent calls. +// +// The returned gauge is safe to use from concurrent goroutines. +func NewGauge(name string, f func() float64) *Gauge { + g := &Gauge{ + f: f, + } + registerMetric(name, g) + return g +} + +// Gauge is a float64 gauge. +type Gauge struct { + f func() float64 +} + +// Get returns the current value for g. +func (g *Gauge) Get() float64 { + return g.f() +} + +func (g *Gauge) marshalTo(prefix string, w io.Writer) { + v := g.f() + if float64(int64(v)) == v { + // Marshal integer values without scientific notations + fmt.Fprintf(w, "%s %d\n", prefix, int64(v)) + } else { + fmt.Fprintf(w, "%s %g\n", prefix, v) + } +} diff --git a/gauge_test.go b/gauge_test.go new file mode 100644 index 0000000..3bb199f --- /dev/null +++ b/gauge_test.go @@ -0,0 +1,55 @@ +package metrics + +import ( + "fmt" + "sync" + "testing" +) + +func TestGaugeSerial(t *testing.T) { + name := "GaugeSerial" + n := 1.23 + var nLock sync.Mutex + g := NewGauge(name, func() float64 { + nLock.Lock() + defer nLock.Unlock() + n++ + return n + }) + for i := 0; i < 10; i++ { + if nn := g.Get(); nn != n { + t.Fatalf("unexpected gauge value; got %v; want %v", nn, n) + } + } + + // Verify marshalTo + testMarshalTo(t, g, "foobar", "foobar 12.23\n") + + // Verify big numbers marshaling + n = 1234567899 + testMarshalTo(t, g, "prefix", "prefix 1234567900\n") +} + +func TestGaugeConcurrent(t *testing.T) { + name := "GaugeConcurrent" + var n int + var nLock sync.Mutex + g := NewGauge(name, func() float64 { + nLock.Lock() + defer nLock.Unlock() + n++ + return float64(n) + }) + err := testConcurrent(func() error { + nPrev := g.Get() + for i := 0; i < 10; i++ { + if n := g.Get(); n <= nPrev { + return fmt.Errorf("gauge value must be greater than %v; got %v", nPrev, n) + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } +} diff --git a/metrics.go b/metrics.go index 717a5ae..324cdc6 100644 --- a/metrics.go +++ b/metrics.go @@ -16,103 +16,15 @@ import ( "runtime" "sort" "sync" - "sync/atomic" "time" "github.com/valyala/histogram" ) -// NewGauge registers and returns gauge with the given name, which calls f -// to obtain gauge value. -// -// name must be valid Prometheus-compatible metric with possible labels. -// For instance, -// -// * foo -// * foo{bar="baz"} -// * foo{bar="baz",aaa="b"} -// -// f must be safe for concurrent calls. -func NewGauge(name string, f func() float64) *Gauge { - g := &Gauge{ - f: f, - } - registerMetric(name, g) - return g -} - -// Gauge is a float64 gauge. -type Gauge struct { - f func() float64 -} - -// Get returns the current value for g. -func (g *Gauge) Get() float64 { - return g.f() -} - -func (g *Gauge) marshalTo(prefix string, w io.Writer) { - v := g.f() - fmt.Fprintf(w, "%s %g\n", prefix, v) -} - -// NewCounter registers and returns new counter with the given name. -// -// name must be valid Prometheus-compatible metric with possible lables. -// For instance, -// -// * foo -// * foo{bar="baz"} -// * foo{bar="baz",aaa="b"} -// -// The returned counter is safe to use from concurrent goroutines. -func NewCounter(name string) *Counter { - c := &Counter{} - registerMetric(name, c) - return c -} - -// Counter is a counter. -// -// It may be used as a gauge if Dec and Set are called. -type Counter struct { - n uint64 -} - -// Inc increments c. -func (c *Counter) Inc() { - atomic.AddUint64(&c.n, 1) -} - -// Dec decrements c. -func (c *Counter) Dec() { - atomic.AddUint64(&c.n, ^uint64(0)) -} - -// Add adds n to c. -func (c *Counter) Add(n int) { - atomic.AddUint64(&c.n, uint64(n)) -} - -// Get returns the current value for c. -func (c *Counter) Get() uint64 { - return atomic.LoadUint64(&c.n) -} - -// Set sets c value to n. -func (c *Counter) Set(n uint64) { - atomic.StoreUint64(&c.n, n) -} - -// marshalTo marshals c with the given prefix to w. -func (c *Counter) marshalTo(prefix string, w io.Writer) { - v := c.Get() - fmt.Fprintf(w, "%s %d\n", prefix, v) -} - var ( metricsMapLock sync.Mutex - metricsMap []namedMetric + metricsList []*namedMetric + metricsMap = make(map[string]*namedMetric) ) type namedMetric struct { @@ -120,24 +32,6 @@ type namedMetric struct { metric metric } -func isRegisteredMetric(mm []namedMetric, name string) bool { - for _, nm := range mm { - if nm.name == name { - return true - } - } - return false -} - -func sortMetrics(mm []namedMetric) { - lessFunc := func(i, j int) bool { - return mm[i].name < mm[j].name - } - if !sort.SliceIsSorted(mm, lessFunc) { - sort.Slice(mm, lessFunc) - } -} - type metric interface { marshalTo(prefix string, w io.Writer) } @@ -154,19 +48,23 @@ type metric interface { // }) // func WritePrometheus(w io.Writer, exposeProcessMetrics bool) { - // Export user-defined metrics. + lessFunc := func(i, j int) bool { + return metricsList[i].name < metricsList[j].name + } metricsMapLock.Lock() - sortMetrics(metricsMap) - for _, nm := range metricsMap { + if !sort.SliceIsSorted(metricsList, lessFunc) { + sort.Slice(metricsList, lessFunc) + } + for _, nm := range metricsList { nm.metric.marshalTo(nm.name, w) } metricsMapLock.Unlock() - - if !exposeProcessMetrics { - return + if exposeProcessMetrics { + writeProcessMetrics(w) } +} - // Export memory stats. +func writeProcessMetrics(w io.Writer) { var ms runtime.MemStats runtime.ReadMemStats(&ms) fmt.Fprintf(w, `go_memstats_alloc_bytes %d`+"\n", ms.Alloc) @@ -225,21 +123,20 @@ var startTime = time.Now() func registerMetric(name string, m metric) { if err := validateMetric(name); err != nil { - // Do not use logger.Panicf here, since it may be uninitialized yet. panic(fmt.Errorf("BUG: invalid metric name %q: %s", name, err)) } metricsMapLock.Lock() - ok := isRegisteredMetric(metricsMap, name) + nm, ok := metricsMap[name] if !ok { - nm := namedMetric{ + nm = &namedMetric{ name: name, metric: m, } - metricsMap = append(metricsMap, nm) + metricsMap[name] = nm + metricsList = append(metricsList, nm) } metricsMapLock.Unlock() if ok { - // Do not use logger.Panicf here, since it may be uninitialized yet. - panic(fmt.Errorf("BUG: metric with name %q is already registered", name)) + panic(fmt.Errorf("BUG: metric %q is already registered", name)) } } diff --git a/metrics_test.go b/metrics_test.go new file mode 100644 index 0000000..c3b5f38 --- /dev/null +++ b/metrics_test.go @@ -0,0 +1,119 @@ +package metrics + +import ( + "bytes" + "fmt" + "testing" + "time" +) + +func TestInvalidName(t *testing.T) { + f := func(name string) { + t.Helper() + expectPanic(t, fmt.Sprintf("NewCounter(%q)", name), func() { NewCounter(name) }) + expectPanic(t, fmt.Sprintf("GetOrCreateCounter(%q)", name), func() { GetOrCreateCounter(name) }) + expectPanic(t, fmt.Sprintf("NewGauge(%q)", name), func() { NewGauge(name, func() float64 { return 0 }) }) + expectPanic(t, fmt.Sprintf("NewSummary(%q)", name), func() { NewSummary(name) }) + } + f("") + f("foo{") + f("foo}") + f("foo{bar") + f("foo{bar=") + f(`foo{bar="`) + f(`foo{bar="baz`) + f(`foo{bar="baz"`) + f(`foo{bar="baz",`) + f(`foo{bar="baz",}`) +} + +func TestDoubleRegister(t *testing.T) { + t.Run("NewCounter", func(t *testing.T) { + name := "NewCounterDoubleRegister" + NewCounter(name) + expectPanic(t, name, func() { NewCounter(name) }) + }) + t.Run("NewGauge", func(t *testing.T) { + name := "NewGaugeDoubleRegister" + NewGauge(name, func() float64 { return 0 }) + expectPanic(t, name, func() { NewGauge(name, func() float64 { return 0 }) }) + }) + t.Run("NewSummary", func(t *testing.T) { + name := "NewSummaryDoubleRegister" + NewSummary(name) + expectPanic(t, name, func() { NewSummary(name) }) + }) +} + +func TestGetOrCreateNotCounter(t *testing.T) { + name := "GetOrCreateNotCounter" + NewSummary(name) + expectPanic(t, name, func() { GetOrCreateCounter(name) }) +} + +func TestWritePrometheusSerial(t *testing.T) { + if err := testWritePrometheus(); err != nil { + t.Fatal(err) + } +} + +func TestWritePrometheusConcurrent(t *testing.T) { + if err := testConcurrent(testWritePrometheus); err != nil { + t.Fatal(err) + } +} + +func testWritePrometheus() error { + var bb bytes.Buffer + WritePrometheus(&bb, false) + resultWithoutProcessMetrics := bb.String() + bb.Reset() + WritePrometheus(&bb, true) + resultWithProcessMetrics := bb.String() + if len(resultWithProcessMetrics) <= len(resultWithoutProcessMetrics) { + return fmt.Errorf("result with process metrics must contain more data than the result without process metrics; got\n%q\nvs\n%q", + resultWithProcessMetrics, resultWithoutProcessMetrics) + } + return nil +} + +func expectPanic(t *testing.T, context string, f func()) { + t.Helper() + defer func() { + if r := recover(); r == nil { + t.Fatalf("expecting panic in %s", context) + } + }() + f() +} + +func testConcurrent(f func() error) error { + const concurrency = 5 + resultsCh := make(chan error, concurrency) + for i := 0; i < concurrency; i++ { + go func() { + resultsCh <- f() + }() + } + for i := 0; i < concurrency; i++ { + select { + case err := <-resultsCh: + if err != nil { + return fmt.Errorf("unexpected error: %s", err) + } + case <-time.After(time.Second * 5): + return fmt.Errorf("timeout") + } + } + return nil +} + +func testMarshalTo(t *testing.T, m metric, prefix, resultExpected string) { + t.Helper() + var bb bytes.Buffer + m.marshalTo(prefix, &bb) + result := bb.String() + if result != resultExpected { + t.Fatalf("unexpected marshaled metric; got %q; want %q", result, resultExpected) + } +} diff --git a/summary.go b/summary.go index 7f2e560..62d6fcf 100644 --- a/summary.go +++ b/summary.go @@ -3,6 +3,7 @@ package metrics import ( "fmt" "io" + "math" "sync" "time" @@ -50,6 +51,7 @@ func NewSummary(name string) *Summary { // // The returned summary is safe to use from concurrent goroutines. func NewSummaryExt(name string, window time.Duration, quantiles []float64) *Summary { + validateQuantiles(quantiles) s := &Summary{ curr: histogram.NewFast(), next: histogram.NewFast(), @@ -57,7 +59,7 @@ func NewSummaryExt(name string, window time.Duration, quantiles []float64) *Summ quantileValues: make([]float64, len(quantiles)), } registerSummary(s, window) - registerMetric(fmt.Sprintf("\x00%s", name), s) + registerMetric(name, s) for i, q := range quantiles { quantileValueName := addTag(name, fmt.Sprintf(`quantile="%g"`, q)) qv := &quantileValue{ @@ -69,6 +71,14 @@ func NewSummaryExt(name string, window time.Duration, quantiles []float64) *Summ return s } +func validateQuantiles(quantiles []float64) { + for _, q := range quantiles { + if q < 0 || q > 1 { + panic(fmt.Errorf("BUG: quantile must be in the range [0..1]; got %v", q)) + } + } +} + // Update updates the summary. func (s *Summary) Update(v float64) { s.mu.Lock() @@ -104,14 +114,16 @@ func (qv *quantileValue) marshalTo(prefix string, w io.Writer) { qv.s.mu.Lock() v := qv.s.quantileValues[qv.idx] qv.s.mu.Unlock() - fmt.Fprintf(w, "%s %g\n", prefix, v) + if !math.IsNaN(v) { + fmt.Fprintf(w, "%s %g\n", prefix, v) + } } func addTag(name, tag string) string { if len(name) == 0 || name[len(name)-1] != '}' { return fmt.Sprintf("%s{%s}", name, tag) } - return fmt.Sprintf("%s, %s}", name[:len(name)-1], tag) + return fmt.Sprintf("%s,%s}", name[:len(name)-1], tag) } func registerSummary(s *Summary, window time.Duration) { diff --git a/summary_test.go b/summary_test.go new file mode 100644 index 0000000..72c3da6 --- /dev/null +++ b/summary_test.go @@ -0,0 +1,101 @@ +package metrics + +import ( + "bytes" + "strings" + "testing" + "time" +) + +func TestSummarySerial(t *testing.T) { + name := `TestSummarySerial` + s := NewSummary(name) + + // Verify that the summary isn't visible in the output of WritePrometheus, + // since it doesn't contain any values yet. + var bb bytes.Buffer + WritePrometheus(&bb, false) + result := bb.String() + if strings.Contains(result, name) { + t.Fatalf("summary %s shouldn't be visible in the WritePrometheus output; got\n%s", name, result) + } + + // Write data to summary + for i := 0; i < 2000; i++ { + s.Update(float64(i)) + t := time.Now() + s.UpdateDuration(t.Add(-time.Millisecond * time.Duration(i))) + } + + // Make sure the summary doesn't print anything on marshalTo call + // and updates s.quantileValues. + testMarshalTo(t, s, "prefix", "") + + // Verify s.quantileValues + if s.quantileValues[len(s.quantileValues)-1] != 1999 { + t.Fatalf("unexpected quantileValues[last]; got %v; want %v", s.quantileValues[len(s.quantileValues)-1], 1999) + } + + // Make sure the summary becomes visible in the output of WritePrometheus, + // since now it contains values. + bb.Reset() + WritePrometheus(&bb, false) + result = bb.String() + if !strings.Contains(result, name) { + t.Fatalf("missing summary %s in the WritePrometheus output; got\n%s", name, result) + } +} + +func TestSummaryConcurrent(t *testing.T) { + name := "SummaryConcurrent" + s := NewSummary(name) + err := testConcurrent(func() error { + for i := 0; i < 10; i++ { + s.Update(float64(i)) + } + testMarshalTo(t, s, "prefix", "") + return nil + }) + if err != nil { + t.Fatal(err) + } +} + +func TestSummaryWithTags(t *testing.T) { + name := `TestSummary{tag="foo"}` + s := NewSummary(name) + s.Update(123) + + var bb bytes.Buffer + WritePrometheus(&bb, false) + result := bb.String() + namePrefixWithTag := `TestSummary{tag="foo",quantile="` + if !strings.Contains(result, namePrefixWithTag) { + t.Fatalf("missing summary prefix %s in the WritePrometheus output; got\n%s", namePrefixWithTag, result) + } +} + +func TestSummaryInvalidQuantiles(t *testing.T) { + name := "SummaryInvalidQuantiles" + expectPanic(t, name, func() { + NewSummaryExt(name, time.Minute, []float64{123, -234}) + }) +} + +func TestSummarySmallWindow(t *testing.T) { + name := "SummarySmallWindow" + window := time.Millisecond * 20 + quantiles := []float64{0.1, 0.2, 0.3} + s := NewSummaryExt(name, window, quantiles) + for i := 0; i < 2000; i++ { + s.Update(123) + } + // Wait for window update and verify that the summary has been cleared. + time.Sleep(5 * window) + var bb bytes.Buffer + WritePrometheus(&bb, false) + result := bb.String() + if strings.Contains(result, name) { + t.Fatalf("summary %s cannot be present in the WritePrometheus output; got\n%s", name, result) + } +} diff --git a/validator.go b/validator.go index 6b74d21..27e88ca 100644 --- a/validator.go +++ b/validator.go @@ -10,10 +10,6 @@ func validateMetric(s string) error { if len(s) == 0 { return fmt.Errorf("metric cannot be empty") } - if s[0] == 0 { - // Skip special case metrics. See Histogram for details. - return nil - } n := strings.IndexByte(s, '{') if n < 0 { return validateIdent(s) @@ -64,13 +60,20 @@ func validateTags(s string) error { if len(s) == 0 { return nil } - if !strings.HasPrefix(s, ", ") { - return fmt.Errorf("missing `, ` after %q value; tail=%q", ident, s) + if !strings.HasPrefix(s, ",") { + return fmt.Errorf("missing `,` after %q value; tail=%q", ident, s) } - s = s[2:] + s = skipSpace(s[1:]) } } +func skipSpace(s string) string { + for len(s) > 0 && s[0] == ' ' { + s = s[1:] + } + return s +} + func validateIdent(s string) error { if !identRegexp.MatchString(s) { return fmt.Errorf("invalid identifier %q", s) diff --git a/validator_test.go b/validator_test.go index dd3c338..d90b62d 100644 --- a/validator_test.go +++ b/validator_test.go @@ -17,6 +17,7 @@ func TestValidateMetricSuccess(t *testing.T) { f(`a{foo="bar"}`) f(`foo{bar="baz", x="y\"z"}`) f(`foo{bar="b}az"}`) + f(`:foo:bar{bar="a",baz="b"}`) } func TestValidateMetricError(t *testing.T) { @@ -40,7 +41,6 @@ func TestValidateMetricError(t *testing.T) { f(`a{ foo="bar"}`) f(`a{foo= "bar"}`) f(`a{foo="bar" }`) - f(`a{foo="bar",baz="a"}`) f(`a{foo="bar" ,baz="a"}`) // invalid tags