From 0b30e065c87d82f746a3c1f899ce95a4524cf3db Mon Sep 17 00:00:00 2001 From: Bernerd Schaefer Date: Fri, 19 Apr 2013 13:22:59 +0200 Subject: [PATCH 1/3] Metrics explicitly implement Metric interface --- prometheus/counter.go | 5 ++--- prometheus/gauge.go | 4 +--- prometheus/histogram.go | 4 +--- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/prometheus/counter.go b/prometheus/counter.go index 74b76dd..ba4a776 100644 --- a/prometheus/counter.go +++ b/prometheus/counter.go @@ -14,14 +14,13 @@ import ( // TODO(matt): Refactor to de-duplicate behaviors. type Counter interface { - AsMarshallable() map[string]interface{} + Metric + Decrement(labels map[string]string) float64 DecrementBy(labels map[string]string, value float64) float64 Increment(labels map[string]string) float64 IncrementBy(labels map[string]string, value float64) float64 - ResetAll() Set(labels map[string]string, value float64) float64 - String() string } type counterVector struct { diff --git a/prometheus/gauge.go b/prometheus/gauge.go index e5f6553..2fa255d 100644 --- a/prometheus/gauge.go +++ b/prometheus/gauge.go @@ -16,10 +16,8 @@ import ( // temperature or the hitherto bandwidth used, this would be the metric for such // circumstances. type Gauge interface { - AsMarshallable() map[string]interface{} - ResetAll() + Metric Set(labels map[string]string, value float64) float64 - String() string } type gaugeVector struct { diff --git a/prometheus/histogram.go b/prometheus/histogram.go index ca3ce93..e696f4d 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -52,10 +52,8 @@ type HistogramSpecification struct { } type Histogram interface { + Metric Add(labels map[string]string, value float64) - AsMarshallable() map[string]interface{} - ResetAll() - String() string } // The histogram is an accumulator for samples. It merely routes into which From 3433b798b390273f731ac206835ddbe2ee3bb043 Mon Sep 17 00:00:00 2001 From: Bernerd Schaefer Date: Fri, 19 Apr 2013 13:29:24 +0200 Subject: [PATCH 2/3] Use raw string literals in tests --- prometheus/counter_test.go | 24 ++++++++++++------------ prometheus/gauge_test.go | 12 ++++++------ prometheus/registry_test.go | 4 ++-- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/prometheus/counter_test.go b/prometheus/counter_test.go index b2023f0..e11b97e 100644 --- a/prometheus/counter_test.go +++ b/prometheus/counter_test.go @@ -28,7 +28,7 @@ func testCounter(t tester) { steps: []func(g Counter){}, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[]}", + value: `{"type":"counter","value":[]}`, }, }, { @@ -40,7 +40,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[{\"labels\":{},\"value\":1}]}", + value: `{"type":"counter","value":[{"labels":{},"value":1}]}`, }, }, { @@ -52,7 +52,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[{\"labels\":{},\"value\":2}]}", + value: `{"type":"counter","value":[{"labels":{},"value":2}]}`, }, }, { @@ -67,7 +67,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[{\"labels\":{},\"value\":5}]}", + value: `{"type":"counter","value":[{"labels":{},"value":5}]}`, }, }, { @@ -85,7 +85,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[]}", + value: `{"type":"counter","value":[]}`, }, }, { @@ -97,7 +97,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[{\"labels\":{\"handler\":\"/foo\"},\"value\":19}]}", + value: `{"type":"counter","value":[{"labels":{"handler":"/foo"},"value":19}]}`, }, }, { @@ -112,7 +112,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[{\"labels\":{\"handler\":\"/foo\"},\"value\":24}]}", + value: `{"type":"counter","value":[{"labels":{"handler":"/foo"},"value":24}]}`, }, }, { @@ -124,7 +124,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[{\"labels\":{\"handler\":\"/foo\"},\"value\":1}]}", + value: `{"type":"counter","value":[{"labels":{"handler":"/foo"},"value":1}]}`, }, }, { @@ -136,7 +136,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[{\"labels\":{\"handler\":\"/foo\"},\"value\":-1}]}", + value: `{"type":"counter","value":[{"labels":{"handler":"/foo"},"value":-1}]}`, }, }, { @@ -151,7 +151,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[{\"labels\":{\"handler\":\"/foo\"},\"value\":28}]}", + value: `{"type":"counter","value":[{"labels":{"handler":"/foo"},"value":28}]}`, }, }, { @@ -166,7 +166,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[{\"labels\":{\"handler\":\"/foo\"},\"value\":36}]}", + value: `{"type":"counter","value":[{"labels":{"handler":"/foo"},"value":36}]}`, }, }, { @@ -181,7 +181,7 @@ func testCounter(t tester) { }, }, out: output{ - value: "{\"type\":\"counter\",\"value\":[{\"labels\":{\"handler\":\"/foo\"},\"value\":27}]}", + value: `{"type":"counter","value":[{"labels":{"handler":"/foo"},"value":27}]}`, }, }, } diff --git a/prometheus/gauge_test.go b/prometheus/gauge_test.go index 8e7e5f1..ee3d425 100644 --- a/prometheus/gauge_test.go +++ b/prometheus/gauge_test.go @@ -28,7 +28,7 @@ func testGauge(t tester) { steps: []func(g Gauge){}, }, out: output{ - value: "{\"type\":\"gauge\",\"value\":[]}", + value: `{"type":"gauge","value":[]}`, }, }, { @@ -40,7 +40,7 @@ func testGauge(t tester) { }, }, out: output{ - value: "{\"type\":\"gauge\",\"value\":[{\"labels\":{},\"value\":1}]}", + value: `{"type":"gauge","value":[{"labels":{},"value":1}]}`, }, }, { @@ -52,7 +52,7 @@ func testGauge(t tester) { }, }, out: output{ - value: "{\"type\":\"gauge\",\"value\":[{\"labels\":{},\"value\":2}]}", + value: `{"type":"gauge","value":[{"labels":{},"value":2}]}`, }, }, { @@ -67,7 +67,7 @@ func testGauge(t tester) { }, }, out: output{ - value: "{\"type\":\"gauge\",\"value\":[{\"labels\":{},\"value\":5}]}", + value: `{"type":"gauge","value":[{"labels":{},"value":5}]}`, }, }, { @@ -85,7 +85,7 @@ func testGauge(t tester) { }, }, out: output{ - value: "{\"type\":\"gauge\",\"value\":[]}", + value: `{"type":"gauge","value":[]}`, }, }, { @@ -97,7 +97,7 @@ func testGauge(t tester) { }, }, out: output{ - value: "{\"type\":\"gauge\",\"value\":[{\"labels\":{\"handler\":\"/foo\"},\"value\":19}]}", + value: `{"type":"gauge","value":[{"labels":{"handler":"/foo"},"value":19}]}`, }, }, } diff --git a/prometheus/registry_test.go b/prometheus/registry_test.go index df0f1c2..b1d8379 100644 --- a/prometheus/registry_test.go +++ b/prometheus/registry_test.go @@ -282,7 +282,7 @@ func testDumpToWriter(t tester) { "foo": NewCounter(), }, }, - out: []byte("[{\"baseLabels\":{\"label_foo\":\"foo\",\"name\":\"foo\"},\"docstring\":\"metric foo\",\"metric\":{\"type\":\"counter\",\"value\":[]}}]"), + out: []byte(`[{"baseLabels":{"label_foo":"foo","name":"foo"},"docstring":"metric foo","metric":{"type":"counter","value":[]}}]`), }, { in: input{ @@ -291,7 +291,7 @@ func testDumpToWriter(t tester) { "bar": NewCounter(), }, }, - out: []byte("[{\"baseLabels\":{\"label_bar\":\"bar\",\"name\":\"bar\"},\"docstring\":\"metric bar\",\"metric\":{\"type\":\"counter\",\"value\":[]}},{\"baseLabels\":{\"label_foo\":\"foo\",\"name\":\"foo\"},\"docstring\":\"metric foo\",\"metric\":{\"type\":\"counter\",\"value\":[]}}]"), + out: []byte(`[{"baseLabels":{"label_bar":"bar","name":"bar"},"docstring":"metric bar","metric":{"type":"counter","value":[]}},{"baseLabels":{"label_foo":"foo","name":"foo"},"docstring":"metric foo","metric":{"type":"counter","value":[]}}]`), }, } From 71dd60e431178eca416f3aef508b52e70c6c55ba Mon Sep 17 00:00:00 2001 From: Bernerd Schaefer Date: Fri, 19 Apr 2013 14:11:01 +0200 Subject: [PATCH 3/3] Registry and Metrics implement json.Marshaler * Drop `AsMarshallable()` from the Metric interface. Use `json.Marshaler` and `MarshalJSON()`, and leverage JSON struct tags where possible. * Add `MarshalJSON()` to Registry and remove `dumpToWriter`, which makes the registry handler much simpler. In addition to simplifying some of the marshalling behavior, this also has the nice side effect of cutting down the number of `map[string]interface{}` instances. --- prometheus/counter.go | 41 +++++++++-------- prometheus/counter_test.go | 4 +- prometheus/gauge.go | 28 ++++++------ prometheus/gauge_test.go | 4 +- prometheus/histogram.go | 26 ++++++----- prometheus/metric.go | 6 ++- prometheus/registry.go | 90 +++++++++++-------------------------- prometheus/registry_test.go | 8 ++-- prometheus/telemetry.go | 3 -- 9 files changed, 83 insertions(+), 127 deletions(-) diff --git a/prometheus/counter.go b/prometheus/counter.go index ba4a776..40f0532 100644 --- a/prometheus/counter.go +++ b/prometheus/counter.go @@ -7,6 +7,7 @@ package prometheus import ( + "encoding/json" "fmt" "sync" ) @@ -24,8 +25,8 @@ type Counter interface { } type counterVector struct { - labels map[string]string - value float64 + Labels map[string]string `json:"labels"` + Value float64 `json:"value"` } func NewCounter() Counter { @@ -49,11 +50,11 @@ func (metric *counter) Set(labels map[string]string, value float64) float64 { signature := labelsToSignature(labels) if original, ok := metric.values[signature]; ok { - original.value = value + original.Value = value } else { metric.values[signature] = &counterVector{ - labels: labels, - value: value, + Labels: labels, + Value: value, } } @@ -65,8 +66,8 @@ func (metric *counter) ResetAll() { defer metric.mutex.Unlock() for key, value := range metric.values { - for label := range value.labels { - delete(value.labels, label) + for label := range value.Labels { + delete(value.Labels, label) } delete(metric.values, key) } @@ -91,11 +92,11 @@ func (metric *counter) IncrementBy(labels map[string]string, value float64) floa signature := labelsToSignature(labels) if original, ok := metric.values[signature]; ok { - original.value += value + original.Value += value } else { metric.values[signature] = &counterVector{ - labels: labels, - value: value, + Labels: labels, + Value: value, } } @@ -116,11 +117,11 @@ func (metric *counter) DecrementBy(labels map[string]string, value float64) floa signature := labelsToSignature(labels) if original, ok := metric.values[signature]; ok { - original.value -= value + original.Value -= value } else { metric.values[signature] = &counterVector{ - labels: labels, - value: -1 * value, + Labels: labels, + Value: -1 * value, } } @@ -131,20 +132,18 @@ func (metric *counter) Decrement(labels map[string]string) float64 { return metric.DecrementBy(labels, 1) } -func (metric counter) AsMarshallable() map[string]interface{} { +func (metric counter) MarshalJSON() ([]byte, error) { metric.mutex.RLock() defer metric.mutex.RUnlock() - values := make([]map[string]interface{}, 0, len(metric.values)) + values := make([]*counterVector, 0, len(metric.values)) + for _, value := range metric.values { - values = append(values, map[string]interface{}{ - labelsKey: value.labels, - valueKey: value.value, - }) + values = append(values, value) } - return map[string]interface{}{ + return json.Marshal(map[string]interface{}{ valueKey: values, typeKey: counterTypeValue, - } + }) } diff --git a/prometheus/counter_test.go b/prometheus/counter_test.go index e11b97e..e9db571 100644 --- a/prometheus/counter_test.go +++ b/prometheus/counter_test.go @@ -193,9 +193,7 @@ func testCounter(t tester) { step(counter) } - marshallable := counter.AsMarshallable() - - bytes, err := json.Marshal(marshallable) + bytes, err := json.Marshal(counter) if err != nil { t.Errorf("%d. could not marshal into JSON %s", i, err) continue diff --git a/prometheus/gauge.go b/prometheus/gauge.go index 2fa255d..dd99976 100644 --- a/prometheus/gauge.go +++ b/prometheus/gauge.go @@ -7,6 +7,7 @@ package prometheus import ( + "encoding/json" "fmt" "sync" ) @@ -21,8 +22,8 @@ type Gauge interface { } type gaugeVector struct { - labels map[string]string - value float64 + Labels map[string]string `json:"labels"` + Value float64 `json:"value"` } func NewGauge() Gauge { @@ -56,11 +57,11 @@ func (metric *gauge) Set(labels map[string]string, value float64) float64 { signature := labelsToSignature(labels) if original, ok := metric.values[signature]; ok { - original.value = value + original.Value = value } else { metric.values[signature] = &gaugeVector{ - labels: labels, - value: value, + Labels: labels, + Value: value, } } @@ -72,27 +73,24 @@ func (metric *gauge) ResetAll() { defer metric.mutex.Unlock() for key, value := range metric.values { - for label := range value.labels { - delete(value.labels, label) + for label := range value.Labels { + delete(value.Labels, label) } delete(metric.values, key) } } -func (metric gauge) AsMarshallable() map[string]interface{} { +func (metric gauge) MarshalJSON() ([]byte, error) { metric.mutex.RLock() defer metric.mutex.RUnlock() - values := make([]map[string]interface{}, 0, len(metric.values)) + values := make([]*gaugeVector, 0, len(metric.values)) for _, value := range metric.values { - values = append(values, map[string]interface{}{ - labelsKey: value.labels, - valueKey: value.value, - }) + values = append(values, value) } - return map[string]interface{}{ + return json.Marshal(map[string]interface{}{ typeKey: gaugeTypeValue, valueKey: values, - } + }) } diff --git a/prometheus/gauge_test.go b/prometheus/gauge_test.go index ee3d425..103bbc5 100644 --- a/prometheus/gauge_test.go +++ b/prometheus/gauge_test.go @@ -109,9 +109,7 @@ func testGauge(t tester) { step(gauge) } - marshallable := gauge.AsMarshallable() - - bytes, err := json.Marshal(marshallable) + bytes, err := json.Marshal(gauge) if err != nil { t.Errorf("%d. could not marshal into JSON %s", i, err) continue diff --git a/prometheus/histogram.go b/prometheus/histogram.go index e696f4d..9988ac2 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -8,6 +8,7 @@ package prometheus import ( "bytes" + "encoding/json" "fmt" "math" "strconv" @@ -238,29 +239,30 @@ func formatFloat(value float64) string { return strconv.FormatFloat(value, floatFormat, floatPrecision, floatBitCount) } -func (h histogram) AsMarshallable() map[string]interface{} { +func (h histogram) MarshalJSON() ([]byte, error) { h.mutex.RLock() defer h.mutex.RUnlock() - result := make(map[string]interface{}, 2) - result[typeKey] = histogramTypeValue values := make([]map[string]interface{}, 0, len(h.values)) for signature, value := range h.values { - metricContainer := map[string]interface{}{} - metricContainer[labelsKey] = value.labels - intermediate := map[string]interface{}{} + percentiles := make(map[string]float64, len(h.reportablePercentiles)) + for _, percentile := range h.reportablePercentiles { formatted := formatFloat(percentile) - intermediate[formatted] = h.percentile(signature, percentile) + percentiles[formatted] = h.percentile(signature, percentile) } - metricContainer[valueKey] = intermediate - values = append(values, metricContainer) + + values = append(values, map[string]interface{}{ + labelsKey: value.labels, + valueKey: percentiles, + }) } - result[valueKey] = values - - return result + return json.Marshal(map[string]interface{}{ + typeKey: histogramTypeValue, + valueKey: values, + }) } func (h *histogram) ResetAll() { diff --git a/prometheus/metric.go b/prometheus/metric.go index 9753130..8d4d5da 100644 --- a/prometheus/metric.go +++ b/prometheus/metric.go @@ -6,10 +6,12 @@ package prometheus +import "encoding/json" + // A Metric is something that can be exposed via the registry framework. type Metric interface { - // Produce a JSON-consumable representation of the metric. - AsMarshallable() map[string]interface{} + // Produce a JSON representation of the metric. + json.Marshaler // Reset the parent metrics and delete all child metrics. ResetAll() // Produce a human-consumable representation of the metric. diff --git a/prometheus/registry.go b/prometheus/registry.go index d968d3f..ebe9871 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -43,9 +43,9 @@ var ( // container represents a top-level registered metric that encompasses its // static metadata. type container struct { - baseLabels map[string]string - docstring string - metric Metric + BaseLabels map[string]string `json:"baseLabels"` + Docstring string `json:"docstring"` + Metric Metric `json:"metric"` name string } @@ -82,6 +82,24 @@ func Register(name, docstring string, baseLabels map[string]string, metric Metri return DefaultRegistry.Register(name, docstring, baseLabels, metric) } +// Implements json.Marshaler +func (r registry) MarshalJSON() (_ []byte, err error) { + metrics := make([]interface{}, 0, len(r.signatureContainers)) + + keys := make([]string, 0, len(metrics)) + for key := range r.signatureContainers { + keys = append(keys, key) + } + + sort.Strings(keys) + + for _, key := range keys { + metrics = append(metrics, r.signatureContainers[key]) + } + + return json.Marshal(metrics) +} + // isValidCandidate returns true if the candidate is acceptable for use. In the // event of any apparent incorrect use it will report the problem, invalidate // the candidate, or outright abort. @@ -125,7 +143,7 @@ func (r registry) isValidCandidate(name string, baseLabels map[string]string) (s if useAggressiveSanityChecks { for _, container := range r.signatureContainers { if container.name == name { - err = fmt.Errorf("metric named %s with baseLabels %s is already registered as %s and risks causing confusion", name, baseLabels, container.baseLabels) + err = fmt.Errorf("metric named %s with baseLabels %s is already registered as %s and risks causing confusion", name, baseLabels, container.BaseLabels) if abortOnMisuse { panic(err) } else if debugRegistration { @@ -154,9 +172,9 @@ func (r registry) Register(name, docstring string, baseLabels map[string]string, } r.signatureContainers[signature] = container{ - baseLabels: baseLabels, - docstring: docstring, - metric: metric, + BaseLabels: baseLabels, + Docstring: docstring, + Metric: metric, name: name, } @@ -194,61 +212,6 @@ func (register registry) YieldBasicAuthExporter(username, password string) http. }) } -func (registry registry) dumpToWriter(writer io.Writer) (err error) { - defer func() { - if err != nil { - dumpErrorCount.Increment(nil) - } - }() - - numberOfMetrics := len(registry.signatureContainers) - keys := make([]string, 0, numberOfMetrics) - for key := range registry.signatureContainers { - keys = append(keys, key) - } - - sort.Strings(keys) - - _, err = writer.Write([]byte("[")) - if err != nil { - return - } - - index := 0 - - for _, key := range keys { - container := registry.signatureContainers[key] - intermediate := map[string]interface{}{ - baseLabelsKey: container.baseLabels, - docstringKey: container.docstring, - metricKey: container.metric.AsMarshallable(), - } - marshaled, err := json.Marshal(intermediate) - if err != nil { - marshalErrorCount.Increment(nil) - index++ - continue - } - - if index > 0 && index < numberOfMetrics { - _, err = writer.Write([]byte(",")) - if err != nil { - return err - } - } - - _, err = writer.Write(marshaled) - if err != nil { - return err - } - index++ - } - - _, err = writer.Write([]byte("]")) - - return -} - // decorateWriter annotates the response writer to handle any other behaviors // that might be beneficial to the client---e.g., GZIP encoding. func decorateWriter(request *http.Request, writer http.ResponseWriter) io.Writer { @@ -286,8 +249,7 @@ func (registry registry) Handler() http.HandlerFunc { defer closer.Close() } - registry.dumpToWriter(writer) - + json.NewEncoder(writer).Encode(registry) } else { w.WriteHeader(http.StatusNotFound) } diff --git a/prometheus/registry_test.go b/prometheus/registry_test.go index b1d8379..f756b84 100644 --- a/prometheus/registry_test.go +++ b/prometheus/registry_test.go @@ -8,6 +8,7 @@ package prometheus import ( "bytes" + "encoding/json" "fmt" "io" "net/http" @@ -305,15 +306,14 @@ func testDumpToWriter(t tester) { } } - actual := &bytes.Buffer{} + actual, err := json.Marshal(registry) - err := registry.dumpToWriter(actual) if err != nil { t.Errorf("%d. encountered error while dumping %s", i, err) } - if !bytes.Equal(scenario.out, actual.Bytes()) { - t.Errorf("%d. expected %q for dumping, got %q", i, scenario.out, actual.Bytes()) + if !bytes.Equal(scenario.out, actual) { + t.Errorf("%d. expected %q for dumping, got %q", i, scenario.out, actual) } } } diff --git a/prometheus/telemetry.go b/prometheus/telemetry.go index dc048bd..011c483 100644 --- a/prometheus/telemetry.go +++ b/prometheus/telemetry.go @@ -15,9 +15,6 @@ import ( // exposed if the DefaultRegistry's exporter is hooked into the HTTP request // handler. var ( - marshalErrorCount = NewCounter() - dumpErrorCount = NewCounter() - requestCount = NewCounter() requestLatencyBuckets = LogarithmicSizedBucketsFor(0, 1000) requestLatency = NewHistogram(&HistogramSpecification{