From 618e4e5609af2641f11cec949eeb09d5d194b744 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Mon, 12 Aug 2019 20:01:06 +0200 Subject: [PATCH 1/5] Use struct instead of interface{} when parsing query user --- cmd/postgres_exporter/postgres_exporter.go | 92 ++++++++++------------ 1 file changed, 42 insertions(+), 50 deletions(-) diff --git a/cmd/postgres_exporter/postgres_exporter.go b/cmd/postgres_exporter/postgres_exporter.go index 3522f7abc..ed5b8d688 100644 --- a/cmd/postgres_exporter/postgres_exporter.go +++ b/cmd/postgres_exporter/postgres_exporter.go @@ -85,6 +85,26 @@ func (cu *ColumnUsage) UnmarshalYAML(unmarshal func(interface{}) error) error { return nil } +// MappingOptions is a copy of ColumnMapping used only for parsing +type MappingOptions struct { + Usage string `yaml:"usage"` + Description string `yaml:"description"` + Mapping map[string]float64 `yaml:"metric_mapping"` // Optional column mapping for MAPPEDMETRIC + SupportedVersions semver.Range `yaml:"pg_version"` // Semantic version ranges which are supported. Unsupported columns are not queried (internally converted to DISCARD). +} + +// nolint: golint +type Mapping map[string]ColumnMapping + +// nolint: golint +type UserQuery struct { + Query string `yaml:"query"` + Metrics []Mapping `yaml:"metrics"` +} + +// nolint: golint +type UserNamespaces map[string]UserQuery + // Regex used to get the "short-version" from the postgres version field. var versionRegex = regexp.MustCompile(`^\w+ ((\d+)(\.\d+)?(\.\d+)?)`) var lowestSupportedVersion = semver.MustParse("9.1.0") @@ -397,12 +417,11 @@ func makeQueryOverrideMap(pgVersion semver.Version, queryOverrides map[string][] // This function modifies metricMap and queryOverrideMap to contain the new // queries. // TODO: test code for all cu. -// TODO: use proper struct type system // TODO: the YAML this supports is "non-standard" - we should move away from it. func addQueries(content []byte, pgVersion semver.Version, server *Server) error { - var extra map[string]interface{} + var userNs UserNamespaces - err := yaml.Unmarshal(content, &extra) + err := yaml.Unmarshal(content, &userNs) if err != nil { return err } @@ -411,53 +430,27 @@ func addQueries(content []byte, pgVersion semver.Version, server *Server) error metricMaps := make(map[string]map[string]ColumnMapping) newQueryOverrides := make(map[string]string) - for metric, specs := range extra { + for metric, specs := range userNs { log.Debugln("New user metric namespace from YAML:", metric) - for key, value := range specs.(map[interface{}]interface{}) { - switch key.(string) { - case "query": - query := value.(string) - newQueryOverrides[metric] = query - - case "metrics": - for _, c := range value.([]interface{}) { - column := c.(map[interface{}]interface{}) - - for n, a := range column { - var columnMapping ColumnMapping - - // Fetch the metric map we want to work on. - metricMap, ok := metricMaps[metric] - if !ok { - // Namespace for metric not found - add it. - metricMap = make(map[string]ColumnMapping) - metricMaps[metric] = metricMap - } - - // Get name. - name := n.(string) - - for attrKey, attrVal := range a.(map[interface{}]interface{}) { - switch attrKey.(string) { - case "usage": - usage, err := stringToColumnUsage(attrVal.(string)) - if err != nil { - return err - } - columnMapping.usage = usage - case "description": - columnMapping.description = attrVal.(string) - } - } - - // TODO: we should support cu - columnMapping.mapping = nil - // Should we support this for users? - columnMapping.supportedVersions = nil - - metricMap[name] = columnMapping - } - } + newQueryOverrides[metric] = specs.Query + metricMap, ok := metricMaps[metric] + if !ok { + // Namespace for metric not found - add it. + metricMap = make(map[string]ColumnMapping) + metricMaps[metric] = metricMap + } + for _, metric := range specs.Metrics { + for name, mappingOption := range metric { + var columnMapping ColumnMapping + tmpUsage, _ := stringToColumnUsage(mappingOption.Usage) + columnMapping.usage = tmpUsage + columnMapping.description = mappingOption.Description + + // TODO: we should support cu + columnMapping.mapping = nil + // Should we support this for users? + columnMapping.supportedVersions = nil + metricMap[name] = columnMapping } } } @@ -486,7 +479,6 @@ func addQueries(content []byte, pgVersion semver.Version, server *Server) error } server.queryOverrides[k] = v } - return nil } From 074c1bcbe8b05b91bd4c8c3e4a6b6e03e8152e57 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Mon, 12 Aug 2019 20:06:17 +0200 Subject: [PATCH 2/5] Use MappingOptions --- cmd/postgres_exporter/postgres_exporter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/postgres_exporter/postgres_exporter.go b/cmd/postgres_exporter/postgres_exporter.go index ed5b8d688..c036324a0 100644 --- a/cmd/postgres_exporter/postgres_exporter.go +++ b/cmd/postgres_exporter/postgres_exporter.go @@ -94,7 +94,7 @@ type MappingOptions struct { } // nolint: golint -type Mapping map[string]ColumnMapping +type Mapping map[string]MappingOptions // nolint: golint type UserQuery struct { From f76488d65e6c3e57003010eba13e31a3674f96df Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Mon, 12 Aug 2019 20:57:15 +0200 Subject: [PATCH 3/5] Split function to be more testable --- cmd/postgres_exporter/postgres_exporter.go | 27 ++++++++++++++-------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/cmd/postgres_exporter/postgres_exporter.go b/cmd/postgres_exporter/postgres_exporter.go index c036324a0..0392d533d 100644 --- a/cmd/postgres_exporter/postgres_exporter.go +++ b/cmd/postgres_exporter/postgres_exporter.go @@ -410,20 +410,12 @@ func makeQueryOverrideMap(pgVersion semver.Version, queryOverrides map[string][] return resultMap } -// Add queries to the builtinMetricMaps and queryOverrides maps. Added queries do not -// respect version requirements, because it is assumed that the user knows -// what they are doing with their version of postgres. -// -// This function modifies metricMap and queryOverrideMap to contain the new -// queries. -// TODO: test code for all cu. -// TODO: the YAML this supports is "non-standard" - we should move away from it. -func addQueries(content []byte, pgVersion semver.Version, server *Server) error { +func parseUserNamespaces(content []byte) (map[string]map[string]ColumnMapping, map[string]string, error) { var userNs UserNamespaces err := yaml.Unmarshal(content, &userNs) if err != nil { - return err + return nil, nil, err } // Stores the loaded map representation @@ -454,7 +446,22 @@ func addQueries(content []byte, pgVersion semver.Version, server *Server) error } } } + return metricMaps, newQueryOverrides, nil +} +// Add queries to the builtinMetricMaps and queryOverrides maps. Added queries do not +// respect version requirements, because it is assumed that the user knows +// what they are doing with their version of postgres. +// +// This function modifies metricMap and queryOverrideMap to contain the new +// queries. +// TODO: test code for all cu. +// TODO: the YAML this supports is "non-standard" - we should move away from it. +func addQueries(content []byte, pgVersion semver.Version, server *Server) error { + metricMaps, newQueryOverrides, err := parseUserNamespaces(content) + if err != nil { + return nil + } // Convert the loaded metric map into exporter representation partialExporterMap := makeDescMap(pgVersion, server.labels, metricMaps) From 2a48292bb0d9a2b95a754fb5c37694c5effeee1a Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Mon, 12 Aug 2019 21:22:32 +0200 Subject: [PATCH 4/5] Rename function to parseUserQueries --- cmd/postgres_exporter/postgres_exporter.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/postgres_exporter/postgres_exporter.go b/cmd/postgres_exporter/postgres_exporter.go index 0392d533d..a7e744e4b 100644 --- a/cmd/postgres_exporter/postgres_exporter.go +++ b/cmd/postgres_exporter/postgres_exporter.go @@ -103,7 +103,7 @@ type UserQuery struct { } // nolint: golint -type UserNamespaces map[string]UserQuery +type UserQueries map[string]UserQuery // Regex used to get the "short-version" from the postgres version field. var versionRegex = regexp.MustCompile(`^\w+ ((\d+)(\.\d+)?(\.\d+)?)`) @@ -410,10 +410,10 @@ func makeQueryOverrideMap(pgVersion semver.Version, queryOverrides map[string][] return resultMap } -func parseUserNamespaces(content []byte) (map[string]map[string]ColumnMapping, map[string]string, error) { - var userNs UserNamespaces +func parseUserQueries(content []byte) (map[string]map[string]ColumnMapping, map[string]string, error) { + var userQueries UserQueries - err := yaml.Unmarshal(content, &userNs) + err := yaml.Unmarshal(content, &userQueries) if err != nil { return nil, nil, err } @@ -422,7 +422,7 @@ func parseUserNamespaces(content []byte) (map[string]map[string]ColumnMapping, m metricMaps := make(map[string]map[string]ColumnMapping) newQueryOverrides := make(map[string]string) - for metric, specs := range userNs { + for metric, specs := range userQueries { log.Debugln("New user metric namespace from YAML:", metric) newQueryOverrides[metric] = specs.Query metricMap, ok := metricMaps[metric] @@ -458,7 +458,7 @@ func parseUserNamespaces(content []byte) (map[string]map[string]ColumnMapping, m // TODO: test code for all cu. // TODO: the YAML this supports is "non-standard" - we should move away from it. func addQueries(content []byte, pgVersion semver.Version, server *Server) error { - metricMaps, newQueryOverrides, err := parseUserNamespaces(content) + metricMaps, newQueryOverrides, err := parseUserQueries(content) if err != nil { return nil } From d72ee8aad87c367dad57fa66efbdb107e322b7be Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Mon, 12 Aug 2019 21:35:48 +0200 Subject: [PATCH 5/5] Start to add test about query parsing --- .../postgres_exporter_test.go | 15 ++++++++++++ .../tests/user_queries_ok.yaml | 23 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 cmd/postgres_exporter/tests/user_queries_ok.yaml diff --git a/cmd/postgres_exporter/postgres_exporter_test.go b/cmd/postgres_exporter/postgres_exporter_test.go index 63cbbd8e2..c3a756589 100644 --- a/cmd/postgres_exporter/postgres_exporter_test.go +++ b/cmd/postgres_exporter/postgres_exporter_test.go @@ -3,6 +3,7 @@ package main import ( + "io/ioutil" "os" "reflect" "testing" @@ -301,3 +302,17 @@ func (s *FunctionalSuite) TestBooleanConversionToValueAndString(c *C) { c.Assert(ok, Equals, cs.expectedOK) } } + +func (s *FunctionalSuite) TestParseUserQueries(c *C) { + userQueriesData, err := ioutil.ReadFile("./tests/user_queries_ok.yaml") + if err == nil { + metricMaps, newQueryOverrides, err := parseUserQueries(userQueriesData) + c.Assert(err, Equals, nil) + c.Assert(metricMaps, NotNil) + c.Assert(newQueryOverrides, NotNil) + + if len(metricMaps) != 2 { + c.Errorf("Expected 2 metrics from user file, got %d", len(metricMaps)) + } + } +} diff --git a/cmd/postgres_exporter/tests/user_queries_ok.yaml b/cmd/postgres_exporter/tests/user_queries_ok.yaml new file mode 100644 index 000000000..e5ecec948 --- /dev/null +++ b/cmd/postgres_exporter/tests/user_queries_ok.yaml @@ -0,0 +1,23 @@ +pg_locks_mode: + query: "WITH q_locks AS (select * from pg_locks where pid != pg_backend_pid() and database = (select oid from pg_database where datname = current_database())) SELECT (select current_database()) as datname, + lockmodes AS tag_lockmode, coalesce((select count(*) FROM q_locks WHERE mode = lockmodes), 0) AS count FROM + unnest('{AccessShareLock, ExclusiveLock, RowShareLock, RowExclusiveLock, ShareLock, ShareRowExclusiveLock, AccessExclusiveLock, ShareUpdateExclusiveLock}'::text[]) lockmodes;" + metrics: + - datname: + usage: "LABEL" + description: "Database name" + - tag_lockmode: + usage: "LABEL" + description: "Lock type" + - count: + usage: "GAUGE" + description: "Number of lock" +pg_wal: + query: "select current_database() as datname, case when pg_is_in_recovery() = false then pg_xlog_location_diff(pg_current_xlog_location(), '0/0')::int8 else pg_xlog_location_diff(pg_last_xlog_replay_location(), '0/0')::int8 end as xlog_location_b;" + metrics: + - datname: + usage: "LABEL" + description: "Database name" + - xlog_location_b: + usage: "COUNTER" + description: "current transaction log write location"