From c78436d707276aba0cda318c78a88613c67b82e1 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Fri, 20 Mar 2015 23:10:26 +0100 Subject: [PATCH 1/6] Remove unused API time dependency injection. --- web/api/api.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/web/api/api.go b/web/api/api.go index 90bb06627b..fe50752d89 100644 --- a/web/api/api.go +++ b/web/api/api.go @@ -21,13 +21,11 @@ import ( "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/retrieval" "github.com/prometheus/prometheus/storage/local" - "github.com/prometheus/prometheus/utility" "github.com/prometheus/prometheus/web/httputils" ) // MetricsService manages the /api HTTP endpoint. type MetricsService struct { - time utility.Time Config *config.Config TargetManager retrieval.TargetManager Storage local.Storage From 8a4acefd66275ecc51878f7dc3f054dae1c21f86 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Fri, 20 Mar 2015 23:10:58 +0100 Subject: [PATCH 2/6] Allow custom timestamps in instant query API. --- web/api/query.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/web/api/query.go b/web/api/query.go index 7f5bfb16ab..5eb4523062 100644 --- a/web/api/query.go +++ b/web/api/query.go @@ -47,6 +47,14 @@ func (serv MetricsService) Query(w http.ResponseWriter, r *http.Request) { params := httputils.GetQueryParams(r) expr := params.Get("expr") asText := params.Get("asText") + tsFloat, _ := strconv.ParseFloat(params.Get("timestamp"), 64) + + var timestamp clientmodel.Timestamp + if tsFloat == 0 { + timestamp = clientmodel.Now() + } else { + timestamp = clientmodel.TimestampFromUnixNano(int64(tsFloat) * int64(time.Second/time.Nanosecond)) + } var format ast.OutputFormat // BUG(julius): Use Content-Type negotiation. @@ -64,7 +72,6 @@ func (serv MetricsService) Query(w http.ResponseWriter, r *http.Request) { return } - timestamp := clientmodel.TimestampFromTime(serv.time.Now()) queryStats := stats.NewTimerGroup() result := ast.EvalToString(exprNode, timestamp, format, serv.Storage, queryStats) glog.V(1).Infof("Instant query: %s\nQuery stats:\n%s\n", expr, queryStats) From cb816ea14aa43361027024b9b7c5865beec2398d Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Sat, 21 Mar 2015 16:58:45 +0100 Subject: [PATCH 3/6] Improve timestamp/duration parsing in query API. Don't handle `0` as a special timestamp value for "now" anymore, except in the `QueryRange()` case, where existing API consumers still expect `0` to mean "now". Also, properly return errors now for malformed timestamp/duration float values. --- web/api/query.go | 82 ++++++++++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 30 deletions(-) diff --git a/web/api/query.go b/web/api/query.go index 5eb4523062..c3264148c5 100644 --- a/web/api/query.go +++ b/web/api/query.go @@ -40,6 +40,26 @@ func setAccessControlHeaders(w http.ResponseWriter) { w.Header().Set("Access-Control-Expose-Headers", "Date") } +func parseTimestampOrNow(t string) (clientmodel.Timestamp, error) { + if t == "" { + return clientmodel.Now(), nil + } else { + tFloat, err := strconv.ParseFloat(t, 64) + if err != nil { + return 0, err + } + return clientmodel.TimestampFromUnixNano(int64(tFloat) * int64(time.Second/time.Nanosecond)), nil + } +} + +func parseDuration(d string) (time.Duration, error) { + dFloat, err := strconv.ParseFloat(d, 64) + if err != nil { + return 0, err + } + return time.Duration(dFloat) * (time.Second / time.Nanosecond), nil +} + // Query handles the /api/query endpoint. func (serv MetricsService) Query(w http.ResponseWriter, r *http.Request) { setAccessControlHeaders(w) @@ -47,13 +67,11 @@ func (serv MetricsService) Query(w http.ResponseWriter, r *http.Request) { params := httputils.GetQueryParams(r) expr := params.Get("expr") asText := params.Get("asText") - tsFloat, _ := strconv.ParseFloat(params.Get("timestamp"), 64) - var timestamp clientmodel.Timestamp - if tsFloat == 0 { - timestamp = clientmodel.Now() - } else { - timestamp = clientmodel.TimestampFromUnixNano(int64(tsFloat) * int64(time.Second/time.Nanosecond)) + timestamp, err := parseTimestampOrNow(params.Get("timestamp")) + if err != nil { + http.Error(w, fmt.Sprintf("invalid query timestamp %s", err), http.StatusBadRequest) + return } var format ast.OutputFormat @@ -86,14 +104,30 @@ func (serv MetricsService) QueryRange(w http.ResponseWriter, r *http.Request) { params := httputils.GetQueryParams(r) expr := params.Get("expr") - // Input times and durations are in seconds and get converted to nanoseconds. - endFloat, _ := strconv.ParseFloat(params.Get("end"), 64) - durationFloat, _ := strconv.ParseFloat(params.Get("range"), 64) - stepFloat, _ := strconv.ParseFloat(params.Get("step"), 64) - nanosPerSecond := int64(time.Second / time.Nanosecond) - end := int64(endFloat) * nanosPerSecond - duration := int64(durationFloat) * nanosPerSecond - step := int64(stepFloat) * nanosPerSecond + duration, err := parseDuration(params.Get("range")) + if err != nil { + http.Error(w, fmt.Sprintf("invalid query range: %s", err), http.StatusBadRequest) + return + } + + step, err := parseDuration(params.Get("step")) + if err != nil { + http.Error(w, fmt.Sprintf("invalid query resolution: %s", err), http.StatusBadRequest) + return + } + + end, err := parseTimestampOrNow(params.Get("end")) + if err != nil { + http.Error(w, fmt.Sprintf("invalid query timestamp: %s", err), http.StatusBadRequest) + return + } + // TODO(julius): Remove this special-case handling a while after PromDash and + // other API consumers have been changed to no longer set "end=0" for setting + // the current time as the end time. Instead, the "end" parameter should + // simply be omitted or set to an empty string for that case. + if end == 0 { + end = clientmodel.Now() + } exprNode, err := rules.LoadExprFromString(expr) if err != nil { @@ -105,18 +139,6 @@ func (serv MetricsService) QueryRange(w http.ResponseWriter, r *http.Request) { return } - if end == 0 { - end = clientmodel.Now().UnixNano() - } - - if step <= 0 { - step = nanosPerSecond - } - - if end-duration < 0 { - duration = end - } - // For safety, limit the number of returned points per timeseries. // This is sufficient for 60s resolution for a week or 1h resolution for a year. if duration/step > 11000 { @@ -125,15 +147,15 @@ func (serv MetricsService) QueryRange(w http.ResponseWriter, r *http.Request) { } // Align the start to step "tick" boundary. - end -= end % step + end = end.Add(-time.Duration(end.UnixNano() % int64(step))) queryStats := stats.NewTimerGroup() matrix, err := ast.EvalVectorRange( exprNode.(ast.VectorNode), - clientmodel.TimestampFromUnixNano(end-duration), - clientmodel.TimestampFromUnixNano(end), - time.Duration(step), + end.Add(-duration), + end, + step, serv.Storage, queryStats) if err != nil { From a68b880c2728795d8f818b0c3aa941cb471b1911 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Sat, 21 Mar 2015 17:50:45 +0100 Subject: [PATCH 4/6] Add tests for new timestamp/duration functions. ...and fix the first bugs in them where they truncate precision below a second. --- web/api/query.go | 4 +-- web/api/query_test.go | 67 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 web/api/query_test.go diff --git a/web/api/query.go b/web/api/query.go index c3264148c5..3fd54773fd 100644 --- a/web/api/query.go +++ b/web/api/query.go @@ -48,7 +48,7 @@ func parseTimestampOrNow(t string) (clientmodel.Timestamp, error) { if err != nil { return 0, err } - return clientmodel.TimestampFromUnixNano(int64(tFloat) * int64(time.Second/time.Nanosecond)), nil + return clientmodel.TimestampFromUnixNano(int64(tFloat * float64(time.Second/time.Nanosecond))), nil } } @@ -57,7 +57,7 @@ func parseDuration(d string) (time.Duration, error) { if err != nil { return 0, err } - return time.Duration(dFloat) * (time.Second / time.Nanosecond), nil + return time.Duration(dFloat * float64(time.Second/time.Nanosecond)), nil } // Query handles the /api/query endpoint. diff --git a/web/api/query_test.go b/web/api/query_test.go new file mode 100644 index 0000000000..b311180df6 --- /dev/null +++ b/web/api/query_test.go @@ -0,0 +1,67 @@ +// Copyright 2015 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 api + +import ( + "testing" + "time" + + clientmodel "github.com/prometheus/client_golang/model" +) + +func TestParseTimestampOrNow(t *testing.T) { + ts, err := parseTimestampOrNow("") + if err != nil { + t.Fatalf("err = %s; want nil", err) + } + now := clientmodel.Now() + if now.Sub(ts) > time.Second || now.Sub(ts) < 0 { + t.Fatalf("ts = %v; want %v <= ts <= %v", ts, now.Sub(ts), now) + } + + ts, err = parseTimestampOrNow("1426956073.12345") + if err != nil { + t.Fatalf("err = %s; want nil", err) + } + expTS := clientmodel.TimestampFromUnixNano(1426956073123000000) + if !ts.Equal(expTS) { + t.Fatalf("ts = %v; want %v", ts, expTS) + } + + _, err = parseTimestampOrNow("123.45foo") + if err == nil { + t.Fatalf("err = nil; want %s", err) + } +} + +func TestParseDuration(t *testing.T) { + _, err := parseDuration("") + if err == nil { + t.Fatalf("err = nil; want %s", err) + } + + _, err = parseDuration("1234.56foo") + if err == nil { + t.Fatalf("err = nil; want %s", err) + } + + d, err := parseDuration("1234.56789") + if err != nil { + t.Fatalf("err = %s; want nil", err) + } + expD := time.Duration(1234.56789 * float64(time.Second)) + if d != expD { + t.Fatalf("d = %v; want %v", d, expD) + } +} From df314ead849d7a66a9ef5c79918c459e7811fc6c Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Sat, 21 Mar 2015 17:54:30 +0100 Subject: [PATCH 5/6] Remove unnecessary "else" branch in query API. --- web/api/query.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/web/api/query.go b/web/api/query.go index 3fd54773fd..4e764088e6 100644 --- a/web/api/query.go +++ b/web/api/query.go @@ -43,13 +43,13 @@ func setAccessControlHeaders(w http.ResponseWriter) { func parseTimestampOrNow(t string) (clientmodel.Timestamp, error) { if t == "" { return clientmodel.Now(), nil - } else { - tFloat, err := strconv.ParseFloat(t, 64) - if err != nil { - return 0, err - } - return clientmodel.TimestampFromUnixNano(int64(tFloat * float64(time.Second/time.Nanosecond))), nil } + + tFloat, err := strconv.ParseFloat(t, 64) + if err != nil { + return 0, err + } + return clientmodel.TimestampFromUnixNano(int64(tFloat * float64(time.Second/time.Nanosecond))), nil } func parseDuration(d string) (time.Duration, error) { From c9b76def4ca5f5e7450b22e6c00a750b2e3c920c Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Fri, 27 Mar 2015 16:48:03 +0100 Subject: [PATCH 6/6] Report all query API HTTP errors in JSON format. --- web/api/query.go | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/web/api/query.go b/web/api/query.go index 4e764088e6..ae539dee85 100644 --- a/web/api/query.go +++ b/web/api/query.go @@ -40,6 +40,12 @@ func setAccessControlHeaders(w http.ResponseWriter) { w.Header().Set("Access-Control-Expose-Headers", "Date") } +func httpJSONError(w http.ResponseWriter, err error, code int) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(code) + fmt.Fprintln(w, ast.ErrorToJSON(err)) +} + func parseTimestampOrNow(t string) (clientmodel.Timestamp, error) { if t == "" { return clientmodel.Now(), nil @@ -63,27 +69,17 @@ func parseDuration(d string) (time.Duration, error) { // Query handles the /api/query endpoint. func (serv MetricsService) Query(w http.ResponseWriter, r *http.Request) { setAccessControlHeaders(w) + w.Header().Set("Content-Type", "application/json") params := httputils.GetQueryParams(r) expr := params.Get("expr") - asText := params.Get("asText") timestamp, err := parseTimestampOrNow(params.Get("timestamp")) if err != nil { - http.Error(w, fmt.Sprintf("invalid query timestamp %s", err), http.StatusBadRequest) + httpJSONError(w, fmt.Errorf("invalid query timestamp %s", err), http.StatusBadRequest) return } - var format ast.OutputFormat - // BUG(julius): Use Content-Type negotiation. - if asText == "" { - format = ast.JSON - w.Header().Set("Content-Type", "application/json") - } else { - format = ast.Text - w.Header().Set("Content-Type", "text/plain") - } - exprNode, err := rules.LoadExprFromString(expr) if err != nil { fmt.Fprint(w, ast.ErrorToJSON(err)) @@ -91,7 +87,7 @@ func (serv MetricsService) Query(w http.ResponseWriter, r *http.Request) { } queryStats := stats.NewTimerGroup() - result := ast.EvalToString(exprNode, timestamp, format, serv.Storage, queryStats) + result := ast.EvalToString(exprNode, timestamp, ast.JSON, serv.Storage, queryStats) glog.V(1).Infof("Instant query: %s\nQuery stats:\n%s\n", expr, queryStats) fmt.Fprint(w, result) } @@ -106,19 +102,19 @@ func (serv MetricsService) QueryRange(w http.ResponseWriter, r *http.Request) { duration, err := parseDuration(params.Get("range")) if err != nil { - http.Error(w, fmt.Sprintf("invalid query range: %s", err), http.StatusBadRequest) + httpJSONError(w, fmt.Errorf("invalid query range: %s", err), http.StatusBadRequest) return } step, err := parseDuration(params.Get("step")) if err != nil { - http.Error(w, fmt.Sprintf("invalid query resolution: %s", err), http.StatusBadRequest) + httpJSONError(w, fmt.Errorf("invalid query resolution: %s", err), http.StatusBadRequest) return } end, err := parseTimestampOrNow(params.Get("end")) if err != nil { - http.Error(w, fmt.Sprintf("invalid query timestamp: %s", err), http.StatusBadRequest) + httpJSONError(w, fmt.Errorf("invalid query timestamp: %s", err), http.StatusBadRequest) return } // TODO(julius): Remove this special-case handling a while after PromDash and @@ -178,15 +174,15 @@ func (serv MetricsService) QueryRange(w http.ResponseWriter, r *http.Request) { // Metrics handles the /api/metrics endpoint. func (serv MetricsService) Metrics(w http.ResponseWriter, r *http.Request) { setAccessControlHeaders(w) + w.Header().Set("Content-Type", "application/json") metricNames := serv.Storage.GetLabelValuesForLabelName(clientmodel.MetricNameLabel) sort.Sort(metricNames) resultBytes, err := json.Marshal(metricNames) if err != nil { glog.Error("Error marshalling metric names: ", err) - http.Error(w, err.Error(), http.StatusInternalServerError) + httpJSONError(w, fmt.Errorf("Error marshalling metric names: %s", err), http.StatusInternalServerError) return } - w.Header().Set("Content-Type", "application/json") w.Write(resultBytes) }