From eba01d11192d8336b1552402afd7e79d7fddd467 Mon Sep 17 00:00:00 2001 From: Conor Hennessy Date: Tue, 22 Oct 2013 20:31:52 +0200 Subject: [PATCH] Remove usage of gorest. Due to on going issues, we've decided to remove gorest. It started with gorest not being thread-safe (it does introspection to create a new handler which is an easy process to mess up with multiple threads of execution): https://code.google.com/p/gorest/issues/detail?id=15 While the issue has been marked fixed, it looks like the patch has introduced more problems than the original issue and simply doesn't work properly. I'm not sure the behaviour was thought through properly. If a new instance is needed every request then a handler-factory is needed or the library needs to set expectations about how the new objects should interact with their constructor state. While it was tempting to try out another routing library, I think for now it's better to use dumb vanilla Go routing. At least until we decide which URL format we intend to standardize on. Change-Id: Ica3da135d05f8ab8fc206f51eeca4f684f8efa0e --- web/api/api.go | 28 ++++++---- web/api/query.go | 84 +++++++++++++++++------------ web/api/targets.go | 18 +++++-- web/{ => http_utils}/compression.go | 10 ++-- web/http_utils/http_utils.go | 24 +++++++++ web/web.go | 5 +- 6 files changed, 112 insertions(+), 57 deletions(-) rename web/{ => http_utils}/compression.go (93%) create mode 100644 web/http_utils/http_utils.go diff --git a/web/api/api.go b/web/api/api.go index da69775e75..c640924b7b 100644 --- a/web/api/api.go +++ b/web/api/api.go @@ -14,24 +14,32 @@ package api import ( - "code.google.com/p/gorest" + "net/http" + + "github.com/prometheus/client_golang/prometheus/exp" + "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/retrieval" "github.com/prometheus/prometheus/storage/metric" "github.com/prometheus/prometheus/utility" + "github.com/prometheus/prometheus/web/http_utils" ) type MetricsService struct { - gorest.RestService `root:"/api/" consumes:"application/json" produces:"application/json"` - - query gorest.EndPoint `method:"GET" path:"/query?{expr:string}&{as_text:string}" output:"string"` - queryRange gorest.EndPoint `method:"GET" path:"/query_range?{expr:string}&{end:int64}&{range:int64}&{step:int64}" output:"string"` - metrics gorest.EndPoint `method:"GET" path:"/metrics" output:"string"` - - setTargets gorest.EndPoint `method:"PUT" path:"/jobs/{jobName:string}/targets" postdata:"[]TargetGroup"` - time utility.Time - + time utility.Time Config *config.Config TargetManager retrieval.TargetManager Storage *metric.TieredStorage } + +func (msrv *MetricsService) RegisterHandler() { + handler := func(h func(http.ResponseWriter, *http.Request)) http.Handler { + return http_utils.CompressionHandler{ + Handler: http.HandlerFunc(h), + } + } + exp.Handle("/api/query", handler(msrv.Query)) + exp.Handle("/api/query_range", handler(msrv.QueryRange)) + exp.Handle("/api/metrics", handler(msrv.Metrics)) + exp.Handle("/api/targets", handler(msrv.SetTargets)) +} diff --git a/web/api/query.go b/web/api/query.go index b4fdee5c59..aa0e1233c2 100644 --- a/web/api/query.go +++ b/web/api/query.go @@ -16,11 +16,12 @@ package api import ( "encoding/json" "errors" + "fmt" "net/http" "sort" + "strconv" "time" - "code.google.com/p/gorest" "github.com/golang/glog" clientmodel "github.com/prometheus/client_golang/model" @@ -28,54 +29,67 @@ import ( "github.com/prometheus/prometheus/rules" "github.com/prometheus/prometheus/rules/ast" "github.com/prometheus/prometheus/stats" + "github.com/prometheus/prometheus/web/http_utils" ) -func (serv MetricsService) setAccessControlHeaders(rb *gorest.ResponseBuilder) { - rb.AddHeader("Access-Control-Allow-Headers", "Accept, Authorization, Content-Type, Origin") - rb.AddHeader("Access-Control-Allow-Methods", "GET") - rb.AddHeader("Access-Control-Allow-Origin", "*") - rb.AddHeader("Access-Control-Expose-Headers", "Date") +// Enables cross-site script calls. +func setAccessControlHeaders(w http.ResponseWriter) { + w.Header().Set("Access-Control-Allow-Headers", "Accept, Authorization, Content-Type, Origin") + w.Header().Set("Access-Control-Allow-Methods", "GET") + w.Header().Set("Access-Control-Allow-Origin", "*") + w.Header().Set("Access-Control-Expose-Headers", "Date") } -func (serv MetricsService) Query(expr string, asText string) string { - rb := serv.ResponseBuilder() - serv.setAccessControlHeaders(rb) +func (serv MetricsService) Query(w http.ResponseWriter, r *http.Request) { + setAccessControlHeaders(w) - exprNode, err := rules.LoadExprFromString(expr) - if err != nil { - return ast.ErrorToJSON(err) - } - - timestamp := serv.time.Now() + params := http_utils.GetQueryParams(r) + expr := params.Get("expr") + asText := params.Get("asText") var format ast.OutputFormat // BUG(julius): Use Content-Type negotiation. if asText == "" { format = ast.JSON - rb.SetContentType(gorest.Application_Json) + w.Header().Set("Content-Type", "application/json") } else { format = ast.TEXT - rb.SetContentType(gorest.Text_Plain) + w.Header().Set("Content-Type", "text/plain") } + exprNode, err := rules.LoadExprFromString(expr) + if err != nil { + fmt.Fprint(w, ast.ErrorToJSON(err)) + return + } + + timestamp := serv.time.Now() + queryStats := stats.NewTimerGroup() result := ast.EvalToString(exprNode, timestamp, format, serv.Storage, queryStats) glog.Infof("Instant query: %s\nQuery stats:\n%s\n", expr, queryStats) - return result + fmt.Fprint(w, result) } -func (serv MetricsService) QueryRange(expr string, end int64, duration int64, step int64) string { - rb := serv.ResponseBuilder() - serv.setAccessControlHeaders(rb) +func (serv MetricsService) QueryRange(w http.ResponseWriter, r *http.Request) { + setAccessControlHeaders(w) + w.Header().Set("Content-Type", "application/json") + + params := http_utils.GetQueryParams(r) + expr := params.Get("expr") + end, _ := strconv.ParseInt(params.Get("end"), 0, 64) + duration, _ := strconv.ParseInt(params.Get("range"), 0, 64) + step, _ := strconv.ParseInt(params.Get("step"), 0, 64) exprNode, err := rules.LoadExprFromString(expr) if err != nil { - return ast.ErrorToJSON(err) + fmt.Fprint(w, ast.ErrorToJSON(err)) + return } if exprNode.Type() != ast.VECTOR { - return ast.ErrorToJSON(errors.New("Expression does not evaluate to vector type")) + fmt.Fprint(w, ast.ErrorToJSON(errors.New("Expression does not evaluate to vector type"))) + return } - rb.SetContentType(gorest.Application_Json) if end == 0 { end = serv.time.Now().Unix() @@ -103,7 +117,8 @@ func (serv MetricsService) QueryRange(expr string, end int64, duration int64, st serv.Storage, queryStats) if err != nil { - return ast.ErrorToJSON(err) + fmt.Fprint(w, ast.ErrorToJSON(err)) + return } evalTimer.Stop() @@ -116,26 +131,25 @@ func (serv MetricsService) QueryRange(expr string, end int64, duration int64, st jsonTimer.Stop() glog.Infof("Range query: %s\nQuery stats:\n%s\n", expr, queryStats) - return result + fmt.Fprint(w, result) } -func (serv MetricsService) Metrics() string { - rb := serv.ResponseBuilder() - serv.setAccessControlHeaders(rb) +func (serv MetricsService) Metrics(w http.ResponseWriter, r *http.Request) { + setAccessControlHeaders(w) metricNames, err := serv.Storage.GetAllValuesForLabel(clientmodel.MetricNameLabel) - rb.SetContentType(gorest.Application_Json) + w.Header().Set("Content-Type", "application/json") if err != nil { glog.Error("Error loading metric names: ", err) - rb.SetResponseCode(http.StatusInternalServerError) - return err.Error() + http.Error(w, err.Error(), http.StatusInternalServerError) + return } sort.Sort(metricNames) resultBytes, err := json.Marshal(metricNames) if err != nil { glog.Error("Error marshalling metric names: ", err) - rb.SetResponseCode(http.StatusInternalServerError) - return err.Error() + http.Error(w, err.Error(), http.StatusInternalServerError) + return } - return string(resultBytes) + w.Write(resultBytes) } diff --git a/web/api/targets.go b/web/api/targets.go index f846fce1c3..d500516ba9 100644 --- a/web/api/targets.go +++ b/web/api/targets.go @@ -14,11 +14,13 @@ package api import ( + "encoding/json" "net/http" clientmodel "github.com/prometheus/client_golang/model" "github.com/prometheus/prometheus/retrieval" + "github.com/prometheus/prometheus/web/http_utils" ) type TargetGroup struct { @@ -26,11 +28,21 @@ type TargetGroup struct { BaseLabels map[string]string `json:"baseLabels"` } -func (serv MetricsService) SetTargets(targetGroups []TargetGroup, jobName string) { +func (serv MetricsService) SetTargets(w http.ResponseWriter, r *http.Request) { + params := http_utils.GetQueryParams(r) + jobName := params.Get("job") + + decoder := json.NewDecoder(r.Body) + var targetGroups []TargetGroup + err := decoder.Decode(&targetGroups) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + job := serv.Config.GetJobByName(jobName) if job == nil { - rb := serv.ResponseBuilder() - rb.SetResponseCode(http.StatusNotFound) + http.Error(w, "job not found", http.StatusNotFound) return } diff --git a/web/compression.go b/web/http_utils/compression.go similarity index 93% rename from web/compression.go rename to web/http_utils/compression.go index 92f226a9ab..3b00ca61d3 100644 --- a/web/compression.go +++ b/web/http_utils/compression.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package web +package http_utils import ( "compress/gzip" @@ -80,13 +80,13 @@ func newCompressedResponseWriter(writer http.ResponseWriter, req *http.Request) // Wrapper around http.Handler which adds suitable response compression based // on the client's Accept-Encoding headers. -type compressionHandler struct { - handler http.Handler +type CompressionHandler struct { + Handler http.Handler } // Adds compression to the original http.Handler's ServeHTTP() method. -func (c compressionHandler) ServeHTTP(writer http.ResponseWriter, req *http.Request) { +func (c CompressionHandler) ServeHTTP(writer http.ResponseWriter, req *http.Request) { compWriter := newCompressedResponseWriter(writer, req) - c.handler.ServeHTTP(compWriter, req) + c.Handler.ServeHTTP(compWriter, req) compWriter.Close() } diff --git a/web/http_utils/http_utils.go b/web/http_utils/http_utils.go new file mode 100644 index 0000000000..31476ce0e1 --- /dev/null +++ b/web/http_utils/http_utils.go @@ -0,0 +1,24 @@ +// Copyright 2013 Prometheus Team +// 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 http_utils + +import ( + "net/http" + "net/url" +) + +func GetQueryParams(r *http.Request) url.Values { + r.ParseForm() + return r.Form +} diff --git a/web/web.go b/web/web.go index 53ce2f2886..914c6d8c4b 100644 --- a/web/web.go +++ b/web/web.go @@ -25,7 +25,6 @@ import ( pprof_runtime "runtime/pprof" - "code.google.com/p/gorest" "github.com/golang/glog" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/exp" @@ -49,8 +48,6 @@ type WebService struct { } func (w WebService) ServeForever() error { - gorest.RegisterService(w.MetricsHandler) - exp.Handle("/favicon.ico", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "", 404) })) @@ -68,7 +65,7 @@ func (w WebService) ServeForever() error { exp.HandleFunc("/graph", graphHandler) exp.HandleFunc("/heap", dumpHeap) - exp.Handle("/api/", compressionHandler{handler: gorest.Handle()}) + w.MetricsHandler.RegisterHandler() exp.Handle("/metrics", prometheus.DefaultHandler) if *useLocalAssets { exp.Handle("/static/", http.StripPrefix("/static/", http.FileServer(http.Dir("web/static"))))