diff --git a/CHANGELOG.md b/CHANGELOG.md index adcef04..5d93880 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +## 0.9.1 / 2018-11-03 +* [FEATURE] Add `WriteToTextfile` function to facilitate the creation of + *.prom files for the textfile collector of the node exporter. #489 +* [ENHANCEMENT] More descriptive error messages for inconsistent label + cardinality. #487 +* [ENHANCEMENT] Exposition: Use a GZIP encoder pool to avoid allocations in + high-frequency scrape scenarios. #366 +* [ENHANCEMENT] Exposition: Streaming serving of metrics data while encoding. + #482 +* [ENHANCEMENT] API client: Add a way to return the body of a 5xx response. + #479 + ## 0.9.0 / 2018-10-15 * [CHANGE] Go1.6 is no longer supported. * [CHANGE] More refinements of the `Registry` consistency checks: Duplicated diff --git a/VERSION b/VERSION index ac39a10..f374f66 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.9.0 +0.9.1 diff --git a/api/prometheus/v1/api.go b/api/prometheus/v1/api.go index 6a19fac..255f3ba 100644 --- a/api/prometheus/v1/api.go +++ b/api/prometheus/v1/api.go @@ -60,6 +60,8 @@ const ( ErrCanceled = "canceled" ErrExec = "execution" ErrBadResponse = "bad_response" + ErrServer = "server_error" + ErrClient = "client_error" // Possible values for HealthStatus. HealthGood HealthStatus = "up" @@ -69,8 +71,9 @@ const ( // Error is an error returned by the API. type Error struct { - Type ErrorType - Msg string + Type ErrorType + Msg string + Detail string } func (e *Error) Error() string { @@ -460,6 +463,16 @@ func apiError(code int) bool { return code == statusAPIError || code == http.StatusBadRequest } +func errorTypeAndMsgFor(resp *http.Response) (ErrorType, string) { + switch resp.StatusCode / 100 { + case 4: + return ErrClient, fmt.Sprintf("client error: %d", resp.StatusCode) + case 5: + return ErrServer, fmt.Sprintf("server error: %d", resp.StatusCode) + } + return ErrBadResponse, fmt.Sprintf("bad response code %d", resp.StatusCode) +} + func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, error) { resp, body, err := c.Client.Do(ctx, req) if err != nil { @@ -469,9 +482,11 @@ func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, [ code := resp.StatusCode if code/100 != 2 && !apiError(code) { + errorType, errorMsg := errorTypeAndMsgFor(resp) return resp, body, &Error{ - Type: ErrBadResponse, - Msg: fmt.Sprintf("bad response code %d", resp.StatusCode), + Type: errorType, + Msg: errorMsg, + Detail: string(body), } } diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index 1f7fda7..8492a5c 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -18,6 +18,7 @@ package v1 import ( "context" "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -30,9 +31,10 @@ import ( ) type apiTest struct { - do func() (interface{}, error) - inErr error - inRes interface{} + do func() (interface{}, error) + inErr error + inStatusCode int + inRes interface{} reqPath string reqParam url.Values @@ -75,7 +77,9 @@ func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Respon } resp := &http.Response{} - if test.inErr != nil { + if test.inStatusCode != 0 { + resp.StatusCode = test.inStatusCode + } else if test.inErr != nil { resp.StatusCode = statusAPIError } else { resp.StatusCode = http.StatusOK @@ -194,6 +198,42 @@ func TestAPIs(t *testing.T) { }, err: fmt.Errorf("some error"), }, + { + do: doQuery("2", testTime), + inRes: "some body", + inStatusCode: 500, + inErr: &Error{ + Type: ErrServer, + Msg: "server error: 500", + Detail: "some body", + }, + + reqMethod: "GET", + reqPath: "/api/v1/query", + reqParam: url.Values{ + "query": []string{"2"}, + "time": []string{testTime.Format(time.RFC3339Nano)}, + }, + err: errors.New("server_error: server error: 500"), + }, + { + do: doQuery("2", testTime), + inRes: "some body", + inStatusCode: 404, + inErr: &Error{ + Type: ErrClient, + Msg: "client error: 404", + Detail: "some body", + }, + + reqMethod: "GET", + reqPath: "/api/v1/query", + reqParam: url.Values{ + "query": []string{"2"}, + "time": []string{testTime.Format(time.RFC3339Nano)}, + }, + err: errors.New("client_error: client error: 404"), + }, { do: doQueryRange("2", Range{ @@ -498,29 +538,34 @@ func TestAPIs(t *testing.T) { var tests []apiTest tests = append(tests, queryTests...) - for _, test := range tests { - client.curTest = test + for i, test := range tests { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + client.curTest = test - res, err := test.do() + res, err := test.do() - if test.err != nil { - if err == nil { - t.Errorf("expected error %q but got none", test.err) - continue + if test.err != nil { + if err == nil { + t.Fatalf("expected error %q but got none", test.err) + } + if err.Error() != test.err.Error() { + t.Errorf("unexpected error: want %s, got %s", test.err, err) + } + if apiErr, ok := err.(*Error); ok { + if apiErr.Detail != test.inRes { + t.Errorf("%q should be %q", apiErr.Detail, test.inRes) + } + } + return } - if err.Error() != test.err.Error() { - t.Errorf("unexpected error: want %s, got %s", test.err, err) + if err != nil { + t.Fatalf("unexpected error: %s", err) } - continue - } - if err != nil { - t.Errorf("unexpected error: %s", err) - continue - } - if !reflect.DeepEqual(res, test.res) { - t.Errorf("unexpected result: want %v, got %v", test.res, res) - } + if !reflect.DeepEqual(res, test.res) { + t.Errorf("unexpected result: want %v, got %v", test.res, res) + } + }) } } @@ -532,10 +577,10 @@ type testClient struct { } type apiClientTest struct { - code int - response interface{} - expected string - err *Error + code int + response interface{} + expectedBody string + expectedErr *Error } func (c *testClient) URL(ep string, args map[string]string) *url.URL { @@ -575,98 +620,108 @@ func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response, func TestAPIClientDo(t *testing.T) { tests := []apiClientTest{ { + code: statusAPIError, response: &apiResponse{ Status: "error", Data: json.RawMessage(`null`), ErrorType: ErrBadData, Error: "failed", }, - err: &Error{ + expectedErr: &Error{ Type: ErrBadData, Msg: "failed", }, - code: statusAPIError, - expected: `null`, + expectedBody: `null`, }, { + code: statusAPIError, response: &apiResponse{ Status: "error", Data: json.RawMessage(`"test"`), ErrorType: ErrTimeout, Error: "timed out", }, - err: &Error{ + expectedErr: &Error{ Type: ErrTimeout, Msg: "timed out", }, - code: statusAPIError, - expected: `test`, + expectedBody: `test`, }, { - response: "bad json", - err: &Error{ - Type: ErrBadResponse, - Msg: "bad response code 500", + code: http.StatusInternalServerError, + response: "500 error details", + expectedErr: &Error{ + Type: ErrServer, + Msg: "server error: 500", + Detail: "500 error details", }, - code: http.StatusInternalServerError, }, { + code: http.StatusNotFound, + response: "404 error details", + expectedErr: &Error{ + Type: ErrClient, + Msg: "client error: 404", + Detail: "404 error details", + }, + }, + { + code: http.StatusBadRequest, response: &apiResponse{ Status: "error", Data: json.RawMessage(`null`), ErrorType: ErrBadData, Error: "end timestamp must not be before start time", }, - err: &Error{ + expectedErr: &Error{ Type: ErrBadData, Msg: "end timestamp must not be before start time", }, - code: http.StatusBadRequest, }, { + code: statusAPIError, response: "bad json", - err: &Error{ + expectedErr: &Error{ Type: ErrBadResponse, Msg: "invalid character 'b' looking for beginning of value", }, - code: statusAPIError, }, { + code: statusAPIError, response: &apiResponse{ Status: "success", Data: json.RawMessage(`"test"`), }, - err: &Error{ + expectedErr: &Error{ Type: ErrBadResponse, Msg: "inconsistent body for response code", }, - code: statusAPIError, }, { + code: statusAPIError, response: &apiResponse{ Status: "success", Data: json.RawMessage(`"test"`), ErrorType: ErrTimeout, Error: "timed out", }, - err: &Error{ + expectedErr: &Error{ Type: ErrBadResponse, Msg: "inconsistent body for response code", }, - code: statusAPIError, }, { + code: http.StatusOK, response: &apiResponse{ Status: "error", Data: json.RawMessage(`"test"`), ErrorType: ErrTimeout, Error: "timed out", }, - err: &Error{ + expectedErr: &Error{ Type: ErrBadResponse, Msg: "inconsistent body for response code", }, - code: http.StatusOK, }, } @@ -677,30 +732,37 @@ func TestAPIClientDo(t *testing.T) { } client := &apiClient{tc} - for _, test := range tests { + for i, test := range tests { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - tc.ch <- test + tc.ch <- test - _, body, err := client.Do(context.Background(), tc.req) + _, body, err := client.Do(context.Background(), tc.req) - if test.err != nil { - if err == nil { - t.Errorf("expected error %q but got none", test.err) - continue + if test.expectedErr != nil { + if err == nil { + t.Fatalf("expected error %q but got none", test.expectedErr) + } + if test.expectedErr.Error() != err.Error() { + t.Errorf("unexpected error: want %q, got %q", test.expectedErr, err) + } + if test.expectedErr.Detail != "" { + apiErr := err.(*Error) + if apiErr.Detail != test.expectedErr.Detail { + t.Errorf("unexpected error details: want %q, got %q", test.expectedErr.Detail, apiErr.Detail) + } + } + return } - if test.err.Error() != err.Error() { - t.Errorf("unexpected error: want %q, got %q", test.err, err) + if err != nil { + t.Fatalf("unexpeceted error %s", err) } - continue - } - if err != nil { - t.Errorf("unexpeceted error %s", err) - continue - } - want, got := test.expected, string(body) - if want != got { - t.Errorf("unexpected body: want %q, got %q", want, got) - } + want, got := test.expectedBody, string(body) + if want != got { + t.Errorf("unexpected body: want %q, got %q", want, got) + } + }) + } } diff --git a/prometheus/counter.go b/prometheus/counter.go index 7610d92..e625e6e 100644 --- a/prometheus/counter.go +++ b/prometheus/counter.go @@ -136,7 +136,7 @@ func NewCounterVec(opts CounterOpts, labelNames []string) *CounterVec { return &CounterVec{ metricVec: newMetricVec(desc, func(lvs ...string) Metric { if len(lvs) != len(desc.variableLabels) { - panic(errInconsistentCardinality) + panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels, lvs)) } result := &counter{desc: desc, labelPairs: makeLabelPairs(desc, lvs)} result.init(result) // Init self-collection. diff --git a/prometheus/examples_test.go b/prometheus/examples_test.go index 468e368..ce1be4d 100644 --- a/prometheus/examples_test.go +++ b/prometheus/examples_test.go @@ -264,7 +264,7 @@ func ExampleRegister() { // taskCounter unregistered. // taskCounterVec not registered: a previously registered descriptor with the same fully-qualified name as Desc{fqName: "worker_pool_completed_tasks_total", help: "Total number of tasks completed.", constLabels: {}, variableLabels: [worker_id]} has different label names or a different help string // taskCounterVec registered. - // Worker initialization failed: inconsistent label cardinality + // Worker initialization failed: inconsistent label cardinality: expected 1 label values but got 2 in []string{"42", "spurious arg"} // notMyCounter is nil. // taskCounterForWorker42 registered. // taskCounterForWorker2001 registered. diff --git a/prometheus/gauge.go b/prometheus/gauge.go index 43b1fb6..12d923d 100644 --- a/prometheus/gauge.go +++ b/prometheus/gauge.go @@ -147,7 +147,7 @@ func NewGaugeVec(opts GaugeOpts, labelNames []string) *GaugeVec { return &GaugeVec{ metricVec: newMetricVec(desc, func(lvs ...string) Metric { if len(lvs) != len(desc.variableLabels) { - panic(errInconsistentCardinality) + panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels, lvs)) } result := &gauge{desc: desc, labelPairs: makeLabelPairs(desc, lvs)} result.init(result) // Init self-collection. diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 807d711..f2a599f 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -165,7 +165,7 @@ func NewHistogram(opts HistogramOpts) Histogram { func newHistogram(desc *Desc, opts HistogramOpts, labelValues ...string) Histogram { if len(desc.variableLabels) != len(labelValues) { - panic(errInconsistentCardinality) + panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels, labelValues)) } for _, n := range desc.variableLabels { diff --git a/prometheus/labels.go b/prometheus/labels.go index e68f132..2744443 100644 --- a/prometheus/labels.go +++ b/prometheus/labels.go @@ -37,9 +37,22 @@ const reservedLabelPrefix = "__" var errInconsistentCardinality = errors.New("inconsistent label cardinality") +func makeInconsistentCardinalityError(fqName string, labels, labelValues []string) error { + return fmt.Errorf( + "%s: %q has %d variable labels named %q but %d values %q were provided", + errInconsistentCardinality, fqName, + len(labels), labels, + len(labelValues), labelValues, + ) +} + func validateValuesInLabels(labels Labels, expectedNumberOfValues int) error { if len(labels) != expectedNumberOfValues { - return errInconsistentCardinality + return fmt.Errorf( + "%s: expected %d label values but got %d in %#v", + errInconsistentCardinality, expectedNumberOfValues, + len(labels), labels, + ) } for name, val := range labels { @@ -53,7 +66,11 @@ func validateValuesInLabels(labels Labels, expectedNumberOfValues int) error { func validateLabelValues(vals []string, expectedNumberOfValues int) error { if len(vals) != expectedNumberOfValues { - return errInconsistentCardinality + return fmt.Errorf( + "%s: expected %d label values but got %d in %#v", + errInconsistentCardinality, expectedNumberOfValues, + len(vals), vals, + ) } for _, val := range vals { diff --git a/prometheus/registry.go b/prometheus/registry.go index e422ef3..f98c81a 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -16,6 +16,9 @@ package prometheus import ( "bytes" "fmt" + "io/ioutil" + "os" + "path/filepath" "runtime" "sort" "strings" @@ -23,6 +26,7 @@ import ( "unicode/utf8" "github.com/golang/protobuf/proto" + "github.com/prometheus/common/expfmt" dto "github.com/prometheus/client_model/go" @@ -533,6 +537,38 @@ func (r *Registry) Gather() ([]*dto.MetricFamily, error) { return internal.NormalizeMetricFamilies(metricFamiliesByName), errs.MaybeUnwrap() } +// WriteToTextfile calls Gather on the provided Gatherer, encodes the result in the +// Prometheus text format, and writes it to a temporary file. Upon success, the +// temporary file is renamed to the provided filename. +// +// This is intended for use with the textfile collector of the node exporter. +// Note that the node exporter expects the filename to be suffixed with ".prom". +func WriteToTextfile(filename string, g Gatherer) error { + tmp, err := ioutil.TempFile(filepath.Dir(filename), filepath.Base(filename)) + if err != nil { + return err + } + defer os.Remove(tmp.Name()) + + mfs, err := g.Gather() + if err != nil { + return err + } + for _, mf := range mfs { + if _, err := expfmt.MetricFamilyToText(tmp, mf); err != nil { + return err + } + } + if err := tmp.Close(); err != nil { + return err + } + + if err := os.Chmod(tmp.Name(), 0644); err != nil { + return err + } + return os.Rename(tmp.Name(), filename) +} + // processMetric is an internal helper method only used by the Gather method. func processMetric( metric Metric, diff --git a/prometheus/registry_test.go b/prometheus/registry_test.go index 73354f5..fb70c29 100644 --- a/prometheus/registry_test.go +++ b/prometheus/registry_test.go @@ -21,9 +21,11 @@ package prometheus_test import ( "bytes" + "io/ioutil" "math/rand" "net/http" "net/http/httptest" + "os" "sync" "testing" "time" @@ -872,3 +874,102 @@ func TestHistogramVecRegisterGatherConcurrency(t *testing.T) { close(quit) wg.Wait() } + +func TestWriteToTextfile(t *testing.T) { + expectedOut := `# HELP test_counter test counter +# TYPE test_counter counter +test_counter{name="qux"} 1 +# HELP test_gauge test gauge +# TYPE test_gauge gauge +test_gauge{name="baz"} 1.1 +# HELP test_hist test histogram +# TYPE test_hist histogram +test_hist_bucket{name="bar",le="0.005"} 0 +test_hist_bucket{name="bar",le="0.01"} 0 +test_hist_bucket{name="bar",le="0.025"} 0 +test_hist_bucket{name="bar",le="0.05"} 0 +test_hist_bucket{name="bar",le="0.1"} 0 +test_hist_bucket{name="bar",le="0.25"} 0 +test_hist_bucket{name="bar",le="0.5"} 0 +test_hist_bucket{name="bar",le="1"} 1 +test_hist_bucket{name="bar",le="2.5"} 1 +test_hist_bucket{name="bar",le="5"} 2 +test_hist_bucket{name="bar",le="10"} 2 +test_hist_bucket{name="bar",le="+Inf"} 2 +test_hist_sum{name="bar"} 3.64 +test_hist_count{name="bar"} 2 +# HELP test_summary test summary +# TYPE test_summary summary +test_summary{name="foo",quantile="0.5"} 10 +test_summary{name="foo",quantile="0.9"} 20 +test_summary{name="foo",quantile="0.99"} 20 +test_summary_sum{name="foo"} 30 +test_summary_count{name="foo"} 2 +` + + registry := prometheus.NewRegistry() + + summary := prometheus.NewSummaryVec( + prometheus.SummaryOpts{ + Name: "test_summary", + Help: "test summary", + }, + []string{"name"}, + ) + + histogram := prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "test_hist", + Help: "test histogram", + }, + []string{"name"}, + ) + + gauge := prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Name: "test_gauge", + Help: "test gauge", + }, + []string{"name"}, + ) + + counter := prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "test_counter", + Help: "test counter", + }, + []string{"name"}, + ) + + registry.MustRegister(summary) + registry.MustRegister(histogram) + registry.MustRegister(gauge) + registry.MustRegister(counter) + + summary.With(prometheus.Labels{"name": "foo"}).Observe(10) + summary.With(prometheus.Labels{"name": "foo"}).Observe(20) + histogram.With(prometheus.Labels{"name": "bar"}).Observe(0.93) + histogram.With(prometheus.Labels{"name": "bar"}).Observe(2.71) + gauge.With(prometheus.Labels{"name": "baz"}).Set(1.1) + counter.With(prometheus.Labels{"name": "qux"}).Inc() + + tmpfile, err := ioutil.TempFile("", "prom_registry_test") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tmpfile.Name()) + + if err := prometheus.WriteToTextfile(tmpfile.Name(), registry); err != nil { + t.Fatal(err) + } + + fileBytes, err := ioutil.ReadFile(tmpfile.Name()) + if err != nil { + t.Fatal(err) + } + fileContents := string(fileBytes) + + if fileContents != expectedOut { + t.Error("file contents didn't match unexpected") + } +} diff --git a/prometheus/summary.go b/prometheus/summary.go index 89e6a02..7fd272f 100644 --- a/prometheus/summary.go +++ b/prometheus/summary.go @@ -168,7 +168,7 @@ func NewSummary(opts SummaryOpts) Summary { func newSummary(desc *Desc, opts SummaryOpts, labelValues ...string) Summary { if len(desc.variableLabels) != len(labelValues) { - panic(errInconsistentCardinality) + panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels, labelValues)) } for _, n := range desc.variableLabels {