Merge pull request #101 from bakins/bakins/metric-name

Allow using name in mapping rather than 'name' label

README.md

@@ -101,15 +101,15 @@ An example mapping configuration:
 ```yaml
 mappings:
 - match: test.dispatcher.*.*.*
-  labels:
   name: "dispatcher_events_total"
+  labels:
     processor: "$1"
     action: "$2"
     outcome: "$3"
     job: "test_dispatcher"
-- match: "*.signup.*.*"
+- match: *.signup.*.*
-  labels:
   name: "signup_events_total"
+  labels:
     provider: "$2"
     outcome: "$3"
     job: "${1}_server"
@@ -127,6 +127,7 @@ follows:
     test.web-server.foo.bar
      => test_web_server_foo_bar{}
 
+Each mapping in the configuration file must define a `name` for the metric.
 
 If the default metric help text is insufficient for your needs you may use the YAML
 configuration to specify a custom help text for each mapping:
@@ -134,8 +135,8 @@ configuration to specify a custom help text for each mapping:
 mappings:
 - match: http.request.*
   help: "Total number of http requests"
-  labels:
   name: "http_requests_total"
+  labels:
     code: "$1"
 ```
 
@@ -148,8 +149,8 @@ mappings:
 - match: test.timing.*.*.*
   timer_type: histogram
   buckets: [ 0.01, 0.025, 0.05, 0.1 ]
-  labels: 
   name: "my_timer"
+  labels:
     provider: "$2"
     outcome: "$3"
     job: "${1}_server"
@@ -165,8 +166,8 @@ paramter is specified the default value of `glob` will be assumed:
 mappings:
 - match: (.*)\.(.*)--(.*)\.status\.(.*)\.count
   match_type: regex
-  labels:
   name: "request_total"
+  labels:
     hostname: "$1"
     exec: "$2"
     protocol: "$3"
@@ -192,16 +193,16 @@ defaults:
 mappings:
 # This will be a histogram using the buckets set in `defaults`.
 - match: test.timing.*.*.*
-  labels: 
   name: "my_timer"
+  labels: 
     provider: "$2"
     outcome: "$3"
     job: "${1}_server"
 # This will be a summary timer.
 - match: other.timing.*.*.*
   timer_type: summary
-  labels: 
   name: "other_timer"
+  labels: 
     provider: "$2"
     outcome: "$3"
     job: "${1}_server_other"

exporter.go

