Prevent dogstatsd sink from clobbering the metric key for other sinks (…

…#156)

If using the Fanout sink and including the dogstatsd sink, any sinks sent the metric AFTER dogstatsd could see invalid metric names. This was due to some inadvertent modifications to the original metric key. The new implementation ensure that when modifying the orignial metric key, a new backing array is used and will have key parts copied into it instead of changing elements in the original metric key.

datadog/dogstatsd.go

@@ -70,7 +70,10 @@ func (s *DogStatsdSink) parseKey(key []string) ([]string, []metrics.Label) {
 	// Splice the hostname out of the key
 	for i, el := range key {
 		if el == hostName {
-			key = append(key[:i], key[i+1:]...)
+			// We need an intermediate key to prevent clobbering the
+			// original backing array that other sinks might be consuming.
+			tempKey := append([]string{}, key[:i]...)
+			key = append(tempKey, key[i+1:]...)
 			break
 		}
 	}

datadog/dogstatsd_test.go

@@ -86,6 +86,10 @@ func setupTestServerAndBuffer(t *testing.T) (*net.UDPConn, []byte) {
 func TestParseKey(t *testing.T) {
 	for _, tt := range ParseKeyTests {
 		dog := mockNewDogStatsdSink(DogStatsdAddr, tt.Tags, tt.PropagateHostname)
+		// make a copy of the original key
+		original := make([]string, len(tt.KeyToParse))
+		copy(original, tt.KeyToParse)
+
 		key, tags := dog.parseKey(tt.KeyToParse)
 
 		if !reflect.DeepEqual(key, tt.ExpectedKey) {
@@ -95,6 +99,10 @@ func TestParseKey(t *testing.T) {
 		if !reflect.DeepEqual(tags, tt.ExpectedTags) {
 			t.Fatalf("Tag Parsing Failed for %v, %v != %v", tt.KeyToParse, tags, tt.ExpectedTags)
 		}
+
+		if !reflect.DeepEqual(original, tt.KeyToParse) {
+			t.Fatalf("Key parsing modified the original input key:, original: %v, after parse: %v", original, tt.KeyToParse)
+		}
 	}
 }