pkg/trace/event: make event service & operation matching insensitive (#…

…3113)

* pkg/trace/event: make event service & operation matchin insensitive

This change makes `apm_config.analyzed_rate_by_service` and
`apm_config.analyzed_spans` entries case insensitive. The reasoning
behind this is that Viper always lower-cases keys in the yaml file, make
it impossible to compare against the orginal value. See spf13/viper#411.

pkg/trace/event/extractor_fixed_rate.go

@@ -1,6 +1,8 @@
 package event
 
 import (
+	"strings"
+
 	"github.com/DataDog/datadog-agent/pkg/trace/pb"
 	"github.com/DataDog/datadog-agent/pkg/trace/sampler"
 )
@@ -14,21 +16,28 @@ type fixedRateExtractor struct {
 // NewFixedRateExtractor returns an APM event extractor that decides whether to extract APM events from spans following
 // the provided extraction rates for a span's (service name, operation name) pair.
 func NewFixedRateExtractor(rateByServiceAndName map[string]map[string]float64) Extractor {
-	return &fixedRateExtractor{
-		rateByServiceAndName: rateByServiceAndName,
+	// lower-case keys for case insensitive matching (see #3113)
+	rbs := make(map[string]map[string]float64, len(rateByServiceAndName))
+	for service, opRates := range rateByServiceAndName {
+		k := strings.ToLower(service)
+		rbs[k] = make(map[string]float64, len(opRates))
+		for op, rate := range opRates {
+			rbs[k][strings.ToLower(op)] = rate
+		}
 	}
+	return &fixedRateExtractor{rateByServiceAndName: rbs}
 }
 
 // Extract decides to extract an apm event from a span if its service and name have a corresponding extraction rate
 // on the rateByServiceAndName map passed in the constructor. The extracted event is returned along with the associated
 // extraction rate and a true value. If no extraction happened, false is returned as the third value and the others
 // are invalid.
 func (e *fixedRateExtractor) Extract(s *pb.Span, priority sampler.SamplingPriority) (float64, bool) {
-	operations, ok := e.rateByServiceAndName[s.Service]
+	operations, ok := e.rateByServiceAndName[strings.ToLower(s.Service)]
 	if !ok {
 		return 0, false
 	}
-	extractionRate, ok := operations[s.Name]
+	extractionRate, ok := operations[strings.ToLower(s.Name)]
 	if !ok {
 		return 0, false
 	}

pkg/trace/event/extractor_fixed_rate_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/DataDog/datadog-agent/pkg/trace/pb"
+	"github.com/stretchr/testify/assert"
 )
 
 func createTestSpans(serviceName string, operationName string) []*pb.Span {
@@ -15,6 +16,27 @@ func createTestSpans(serviceName string, operationName string) []*pb.Span {
 	return spans
 }
 
+func TestFixedCases(t *testing.T) {
+	assert := assert.New(t)
+	e := NewFixedRateExtractor(map[string]map[string]float64{
+		"sERvice1": {
+			"OP1": 1,
+			"op2": 0.5,
+		},
+	})
+
+	span1 := &pb.Span{Service: "service1", Name: "op1"}
+	span2 := &pb.Span{Service: "SerVice1", Name: "Op2"}
+
+	rate, ok := e.Extract(span1, 0)
+	assert.Equal(rate, 1.)
+	assert.True(ok)
+
+	rate, ok = e.Extract(span2, 0)
+	assert.Equal(rate, 0.5)
+	assert.True(ok)
+}
+
 func TestAnalyzedExtractor(t *testing.T) {
 	config := make(map[string]map[string]float64)
 	config["serviceA"] = make(map[string]float64)

pkg/trace/event/extractor_legacy.go

@@ -1,6 +1,8 @@
 package event
 
 import (
+	"strings"
+
 	"github.com/DataDog/datadog-agent/pkg/trace/pb"
 	"github.com/DataDog/datadog-agent/pkg/trace/sampler"
 	"github.com/DataDog/datadog-agent/pkg/trace/traceutil"
@@ -15,9 +17,12 @@ type legacyExtractor struct {
 // NewLegacyExtractor returns an APM event extractor that decides whether to extract APM events from spans following the
 // specified extraction rates for a span's service.
 func NewLegacyExtractor(rateByService map[string]float64) Extractor {
-	return &legacyExtractor{
-		rateByService: rateByService,
+	// lower-case keys for case insensitive matching (see #3113)
+	rbs := make(map[string]float64, len(rateByService))
+	for k, v := range rateByService {
+		rbs[strings.ToLower(k)] = v
 	}
+	return &legacyExtractor{rateByService: rbs}
 }
 
 // Extract decides to extract an apm event from the provided span if there's an extraction rate configured for that
@@ -28,7 +33,7 @@ func (e *legacyExtractor) Extract(s *pb.Span, priority sampler.SamplingPriority)
 	if !traceutil.HasTopLevel(s) {
 		return 0, false
 	}
-	extractionRate, ok := e.rateByService[s.Service]
+	extractionRate, ok := e.rateByService[strings.ToLower(s.Service)]
 	if !ok {
 		return 0, false
 	}

pkg/trace/event/extractor_legacy_test.go

@@ -0,0 +1,20 @@
+package event
+
+import (
+	"testing"
+
+	"github.com/DataDog/datadog-agent/pkg/trace/pb"
+	"github.com/DataDog/datadog-agent/pkg/trace/traceutil"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLegacyCases(t *testing.T) {
+	assert := assert.New(t)
+	e := NewLegacyExtractor(map[string]float64{"serviCE1": 1})
+	span := &pb.Span{Service: "SeRvIcE1"}
+	traceutil.SetTopLevel(span, true)
+
+	rate, ok := e.Extract(span, 0)
+	assert.Equal(rate, 1.)
+	assert.True(ok)
+}

releasenotes/notes/apm-analyzed-spans-insensitive-f0c03d742df708df.yaml

@@ -0,0 +1,12 @@
+# Each section from every releasenote are combined when the
+# CHANGELOG.rst is rendered. So the text needs to be worded so that
+# it does not depend on any information only available in another
+# section. This may mean repeating some details, but each section
+# must be readable independently of the other.
+#
+# Each section note must be formatted as reStructuredText.
+---
+fixes:
+  - |
+    APM: Mixing cases in `apm_config.analyzed_spans` and `apm_config.analyzed_rate_by_service`
+    entries is now allowed. Service names and operation names will be treated as case insensitive.