[chore] Add tests and remove unnecessary constants (#4718)

## Which problem is this PR solving?
- Minor clean-up after #4681

## Description of the changes
- Add tests for `model.SamplerType.String()`
- Refactor `cmd/collector/app/metrics.go` to use `model.SamplerType`
more directly

## How was this change tested?
- Unit tests

Signed-off-by: Yuri Shkuro <[email protected]>

cmd/collector/app/metrics.go

@@ -32,24 +32,32 @@ const (
 	// otherServices is the catch-all label when number of services exceeds maxServiceNames
 	otherServices = "other-services"
 
-	samplerTypeKey           = "sampler_type"
-	samplerTypeConst         = "const"
-	samplerTypeProbabilistic = "probabilistic"
-	samplerTypeRateLimiting  = "ratelimiting"
-	samplerTypeLowerBound    = "lowerbound"
-	samplerTypeUnknown       = "unrecognized"
-	// types of samplers: const, probabilistic, ratelimiting, lowerbound
-	numOfSamplerTypes = 4
+	// samplerTypeKey is the name of the metric tag showing sampler type
+	samplerTypeKey = "sampler_type"
 
-	concatenation = "$_$"
+	// // types of samplers: const, probabilistic, ratelimiting, lowerbound
+	// numOfSamplerTypes = 4
 
-	otherServicesConstSampler         = otherServices + concatenation + samplerTypeConst
-	otherServicesProbabilisticSampler = otherServices + concatenation + samplerTypeProbabilistic
-	otherServicesRateLimitingSampler  = otherServices + concatenation + samplerTypeRateLimiting
-	otherServicesLowerBoundSampler    = otherServices + concatenation + samplerTypeLowerBound
-	otherServicesUnknownSampler       = otherServices + concatenation + samplerTypeUnknown
+	concatenation = "$_$"
 )
 
+var otherServicesSamplers map[model.SamplerType]string = initOtherServicesSamplers()
+
+func initOtherServicesSamplers() map[model.SamplerType]string {
+	samplers := []model.SamplerType{
+		model.SamplerTypeUnrecognized,
+		model.SamplerTypeProbabilistic,
+		model.SamplerTypeLowerBound,
+		model.SamplerTypeRateLimiting,
+		model.SamplerTypeConst,
+	}
+	m := make(map[model.SamplerType]string)
+	for _, s := range samplers {
+		m[s] = otherServices + concatenation + s.String()
+	}
+	return m
+}
+
 // SpanProcessorMetrics contains all the necessary metrics for the SpanProcessor
 type SpanProcessorMetrics struct {
 	// TODO - initialize metrics in the traditional factory way. Initialize map afterward.
@@ -149,13 +157,14 @@ func newMetricsBySvc(factory metrics.Factory, category string) metricsBySvc {
 }
 
 func newTraceCountsBySvc(factory metrics.Factory, category string, maxServices int) traceCountsBySvc {
+	extraSlotsForOtherServicesSamples := len(otherServicesSamplers) - 1 // excluding UnrecognizedSampler
 	return traceCountsBySvc{
 		countsBySvc: countsBySvc{
 			counts:          newTraceCountsOtherServices(factory, category, "false"),
 			debugCounts:     newTraceCountsOtherServices(factory, category, "true"),
 			factory:         factory,
 			lock:            &sync.Mutex{},
-			maxServiceNames: maxServices + numOfSamplerTypes, // numOfSamplerType is the offset added to maxServices threshold
+			maxServiceNames: maxServices + extraSlotsForOtherServicesSamples,
 			category:        category,
 		},
 		// use sync.Pool to reduce allocation of stringBuilder
@@ -168,13 +177,19 @@ func newTraceCountsBySvc(factory metrics.Factory, category string, maxServices i
 }
 
 func newTraceCountsOtherServices(factory metrics.Factory, category string, isDebug string) map[string]metrics.Counter {
-	return map[string]metrics.Counter{
-		otherServicesConstSampler:         factory.Counter(metrics.Options{Name: category, Tags: map[string]string{"svc": otherServices, "debug": isDebug, samplerTypeKey: samplerTypeConst}}),
-		otherServicesLowerBoundSampler:    factory.Counter(metrics.Options{Name: category, Tags: map[string]string{"svc": otherServices, "debug": isDebug, samplerTypeKey: samplerTypeLowerBound}}),
-		otherServicesProbabilisticSampler: factory.Counter(metrics.Options{Name: category, Tags: map[string]string{"svc": otherServices, "debug": isDebug, samplerTypeKey: samplerTypeProbabilistic}}),
-		otherServicesRateLimitingSampler:  factory.Counter(metrics.Options{Name: category, Tags: map[string]string{"svc": otherServices, "debug": isDebug, samplerTypeKey: samplerTypeRateLimiting}}),
-		otherServicesUnknownSampler:       factory.Counter(metrics.Options{Name: category, Tags: map[string]string{"svc": otherServices, "debug": isDebug, samplerTypeKey: samplerTypeUnknown}}),
+	m := make(map[string]metrics.Counter)
+	for kSampler, vString := range otherServicesSamplers {
+		m[vString] = factory.Counter(
+			metrics.Options{
+				Name: category,
+				Tags: map[string]string{
+					"svc":          otherServices,
+					"debug":        isDebug,
+					samplerTypeKey: kSampler.String(),
+				},
+			})
 	}
+	return m
 }
 
 func newSpanCountsBySvc(factory metrics.Factory, category string, maxServiceNames int) spanCountsBySvc {
@@ -284,18 +299,11 @@ func (m *traceCountsBySvc) countByServiceName(serviceName string, isDebug bool,
 		counts[key] = c
 		counter = c
 	} else {
-		switch samplerType {
-		case model.SamplerTypeConst:
-			counter = counts[otherServicesConstSampler]
-		case model.SamplerTypeLowerBound:
-			counter = counts[otherServicesLowerBoundSampler]
-		case model.SamplerTypeProbabilistic:
-			counter = counts[otherServicesProbabilisticSampler]
-		case model.SamplerTypeRateLimiting:
-			counter = counts[otherServicesRateLimitingSampler]
-		default:
-			counter = counts[otherServicesUnknownSampler]
+		otherServicesSampler, ok := otherServicesSamplers[samplerType]
+		if !ok {
+			otherServicesSampler = otherServicesSamplers[model.SamplerTypeUnrecognized]
 		}
+		counter = counts[otherServicesSampler]
 	}
 	m.lock.Unlock()
 	counter.Inc(1)

cmd/collector/app/metrics_test.go

@@ -73,7 +73,7 @@ func TestNewTraceCountsBySvc(t *testing.T) {
 	counters, _ := baseMetrics.Backend.Snapshot()
 	assert.EqualValues(t, 1, counters["not_on_my_level|debug=false|sampler_type=unrecognized|svc=fry"])
 	assert.EqualValues(t, 1, counters["not_on_my_level|debug=false|sampler_type=unrecognized|svc=leela"])
-	assert.EqualValues(t, 2, counters["not_on_my_level|debug=false|sampler_type=unrecognized|svc=other-services"])
+	assert.EqualValues(t, 2, counters["not_on_my_level|debug=false|sampler_type=unrecognized|svc=other-services"], counters)
 
 	metrics.countByServiceName("bender", true, model.SamplerTypeConst)
 	metrics.countByServiceName("bender", true, model.SamplerTypeProbabilistic)
@@ -86,7 +86,7 @@ func TestNewTraceCountsBySvc(t *testing.T) {
 	counters, _ = baseMetrics.Backend.Snapshot()
 	assert.EqualValues(t, 1, counters["not_on_my_level|debug=true|sampler_type=const|svc=bender"])
 	assert.EqualValues(t, 1, counters["not_on_my_level|debug=true|sampler_type=probabilistic|svc=bender"])
-	assert.EqualValues(t, 1, counters["not_on_my_level|debug=true|sampler_type=probabilistic|svc=other-services"])
+	assert.EqualValues(t, 1, counters["not_on_my_level|debug=true|sampler_type=probabilistic|svc=other-services"], counters)
 	assert.EqualValues(t, 1, counters["not_on_my_level|debug=true|sampler_type=ratelimiting|svc=other-services"])
 	assert.EqualValues(t, 1, counters["not_on_my_level|debug=true|sampler_type=const|svc=other-services"])
 	assert.EqualValues(t, 1, counters["not_on_my_level|debug=true|sampler_type=lowerbound|svc=other-services"])

model/span.go

@@ -48,7 +48,6 @@ const (
 // Flags is a bit map of flags for a span
 type Flags uint32
 
-// Map from string to trace.SpanKind.
 var toSpanKind = map[string]trace.SpanKind{
 	"client":   trace.SpanKindClient,
 	"server":   trace.SpanKindServer,

model/span_pkg_test.go

@@ -0,0 +1,28 @@
+// Copyright (c) 2023 The Jaeger Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package model
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSamplerTypeToString(t *testing.T) {
+	for kStr, vEnum := range toSamplerType {
+		assert.Equal(t, kStr, vEnum.String())
+	}
+	assert.Equal(t, "", SamplerType(-1).String())
+}