Merge 60aae53 into c7891a4

jaegertracing · May 1, 2018 · 4a8cfcb · 4a8cfcb
2 parents c7891a4 + 60aae53
commit 4a8cfcb
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 45 deletions.
diff --git a/cmd/collector/app/metrics.go b/cmd/collector/app/metrics.go
@@ -26,6 +26,10 @@ const (
 	maxServiceNames = 2000
 )
 
+var (
+	debugCounterTag = map[string]string{"debug": "true"}
+)
+
 // SpanProcessorMetrics contains all the necessary metrics for the SpanProcessor
 type SpanProcessorMetrics struct { //TODO - initialize metrics in the traditional factory way. Initialize map afterward.
 	// SaveLatency measures how long the actual save to storage takes
@@ -47,15 +51,15 @@ type SpanProcessorMetrics struct { //TODO - initialize metrics in the traditiona
 }
 
 type countsBySvc struct {
-	counts  map[string]metrics.Counter // counters per service
-	factory metrics.Factory
-	lock    *sync.Mutex
+	counts      map[string]metrics.Counter // counters per service
+	debugCounts map[string]metrics.Counter // debug counters per service
+	factory     metrics.Factory
+	lock        *sync.Mutex
 }
 
 type metricsBySvc struct {
-	spans      countsBySvc // number of spans received per service
-	debugSpans countsBySvc // number of debug spans received per service
-	traces     countsBySvc // number of traces originated per service
+	spans  countsBySvc // number of spans received per service
+	traces countsBySvc // number of traces originated per service
 }
 
 // CountsBySpanType measures received, rejected, and receivedByService metrics for a format type
@@ -96,19 +100,16 @@ func NewSpanProcessorMetrics(serviceMetrics metrics.Factory, hostMetrics metrics
 func newMetricsBySvc(factory metrics.Factory, category string) metricsBySvc {
 	return metricsBySvc{
 		spans: countsBySvc{
-			counts:  make(map[string]metrics.Counter),
-			factory: factory.Namespace("spans."+category, nil),
-			lock:    &sync.Mutex{},
-		},
-		debugSpans: countsBySvc{
-			counts:  make(map[string]metrics.Counter),
-			factory: factory.Namespace("debug-spans."+category, nil),
-			lock:    &sync.Mutex{},
+			counts:      make(map[string]metrics.Counter),
+			debugCounts: make(map[string]metrics.Counter),
+			factory:     factory.Namespace("spans."+category, nil),
+			lock:        &sync.Mutex{},
 		},
 		traces: countsBySvc{
-			counts:  make(map[string]metrics.Counter),
-			factory: factory.Namespace("traces."+category, nil),
-			lock:    &sync.Mutex{},
+			counts:      make(map[string]metrics.Counter),
+			debugCounts: make(map[string]metrics.Counter),
+			factory:     factory.Namespace("traces."+category, nil),
+			lock:        &sync.Mutex{},
 		},
 	}
 }
@@ -137,29 +138,21 @@ func (m metricsBySvc) ReportServiceNameForSpan(span *model.Span) {
 	if serviceName == "" {
 		return
 	}
-	m.countSpansByServiceName(serviceName)
-	if span.Flags.IsDebug() {
-		m.countDebugSpansByServiceName(serviceName)
-	}
+	m.countSpansByServiceName(serviceName, span.Flags.IsDebug())
 	if span.ParentSpanID == 0 {
-		m.countTracesByServiceName(serviceName)
+		m.countTracesByServiceName(serviceName, span.Flags.IsDebug())
 	}
 }
 
 // countSpansByServiceName counts how many spans are received per service.
-func (m metricsBySvc) countSpansByServiceName(serviceName string) {
-	m.spans.countByServiceName(serviceName)
-}
-
-// countDebugSpansByServiceName counts how many debug spans are received per service.
-func (m metricsBySvc) countDebugSpansByServiceName(serviceName string) {
-	m.debugSpans.countByServiceName(serviceName)
+func (m metricsBySvc) countSpansByServiceName(serviceName string, isDebug bool) {
+	m.spans.countByServiceName(serviceName, isDebug)
 }
 
 // countTracesByServiceName counts how many traces are received per service,
 // i.e. the counter is only incremented for the root spans.
-func (m metricsBySvc) countTracesByServiceName(serviceName string) {
-	m.traces.countByServiceName(serviceName)
+func (m metricsBySvc) countTracesByServiceName(serviceName string, isDebug bool) {
+	m.traces.countByServiceName(serviceName, isDebug)
 }
 
 // countByServiceName maintains a map of counters for each service name it's
@@ -172,16 +165,28 @@ func (m metricsBySvc) countTracesByServiceName(serviceName string) {
 // total number of stored counters, so if it exceeds say the 90% threshold
 // an alert should be raised to investigate what's causing so many unique
 // service names.
-func (m *countsBySvc) countByServiceName(serviceName string) {
+func (m *countsBySvc) countByServiceName(serviceName string, isDebug bool) {
 	serviceName = NormalizeServiceName(serviceName)
+	getOrCreateCounter := func(counts map[string]metrics.Counter, createCounter func() metrics.Counter) metrics.Counter {
+		if c, ok := counts[serviceName]; ok {
+			return c
+		} else if len(counts) < maxServiceNames {
+			c := createCounter()
+			counts[serviceName] = c
+			return c
+		}
+		return nil
+	}
 	var counter metrics.Counter
 	m.lock.Lock()
-	if c, ok := m.counts[serviceName]; ok {
-		counter = c
-	} else if len(m.counts) < maxServiceNames {
-		c := m.factory.Counter(serviceName, nil)
-		m.counts[serviceName] = c
-		counter = c
+	if isDebug {
+		counter = getOrCreateCounter(m.debugCounts, func() metrics.Counter {
+			return m.factory.Counter(serviceName, debugCounterTag)
+		})
+	} else {
+		counter = getOrCreateCounter(m.counts, func() metrics.Counter {
+			return m.factory.Counter(serviceName, nil)
+		})
 	}
 	m.lock.Unlock()
 	if counter != nil {

diff --git a/cmd/collector/app/metrics_test.go b/cmd/collector/app/metrics_test.go
@@ -47,8 +47,8 @@ func TestProcessorMetrics(t *testing.T) {
 	jFormat.ReceivedBySvc.ReportServiceNameForSpan(&mSpan)
 	counters, gauges := baseMetrics.LocalBackend.Snapshot()
 
-	assert.EqualValues(t, 2, counters["service.jaeger.spans.by-svc.fry"])
+	assert.EqualValues(t, 1, counters["service.jaeger.spans.by-svc.fry"])
 	assert.EqualValues(t, 1, counters["service.jaeger.traces.by-svc.fry"])
-	assert.EqualValues(t, 1, counters["service.jaeger.debug-spans.by-svc.fry"])
+	assert.EqualValues(t, 1, counters["service.jaeger.spans.by-svc.fry|debug=true"])
 	assert.Empty(t, gauges)
 }
diff --git a/cmd/collector/app/span_processor_test.go b/cmd/collector/app/span_processor_test.go
@@ -109,18 +109,27 @@ func TestBySvcMetrics(t *testing.T) {
 		}
 		expected := []metricsTest.ExpectedMetric{
 			{Name: metricPrefix + ".spans.recd", Value: 2},
-			{Name: metricPrefix + ".spans.by-svc." + test.serviceName, Value: 2},
 		}
 		if test.debug {
 			expected = append(expected, metricsTest.ExpectedMetric{
-				Name: metricPrefix + ".debug-spans.by-svc." + test.serviceName, Value: 2,
+				Name: metricPrefix + ".spans.by-svc." + test.serviceName, Tags: map[string]string{"debug": "true"}, Value: 2,
 			})
-		}
-		if test.rootSpan {
+		} else {
 			expected = append(expected, metricsTest.ExpectedMetric{
-				Name: metricPrefix + ".traces.by-svc." + test.serviceName, Value: 2,
+				Name: metricPrefix + ".spans.by-svc." + test.serviceName, Value: 2,
 			})
 		}
+		if test.rootSpan {
+			if test.debug {
+				expected = append(expected, metricsTest.ExpectedMetric{
+					Name: metricPrefix + ".traces.by-svc." + test.serviceName, Tags: map[string]string{"debug": "true"}, Value: 2,
+				})
+			} else {
+				expected = append(expected, metricsTest.ExpectedMetric{
+					Name: metricPrefix + ".traces.by-svc." + test.serviceName, Value: 2,
+				})
+			}
+		}
 		if test.serviceName != blackListedService || test.debug {
 			// "error.busy" and "spans.dropped" are both equivalent to a span being accepted,
 			// because both are emitted when attempting to add span to the queue, and since