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 da69775e7..c640924b7 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 b4fdee5c5..aa0e1233c 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 f846fce1c..d500516ba 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 92f226a9a..3b00ca61d 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 000000000..31476ce0e --- /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 53ce2f288..914c6d8c4 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"))))