From c3d31febd6074e6d5b173b0fc823e5e5f247464d Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Wed, 13 Feb 2013 17:41:35 +0100 Subject: [PATCH 1/4] Move durationToString to common place and cleanup error handling. --- config/config.go | 25 ++++++++------- config/helpers.go | 30 ++---------------- config/printer.go | 8 ++--- rules/ast/printer.go | 26 ++-------------- rules/helpers.go | 53 ++++++-------------------------- rules/lexer.l | 4 ++- rules/lexer.l.go | 4 +-- utility/strconv.go | 72 ++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 108 insertions(+), 114 deletions(-) create mode 100644 utility/strconv.go diff --git a/config/config.go b/config/config.go index 290fa2d9cf..01dfee3b26 100644 --- a/config/config.go +++ b/config/config.go @@ -17,6 +17,7 @@ import ( "errors" "fmt" "github.com/prometheus/prometheus/model" + "github.com/prometheus/prometheus/utility" "time" ) @@ -55,7 +56,7 @@ func (config *Config) AddJob(options map[string]string, targets []Targets) error return errors.New("Missing job name") } if len(targets) == 0 { - return errors.New(fmt.Sprintf("No targets configured for job '%v'", name)) + return fmt.Errorf("No targets configured for job '%v'", name) } job := &JobConfig{ Targets: tmpJobTargets, @@ -69,18 +70,18 @@ func (config *Config) AddJob(options map[string]string, targets []Targets) error return nil } -func (config *GlobalConfig) SetOption(option string, value string) error { +func (config *GlobalConfig) SetOption(option string, value string) (err error) { switch option { case "scrape_interval": - config.ScrapeInterval = stringToDuration(value) + config.ScrapeInterval, err = utility.StringToDuration(value) return nil case "evaluation_interval": - config.EvaluationInterval = stringToDuration(value) - return nil + config.EvaluationInterval, err = utility.StringToDuration(value) + return err default: - return errors.New(fmt.Sprintf("Unrecognized global configuration option '%v'", option)) + err = fmt.Errorf("Unrecognized global configuration option '%v'", option) } - return nil + return } func (config *GlobalConfig) SetLabels(labels model.LabelSet) { @@ -95,18 +96,16 @@ func (config *GlobalConfig) AddRuleFiles(ruleFiles []string) { } } -func (job *JobConfig) SetOption(option string, value string) error { +func (job *JobConfig) SetOption(option string, value string) (err error) { switch option { case "name": job.Name = value - return nil case "scrape_interval": - job.ScrapeInterval = stringToDuration(value) - return nil + job.ScrapeInterval, err = utility.StringToDuration(value) default: - return errors.New(fmt.Sprintf("Unrecognized job configuration option '%v'", option)) + err = fmt.Errorf("Unrecognized job configuration option '%v'", option) } - return nil + return } func (job *JobConfig) AddTargets(endpoints []string, labels model.LabelSet) { diff --git a/config/helpers.go b/config/helpers.go index af72519698..8ef989345f 100644 --- a/config/helpers.go +++ b/config/helpers.go @@ -17,9 +17,6 @@ import ( "fmt" "github.com/prometheus/prometheus/model" "log" - "regexp" - "strconv" - "time" ) // Unfortunately, more global variables that are needed for parsing. @@ -30,7 +27,9 @@ var tmpTargetLabels = model.LabelSet{} func configError(error string, v ...interface{}) { message := fmt.Sprintf(error, v...) - log.Fatal(fmt.Sprintf("Line %v, char %v: %s", yyline, yypos, message)) + // TODO: Don't just die here. Pass errors back all the way to the caller + // instead. + log.Fatalf("Line %v, char %v: %s", yyline, yypos, message) } func PushJobOption(option string, value string) { @@ -66,26 +65,3 @@ func PopJob() { tmpJobOptions = map[string]string{} tmpJobTargets = []Targets{} } - -func stringToDuration(durationStr string) time.Duration { - durationRE := regexp.MustCompile("^([0-9]+)([ywdhms]+)$") - matches := durationRE.FindStringSubmatch(durationStr) - if len(matches) != 3 { - configError("Not a valid duration string: '%v'", durationStr) - } - value, _ := strconv.Atoi(matches[1]) - unit := matches[2] - switch unit { - case "y": - value *= 60 * 60 * 24 * 365 - case "w": - value *= 60 * 60 * 24 * 7 - case "d": - value *= 60 * 60 * 24 - case "h": - value *= 60 * 60 - case "m": - value *= 60 - } - return time.Duration(value) * time.Second -} diff --git a/config/printer.go b/config/printer.go index 26d7c6bc12..00e4c7dc1f 100644 --- a/config/printer.go +++ b/config/printer.go @@ -16,6 +16,7 @@ package config import ( "fmt" "github.com/prometheus/prometheus/model" + "github.com/prometheus/prometheus/utility" "strings" ) @@ -47,8 +48,8 @@ func labelsToString(indent int, labels model.LabelSet) string { func (global *GlobalConfig) ToString(indent int) string { str := indentStr(indent, "global {\n") - str += indentStr(indent+1, "scrape_interval = \"%vs\"\n", global.ScrapeInterval) - str += indentStr(indent+1, "evaluation_interval = \"%vs\"\n", global.EvaluationInterval) + str += indentStr(indent+1, "scrape_interval = \"%s\"\n", utility.DurationToString(global.ScrapeInterval)) + str += indentStr(indent+1, "evaluation_interval = \"%s\"\n", utility.DurationToString(global.EvaluationInterval)) str += labelsToString(indent+1, global.Labels) str += indentStr(indent, "}\n") str += indentStr(indent+1, "rule_files = [\n") @@ -61,9 +62,8 @@ func (global *GlobalConfig) ToString(indent int) string { func (job *JobConfig) ToString(indent int) string { str := indentStr(indent, "job {\n") - str += indentStr(indent+1, "job {\n") str += indentStr(indent+1, "name = \"%v\"\n", job.Name) - str += indentStr(indent+1, "scrape_interval = \"%vs\"\n", job.ScrapeInterval) + str += indentStr(indent+1, "scrape_interval = \"%s\"\n", utility.DurationToString(job.ScrapeInterval)) for _, targets := range job.Targets { str += indentStr(indent+1, "targets {\n") str += indentStr(indent+2, "endpoints = [\n") diff --git a/rules/ast/printer.go b/rules/ast/printer.go index 1f38117ac1..4fd16296d8 100644 --- a/rules/ast/printer.go +++ b/rules/ast/printer.go @@ -16,6 +16,7 @@ package ast import ( "encoding/json" "fmt" + "github.com/prometheus/prometheus/utility" "sort" "strings" "time" @@ -65,29 +66,6 @@ func exprTypeToString(exprType ExprType) string { return exprTypeMap[exprType] } -func durationToString(duration time.Duration) string { - seconds := int64(duration / time.Second) - factors := map[string]int64{ - "y": 60 * 60 * 24 * 365, - "d": 60 * 60 * 24, - "h": 60 * 60, - "m": 60, - "s": 1, - } - unit := "s" - switch int64(0) { - case seconds % factors["y"]: - unit = "y" - case seconds % factors["d"]: - unit = "d" - case seconds % factors["h"]: - unit = "h" - case seconds % factors["m"]: - unit = "m" - } - return fmt.Sprintf("%v%v", seconds/factors[unit], unit) -} - func (vector Vector) ToString() string { metricStrings := []string{} for _, sample := range vector { @@ -228,7 +206,7 @@ func (node *VectorLiteral) ToString() string { func (node *MatrixLiteral) ToString() string { vectorString := (&VectorLiteral{labels: node.labels}).ToString() - intervalString := fmt.Sprintf("['%v']", durationToString(node.interval)) + intervalString := fmt.Sprintf("['%v']", utility.DurationToString(node.interval)) return vectorString + intervalString } diff --git a/rules/helpers.go b/rules/helpers.go index 3008768d1b..a5a224c443 100644 --- a/rules/helpers.go +++ b/rules/helpers.go @@ -14,48 +14,15 @@ package rules import ( - "errors" "fmt" "github.com/prometheus/prometheus/model" "github.com/prometheus/prometheus/rules/ast" - "regexp" - "strconv" - "time" + "github.com/prometheus/prometheus/utility" ) -func rulesError(error string, v ...interface{}) error { - return errors.New(fmt.Sprintf(error, v...)) -} - -// TODO move to common place, currently duplicated in config/ -func stringToDuration(durationStr string) (time.Duration, error) { - durationRE := regexp.MustCompile("^([0-9]+)([ywdhms]+)$") - matches := durationRE.FindStringSubmatch(durationStr) - if len(matches) != 3 { - return 0, rulesError("Not a valid duration string: '%v'", durationStr) - } - value, _ := strconv.Atoi(matches[1]) - unit := matches[2] - switch unit { - case "y": - value *= 60 * 60 * 24 * 365 - case "w": - value *= 60 * 60 * 24 - case "d": - value *= 60 * 60 * 24 - case "h": - value *= 60 * 60 - case "m": - value *= 60 - case "s": - value *= 1 - } - return time.Duration(value) * time.Second, nil -} - func CreateRule(name string, labels model.LabelSet, root ast.Node, permanent bool) (*Rule, error) { if root.Type() != ast.VECTOR { - return nil, rulesError("Rule %v does not evaluate to vector type", name) + return nil, fmt.Errorf("Rule %v does not evaluate to vector type", name) } return NewRule(name, labels, root.(ast.VectorNode), permanent), nil } @@ -63,18 +30,18 @@ func CreateRule(name string, labels model.LabelSet, root ast.Node, permanent boo func NewFunctionCall(name string, args []ast.Node) (ast.Node, error) { function, err := ast.GetFunction(name) if err != nil { - return nil, rulesError("Unknown function \"%v\"", name) + return nil, fmt.Errorf("Unknown function \"%v\"", name) } functionCall, err := ast.NewFunctionCall(function, args) if err != nil { - return nil, rulesError(err.Error()) + return nil, fmt.Errorf(err.Error()) } return functionCall, nil } func NewVectorAggregation(aggrTypeStr string, vector ast.Node, groupBy []model.LabelName) (*ast.VectorAggregation, error) { if vector.Type() != ast.VECTOR { - return nil, rulesError("Operand of %v aggregation must be of vector type", aggrTypeStr) + return nil, fmt.Errorf("Operand of %v aggregation must be of vector type", aggrTypeStr) } var aggrTypes = map[string]ast.AggrType{ "SUM": ast.SUM, @@ -84,7 +51,7 @@ func NewVectorAggregation(aggrTypeStr string, vector ast.Node, groupBy []model.L } aggrType, ok := aggrTypes[aggrTypeStr] if !ok { - return nil, rulesError("Unknown aggregation type '%v'", aggrTypeStr) + return nil, fmt.Errorf("Unknown aggregation type '%v'", aggrTypeStr) } return ast.NewVectorAggregation(aggrType, vector.(ast.VectorNode), groupBy), nil } @@ -107,11 +74,11 @@ func NewArithExpr(opTypeStr string, lhs ast.Node, rhs ast.Node) (ast.Node, error } opType, ok := opTypes[opTypeStr] if !ok { - return nil, rulesError("Invalid binary operator \"%v\"", opTypeStr) + return nil, fmt.Errorf("Invalid binary operator \"%v\"", opTypeStr) } expr, err := ast.NewArithExpr(opType, lhs, rhs) if err != nil { - return nil, rulesError(err.Error()) + return nil, fmt.Errorf(err.Error()) } return expr, nil } @@ -123,9 +90,9 @@ func NewMatrix(vector ast.Node, intervalStr string) (ast.MatrixNode, error) { break } default: - return nil, rulesError("Intervals are currently only supported for vector literals.") + return nil, fmt.Errorf("Intervals are currently only supported for vector literals.") } - interval, err := stringToDuration(intervalStr) + interval, err := utility.StringToDuration(intervalStr) if err != nil { return nil, err } diff --git a/rules/lexer.l b/rules/lexer.l index f174f0bbf5..643f52fc6d 100644 --- a/rules/lexer.l +++ b/rules/lexer.l @@ -48,7 +48,9 @@ AVG|SUM|MAX|MIN { yylval.str = yytext; return AGGR_OP } {L}({L}|{D})+ { yylval.str = yytext; return IDENTIFIER } \-?{D}+(\.{D}*)? { num, err := strconv.ParseFloat(yytext, 32); - if (err != nil) { rulesError("Invalid float %v", yytext) } + if (err != nil && err.(*strconv.NumError).Err == strconv.ErrSyntax) { + panic("Invalid float") + } yylval.num = model.SampleValue(num) return NUMBER } diff --git a/rules/lexer.l.go b/rules/lexer.l.go index aceeaec4ae..7ec712fd63 100644 --- a/rules/lexer.l.go +++ b/rules/lexer.l.go @@ -436,8 +436,8 @@ var yyrules []yyrule = []yyrule{{regexp.MustCompile("[^\\n]"), nil, []yystartcon }() { num, err := strconv.ParseFloat(yytext, 32) - if err != nil { - rulesError("Invalid float %v", yytext) + if err != nil && err.(*strconv.NumError).Err == strconv.ErrSyntax { + panic("Invalid float") } yylval.num = model.SampleValue(num) return yyactionreturn{NUMBER, yyRT_USER_RETURN} diff --git a/utility/strconv.go b/utility/strconv.go new file mode 100644 index 0000000000..655395f87f --- /dev/null +++ b/utility/strconv.go @@ -0,0 +1,72 @@ +// 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 utility + +import ( + "fmt" + "regexp" + "strconv" + "time" +) + +var durationRE = regexp.MustCompile("^([0-9]+)([ywdhms]+)$") + +func DurationToString(duration time.Duration) string { + seconds := int64(duration / time.Second) + factors := map[string]int64{ + "y": 60 * 60 * 24 * 365, + "d": 60 * 60 * 24, + "h": 60 * 60, + "m": 60, + "s": 1, + } + unit := "s" + switch int64(0) { + case seconds % factors["y"]: + unit = "y" + case seconds % factors["d"]: + unit = "d" + case seconds % factors["h"]: + unit = "h" + case seconds % factors["m"]: + unit = "m" + } + return fmt.Sprintf("%v%v", seconds/factors[unit], unit) +} + +func StringToDuration(durationStr string) (duration time.Duration, err error) { + matches := durationRE.FindStringSubmatch(durationStr) + if len(matches) != 3 { + err = fmt.Errorf("Not a valid duration string: '%v'", durationStr) + return + } + durationSeconds, _ := strconv.Atoi(matches[1]) + duration = time.Duration(durationSeconds) * time.Second + unit := matches[2] + switch unit { + case "y": + duration *= 60 * 60 * 24 * 365 + case "w": + duration *= 60 * 60 * 24 * 7 + case "d": + duration *= 60 * 60 * 24 + case "h": + duration *= 60 * 60 + case "m": + duration *= 60 + case "s": + duration *= 1 + } + return +} From 23374788d34956beee9e93191517c3c327867a57 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Tue, 12 Feb 2013 13:15:40 +0100 Subject: [PATCH 2/4] Beginnings of a Prometheus status page. --- appstate/appstate.go | 28 +++++++++++++++++++++++ main.go | 10 ++++++++- web/api/api.go | 4 ++-- web/static/css/graph.css | 23 +++++++++++++++++++ web/static/css/prometheus.css | 21 ------------------ web/static/graph.html | 1 + web/static/index.html | 1 + web/status.go | 42 +++++++++++++++++++++++++++++++++++ web/templates/status.html | 33 +++++++++++++++++++++++++++ web/web.go | 12 +++++----- 10 files changed, 146 insertions(+), 29 deletions(-) create mode 100644 appstate/appstate.go create mode 100644 web/static/css/graph.css create mode 100644 web/status.go create mode 100644 web/templates/status.html diff --git a/appstate/appstate.go b/appstate/appstate.go new file mode 100644 index 0000000000..2435629a8b --- /dev/null +++ b/appstate/appstate.go @@ -0,0 +1,28 @@ +// 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 appstate + +import ( + "github.com/prometheus/prometheus/config" + "github.com/prometheus/prometheus/retrieval" + "github.com/prometheus/prometheus/rules" + "github.com/prometheus/prometheus/storage/metric" +) + +type ApplicationState struct { + Config *config.Config + RuleManager rules.RuleManager + Persistence metric.MetricPersistence + TargetManager retrieval.TargetManager +} diff --git a/main.go b/main.go index 39af0be2f2..35f6b76005 100644 --- a/main.go +++ b/main.go @@ -15,6 +15,7 @@ package main import ( "flag" + "github.com/prometheus/prometheus/appstate" "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/retrieval" "github.com/prometheus/prometheus/retrieval/format" @@ -73,7 +74,14 @@ func main() { log.Fatalf("Error loading rule files: %v", err) } - web.StartServing(persistence) + appState := &appstate.ApplicationState{ + Config: conf, + RuleManager: ruleManager, + Persistence: persistence, + TargetManager: targetManager, + } + + web.StartServing(appState) for { select { diff --git a/web/api/api.go b/web/api/api.go index 7aa4e891a6..7c7b6155af 100644 --- a/web/api/api.go +++ b/web/api/api.go @@ -17,8 +17,8 @@ type MetricsService struct { time utility.Time } -func NewMetricsService(p metric.MetricPersistence) *MetricsService { +func NewMetricsService(persistence metric.MetricPersistence) *MetricsService { return &MetricsService{ - persistence: p, + persistence: persistence, } } diff --git a/web/static/css/graph.css b/web/static/css/graph.css new file mode 100644 index 0000000000..5a84cbaabe --- /dev/null +++ b/web/static/css/graph.css @@ -0,0 +1,23 @@ +body { + margin: 0; +} + +.graph_container { + font-family: Arial, Helvetica, sans-serif; +} + +.graph { + position: relative; +} + +svg { + border: 1px solid #aaa; + margin-bottom: 5px; +} + +.legend { + display: inline-block; + vertical-align: top; + margin: 0 0 0 0px; + background- +} diff --git a/web/static/css/prometheus.css b/web/static/css/prometheus.css index 52979a4c46..2a563b085f 100644 --- a/web/static/css/prometheus.css +++ b/web/static/css/prometheus.css @@ -1,5 +1,4 @@ body { - margin: 0; font-family: "Helvetica Neue", Helvetica, Arial, sans-serif; font-size: 14px; line-height: 20px; @@ -7,26 +6,6 @@ body { background-color: #eee; } -.graph_container { - font-family: Arial, Helvetica, sans-serif; -} - -.graph { - position: relative; -} - -svg { - border: 1px solid #aaa; - margin-bottom: 5px; -} - -.legend { - display: inline-block; - vertical-align: top; - margin: 0 0 0 0px; - background- -} - input:not([type=submit]):not([type=file]):not([type=button]) { border: 1px solid #aaa; -webkit-border-radius: 3px; diff --git a/web/static/graph.html b/web/static/graph.html index 2d64d9e6a3..592f5a3c5e 100644 --- a/web/static/graph.html +++ b/web/static/graph.html @@ -6,6 +6,7 @@ + diff --git a/web/static/index.html b/web/static/index.html index ce04c7bd44..df6e32cfa1 100644 --- a/web/static/index.html +++ b/web/static/index.html @@ -4,6 +4,7 @@ Prometheus Expression Browser + diff --git a/web/status.go b/web/status.go new file mode 100644 index 0000000000..a0403b0e4e --- /dev/null +++ b/web/status.go @@ -0,0 +1,42 @@ +// 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 web + +import ( + "github.com/prometheus/prometheus/appstate" + "html/template" + "net/http" +) + +type PrometheusStatus struct { + Status string + Config string + Rules string + Targets string +} + +type StatusHandler struct { + appState *appstate.ApplicationState +} + +func (h *StatusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + status := &PrometheusStatus{ + Status: "TODO: add status information here", + Config: h.appState.Config.ToString(0), + Rules: "TODO: list rules here", + Targets: "TODO: list targets here", + } + t, _ := template.ParseFiles("web/templates/status.html") + t.Execute(w, status) +} diff --git a/web/templates/status.html b/web/templates/status.html new file mode 100644 index 0000000000..8820846d71 --- /dev/null +++ b/web/templates/status.html @@ -0,0 +1,33 @@ + + + + + Prometheus Status + + + + + +

Status

+
+{{.Status}} +
+ +

Configuration

+
+ 
+{{.Config}}
+
+
+ +

Rules

+
+{{.Rules}} +
+ +

Targets

+
+{{.Targets}} +
+ + diff --git a/web/web.go b/web/web.go index 021391fbe8..50907dbd53 100644 --- a/web/web.go +++ b/web/web.go @@ -17,7 +17,7 @@ import ( "code.google.com/p/gorest" "flag" "github.com/prometheus/client_golang" - "github.com/prometheus/prometheus/storage/metric" + "github.com/prometheus/prometheus/appstate" "github.com/prometheus/prometheus/web/api" "net/http" _ "net/http/pprof" @@ -28,11 +28,13 @@ var ( listenAddress = flag.String("listenAddress", ":9090", "Address to listen on for web interface.") ) -func StartServing(persistence metric.MetricPersistence) { - gorest.RegisterService(api.NewMetricsService(persistence)) +func StartServing(appState *appstate.ApplicationState) { + gorest.RegisterService(api.NewMetricsService(appState.Persistence)) + exporter := registry.DefaultRegistry.YieldExporter() - http.Handle("/", gorest.Handle()) - http.Handle("/metrics.json", registry.DefaultHandler) + http.Handle("/status", &StatusHandler{appState: appState}) + http.Handle("/api/", gorest.Handle()) + http.Handle("/metrics.json", exporter) http.Handle("/static/", http.StripPrefix("/static/", http.FileServer(http.Dir("web/static")))) go http.ListenAndServe(*listenAddress, nil) From d137362257f691bf79d8dbe1b1cd4b0e9bf40554 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Thu, 14 Feb 2013 01:04:07 +0100 Subject: [PATCH 3/4] Config tests and config cleanups+fixes. --- config/config_test.go | 86 ++++++++++++++++++++++++++++ config/fixtures/empty.conf.printed | 8 +++ config/fixtures/minimal.conf.printed | 20 +++++++ config/fixtures/sample.conf.printed | 49 ++++++++++++++++ config/load.go | 3 +- config/printer.go | 32 +++++++---- 6 files changed, 186 insertions(+), 12 deletions(-) create mode 100644 config/config_test.go create mode 100644 config/fixtures/empty.conf.printed create mode 100644 config/fixtures/minimal.conf.printed create mode 100644 config/fixtures/sample.conf.printed diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 0000000000..477ae067f7 --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,86 @@ +// 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 config + +import ( + "fmt" + "io/ioutil" + "path" + "strings" + "testing" +) + +var fixturesPath = "fixtures" + +var configTests = []struct { + inputFile string + printedFile string + shouldFail bool + errContains string +}{ + { + inputFile: "minimal.conf", + printedFile: "minimal.conf.printed", + }, { + inputFile: "sample.conf", + printedFile: "sample.conf.printed", + }, { + // TODO: Options that are not provided should be set to sane defaults or + // create errors during config loading (as appropriate). Right now, these + // options remain at their zero-values, which is probably not what we want. + inputFile: "empty.conf", + printedFile: "empty.conf.printed", + }, + // TODO: To enable testing of bad configs, we first need to change config + // loading so that it doesn't exit when loading a bad config. Instead, the + // configuration error should be passed back all the way to the caller. + // + //{ + // inputFile: "bad_job_option.conf", + // shouldFail: true, + // errContains: "Missing job name", + //}, +} + +func TestConfigs(t *testing.T) { + for _, configTest := range configTests { + testConfig, err := LoadFromFile(path.Join(fixturesPath, configTest.inputFile)) + + if err != nil { + if !configTest.shouldFail { + t.Errorf("Error parsing config %v: %v", configTest.inputFile, err) + } else { + if !strings.Contains(err.Error(), configTest.errContains) { + t.Errorf("Expected error containing '%v', got: %v", configTest.errContains, err) + } + } + } else { + printedConfig, err := ioutil.ReadFile(path.Join(fixturesPath, configTest.printedFile)) + if err != nil { + t.Errorf("Error reading config %v: %v", configTest.inputFile, err) + continue + } + expected := string(printedConfig) + actual := testConfig.ToString(0) + + if actual != expected { + t.Errorf("%v: printed config doesn't match expected output", configTest.inputFile) + t.Errorf("Expected:\n%v\n\nActual:\n%v\n", expected, actual) + t.Errorf("Writing expected and actual printed configs to /tmp for diffing (see test source for paths)") + ioutil.WriteFile(fmt.Sprintf("/tmp/%s.expected", configTest.printedFile), []byte(expected), 0600) + ioutil.WriteFile(fmt.Sprintf("/tmp/%s.actual", configTest.printedFile), []byte(actual), 0600) + } + } + } +} diff --git a/config/fixtures/empty.conf.printed b/config/fixtures/empty.conf.printed new file mode 100644 index 0000000000..b2c9c87380 --- /dev/null +++ b/config/fixtures/empty.conf.printed @@ -0,0 +1,8 @@ +global { + scrape_interval = "0y" + evaluation_interval = "0y" + rule_files = [ + + ] +} + diff --git a/config/fixtures/minimal.conf.printed b/config/fixtures/minimal.conf.printed new file mode 100644 index 0000000000..833f635bff --- /dev/null +++ b/config/fixtures/minimal.conf.printed @@ -0,0 +1,20 @@ +global { + scrape_interval = "30s" + evaluation_interval = "30s" + labels { + monitor = "test" + } + rule_files = [ + "prometheus.rules" + ] +} + +job { + name = "prometheus" + scrape_interval = "15s" + targets { + endpoints = [ + "http://localhost:9090/metrics.json" + ] + } +} diff --git a/config/fixtures/sample.conf.printed b/config/fixtures/sample.conf.printed new file mode 100644 index 0000000000..aa46fa82fc --- /dev/null +++ b/config/fixtures/sample.conf.printed @@ -0,0 +1,49 @@ +global { + scrape_interval = "30s" + evaluation_interval = "30s" + labels { + monitor = "test" + } + rule_files = [ + "prometheus.rules" + ] +} + +job { + name = "prometheus" + scrape_interval = "15s" + targets { + endpoints = [ + "http://localhost:9090/metrics.json" + ] + labels { + group = "canary" + } + } +} + +job { + name = "random" + scrape_interval = "30s" + targets { + endpoints = [ + "http://random.com:8080/metrics.json", + "http://random.com:8081/metrics.json", + "http://random.com:8082/metrics.json", + "http://random.com:8083/metrics.json", + "http://random.com:8084/metrics.json" + ] + labels { + group = "production" + } + } + targets { + endpoints = [ + "http://random.com:8085/metrics.json", + "http://random.com:8086/metrics.json" + ] + labels { + group = "canary" + } + } +} diff --git a/config/load.go b/config/load.go index f998cb3378..f0b7407975 100644 --- a/config/load.go +++ b/config/load.go @@ -27,7 +27,7 @@ import ( var yylval *yySymType // For storing extra token information, like the contents of a string. var yyline int // Line number within the current file or buffer. var yypos int // Character position within the current line. -var parsedConfig = New() // Temporary variable for storing the parsed configuration. +var parsedConfig *Config // Temporary variable for storing the parsed configuration. type ConfigLexer struct { errors []string @@ -45,6 +45,7 @@ func (lexer *ConfigLexer) Error(errorStr string) { } func LoadFromReader(configReader io.Reader) (*Config, error) { + parsedConfig = New() yyin = configReader yypos = 1 yyline = 1 diff --git a/config/printer.go b/config/printer.go index 00e4c7dc1f..c977d945bf 100644 --- a/config/printer.go +++ b/config/printer.go @@ -34,29 +34,39 @@ func (config *Config) ToString(indent int) string { for _, job := range config.Jobs { jobs = append(jobs, job.ToString(indent)) } - return indentStr(indent, "%v\n%v\n", global, strings.Join(jobs, "\n")) + return indentStr(indent, "%v\n%v", global, strings.Join(jobs, "\n")) } func labelsToString(indent int, labels model.LabelSet) string { str := indentStr(indent, "labels {\n") + labelStrings := []string{} for label, value := range labels { - str += indentStr(indent+1, "%v = \"%v\",\n", label, value) + labelStrings = append(labelStrings, indentStr(indent+1, "%v = \"%v\"", label, value)) } + str += strings.Join(labelStrings, ",\n") + "\n" str += indentStr(indent, "}\n") return str } +func stringListToString(indent int, list []string) string { + listString := []string{} + for _, item := range list { + listString = append(listString, indentStr(indent, "\"%v\"", item)) + } + return strings.Join(listString, ",\n") + "\n" +} + func (global *GlobalConfig) ToString(indent int) string { str := indentStr(indent, "global {\n") str += indentStr(indent+1, "scrape_interval = \"%s\"\n", utility.DurationToString(global.ScrapeInterval)) str += indentStr(indent+1, "evaluation_interval = \"%s\"\n", utility.DurationToString(global.EvaluationInterval)) - str += labelsToString(indent+1, global.Labels) - str += indentStr(indent, "}\n") - str += indentStr(indent+1, "rule_files = [\n") - for _, ruleFile := range global.RuleFiles { - str += indentStr(indent+2, "\"%v\",\n", ruleFile) + if len(global.Labels) > 0 { + str += labelsToString(indent+1, global.Labels) } + str += indentStr(indent+1, "rule_files = [\n") + str += stringListToString(indent+2, global.RuleFiles) str += indentStr(indent+1, "]\n") + str += indentStr(indent, "}\n") return str } @@ -67,11 +77,11 @@ func (job *JobConfig) ToString(indent int) string { for _, targets := range job.Targets { str += indentStr(indent+1, "targets {\n") str += indentStr(indent+2, "endpoints = [\n") - for _, endpoint := range targets.Endpoints { - str += indentStr(indent+3, "\"%v\",\n", endpoint) - } + str += stringListToString(indent+3, targets.Endpoints) str += indentStr(indent+2, "]\n") - str += labelsToString(indent+2, targets.Labels) + if len(targets.Labels) > 0 { + str += labelsToString(indent+2, targets.Labels) + } str += indentStr(indent+1, "}\n") } str += indentStr(indent, "}\n") From a908e397bcbf6c112fcb4d6b92b18f52c1f79b2d Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Thu, 14 Feb 2013 18:21:21 +0100 Subject: [PATCH 4/4] Integrate cleanups for comments in PR70. --- appstate/appstate.go | 5 ++++- main.go | 2 +- utility/strconv.go | 2 ++ web/status.go | 4 ++-- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/appstate/appstate.go b/appstate/appstate.go index 2435629a8b..543f5c4b71 100644 --- a/appstate/appstate.go +++ b/appstate/appstate.go @@ -20,9 +20,12 @@ import ( "github.com/prometheus/prometheus/storage/metric" ) +// ApplicationState is an encapsulation of all relevant Prometheus application +// runtime state. It enables simpler passing of this state to components that +// require it. type ApplicationState struct { Config *config.Config - RuleManager rules.RuleManager Persistence metric.MetricPersistence + RuleManager rules.RuleManager TargetManager retrieval.TargetManager } diff --git a/main.go b/main.go index 35f6b76005..7d256ad87d 100644 --- a/main.go +++ b/main.go @@ -76,8 +76,8 @@ func main() { appState := &appstate.ApplicationState{ Config: conf, - RuleManager: ruleManager, Persistence: persistence, + RuleManager: ruleManager, TargetManager: targetManager, } diff --git a/utility/strconv.go b/utility/strconv.go index 655395f87f..5e13fcfc8c 100644 --- a/utility/strconv.go +++ b/utility/strconv.go @@ -67,6 +67,8 @@ func StringToDuration(durationStr string) (duration time.Duration, err error) { duration *= 60 case "s": duration *= 1 + default: + panic("Invalid time unit in duration string.") } return } diff --git a/web/status.go b/web/status.go index a0403b0e4e..9e01161603 100644 --- a/web/status.go +++ b/web/status.go @@ -20,9 +20,9 @@ import ( ) type PrometheusStatus struct { - Status string Config string Rules string + Status string Targets string } @@ -32,9 +32,9 @@ type StatusHandler struct { func (h *StatusHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { status := &PrometheusStatus{ - Status: "TODO: add status information here", Config: h.appState.Config.ToString(0), Rules: "TODO: list rules here", + Status: "TODO: add status information here", Targets: "TODO: list targets here", } t, _ := template.ParseFiles("web/templates/status.html")