Fixed overwrite global tags in Influx adapter

[kihamo]

Hi.

I found that overwrites global tags in mergeTags in Influx adapter of metrics package. Example,

func TestCounter_RewriteTags(t *testing.T) {
	in := New(map[string]string{}, influxdb.BatchPointsConfig{}, log.NewNopLogger())

	reFirst := regexp.MustCompile(`influx_counter_first,tag_for_counter_first=first count=([0-9\.]+) [0-9]+`)
	counterFirst := in.NewCounter("influx_counter_first").With("tag_for_counter_first", "first")
	counterFirst.Add(11)

	reSecond := regexp.MustCompile(`influx_counter_second,tag_for_counter_second=second count=([0-9\.]+) [0-9]+`)
	counterSecond := in.NewCounter("influx_counter_second").With("tag_for_counter_second", "second")
	counterSecond.Add(22)

	client := &bufWriter{}
	in.WriteTo(client)

	for _, row := range strings.Split(client.buf.String(), string('\n')) {
		if strings.HasPrefix(row, "influx_counter_first") && !reFirst.MatchString(row) {
			t.Fatal(`First counter not equal "`, reFirst.String() , `" want "`, row, `"`)
		} else if strings.HasPrefix(row, "influx_counter_second") && !reSecond.MatchString(row) {
			t.Fatal(`Second counter not equal "`, reSecond.String() , `" want "`, row, `"`)
		}
	}
}

Result

$ go test -run TestCounter_RewriteTags
--- FAIL: TestCounter_RewriteTags (0.00s)
        influx_test.go:51: Second counter not equal " influx_counter_second,tag_for_counter_second=second count=([0-9\.]+) [0-9]+ " want " influx_counter_second,tag_for_counter_first=first,tag_for_counter_second=second count=22 1481159549909954467 "
FAIL
exit status 1
FAIL    github.com/go-kit/kit/metrics/influx    0.010s

As you can see tags from the first counter appear in the second. I think that this is wrong. Map tags map[string]string is the root cause of the error, it is passed by pointer and each time frays from other counters/gauges/histograms.

Please see my PL.

[peterbourgon]

Wow, great find! Thank you so much for the effort. I would like to have a few changes to your test before merging. Can I make a PR on your branch, or would you like to just apply this diff yourself?

diff --git a/metrics/influx/influx.go b/metrics/influx/influx.go
index 8e9ba12..b9a3f66 100644
--- a/metrics/influx/influx.go
+++ b/metrics/influx/influx.go
@@ -168,8 +168,7 @@ func mergeTags(tags map[string]string, labelValues []string) map[string]string {
 	if len(labelValues)%2 != 0 {
 		panic("mergeTags received a labelValues with an odd number of strings")
 	}
-
-	ret := make(map[string]string, len(tags) + len(labelValues) / 2)
+	ret := make(map[string]string, len(tags)+len(labelValues)/2)
 	for k, v := range tags {
 		ret[k] = v
 	}
diff --git a/metrics/influx/influx_test.go b/metrics/influx/influx_test.go
index 56d35bd..57950f3 100644
--- a/metrics/influx/influx_test.go
+++ b/metrics/influx/influx_test.go
@@ -30,29 +30,6 @@ func TestCounter(t *testing.T) {
 	}
 }
 
-func TestCounter_RewriteTags(t *testing.T) {
-	in := New(map[string]string{}, influxdb.BatchPointsConfig{}, log.NewNopLogger())
-
-	reFirst := regexp.MustCompile(`influx_counter_first,tag_for_counter_first=first count=([0-9\.]+) [0-9]+`)
-	counterFirst := in.NewCounter("influx_counter_first").With("tag_for_counter_first", "first")
-	counterFirst.Add(11)
-
-	reSecond := regexp.MustCompile(`influx_counter_second,tag_for_counter_second=second count=([0-9\.]+) [0-9]+`)
-	counterSecond := in.NewCounter("influx_counter_second").With("tag_for_counter_second", "second")
-	counterSecond.Add(22)
-
-	client := &bufWriter{}
-	in.WriteTo(client)
-
-	for _, row := range strings.Split(client.buf.String(), string('\n')) {
-		if strings.HasPrefix(row, "influx_counter_first") && !reFirst.MatchString(row) {
-			t.Fatal(`First counter not equal "`, reFirst.String() , `" want "`, row, `"`)
-		} else if strings.HasPrefix(row, "influx_counter_second") && !reSecond.MatchString(row) {
-			t.Fatal(`Second counter not equal "`, reSecond.String() , `" want "`, row, `"`)
-		}
-	}
-}
-
 func TestGauge(t *testing.T) {
 	in := New(map[string]string{"foo": "alpha"}, influxdb.BatchPointsConfig{}, log.NewNopLogger())
 	re := regexp.MustCompile(`influx_gauge,foo=alpha value=([0-9\.]+) [0-9]+`)
@@ -105,6 +82,37 @@ func TestHistogramLabels(t *testing.T) {
 	}
 }
 
+func TestIssue404(t *testing.T) {
+	in := New(map[string]string{}, influxdb.BatchPointsConfig{}, log.NewNopLogger())
+
+	counterOne := in.NewCounter("influx_counter_one").With("a", "b")
+	counterOne.Add(123)
+
+	counterTwo := in.NewCounter("influx_counter_two").With("c", "d")
+	counterTwo.Add(456)
+
+	w := &bufWriter{}
+	in.WriteTo(w)
+
+	lines := strings.Split(strings.TrimSpace(w.buf.String()), "\n")
+	if want, have := 2, len(lines); want != have {
+		t.Fatalf("want %d, have %d", want, have)
+	}
+	for _, line := range lines {
+		if strings.HasPrefix(line, "influx_counter_one") {
+			if !strings.HasPrefix(line, "influx_counter_one,a=b count=123 ") {
+				t.Errorf("invalid influx_counter_one: %s", line)
+			}
+		} else if strings.HasPrefix(line, "influx_counter_two") {
+			if !strings.HasPrefix(line, "influx_counter_two,c=d count=456 ") {
+				t.Errorf("invalid influx_counter_two: %s", line)
+			}
+		} else {
+			t.Errorf("unexpected line: %s", line)
+		}
+	}
+}
+
 type bufWriter struct {
 	buf bytes.Buffer
 }

[kihamo]

@peterbourgon no problem, done in my branch.

[peterbourgon]

Amazing! Thank you so much!