@@ -258,12 +258,10 @@ func (b *Exporter) Listen(e <-chan Events) {
 				help = mapping.HelpText
 			}
 			if present {
-				metricName = labels["name"]
+				metricName = mapping.Name
 				for label, value := range labels {
-					if label != "name" {
 					prometheusLabels[label] = value
 				}
-				}
 			} else {
 				eventsUnmapped.Inc()
 				metricName = escapeMetricName(event.MetricName())

mapper.go

@@ -47,6 +47,7 @@ type metricMapper struct {
 
 type metricMapping struct {
 	Match     string `yaml:"match"`
+	Name      string `yaml:"name"`
 	regex     *regexp.Regexp
 	Labels    prometheus.Labels `yaml:"labels"`
 	TimerType timerType         `yaml:"timer_type"`
@@ -74,18 +75,20 @@ func (m *metricMapper) initFromYAMLString(fileContents string) error {
 		currentMapping := &n.Mappings[i]
 
 		// check that label is correct
-		for k, v := range currentMapping.Labels {
+		for k := range currentMapping.Labels {
 			if !metricNameRE.MatchString(k) {
 				return fmt.Errorf("invalid label key: %s", k)
 			}
-			if k == "name" && !metricNameRE.MatchString(v) {
-				return fmt.Errorf("metric name '%s' doesn't match regex '%s'", v, metricNameRE)
-			}
 		}
 
-		if _, ok := currentMapping.Labels["name"]; !ok {
+		if currentMapping.Name == "" {
 			return fmt.Errorf("line %d: metric mapping didn't set a metric name", i)
 		}
+
+		if !metricNameRE.MatchString(currentMapping.Name) {
+			return fmt.Errorf("metric name '%s' doesn't match regex '%s'", currentMapping.Name, metricNameRE)
+		}
+
 		if currentMapping.MatchType == "" {
 			currentMapping.MatchType = n.Defaults.MatchType
 		}

mapper_test.go

@@ -17,11 +17,17 @@ import (
 	"testing"
 )
 
+type mappings map[string]struct {
+	name       string
+	labels     map[string]string
+	notPresent bool
+}
+
 func TestMetricMapperYAML(t *testing.T) {
 	scenarios := []struct {
 		config    string
 		configBad bool
-		mappings  map[string]map[string]string
+		mappings  mappings
 	}{
 		// Empty config.
 		{},
@@ -30,22 +36,22 @@ func TestMetricMapperYAML(t *testing.T) {
 			config: `---
 mappings:
 - match: test.dispatcher.*.*.*
-  labels: 
   name: "dispatch_events"
+  labels: 
     processor: "$1"
     action: "$2"
     result: "$3"
     job: "test_dispatcher"
 - match: test.my-dispatch-host01.name.dispatcher.*.*.*
-  labels:
   name: "host_dispatch_events"
+  labels:
     processor: "$1"
     action: "$2"
     result: "$3"
     job: "test_dispatcher"
 - match: request_time.*.*.*.*.*.*.*.*.*.*.*.*
-  labels:
   name: "tyk_http_request"
+  labels:
     method_and_path: "${1}"
     response_code: "${2}"
     apikey: "${3}"
@@ -59,39 +65,44 @@ mappings:
     orgid: "${11}"
     oauthid: "${12}"
 - match: "*.*"
-  labels:
   name: "catchall"
+  labels:
     first: "$1"
     second: "$2"
     third: "$3"
     job: "$1-$2-$3"
 - match: (.*)\.(.*)-(.*)\.(.*)
   match_type: regex
-  labels:
   name: "proxy_requests_total"
+  labels:
     job: "$1"
     protocol: "$2"
     endpoint: "$3"
     result: "$4"
 
   `,
-			mappings: map[string]map[string]string{
+			mappings: mappings{
-				"test.dispatcher.FooProcessor.send.succeeded": map[string]string{
+				"test.dispatcher.FooProcessor.send.succeeded": {
-					"name":      "dispatch_events",
+					name: "dispatch_events",
+					labels: map[string]string{
 						"processor": "FooProcessor",
 						"action":    "send",
 						"result":    "succeeded",
 						"job":       "test_dispatcher",
 					},
-				"test.my-dispatch-host01.name.dispatcher.FooProcessor.send.succeeded": map[string]string{
+				},
-					"name":      "host_dispatch_events",
+				"test.my-dispatch-host01.name.dispatcher.FooProcessor.send.succeeded": {
+					name: "host_dispatch_events",
+					labels: map[string]string{
 						"processor": "FooProcessor",
 						"action":    "send",
 						"result":    "succeeded",
 						"job":       "test_dispatcher",
 					},
-				"request_time.get/threads/1/posts.200.00000000.nonversioned.discussions.a11bbcdf0ac64ec243658dc64b7100fb.172.20.0.1.12ba97b7eaa1a50001000001.": map[string]string{
+				},
-					"name":            "tyk_http_request",
+				"request_time.get/threads/1/posts.200.00000000.nonversioned.discussions.a11bbcdf0ac64ec243658dc64b7100fb.172.20.0.1.12ba97b7eaa1a50001000001.": {
+					name: "tyk_http_request",
+					labels: map[string]string{
 						"method_and_path": "get/threads/1/posts",
 						"response_code":   "200",
 						"apikey":          "00000000",
@@ -105,16 +116,20 @@ mappings:
 						"orgid":           "12ba97b7eaa1a50001000001",
 						"oauthid":         "",
 					},
-				"foo.bar": map[string]string{
+				},
-					"name":   "catchall",
+				"foo.bar": {
+					name: "catchall",
+					labels: map[string]string{
 						"first":  "foo",
 						"second": "bar",
 						"third":  "",
 						"job":    "foo-bar-",
 					},
-				"foo.bar.baz": map[string]string{},
+				},
-				"proxy-1.http-goober.success": map[string]string{
+				"foo.bar.baz": {},
-					"name":     "proxy_requests_total",
+				"proxy-1.http-goober.success": {
+					name: "proxy_requests_total",
+					labels: map[string]string{
 						"job":      "proxy-1",
 						"protocol": "http",
 						"endpoint": "goober",
@@ -122,45 +137,50 @@ mappings:
 					},
 				},
 			},
+		},
 		// Config with bad regex reference.
 		{
 			config: `---
 mappings:
 - match: test.*
-  labels:
   name: "name"
+  labels:
     label: "$1_foo"
   `,
-			mappings: map[string]map[string]string{
+			mappings: mappings{
-				"test.a": map[string]string{
+				"test.a": {
-					"name":  "name",
+					name: "name",
+					labels: map[string]string{
 						"label": "",
 					},
 				},
 			},
+		},
 		// Config with good regex reference.
 		{
 			config: `
 mappings:
 - match: test.*
-  labels:
   name: "name"
+  labels:
     label: "${1}_foo"
   `,
-			mappings: map[string]map[string]string{
+			mappings: mappings{
-				"test.a": map[string]string{
+				"test.a": {
-					"name":  "name",
+					name: "name",
+					labels: map[string]string{
 						"label": "a_foo",
 					},
 				},
 			},
+		},
 		// Config with bad metric line.
 		{
 			config: `---
 mappings:
 - match: bad--metric-line.*.*
-  labels:
   name: "foo"
+  labels: {}
   `,
 			configBad: true,
 		},
@@ -169,8 +189,8 @@ mappings:
 			config: `---
 mappings:
 - match: test.*.*
-  labels:
   name: "0foo"
+  labels: {}
   `,
 			configBad: true,
 		},
@@ -187,29 +207,31 @@ mappings:
 		// Config with no mappings.
 		{
 			config:   ``,
-			mappings: map[string]map[string]string{},
+			mappings: mappings{},
 		},
 		// Config without a trailing newline.
 		{
 			config: `mappings:
 - match: test.*
-  labels:
   name: "name"
+  labels:
     label: "${1}_foo"`,
-			mappings: map[string]map[string]string{
+			mappings: mappings{
-				"test.a": map[string]string{
+				"test.a": {
-					"name":  "name",
+					name: "name",
+					labels: map[string]string{
 						"label": "a_foo",
 					},
 				},
 			},
+		},
 		// Config with an improperly escaped *.
 		{
 			config: `
 mappings:
 - match: *.test.*
-  labels:
   name: "name"
+  labels:
     label: "${1}_foo"`,
 			configBad: true,
 		},
@@ -218,28 +240,31 @@ mappings:
 			config: `
 mappings:
 - match: "*.test.*"
-  labels:
   name: "name"
+  labels:
     label: "${2}_foo"`,
-			mappings: map[string]map[string]string{
+			mappings: mappings{
-				"foo.test.a": map[string]string{
+				"foo.test.a": {
-					"name":  "name",
+					name: "name",
+					labels: map[string]string{
 						"label": "a_foo",
 					},
 				},
 			},
+		},
 		// Config with good timer type.
 		{
 			config: `---
 mappings:
 - match: test.*.*
   timer_type: summary
-  labels:
   name: "foo"
+  labels: {}
   `,
-			mappings: map[string]map[string]string{
+			mappings: mappings{
-				"test.*.*": map[string]string{
+				"test.*.*": {
-					"name": "foo",
+					name:   "foo",
+					labels: map[string]string{},
 				},
 			},
 		},
@@ -249,8 +274,8 @@ mappings:
 mappings:
 - match: test.*.*
   timer_type: wrong
-  labels:
   name: "foo"
+  labels: {}
     `,
 			configBad: true,
 		},
@@ -260,8 +285,36 @@ mappings:
 mappings:
 - match: "*\.foo"
   match_type: regex
-  labels:
   name: "foo"
+  labels: {}
+    `,
+			configBad: true,
+		},
+		//Config with non-matched metric.
+		{
+			config: `---
+mappings:
+- match: foo.*.*
+  timer_type: summary
+  name: "foo"
+  labels: {}
+  `,
+			mappings: mappings{
+				"test.1.2": {
+					name:       "test_1_2",
+					labels:     map[string]string{},
+					notPresent: true,
+				},
+			},
+		},
+		//Config with no name.
+		{
+			config: `---
+mappings:
+- match: *\.foo
+  match_type: regex
+  labels:
+    bar: "foo"
     `,
 			configBad: true,
 		},
@@ -270,34 +323,41 @@ mappings:
 			config: `
 mappings:
 - match: test.dispatcher.*.*.*
-  labels:
   name: "dispatcher_events_total"
+  labels:
     processor: "$1"
     action: "$2"
     outcome: "$3"
     job: "test_dispatcher"
 - match: "*.signup.*.*"
-  labels:
   name: "signup_events_total"
+  labels:
     provider: "$2"
     outcome: "$3"
     job: "${1}_server"
 `,
-			mappings: map[string]map[string]string{
+			mappings: mappings{
-				"test.dispatcher.FooProcessor.send.success": map[string]string{
+				"test.dispatcher.FooProcessor.send.success": {
-					"name":      "dispatcher_events_total",
+					name: "dispatcher_events_total",
+					labels: map[string]string{
 						"processor": "FooProcessor",
 						"action":    "send",
 						"outcome":   "success",
 						"job":       "test_dispatcher",
 					},
-				"foo_product.signup.facebook.failure": map[string]string{
+				},
-					"name":     "signup_events_total",
+				"foo_product.signup.facebook.failure": {
+					name: "signup_events_total",
+					labels: map[string]string{
 						"provider": "facebook",
 						"outcome":  "failure",
 						"job":      "foo_product_server",
 					},
-				"test.web-server.foo.bar": map[string]string{},
+				},
+				"test.web-server.foo.bar": {
+					name:   "test_web_server_foo_bar",
+					labels: map[string]string{},
+				},
 			},
 		},
 	}
@@ -313,18 +373,22 @@ mappings:
 		}
 
 		for metric, mapping := range scenario.mappings {
-			_, labels, present := mapper.getMapping(metric)
+			m, labels, present := mapper.getMapping(metric)
-			if len(labels) == 0 && present {
+			if present && m.Name != mapping.name {
+				t.Fatalf("%d.%q: Expected name %v, got %v", i, metric, m.Name, mapping.name)
+			}
+			if mapping.notPresent && present {
 				t.Fatalf("%d.%q: Expected metric to not be present", i, metric)
 			}
-			if len(labels) != len(mapping) {
+			if len(labels) != len(mapping.labels) {
-				t.Fatalf("%d.%q: Expected %d labels, got %d", i, metric, len(mapping), len(labels))
+				t.Fatalf("%d.%q: Expected %d labels, got %d", i, metric, len(mapping.labels), len(labels))
 			}
 			for label, value := range labels {
-				if mapping[label] != value {
+				if mapping.labels[label] != value {
 					t.Fatalf("%d.%q: Expected labels %v, got %v", i, metric, mapping, labels)
 				}
 			}
+
 		}
 	}
 }