Merge pull request #400 from DataDog/benjamin/tag-replace-all

Apply tag replacement to all spans instead of root only

cmd/trace-agent/agent.go

@@ -211,7 +211,7 @@ func (a *Agent) Process(t model.Trace) {
 	}
 
 	for _, f := range a.Filters {
-		if f.Keep(root) {
+		if f.Keep(root, &t) {
 			continue
 		}
 

filters/base.go

@@ -7,7 +7,7 @@ import (
 
 // Filter is the interface implemented by all span-filters
 type Filter interface {
-	Keep(*model.Span) bool
+	Keep(*model.Span, *model.Trace) bool
 }
 
 // Setup returns a slice of all registered filters

filters/resource.go

@@ -15,9 +15,9 @@ type resourceFilter struct {
 }
 
 // Keep returns true if Span.Resource doesn't match any of the filter's rules
-func (f *resourceFilter) Keep(t *model.Span) bool {
+func (f *resourceFilter) Keep(root *model.Span, trace *model.Trace) bool {
 	for _, entry := range f.blacklist {
-		if entry.MatchString(t.Resource) {
+		if entry.MatchString(root.Resource) {
 			return false
 		}
 	}

filters/resource_test.go

@@ -29,9 +29,10 @@ func TestFilter(t *testing.T) {
 
 	for _, test := range tests {
 		span := newTestSpan(test.resource)
+		trace := model.Trace{span}
 		filter := newTestFilter(test.filter)
 
-		assert.Equal(t, test.expectation, filter.Keep(span))
+		assert.Equal(t, test.expectation, filter.Keep(span, &trace))
 	}
 }
 
@@ -41,28 +42,32 @@ func TestRegexCompilationFailure(t *testing.T) {
 
 	for i := 0; i < 100; i++ {
 		span := fixtures.RandomSpan()
-		assert.True(t, filter.Keep(span))
+		trace := model.Trace{span}
+		assert.True(t, filter.Keep(span, &trace))
 	}
 }
 
 func TestRegexEscaping(t *testing.T) {
 	span := newTestSpan("[123")
+	trace := model.Trace{span}
 
 	filter := newTestFilter("[123")
-	assert.True(t, filter.Keep(span))
+	assert.True(t, filter.Keep(span, &trace))
 
 	filter = newTestFilter("\\[123")
-	assert.False(t, filter.Keep(span))
+	assert.False(t, filter.Keep(span, &trace))
 }
 
 func TestMultipleEntries(t *testing.T) {
 	filter := newTestFilter("ABC+", "W+")
 
 	span := newTestSpan("ABCCCC")
-	assert.False(t, filter.Keep(span))
+	trace := model.Trace{span}
+	assert.False(t, filter.Keep(span, &trace))
 
 	span = newTestSpan("WWW")
-	assert.False(t, filter.Keep(span))
+	trace = model.Trace{span}
+	assert.False(t, filter.Keep(span, &trace))
 }
 
 func newTestFilter(blacklist ...string) Filter {

filters/tag.go

@@ -19,26 +19,29 @@ func newTagReplacer(c *config.AgentConfig) *tagReplacer {
 }
 
 // Keep implements Filter.
-func (f tagReplacer) Keep(s *model.Span) bool {
+func (f tagReplacer) Keep(root *model.Span, trace *model.Trace) bool {
 	for _, rule := range f.replace {
 		key, str, re := rule.Name, rule.Repl, rule.Re
-		switch key {
-		case "*":
-			for k := range s.Meta {
-				s.Meta[k] = re.ReplaceAllString(s.Meta[k], str)
+		for _, s := range *trace {
+			switch key {
+			case "*":
+				for k := range s.Meta {
+					s.Meta[k] = re.ReplaceAllString(s.Meta[k], str)
+				}
+				s.Resource = re.ReplaceAllString(s.Resource, str)
+			case "resource.name":
+				s.Resource = re.ReplaceAllString(s.Resource, str)
+			default:
+				if s.Meta == nil {
+					continue
+				}
+				if _, ok := s.Meta[key]; !ok {
+					continue
+				}
+				s.Meta[key] = re.ReplaceAllString(s.Meta[key], str)
 			}
-			s.Resource = re.ReplaceAllString(s.Resource, str)
-		case "resource.name":
-			s.Resource = re.ReplaceAllString(s.Resource, str)
-		default:
-			if s.Meta == nil {
-				continue
-			}
-			if _, ok := s.Meta[key]; !ok {
-				continue
-			}
-			s.Meta[key] = re.ReplaceAllString(s.Meta[key], str)
 		}
 	}
+	// always return true as the goal of this filter is only to mutate data
 	return true
 }

filters/tag_test.go

@@ -55,15 +55,20 @@ func TestTagReplacer(t *testing.T) {
 	} {
 		rules := parseRulesFromString(tt.rules)
 		tr := &tagReplacer{replace: rules}
-		span := replaceFilterTestSpan(tt.got)
-		ok := tr.Keep(span)
+		root := replaceFilterTestSpan(tt.got)
+		childSpan := replaceFilterTestSpan(tt.got)
+		trace := model.Trace{root, childSpan}
+		ok := tr.Keep(root, &trace)
 		assert.True(ok)
 		for k, v := range tt.want {
 			switch k {
 			case "resource.name":
-				assert.Equal(v, span.Resource)
+				// test that the filter applies to all spans, not only the root
+				assert.Equal(v, root.Resource)
+				assert.Equal(v, childSpan.Resource)
 			default:
-				assert.Equal(v, span.Meta[k])
+				assert.Equal(v, root.Meta[k])
+				assert.Equal(v, childSpan.Meta[k])
 			}
 		}
 	}