From 1335ef46bddd1cb120c21240851f292904aa91f6 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Thu, 13 Jun 2019 16:40:59 -0700 Subject: [PATCH 1/8] Pass warnings through on non-error responses (#599) Return warnings as a separate string slice to simplify handling. Signed-off-by: Thomas Jackson --- api/client.go | 56 ++------ api/client_test.go | 4 +- api/prometheus/v1/api.go | 242 +++++++++++++++------------------- api/prometheus/v1/api_test.go | 181 ++++++++++++++++--------- 4 files changed, 241 insertions(+), 242 deletions(-) diff --git a/api/client.go b/api/client.go index 2e6a551..53b87ae 100644 --- a/api/client.go +++ b/api/client.go @@ -25,41 +25,7 @@ import ( "time" ) -func NewErrorAPI(err error, warnings []string) Error { - if err == nil && warnings == nil { - return nil - } - return &ErrorAPI{err, warnings} -} - -type ErrorAPI struct { - err error - warnings []string -} - -func (w *ErrorAPI) Err() error { - return w.err -} - -func (w *ErrorAPI) Error() string { - if w.err != nil { - return w.err.Error() - } - return "Warnings: " + strings.Join(w.warnings, " , ") -} - -func (w *ErrorAPI) Warnings() []string { - return w.warnings -} - -// Error encapsulates an error + warning -type Error interface { - error - // Err returns the underlying error. - Err() error - // Warnings returns a list of warnings. - Warnings() []string -} +type Warnings []string // DefaultRoundTripper is used if no RoundTripper is set in Config. var DefaultRoundTripper http.RoundTripper = &http.Transport{ @@ -91,30 +57,30 @@ func (cfg *Config) roundTripper() http.RoundTripper { // Client is the interface for an API client. type Client interface { URL(ep string, args map[string]string) *url.URL - Do(context.Context, *http.Request) (*http.Response, []byte, Error) + Do(context.Context, *http.Request) (*http.Response, []byte, Warnings, error) } // DoGetFallback will attempt to do the request as-is, and on a 405 it will fallback to a GET request. -func DoGetFallback(c Client, ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, Error) { +func DoGetFallback(c Client, ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, Warnings, error) { req, err := http.NewRequest(http.MethodPost, u.String(), strings.NewReader(args.Encode())) if err != nil { - return nil, nil, NewErrorAPI(err, nil) + return nil, nil, nil, err } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, body, err := c.Do(ctx, req) + resp, body, warnings, err := c.Do(ctx, req) if resp != nil && resp.StatusCode == http.StatusMethodNotAllowed { u.RawQuery = args.Encode() req, err = http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, nil, NewErrorAPI(err, nil) + return nil, nil, warnings, err } } else { if err != nil { - return resp, body, NewErrorAPI(err, nil) + return resp, body, warnings, err } - return resp, body, nil + return resp, body, warnings, nil } return c.Do(ctx, req) } @@ -154,7 +120,7 @@ func (c *httpClient) URL(ep string, args map[string]string) *url.URL { return &u } -func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, Error) { +func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, Warnings, error) { if ctx != nil { req = req.WithContext(ctx) } @@ -166,7 +132,7 @@ func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, }() if err != nil { - return nil, nil, NewErrorAPI(err, nil) + return nil, nil, nil, err } var body []byte @@ -186,5 +152,5 @@ func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, case <-done: } - return resp, body, NewErrorAPI(err, nil) + return resp, body, nil, err } diff --git a/api/client_test.go b/api/client_test.go index b0ea306..b3c95ee 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -152,7 +152,7 @@ func TestDoGetFallback(t *testing.T) { client := &httpClient{client: *(server.Client())} // Do a post, and ensure that the post succeeds. - _, b, err := DoGetFallback(client, context.TODO(), u, v) + _, b, _, err := DoGetFallback(client, context.TODO(), u, v) if err != nil { t.Fatalf("Error doing local request: %v", err) } @@ -169,7 +169,7 @@ func TestDoGetFallback(t *testing.T) { // Do a fallbcak to a get. u.Path = "/blockPost" - _, b, err = DoGetFallback(client, context.TODO(), u, v) + _, b, _, err = DoGetFallback(client, context.TODO(), u, v) if err != nil { t.Fatalf("Error doing local request: %v", err) } diff --git a/api/prometheus/v1/api.go b/api/prometheus/v1/api.go index 28cdaef..694c7cd 100644 --- a/api/prometheus/v1/api.go +++ b/api/prometheus/v1/api.go @@ -22,7 +22,6 @@ import ( "math" "net/http" "strconv" - "strings" "time" "unsafe" @@ -197,26 +196,13 @@ const ( // Error is an error returned by the API. type Error struct { - Type ErrorType - Msg string - Detail string - warnings []string + Type ErrorType + Msg string + Detail string } func (e *Error) Error() string { - if e.Type != "" || e.Msg != "" { - return fmt.Sprintf("%s: %s", e.Type, e.Msg) - } - - return "Warnings: " + strings.Join(e.warnings, " , ") -} - -func (w *Error) Err() error { - return w -} - -func (w *Error) Warnings() []string { - return w.warnings + return fmt.Sprintf("%s: %s", e.Type, e.Msg) } // Range represents a sliced time range. @@ -230,34 +216,34 @@ type Range struct { // API provides bindings for Prometheus's v1 API. type API interface { // Alerts returns a list of all active alerts. - Alerts(ctx context.Context) (AlertsResult, api.Error) + Alerts(ctx context.Context) (AlertsResult, error) // AlertManagers returns an overview of the current state of the Prometheus alert manager discovery. - AlertManagers(ctx context.Context) (AlertManagersResult, api.Error) + AlertManagers(ctx context.Context) (AlertManagersResult, error) // CleanTombstones removes the deleted data from disk and cleans up the existing tombstones. - CleanTombstones(ctx context.Context) api.Error + CleanTombstones(ctx context.Context) error // Config returns the current Prometheus configuration. - Config(ctx context.Context) (ConfigResult, api.Error) + Config(ctx context.Context) (ConfigResult, error) // DeleteSeries deletes data for a selection of series in a time range. - DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) api.Error + DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) error // Flags returns the flag values that Prometheus was launched with. - Flags(ctx context.Context) (FlagsResult, api.Error) + Flags(ctx context.Context) (FlagsResult, error) // LabelValues performs a query for the values of the given label. - LabelValues(ctx context.Context, label string) (model.LabelValues, api.Error) + LabelValues(ctx context.Context, label string) (model.LabelValues, error) // Query performs a query for the given time. - Query(ctx context.Context, query string, ts time.Time) (model.Value, api.Error) + Query(ctx context.Context, query string, ts time.Time) (model.Value, api.Warnings, error) // QueryRange performs a query for the given range. - QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Error) + QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Warnings, error) // Series finds series by label matchers. - Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, api.Error) + Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, error) // Snapshot creates a snapshot of all current data into snapshots/- // under the TSDB's data directory and returns the directory as response. - Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, api.Error) + Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, error) // Rules returns a list of alerting and recording rules that are currently loaded. - Rules(ctx context.Context) (RulesResult, api.Error) + Rules(ctx context.Context) (RulesResult, error) // Targets returns an overview of the current state of the Prometheus target discovery. - Targets(ctx context.Context) (TargetsResult, api.Error) + Targets(ctx context.Context) (TargetsResult, error) // TargetsMetadata returns metadata about metrics currently scraped by the target. - TargetsMetadata(ctx context.Context, matchTarget string, metric string, limit string) ([]MetricMetadata, api.Error) + TargetsMetadata(ctx context.Context, matchTarget string, metric string, limit string) ([]MetricMetadata, error) } // AlertsResult contains the result from querying the alerts endpoint. @@ -534,73 +520,70 @@ type httpAPI struct { client api.Client } -func (h *httpAPI) Alerts(ctx context.Context) (AlertsResult, api.Error) { +func (h *httpAPI) Alerts(ctx context.Context) (AlertsResult, error) { u := h.client.URL(epAlerts, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return AlertsResult{}, api.NewErrorAPI(err, nil) + return AlertsResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return AlertsResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return AlertsResult{}, err } var res AlertsResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) AlertManagers(ctx context.Context) (AlertManagersResult, api.Error) { +func (h *httpAPI) AlertManagers(ctx context.Context) (AlertManagersResult, error) { u := h.client.URL(epAlertManagers, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return AlertManagersResult{}, api.NewErrorAPI(err, nil) + return AlertManagersResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return AlertManagersResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return AlertManagersResult{}, err } var res AlertManagersResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) CleanTombstones(ctx context.Context) api.Error { +func (h *httpAPI) CleanTombstones(ctx context.Context) error { u := h.client.URL(epCleanTombstones, nil) req, err := http.NewRequest(http.MethodPost, u.String(), nil) if err != nil { - return api.NewErrorAPI(err, nil) + return err } - _, _, apiErr := h.client.Do(ctx, req) - return apiErr + _, _, _, err = h.client.Do(ctx, req) + return err } -func (h *httpAPI) Config(ctx context.Context) (ConfigResult, api.Error) { +func (h *httpAPI) Config(ctx context.Context) (ConfigResult, error) { u := h.client.URL(epConfig, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return ConfigResult{}, api.NewErrorAPI(err, nil) + return ConfigResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return ConfigResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return ConfigResult{}, err } var res ConfigResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) api.Error { +func (h *httpAPI) DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) error { u := h.client.URL(epDeleteSeries, nil) q := u.Query() @@ -615,47 +598,45 @@ func (h *httpAPI) DeleteSeries(ctx context.Context, matches []string, startTime req, err := http.NewRequest(http.MethodPost, u.String(), nil) if err != nil { - return api.NewErrorAPI(err, nil) + return err } - _, _, apiErr := h.client.Do(ctx, req) - return apiErr + _, _, _, err = h.client.Do(ctx, req) + return err } -func (h *httpAPI) Flags(ctx context.Context) (FlagsResult, api.Error) { +func (h *httpAPI) Flags(ctx context.Context) (FlagsResult, error) { u := h.client.URL(epFlags, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return FlagsResult{}, api.NewErrorAPI(err, nil) + return FlagsResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return FlagsResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return FlagsResult{}, err } var res FlagsResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) LabelValues(ctx context.Context, label string) (model.LabelValues, api.Error) { +func (h *httpAPI) LabelValues(ctx context.Context, label string) (model.LabelValues, error) { u := h.client.URL(epLabelValues, map[string]string{"name": label}) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, api.NewErrorAPI(err, nil) + return nil, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return nil, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return nil, err } var labelValues model.LabelValues - err = json.Unmarshal(body, &labelValues) - return labelValues, api.NewErrorAPI(err, nil) + return labelValues, json.Unmarshal(body, &labelValues) } -func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model.Value, api.Error) { +func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model.Value, api.Warnings, error) { u := h.client.URL(epQuery, nil) q := u.Query() @@ -664,16 +645,16 @@ func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model. q.Set("time", ts.Format(time.RFC3339Nano)) } - _, body, apiErr := api.DoGetFallback(h.client, ctx, u, q) - if apiErr != nil { - return nil, apiErr + _, body, warnings, err := api.DoGetFallback(h.client, ctx, u, q) + if err != nil { + return nil, warnings, err } var qres queryResult - return model.Value(qres.v), api.NewErrorAPI(json.Unmarshal(body, &qres), nil) + return model.Value(qres.v), warnings, json.Unmarshal(body, &qres) } -func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Error) { +func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Warnings, error) { u := h.client.URL(epQueryRange, nil) q := u.Query() @@ -688,17 +669,17 @@ func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model. q.Set("end", end) q.Set("step", step) - _, body, apiErr := api.DoGetFallback(h.client, ctx, u, q) - if apiErr != nil { - return nil, apiErr + _, body, warnings, err := api.DoGetFallback(h.client, ctx, u, q) + if err != nil { + return nil, warnings, err } var qres queryResult - return model.Value(qres.v), api.NewErrorAPI(json.Unmarshal(body, &qres), nil) + return model.Value(qres.v), warnings, json.Unmarshal(body, &qres) } -func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, api.Error) { +func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, error) { u := h.client.URL(epSeries, nil) q := u.Query() @@ -713,20 +694,19 @@ func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.T req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, api.NewErrorAPI(err, nil) + return nil, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return nil, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return nil, err } var mset []model.LabelSet - err = json.Unmarshal(body, &mset) - return mset, api.NewErrorAPI(err, nil) + return mset, json.Unmarshal(body, &mset) } -func (h *httpAPI) Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, api.Error) { +func (h *httpAPI) Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, error) { u := h.client.URL(epSnapshot, nil) q := u.Query() @@ -736,56 +716,53 @@ func (h *httpAPI) Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, req, err := http.NewRequest(http.MethodPost, u.String(), nil) if err != nil { - return SnapshotResult{}, api.NewErrorAPI(err, nil) + return SnapshotResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return SnapshotResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return SnapshotResult{}, err } var res SnapshotResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) Rules(ctx context.Context) (RulesResult, api.Error) { +func (h *httpAPI) Rules(ctx context.Context) (RulesResult, error) { u := h.client.URL(epRules, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return RulesResult{}, api.NewErrorAPI(err, nil) + return RulesResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return RulesResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return RulesResult{}, err } var res RulesResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) Targets(ctx context.Context) (TargetsResult, api.Error) { +func (h *httpAPI) Targets(ctx context.Context) (TargetsResult, error) { u := h.client.URL(epTargets, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return TargetsResult{}, api.NewErrorAPI(err, nil) + return TargetsResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return TargetsResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return TargetsResult{}, err } var res TargetsResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metric string, limit string) ([]MetricMetadata, api.Error) { +func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metric string, limit string) ([]MetricMetadata, error) { u := h.client.URL(epTargetsMetadata, nil) q := u.Query() @@ -797,17 +774,16 @@ func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metri req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, api.NewErrorAPI(err, nil) + return nil, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return nil, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return nil, err } var res []MetricMetadata - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } // apiClient wraps a regular client and processes successful API responses. @@ -839,19 +815,17 @@ func errorTypeAndMsgFor(resp *http.Response) (ErrorType, string) { return ErrBadResponse, fmt.Sprintf("bad response code %d", resp.StatusCode) } -func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Error) { - resp, body, apiErr := c.Client.Do(ctx, req) - if apiErr != nil { - return resp, body, apiErr +func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Warnings, error) { + resp, body, warnings, err := c.Client.Do(ctx, req) + if err != nil { + return resp, body, warnings, err } code := resp.StatusCode - var err api.Error - if code/100 != 2 && !apiError(code) { errorType, errorMsg := errorTypeAndMsgFor(resp) - return resp, body, &Error{ + return resp, body, warnings, &Error{ Type: errorType, Msg: errorMsg, Detail: string(body), @@ -862,7 +836,7 @@ func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, [ if http.StatusNoContent != code { if jsonErr := json.Unmarshal(body, &result); jsonErr != nil { - return resp, body, &Error{ + return resp, body, warnings, &Error{ Type: ErrBadResponse, Msg: jsonErr.Error(), } @@ -871,20 +845,18 @@ func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, [ if apiError(code) != (result.Status == "error") { err = &Error{ - Type: ErrBadResponse, - Msg: "inconsistent body for response code", - warnings: result.Warnings, + Type: ErrBadResponse, + Msg: "inconsistent body for response code", } } if apiError(code) && result.Status == "error" { err = &Error{ - Type: result.ErrorType, - Msg: result.Error, - warnings: result.Warnings, + Type: result.ErrorType, + Msg: result.Error, } } - return resp, []byte(result.Data), err + return resp, []byte(result.Data), warnings, err } diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index 0b8fe2b..49c90d6 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -33,7 +33,7 @@ import ( ) type apiTest struct { - do func() (interface{}, api.Error) + do func() (interface{}, api.Warnings, error) inWarnings []string inErr error inStatusCode int @@ -43,6 +43,7 @@ type apiTest struct { reqParam url.Values reqMethod string res interface{} + warnings api.Warnings err error } @@ -63,7 +64,7 @@ func (c *apiTestClient) URL(ep string, args map[string]string) *url.URL { return u } -func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Error) { +func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Warnings, error) { test := c.curTest @@ -88,7 +89,7 @@ func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Respon resp.StatusCode = http.StatusOK } - return resp, b, api.NewErrorAPI(test.inErr, test.inWarnings) + return resp, b, test.inWarnings, test.inErr } func TestAPIs(t *testing.T) { @@ -101,81 +102,90 @@ func TestAPIs(t *testing.T) { client: client, } - doAlertManagers := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.AlertManagers(context.Background()) + doAlertManagers := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.AlertManagers(context.Background()) + return v, nil, err } } - doCleanTombstones := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return nil, promAPI.CleanTombstones(context.Background()) + doCleanTombstones := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + return nil, nil, promAPI.CleanTombstones(context.Background()) } } - doConfig := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Config(context.Background()) + doConfig := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Config(context.Background()) + return v, nil, err } } - doDeleteSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return nil, promAPI.DeleteSeries(context.Background(), []string{matcher}, startTime, endTime) + doDeleteSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + return nil, nil, promAPI.DeleteSeries(context.Background(), []string{matcher}, startTime, endTime) } } - doFlags := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Flags(context.Background()) + doFlags := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Flags(context.Background()) + return v, nil, err } } - doLabelValues := func(label string) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.LabelValues(context.Background(), label) + doLabelValues := func(label string) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.LabelValues(context.Background(), label) + return v, nil, err } } - doQuery := func(q string, ts time.Time) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { + doQuery := func(q string, ts time.Time) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { return promAPI.Query(context.Background(), q, ts) } } - doQueryRange := func(q string, rng Range) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { + doQueryRange := func(q string, rng Range) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { return promAPI.QueryRange(context.Background(), q, rng) } } - doSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Series(context.Background(), []string{matcher}, startTime, endTime) + doSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Series(context.Background(), []string{matcher}, startTime, endTime) + return v, nil, err } } - doSnapshot := func(skipHead bool) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Snapshot(context.Background(), skipHead) + doSnapshot := func(skipHead bool) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Snapshot(context.Background(), skipHead) + return v, nil, err } } - doRules := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Rules(context.Background()) + doRules := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Rules(context.Background()) + return v, nil, err } } - doTargets := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Targets(context.Background()) + doTargets := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Targets(context.Background()) + return v, nil, err } } - doTargetsMetadata := func(matchTarget string, metric string, limit string) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.TargetsMetadata(context.Background(), matchTarget, metric, limit) + doTargetsMetadata := func(matchTarget string, metric string, limit string) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.TargetsMetadata(context.Background(), matchTarget, metric, limit) + return v, nil, err } } @@ -249,6 +259,51 @@ func TestAPIs(t *testing.T) { }, err: errors.New("client_error: client error: 404"), }, + // Warning only. + { + do: doQuery("2", testTime), + inWarnings: []string{"warning"}, + inRes: &queryResult{ + Type: model.ValScalar, + Result: &model.Scalar{ + Value: 2, + Timestamp: model.TimeFromUnix(testTime.Unix()), + }, + }, + + reqMethod: "POST", + reqPath: "/api/v1/query", + reqParam: url.Values{ + "query": []string{"2"}, + "time": []string{testTime.Format(time.RFC3339Nano)}, + }, + res: &model.Scalar{ + Value: 2, + Timestamp: model.TimeFromUnix(testTime.Unix()), + }, + warnings: []string{"warning"}, + }, + // Warning + error. + { + do: doQuery("2", testTime), + inWarnings: []string{"warning"}, + inRes: "some body", + inStatusCode: 404, + inErr: &Error{ + Type: ErrClient, + Msg: "client error: 404", + Detail: "some body", + }, + + reqMethod: "POST", + reqPath: "/api/v1/query", + reqParam: url.Values{ + "query": []string{"2"}, + "time": []string{testTime.Format(time.RFC3339Nano)}, + }, + err: errors.New("client_error: client error: 404"), + warnings: []string{"warning"}, + }, { do: doQueryRange("2", Range{ @@ -705,7 +760,11 @@ func TestAPIs(t *testing.T) { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { client.curTest = test - res, err := test.do() + res, warnings, err := test.do() + + if (test.inWarnings == nil) != (warnings == nil) && !reflect.DeepEqual(test.inWarnings, warnings) { + t.Fatalf("mismatch in warnings expected=%v actual=%v", test.inWarnings, warnings) + } if test.err != nil { if err == nil { @@ -740,17 +799,18 @@ type testClient struct { } type apiClientTest struct { - code int - response interface{} - expectedBody string - expectedErr *Error + code int + response interface{} + expectedBody string + expectedErr *Error + expectedWarnings api.Warnings } func (c *testClient) URL(ep string, args map[string]string) *url.URL { return nil } -func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Error) { +func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Warnings, error) { if ctx == nil { c.Fatalf("context was not passed down") } @@ -777,7 +837,7 @@ func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response, StatusCode: test.code, } - return resp, b, nil + return resp, b, test.expectedWarnings, nil } func TestAPIClientDo(t *testing.T) { @@ -896,10 +956,10 @@ func TestAPIClientDo(t *testing.T) { Warnings: []string{"a"}, }, expectedErr: &Error{ - Type: ErrBadResponse, - Msg: "inconsistent body for response code", - warnings: []string{"a"}, + Type: ErrBadResponse, + Msg: "inconsistent body for response code", }, + expectedWarnings: []string{"a"}, }, } @@ -915,7 +975,17 @@ func TestAPIClientDo(t *testing.T) { tc.ch <- test - _, body, err := client.Do(context.Background(), tc.req) + _, body, warnings, err := client.Do(context.Background(), tc.req) + + if test.expectedWarnings != nil { + if !reflect.DeepEqual(test.expectedWarnings, warnings) { + t.Fatalf("mismatch in warnings expected=%v actual=%v", test.expectedWarnings, warnings) + } + } else { + if warnings != nil { + t.Fatalf("unexpexted warnings: %v", warnings) + } + } if test.expectedErr != nil { if err == nil { @@ -933,15 +1003,6 @@ func TestAPIClientDo(t *testing.T) { } } - if len(test.expectedErr.Warnings()) != len(err.Warnings()) { - t.Fatalf("expected warnings length :%v, but got:%v", len(test.expectedErr.Warnings()), len(err.Warnings())) - } - - for x, warning := range test.expectedErr.Warnings() { - if warning != err.Warnings()[x] { - t.Fatalf("expected warning :%v, but got:%v", warning, err.Warnings()[x]) - } - } return } From 6fdbbd8882316de46b5ec90daa08ee7aa3a8cc39 Mon Sep 17 00:00:00 2001 From: prombot Date: Fri, 14 Jun 2019 00:01:17 +0000 Subject: [PATCH 2/8] makefile: update Makefile.common with newer version Signed-off-by: prombot --- Makefile.common | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile.common b/Makefile.common index 6b94f83..d7aea1b 100644 --- a/Makefile.common +++ b/Makefile.common @@ -248,7 +248,9 @@ proto: ifdef GOLANGCI_LINT $(GOLANGCI_LINT): mkdir -p $(FIRST_GOPATH)/bin - curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(FIRST_GOPATH)/bin $(GOLANGCI_LINT_VERSION) + curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/$(GOLANGCI_LINT_VERSION)/install.sh \ + | sed -e '/install -d/d' \ + | sh -s -- -b $(FIRST_GOPATH)/bin $(GOLANGCI_LINT_VERSION) endif ifdef GOVENDOR From c5f419033887d835ba94f0c2eef1748ebe792f74 Mon Sep 17 00:00:00 2001 From: Calle Pettersson Date: Fri, 14 Jun 2019 12:26:55 +0200 Subject: [PATCH 3/8] Implement process collector for Windows (#596) * Implement process collector for Windows Signed-off-by: Calle Pettersson --- go.mod | 1 + go.sum | 1 + prometheus/process_collector.go | 44 +------- prometheus/process_collector_other.go | 65 +++++++++++ prometheus/process_collector_windows.go | 112 +++++++++++++++++++ prometheus/process_collector_windows_test.go | 70 ++++++++++++ 6 files changed, 250 insertions(+), 43 deletions(-) create mode 100644 prometheus/process_collector_other.go create mode 100644 prometheus/process_collector_windows.go create mode 100644 prometheus/process_collector_windows_test.go diff --git a/go.mod b/go.mod index 97cee81..3aee077 100644 --- a/go.mod +++ b/go.mod @@ -10,4 +10,5 @@ require ( github.com/prometheus/common v0.4.1 github.com/prometheus/procfs v0.0.2 github.com/stretchr/testify v1.3.0 // indirect + golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5 ) diff --git a/go.sum b/go.sum index 21c7bc0..1b68464 100644 --- a/go.sum +++ b/go.sum @@ -58,6 +58,7 @@ golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5 h1:mzjBh+S5frKOsOBobWIMAbXavqjmgO17k/2puhcFR94= golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/prometheus/process_collector.go b/prometheus/process_collector.go index 37d2026..5d01c31 100644 --- a/prometheus/process_collector.go +++ b/prometheus/process_collector.go @@ -16,8 +16,6 @@ package prometheus import ( "errors" "os" - - "github.com/prometheus/procfs" ) type processCollector struct { @@ -126,7 +124,7 @@ func NewProcessCollector(opts ProcessCollectorOpts) Collector { } // Set up process metric collection if supported by the runtime. - if _, err := procfs.NewDefaultFS(); err == nil { + if canCollectProcess() { c.collectFn = c.processCollect } else { c.collectFn = func(ch chan<- Metric) { @@ -153,46 +151,6 @@ func (c *processCollector) Collect(ch chan<- Metric) { c.collectFn(ch) } -func (c *processCollector) processCollect(ch chan<- Metric) { - pid, err := c.pidFn() - if err != nil { - c.reportError(ch, nil, err) - return - } - - p, err := procfs.NewProc(pid) - if err != nil { - c.reportError(ch, nil, err) - return - } - - if stat, err := p.Stat(); err == nil { - ch <- MustNewConstMetric(c.cpuTotal, CounterValue, stat.CPUTime()) - ch <- MustNewConstMetric(c.vsize, GaugeValue, float64(stat.VirtualMemory())) - ch <- MustNewConstMetric(c.rss, GaugeValue, float64(stat.ResidentMemory())) - if startTime, err := stat.StartTime(); err == nil { - ch <- MustNewConstMetric(c.startTime, GaugeValue, startTime) - } else { - c.reportError(ch, c.startTime, err) - } - } else { - c.reportError(ch, nil, err) - } - - if fds, err := p.FileDescriptorsLen(); err == nil { - ch <- MustNewConstMetric(c.openFDs, GaugeValue, float64(fds)) - } else { - c.reportError(ch, c.openFDs, err) - } - - if limits, err := p.Limits(); err == nil { - ch <- MustNewConstMetric(c.maxFDs, GaugeValue, float64(limits.OpenFiles)) - ch <- MustNewConstMetric(c.maxVsize, GaugeValue, float64(limits.AddressSpace)) - } else { - c.reportError(ch, nil, err) - } -} - func (c *processCollector) reportError(ch chan<- Metric, desc *Desc, err error) { if !c.reportErrors { return diff --git a/prometheus/process_collector_other.go b/prometheus/process_collector_other.go new file mode 100644 index 0000000..3117461 --- /dev/null +++ b/prometheus/process_collector_other.go @@ -0,0 +1,65 @@ +// Copyright 2019 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build !windows + +package prometheus + +import ( + "github.com/prometheus/procfs" +) + +func canCollectProcess() bool { + _, err := procfs.NewDefaultFS() + return err == nil +} + +func (c *processCollector) processCollect(ch chan<- Metric) { + pid, err := c.pidFn() + if err != nil { + c.reportError(ch, nil, err) + return + } + + p, err := procfs.NewProc(pid) + if err != nil { + c.reportError(ch, nil, err) + return + } + + if stat, err := p.Stat(); err == nil { + ch <- MustNewConstMetric(c.cpuTotal, CounterValue, stat.CPUTime()) + ch <- MustNewConstMetric(c.vsize, GaugeValue, float64(stat.VirtualMemory())) + ch <- MustNewConstMetric(c.rss, GaugeValue, float64(stat.ResidentMemory())) + if startTime, err := stat.StartTime(); err == nil { + ch <- MustNewConstMetric(c.startTime, GaugeValue, startTime) + } else { + c.reportError(ch, c.startTime, err) + } + } else { + c.reportError(ch, nil, err) + } + + if fds, err := p.FileDescriptorsLen(); err == nil { + ch <- MustNewConstMetric(c.openFDs, GaugeValue, float64(fds)) + } else { + c.reportError(ch, c.openFDs, err) + } + + if limits, err := p.Limits(); err == nil { + ch <- MustNewConstMetric(c.maxFDs, GaugeValue, float64(limits.OpenFiles)) + ch <- MustNewConstMetric(c.maxVsize, GaugeValue, float64(limits.AddressSpace)) + } else { + c.reportError(ch, nil, err) + } +} diff --git a/prometheus/process_collector_windows.go b/prometheus/process_collector_windows.go new file mode 100644 index 0000000..e0b935d --- /dev/null +++ b/prometheus/process_collector_windows.go @@ -0,0 +1,112 @@ +// Copyright 2019 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prometheus + +import ( + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +func canCollectProcess() bool { + return true +} + +var ( + modpsapi = syscall.NewLazyDLL("psapi.dll") + modkernel32 = syscall.NewLazyDLL("kernel32.dll") + + procGetProcessMemoryInfo = modpsapi.NewProc("GetProcessMemoryInfo") + procGetProcessHandleCount = modkernel32.NewProc("GetProcessHandleCount") +) + +type processMemoryCounters struct { + // https://docs.microsoft.com/en-us/windows/desktop/api/psapi/ns-psapi-_process_memory_counters_ex + _ uint32 + PageFaultCount uint32 + PeakWorkingSetSize uint64 + WorkingSetSize uint64 + QuotaPeakPagedPoolUsage uint64 + QuotaPagedPoolUsage uint64 + QuotaPeakNonPagedPoolUsage uint64 + QuotaNonPagedPoolUsage uint64 + PagefileUsage uint64 + PeakPagefileUsage uint64 + PrivateUsage uint64 +} + +func getProcessMemoryInfo(handle windows.Handle) (processMemoryCounters, error) { + mem := processMemoryCounters{} + r1, _, err := procGetProcessMemoryInfo.Call( + uintptr(handle), + uintptr(unsafe.Pointer(&mem)), + uintptr(unsafe.Sizeof(mem)), + ) + if r1 != 1 { + return mem, err + } else { + return mem, nil + } +} + +func getProcessHandleCount(handle windows.Handle) (uint32, error) { + var count uint32 + r1, _, err := procGetProcessHandleCount.Call( + uintptr(handle), + uintptr(unsafe.Pointer(&count)), + ) + if r1 != 1 { + return 0, err + } else { + return count, nil + } +} + +func (c *processCollector) processCollect(ch chan<- Metric) { + h, err := windows.GetCurrentProcess() + if err != nil { + c.reportError(ch, nil, err) + return + } + + var startTime, exitTime, kernelTime, userTime windows.Filetime + err = windows.GetProcessTimes(h, &startTime, &exitTime, &kernelTime, &userTime) + if err != nil { + c.reportError(ch, nil, err) + return + } + ch <- MustNewConstMetric(c.startTime, GaugeValue, float64(startTime.Nanoseconds()/1e9)) + ch <- MustNewConstMetric(c.cpuTotal, CounterValue, fileTimeToSeconds(kernelTime)+fileTimeToSeconds(userTime)) + + mem, err := getProcessMemoryInfo(h) + if err != nil { + c.reportError(ch, nil, err) + return + } + ch <- MustNewConstMetric(c.vsize, GaugeValue, float64(mem.PrivateUsage)) + ch <- MustNewConstMetric(c.rss, GaugeValue, float64(mem.WorkingSetSize)) + + handles, err := getProcessHandleCount(h) + if err != nil { + c.reportError(ch, nil, err) + return + } + ch <- MustNewConstMetric(c.openFDs, GaugeValue, float64(handles)) + ch <- MustNewConstMetric(c.maxFDs, GaugeValue, float64(16*1024*1024)) // Windows has a hard-coded max limit, not per-process. +} + +func fileTimeToSeconds(ft windows.Filetime) float64 { + return float64(uint64(ft.HighDateTime)<<32+uint64(ft.LowDateTime)) / 1e7 +} diff --git a/prometheus/process_collector_windows_test.go b/prometheus/process_collector_windows_test.go new file mode 100644 index 0000000..6f687b9 --- /dev/null +++ b/prometheus/process_collector_windows_test.go @@ -0,0 +1,70 @@ +// Copyright 2019 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prometheus + +import ( + "bytes" + "os" + "regexp" + "testing" + + "github.com/prometheus/common/expfmt" +) + +func TestWindowsProcessCollector(t *testing.T) { + registry := NewRegistry() + if err := registry.Register(NewProcessCollector(ProcessCollectorOpts{})); err != nil { + t.Fatal(err) + } + if err := registry.Register(NewProcessCollector(ProcessCollectorOpts{ + PidFn: func() (int, error) { return os.Getpid(), nil }, + Namespace: "foobar", + ReportErrors: true, // No errors expected, just to see if none are reported. + })); err != nil { + t.Fatal(err) + } + + mfs, err := registry.Gather() + if err != nil { + t.Fatal(err) + } + + var buf bytes.Buffer + for _, mf := range mfs { + if _, err := expfmt.MetricFamilyToText(&buf, mf); err != nil { + t.Fatal(err) + } + } + + for _, re := range []*regexp.Regexp{ + regexp.MustCompile("\nprocess_cpu_seconds_total [0-9]"), + regexp.MustCompile("\nprocess_max_fds [1-9]"), + regexp.MustCompile("\nprocess_open_fds [1-9]"), + regexp.MustCompile("\nprocess_virtual_memory_max_bytes (-1|[1-9])"), + regexp.MustCompile("\nprocess_virtual_memory_bytes [1-9]"), + regexp.MustCompile("\nprocess_resident_memory_bytes [1-9]"), + regexp.MustCompile("\nprocess_start_time_seconds [0-9.]{10,}"), + regexp.MustCompile("\nfoobar_process_cpu_seconds_total [0-9]"), + regexp.MustCompile("\nfoobar_process_max_fds [1-9]"), + regexp.MustCompile("\nfoobar_process_open_fds [1-9]"), + regexp.MustCompile("\nfoobar_process_virtual_memory_max_bytes (-1|[1-9])"), + regexp.MustCompile("\nfoobar_process_virtual_memory_bytes [1-9]"), + regexp.MustCompile("\nfoobar_process_resident_memory_bytes [1-9]"), + regexp.MustCompile("\nfoobar_process_start_time_seconds [0-9.]{10,}"), + } { + if !re.Match(buf.Bytes()) { + t.Errorf("want body to match %s\n%s", re, buf.String()) + } + } +} From f61dbeadeda7e4b6336b49de8753a301c0d80b6f Mon Sep 17 00:00:00 2001 From: beorn7 Date: Fri, 14 Jun 2019 12:31:33 +0200 Subject: [PATCH 4/8] Update doc comment of NewProcessCollector - Now also works on MS Windows. - The hints for updating from older versions is obsolete by now. Signed-off-by: beorn7 --- prometheus/process_collector.go | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/prometheus/process_collector.go b/prometheus/process_collector.go index 5d01c31..9b80979 100644 --- a/prometheus/process_collector.go +++ b/prometheus/process_collector.go @@ -57,20 +57,9 @@ type ProcessCollectorOpts struct { // collector for the current process with an empty namespace string and no error // reporting. // -// Currently, the collector depends on a Linux-style proc filesystem and -// therefore only exports metrics for Linux. -// -// Note: An older version of this function had the following signature: -// -// NewProcessCollector(pid int, namespace string) Collector -// -// Most commonly, it was called as -// -// NewProcessCollector(os.Getpid(), "") -// -// The following call of the current version is equivalent to the above: -// -// NewProcessCollector(ProcessCollectorOpts{}) +// The collector only works on operating systems with a Linux-style proc +// filesystem and on Microsoft Windows. On other operating systems, it will not +// collect any metrics. func NewProcessCollector(opts ProcessCollectorOpts) Collector { ns := "" if len(opts.Namespace) > 0 { From f213ad9bfc4833a54781ee135067aaded9321972 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 14 Jun 2019 07:49:58 -0700 Subject: [PATCH 5/8] Add /labels API to client (#604) API ref https://prometheus.io/docs/prometheus/latest/querying/api/#getting-label-names Signed-off-by: Thomas Jackson --- api/prometheus/v1/api.go | 17 +++++++++++++++++ api/prometheus/v1/api_test.go | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/api/prometheus/v1/api.go b/api/prometheus/v1/api.go index 694c7cd..77e7348 100644 --- a/api/prometheus/v1/api.go +++ b/api/prometheus/v1/api.go @@ -124,6 +124,7 @@ const ( epAlertManagers = apiPrefix + "/alertmanagers" epQuery = apiPrefix + "/query" epQueryRange = apiPrefix + "/query_range" + epLabels = apiPrefix + "/labels" epLabelValues = apiPrefix + "/label/:name/values" epSeries = apiPrefix + "/series" epTargets = apiPrefix + "/targets" @@ -227,6 +228,8 @@ type API interface { DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) error // Flags returns the flag values that Prometheus was launched with. Flags(ctx context.Context) (FlagsResult, error) + // LabelNames returns all the unique label names present in the block in sorted order. + LabelNames(ctx context.Context) ([]string, error) // LabelValues performs a query for the values of the given label. LabelValues(ctx context.Context, label string) (model.LabelValues, error) // Query performs a query for the given time. @@ -622,6 +625,20 @@ func (h *httpAPI) Flags(ctx context.Context) (FlagsResult, error) { return res, json.Unmarshal(body, &res) } +func (h *httpAPI) LabelNames(ctx context.Context) ([]string, error) { + u := h.client.URL(epLabels, nil) + req, err := http.NewRequest(http.MethodGet, u.String(), nil) + if err != nil { + return nil, err + } + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return nil, err + } + var labelNames []string + return labelNames, json.Unmarshal(body, &labelNames) +} + func (h *httpAPI) LabelValues(ctx context.Context, label string) (model.LabelValues, error) { u := h.client.URL(epLabelValues, map[string]string{"name": label}) req, err := http.NewRequest(http.MethodGet, u.String(), nil) diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index 49c90d6..f6b4897 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -135,6 +135,13 @@ func TestAPIs(t *testing.T) { } } + doLabelNames := func(label string) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.LabelNames(context.Background()) + return v, nil, err + } + } + doLabelValues := func(label string) func() (interface{}, api.Warnings, error) { return func() (interface{}, api.Warnings, error) { v, err := promAPI.LabelValues(context.Background(), label) @@ -324,6 +331,22 @@ func TestAPIs(t *testing.T) { err: fmt.Errorf("some error"), }, + { + do: doLabelNames("mylabel"), + inRes: []string{"val1", "val2"}, + reqMethod: "GET", + reqPath: "/api/v1/labels", + res: []string{"val1", "val2"}, + }, + + { + do: doLabelNames("mylabel"), + inErr: fmt.Errorf("some error"), + reqMethod: "GET", + reqPath: "/api/v1/labels", + err: fmt.Errorf("some error"), + }, + { do: doLabelValues("mylabel"), inRes: []string{"val1", "val2"}, From 2f3a0f8f2e631afb1ec7366101d7649c6155b980 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Fri, 14 Jun 2019 16:16:15 +0200 Subject: [PATCH 6/8] Make the AlreadyRegisteredError useful for wrapped registries Signed-off-by: beorn7 --- prometheus/registry.go | 14 ++++- prometheus/registry_test.go | 119 ++++++++++++++++++++++++++++++------ prometheus/wrap.go | 21 +++++++ 3 files changed, 133 insertions(+), 21 deletions(-) diff --git a/prometheus/registry.go b/prometheus/registry.go index f2fb67a..6c32516 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -325,9 +325,17 @@ func (r *Registry) Register(c Collector) error { return nil } if existing, exists := r.collectorsByID[collectorID]; exists { - return AlreadyRegisteredError{ - ExistingCollector: existing, - NewCollector: c, + switch e := existing.(type) { + case *wrappingCollector: + return AlreadyRegisteredError{ + ExistingCollector: e.unwrapRecursively(), + NewCollector: c, + } + default: + return AlreadyRegisteredError{ + ExistingCollector: e, + NewCollector: c, + } } } // If the collectorID is new, but at least one of the descs existed diff --git a/prometheus/registry_test.go b/prometheus/registry_test.go index 3324c7c..2062e67 100644 --- a/prometheus/registry_test.go +++ b/prometheus/registry_test.go @@ -746,37 +746,120 @@ func BenchmarkHandler(b *testing.B) { } func TestAlreadyRegistered(t *testing.T) { - reg := prometheus.NewRegistry() original := prometheus.NewCounterVec( prometheus.CounterOpts{ - Name: "test", - Help: "help", + Name: "test", + Help: "help", + ConstLabels: prometheus.Labels{"const": "label"}, }, []string{"foo", "bar"}, ) equalButNotSame := prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "test", + Help: "help", + ConstLabels: prometheus.Labels{"const": "label"}, + }, + []string{"foo", "bar"}, + ) + originalWithoutConstLabel := prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "test", Help: "help", }, []string{"foo", "bar"}, ) - var err error - if err = reg.Register(original); err != nil { - t.Fatal(err) + equalButNotSameWithoutConstLabel := prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "test", + Help: "help", + }, + []string{"foo", "bar"}, + ) + + scenarios := []struct { + name string + originalCollector prometheus.Collector + registerWith func(prometheus.Registerer) prometheus.Registerer + newCollector prometheus.Collector + reRegisterWith func(prometheus.Registerer) prometheus.Registerer + }{ + { + "RegisterNormallyReregisterNormally", + original, + func(r prometheus.Registerer) prometheus.Registerer { return r }, + equalButNotSame, + func(r prometheus.Registerer) prometheus.Registerer { return r }, + }, + { + "RegisterNormallyReregisterWrapped", + original, + func(r prometheus.Registerer) prometheus.Registerer { return r }, + equalButNotSameWithoutConstLabel, + func(r prometheus.Registerer) prometheus.Registerer { + return prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r) + }, + }, + { + "RegisterWrappedReregisterWrapped", + originalWithoutConstLabel, + func(r prometheus.Registerer) prometheus.Registerer { + return prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r) + }, + equalButNotSameWithoutConstLabel, + func(r prometheus.Registerer) prometheus.Registerer { + return prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r) + }, + }, + { + "RegisterWrappedReregisterNormally", + originalWithoutConstLabel, + func(r prometheus.Registerer) prometheus.Registerer { + return prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r) + }, + equalButNotSame, + func(r prometheus.Registerer) prometheus.Registerer { return r }, + }, + { + "RegisterDoublyWrappedReregisterDoublyWrapped", + originalWithoutConstLabel, + func(r prometheus.Registerer) prometheus.Registerer { + return prometheus.WrapRegistererWithPrefix( + "wrap_", + prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r), + ) + }, + equalButNotSameWithoutConstLabel, + func(r prometheus.Registerer) prometheus.Registerer { + return prometheus.WrapRegistererWithPrefix( + "wrap_", + prometheus.WrapRegistererWith(prometheus.Labels{"const": "label"}, r), + ) + }, + }, } - if err = reg.Register(equalButNotSame); err == nil { - t.Fatal("expected error when registering equal collector") - } - if are, ok := err.(prometheus.AlreadyRegisteredError); ok { - if are.ExistingCollector != original { - t.Error("expected original collector but got something else") - } - if are.ExistingCollector == equalButNotSame { - t.Error("expected original callector but got new one") - } - } else { - t.Error("unexpected error:", err) + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + var err error + reg := prometheus.NewRegistry() + if err = s.registerWith(reg).Register(s.originalCollector); err != nil { + t.Fatal(err) + } + if err = s.reRegisterWith(reg).Register(s.newCollector); err == nil { + t.Fatal("expected error when registering new collector") + } + if are, ok := err.(prometheus.AlreadyRegisteredError); ok { + if are.ExistingCollector != s.originalCollector { + t.Error("expected original collector but got something else") + } + if are.ExistingCollector == s.newCollector { + t.Error("expected original collector but got new one") + } + } else { + t.Error("unexpected error:", err) + } + }) } } diff --git a/prometheus/wrap.go b/prometheus/wrap.go index 49159bf..e303eef 100644 --- a/prometheus/wrap.go +++ b/prometheus/wrap.go @@ -32,6 +32,12 @@ import ( // WrapRegistererWith provides a way to add fixed labels to a subset of // Collectors. It should not be used to add fixed labels to all metrics exposed. // +// Conflicts between Collectors registered through the original Registerer with +// Collectors registered through the wrapping Registerer will still be +// detected. Any AlreadyRegisteredError returned by the Register method of +// either Registerer will contain the ExistingCollector in the form it was +// provided to the respective registry. +// // The Collector example demonstrates a use of WrapRegistererWith. func WrapRegistererWith(labels Labels, reg Registerer) Registerer { return &wrappingRegisterer{ @@ -54,6 +60,12 @@ func WrapRegistererWith(labels Labels, reg Registerer) Registerer { // (see NewGoCollector) and the process collector (see NewProcessCollector). (In // fact, those metrics are already prefixed with “go_” or “process_”, // respectively.) +// +// Conflicts between Collectors registered through the original Registerer with +// Collectors registered through the wrapping Registerer will still be +// detected. Any AlreadyRegisteredError returned by the Register method of +// either Registerer will contain the ExistingCollector in the form it was +// provided to the respective registry. func WrapRegistererWithPrefix(prefix string, reg Registerer) Registerer { return &wrappingRegisterer{ wrappedRegisterer: reg, @@ -123,6 +135,15 @@ func (c *wrappingCollector) Describe(ch chan<- *Desc) { } } +func (c *wrappingCollector) unwrapRecursively() Collector { + switch wc := c.wrappedCollector.(type) { + case *wrappingCollector: + return wc.unwrapRecursively() + default: + return wc + } +} + type wrappingMetric struct { wrappedMetric Metric prefix string From 063470a3c9c980049e07be61cfbf7c820204c517 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 14 Jun 2019 14:28:28 -0700 Subject: [PATCH 7/8] Add warnings to series (#603) Signed-off-by: Thomas Jackson --- api/prometheus/v1/api.go | 12 +++++----- api/prometheus/v1/api_test.go | 44 +++++++++++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/api/prometheus/v1/api.go b/api/prometheus/v1/api.go index 77e7348..c081bc8 100644 --- a/api/prometheus/v1/api.go +++ b/api/prometheus/v1/api.go @@ -237,7 +237,7 @@ type API interface { // QueryRange performs a query for the given range. QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Warnings, error) // Series finds series by label matchers. - Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, error) + Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, api.Warnings, error) // Snapshot creates a snapshot of all current data into snapshots/- // under the TSDB's data directory and returns the directory as response. Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, error) @@ -696,7 +696,7 @@ func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model. return model.Value(qres.v), warnings, json.Unmarshal(body, &qres) } -func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, error) { +func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, api.Warnings, error) { u := h.client.URL(epSeries, nil) q := u.Query() @@ -711,16 +711,16 @@ func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.T req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, err + return nil, nil, err } - _, body, _, err := h.client.Do(ctx, req) + _, body, warnings, err := h.client.Do(ctx, req) if err != nil { - return nil, err + return nil, warnings, err } var mset []model.LabelSet - return mset, json.Unmarshal(body, &mset) + return mset, warnings, json.Unmarshal(body, &mset) } func (h *httpAPI) Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, error) { diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index f6b4897..28e7214 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -163,8 +163,7 @@ func TestAPIs(t *testing.T) { doSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, api.Warnings, error) { return func() (interface{}, api.Warnings, error) { - v, err := promAPI.Series(context.Background(), []string{matcher}, startTime, endTime) - return v, nil, err + return promAPI.Series(context.Background(), []string{matcher}, startTime, endTime) } } @@ -386,6 +385,32 @@ func TestAPIs(t *testing.T) { }, }, }, + // Series with data + warning. + { + do: doSeries("up", testTime.Add(-time.Minute), testTime), + inRes: []map[string]string{ + { + "__name__": "up", + "job": "prometheus", + "instance": "localhost:9090"}, + }, + inWarnings: []string{"a"}, + reqMethod: "GET", + reqPath: "/api/v1/series", + reqParam: url.Values{ + "match": []string{"up"}, + "start": []string{testTime.Add(-time.Minute).Format(time.RFC3339Nano)}, + "end": []string{testTime.Format(time.RFC3339Nano)}, + }, + res: []model.LabelSet{ + { + "__name__": "up", + "job": "prometheus", + "instance": "localhost:9090", + }, + }, + warnings: []string{"a"}, + }, { do: doSeries("up", testTime.Add(-time.Minute), testTime), @@ -399,6 +424,21 @@ func TestAPIs(t *testing.T) { }, err: fmt.Errorf("some error"), }, + // Series with error and warning. + { + do: doSeries("up", testTime.Add(-time.Minute), testTime), + inErr: fmt.Errorf("some error"), + inWarnings: []string{"a"}, + reqMethod: "GET", + reqPath: "/api/v1/series", + reqParam: url.Values{ + "match": []string{"up"}, + "start": []string{testTime.Add(-time.Minute).Format(time.RFC3339Nano)}, + "end": []string{testTime.Format(time.RFC3339Nano)}, + }, + err: fmt.Errorf("some error"), + warnings: []string{"a"}, + }, { do: doSnapshot(true), From 505041cff1d2b843800a4f384694dcf62fb3e69c Mon Sep 17 00:00:00 2001 From: beorn7 Date: Fri, 14 Jun 2019 19:43:08 +0200 Subject: [PATCH 8/8] Cut release v1.0.0 Signed-off-by: beorn7 --- CHANGELOG.md | 13 +++++++++++++ README.md | 48 +++++++++++++++--------------------------------- VERSION | 2 +- 3 files changed, 29 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e1588f..8ea4c5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,16 @@ +## 1.0.0 / 2019-06-15 + +_This release removes all previously deprecated features, resulting in the breaking changes listed below. As this is v1.0.0, semantic versioning applies from now on, with the exception of the API client and parts marked explicitly as experimental._ + +* [CHANGE] Remove objectives from the default `Summary`. (Objectives have to be set explicitly in the `SummaryOpts`.) #600 +* [CHANGE] Remove all HTTP related feature in the `prometheus` package. (Use the `promhttp` package instead.) #600 +* [CHANGE] Remove `push.FromGatherer`, `push.AddFromGatherer`, `push.Collectors`. (Use `push.New` instead.) #600 +* [CHANGE] API client: Pass warnings through on non-error responses. #599 +* [CHANGE] API client: Add warnings to `Series` call. #603 +* [FEATURE] Make process collector work on Microsoft Windows. **EXPERIMENTAL** #596 +* [FEATURE] API client: Add `/labels` call. #604 +* [BUGFIX] Make `AlreadyRegisteredError` usable for wrapped registries. #607 + ## 0.9.4 / 2019-06-07 * [CHANGE] API client: Switch to alert values as strings. #585 * [FEATURE] Add a collector for Go module build information. #595 diff --git a/README.md b/README.md index 2336eb1..f02060a 100644 --- a/README.md +++ b/README.md @@ -11,41 +11,23 @@ Prometheus HTTP API. __This library requires Go1.9 or later.__ -## Important note about releases, versioning, tagging, and stability +## Important note about releases and stability -In this repository, we used to mostly ignore the many coming and going -dependency management tools for Go and instead wait for a tool that most of the -community would converge on. Our bet is that this tool has arrived now in the -form of [Go -Modules](https://github.com/golang/go/wiki/Modules#how-to-upgrade-and-downgrade-dependencies). +This repository generally follows [Semantic +Versioning](https://semver.org/). However, the API client in +prometheus/client_golang/api/… is still considered experimental. Breaking +changes of the API client will _not_ trigger a new major release. The same is +true for selected other new features explicitly marked as **EXPERIMENTAL** in +CHANGELOG.md. -To make full use of what Go Modules are offering, the previous versioning -roadmap for this repository had to be changed. In particular, Go Modules -finally provide a way for incompatible versions of the same package to coexist -in the same binary. For that, however, the versions must be tagged with -different major versions of 1 or greater (following [Semantic -Versioning](https://semver.org/)). Thus, we decided to abandon the original -plan of introducing a lot of breaking changes _before_ releasing v1 of this -repository, mostly driven by the widespread use this repository already has and -the relatively stable state it is in. - -To leverage the mechanism Go Modules offers for a transition between major -version, the current plan is the following: - -- The v0.9.x series of releases will see a small number of bugfix releases to - deal with a few remaining minor issues (#543, #542, #539). -- After that, all features currently marked as _deprecated_ will be removed, - and the result will be released as v1.0.0. -- The planned breaking changes previously gathered as part of the v0.10 - milestone will now go into the v2 milestone. The v2 development happens in a - [separate branch](https://github.com/prometheus/client_golang/tree/dev-v2) - for the time being. v2 releases off that branch will happen once sufficient - stability is reached. v1 and v2 will coexist for a while to enable a - convenient transition. -- The API client in prometheus/client_golang/api/… is still considered - experimental. While it will be tagged alongside the rest of the code - according to the plan above, we cannot strictly guarantee semver semantics - for it. +Features that require breaking changes in the stable parts of the repository +are being batched up and tracked in the [v2 +milestone](https://github.com/prometheus/client_golang/milestone/2). The v2 +development happens in a [separate +branch](https://github.com/prometheus/client_golang/tree/dev-v2) for the time +being. v2 releases off that branch will happen once sufficient stability is +reached. In view of the widespread use of this repository, v1 and v2 will +coexist for a while to enable a convenient transition. ## Instrumenting applications diff --git a/VERSION b/VERSION index a602fc9..3eefcb9 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.9.4 +1.0.0