[refractor] Switch to enums for ES mappings (jaegertracing#6091)

## Which problem is this PR solving?
- Resolves jaegertracing#6067 
- Follows up jaegertracing#6068 

## Description of the changes
- Currently in `plugin/storage/es/mappings/mapping.go` we are using bare
string inorder to for the file mapping. This PR refractors this approach
to use enums instead of string

## How was this change tested?
- running `go test` in respective files/directories

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Saumya Shah <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>

Author: Saumya40-codes

cmd/es-rollover/app/init/action.go

@@ -28,7 +28,7 @@ type Action struct {
 	ILMClient     client.IndexManagementLifecycleAPI
 }
 
-func (c Action) getMapping(version uint, templateName string) (string, error) {
+func (c Action) getMapping(version uint, mappingType mappings.MappingType) (string, error) {
 	c.Config.Indices.IndexPrefix = config.IndexPrefix(c.Config.Config.IndexPrefix)
 	mappingBuilder := mappings.MappingBuilder{
 		TemplateBuilder: es.TextTemplateBuilder{},
@@ -38,7 +38,7 @@ func (c Action) getMapping(version uint, templateName string) (string, error) {
 		EsVersion:       version,
 	}
 
-	return mappingBuilder.GetMapping(templateName)
+	return mappingBuilder.GetMapping(mappingType)
 }
 
 // Do the init action
@@ -96,10 +96,16 @@ func createIndexIfNotExist(c client.IndexAPI, index string) error {
 }
 
 func (c Action) init(version uint, indexopt app.IndexOption) error {
-	mapping, err := c.getMapping(version, indexopt.Mapping)
+	mappingType, err := mappings.MappingTypeFromString(indexopt.Mapping)
 	if err != nil {
 		return err
 	}
+
+	mapping, err := c.getMapping(version, mappingType)
+	if err != nil {
+		return err
+	}
+
 	err = c.IndicesClient.CreateTemplate(mapping, indexopt.TemplateName())
 	if err != nil {
 		return err

cmd/esmapping-generator/app/renderer/render.go

@@ -12,13 +12,6 @@ import (
 	"github.com/jaegertracing/jaeger/plugin/storage/es/mappings"
 )
 
-var supportedMappings = map[string]struct{}{
-	"jaeger-span":         {},
-	"jaeger-service":      {},
-	"jaeger-dependencies": {},
-	"jaeger-sampling":     {},
-}
-
 // GetMappingAsString returns rendered index templates as string
 func GetMappingAsString(builder es.TemplateBuilder, opt *app.Options) (string, error) {
 	enableILM, err := strconv.ParseBool(opt.UseILM)
@@ -43,11 +36,11 @@ func GetMappingAsString(builder es.TemplateBuilder, opt *app.Options) (string, e
 		UseILM:        enableILM,
 		ILMPolicyName: opt.ILMPolicyName,
 	}
-	return mappingBuilder.GetMapping(opt.Mapping)
-}
 
-// IsValidOption checks if passed option is a valid index template.
-func IsValidOption(val string) bool {
-	_, ok := supportedMappings[val]
-	return ok
+	mappingType, err := mappings.MappingTypeFromString(opt.Mapping)
+	if err != nil {
+		return "", err
+	}
+
+	return mappingBuilder.GetMapping(mappingType)
 }

cmd/esmapping-generator/app/renderer/render_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/esmapping-generator/app"
 	"github.com/jaegertracing/jaeger/pkg/es/mocks"
 	"github.com/jaegertracing/jaeger/pkg/testutils"
+	"github.com/jaegertracing/jaeger/plugin/storage/es/mappings"
 )
 
 func TestIsValidOption(t *testing.T) {
@@ -29,7 +30,12 @@ func TestIsValidOption(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			assert.Equal(t, test.expectedValue, IsValidOption(test.arg))
+			_, err := mappings.MappingTypeFromString(test.arg)
+			if test.expectedValue {
+				assert.NoError(t, err)
+			} else {
+				assert.Error(t, err)
+			}
 		})
 	}
 }
@@ -53,6 +59,10 @@ func Test_getMappingAsString(t *testing.T) {
 			name: "Parse bool error", args: app.Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: 1, IndexPrefix: "test", UseILM: "foo", ILMPolicyName: "jaeger-test-policy"},
 			wantErr: errors.New("strconv.ParseBool: parsing \"foo\": invalid syntax"),
 		},
+		{
+			name: "Invalid Mapping type", args: app.Options{Mapping: "invalid-mapping", EsVersion: 7, Shards: 5, Replicas: 1, IndexPrefix: "test", UseILM: "true", ILMPolicyName: "jaeger-test-policy"},
+			wantErr: errors.New("invalid mapping type: invalid-mapping"),
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

cmd/esmapping-generator/main.go

@@ -14,6 +14,7 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/esmapping-generator/app/renderer"
 	"github.com/jaegertracing/jaeger/pkg/es"
 	"github.com/jaegertracing/jaeger/pkg/version"
+	"github.com/jaegertracing/jaeger/plugin/storage/es/mappings"
 )
 
 func main() {
@@ -24,7 +25,7 @@ func main() {
 		Short: "Jaeger esmapping-generator prints rendered mappings as string",
 		Long:  `Jaeger esmapping-generator renders passed templates with provided values and prints rendered output to stdout`,
 		Run: func(_ *cobra.Command, _ /* args */ []string) {
-			if !renderer.IsValidOption(options.Mapping) {
+			if _, err := mappings.MappingTypeFromString(options.Mapping); err != nil {
 				logger.Fatal("please pass either 'jaeger-service' or 'jaeger-span' as argument")
 			}
 

plugin/storage/es/mappings/mapping.go

@@ -6,7 +6,7 @@ package mappings
 import (
 	"bytes"
 	"embed"
-	"strings"
+	"fmt"
 
 	"github.com/jaegertracing/jaeger/pkg/es"
 	"github.com/jaegertracing/jaeger/pkg/es/config"
@@ -17,6 +17,16 @@ import (
 //go:embed *.json
 var MAPPINGS embed.FS
 
+// MappingType represents the type of Elasticsearch mapping
+type MappingType int
+
+const (
+	SpanMapping MappingType = iota
+	ServiceMapping
+	DependenciesMapping
+	SamplingMapping
+)
+
 // MappingBuilder holds common parameters required to render an elasticsearch index template
 type MappingBuilder struct {
 	TemplateBuilder es.TemplateBuilder
@@ -36,25 +46,25 @@ type templateParams struct {
 	Priority      int64
 }
 
-func (mb MappingBuilder) getMappingTemplateOptions(mapping string) templateParams {
+func (mb MappingBuilder) getMappingTemplateOptions(mappingType MappingType) templateParams {
 	mappingOpts := templateParams{}
 	mappingOpts.UseILM = mb.UseILM
 	mappingOpts.ILMPolicyName = mb.ILMPolicyName
 
-	switch {
-	case strings.Contains(mapping, "span"):
+	switch mappingType {
+	case SpanMapping:
 		mappingOpts.Shards = mb.Indices.Spans.Shards
 		mappingOpts.Replicas = mb.Indices.Spans.Replicas
 		mappingOpts.Priority = mb.Indices.Spans.Priority
-	case strings.Contains(mapping, "service"):
+	case ServiceMapping:
 		mappingOpts.Shards = mb.Indices.Services.Shards
 		mappingOpts.Replicas = mb.Indices.Services.Replicas
 		mappingOpts.Priority = mb.Indices.Services.Priority
-	case strings.Contains(mapping, "dependencies"):
+	case DependenciesMapping:
 		mappingOpts.Shards = mb.Indices.Dependencies.Shards
 		mappingOpts.Replicas = mb.Indices.Dependencies.Replicas
 		mappingOpts.Priority = mb.Indices.Dependencies.Priority
-	case strings.Contains(mapping, "sampling"):
+	case SamplingMapping:
 		mappingOpts.Shards = mb.Indices.Sampling.Shards
 		mappingOpts.Replicas = mb.Indices.Sampling.Replicas
 		mappingOpts.Priority = mb.Indices.Sampling.Priority
@@ -63,24 +73,50 @@ func (mb MappingBuilder) getMappingTemplateOptions(mapping string) templateParam
 	return mappingOpts
 }
 
-// GetMapping returns the rendered mapping based on elasticsearch version
-func (mb *MappingBuilder) GetMapping(mapping string) (string, error) {
-	templateOpts := mb.getMappingTemplateOptions(mapping)
-	if mb.EsVersion == 8 {
-		return mb.renderMapping(mapping+"-8.json", templateOpts)
-	} else if mb.EsVersion == 7 {
-		return mb.renderMapping(mapping+"-7.json", templateOpts)
+func (mt MappingType) String() string {
+	switch mt {
+	case SpanMapping:
+		return "jaeger-span"
+	case ServiceMapping:
+		return "jaeger-service"
+	case DependenciesMapping:
+		return "jaeger-dependencies"
+	case SamplingMapping:
+		return "jaeger-sampling"
+	default:
+		return "unknown"
+	}
+}
+
+// MappingTypeFromString converts a string to a MappingType
+func MappingTypeFromString(val string) (MappingType, error) {
+	switch val {
+	case "jaeger-span":
+		return SpanMapping, nil
+	case "jaeger-service":
+		return ServiceMapping, nil
+	case "jaeger-dependencies":
+		return DependenciesMapping, nil
+	case "jaeger-sampling":
+		return SamplingMapping, nil
+	default:
+		return -1, fmt.Errorf("invalid mapping type: %s", val)
 	}
-	return mb.renderMapping(mapping+"-6.json", templateOpts)
+}
+
+// GetMapping returns the rendered mapping based on elasticsearch version
+func (mb *MappingBuilder) GetMapping(mappingType MappingType) (string, error) {
+	templateOpts := mb.getMappingTemplateOptions(mappingType)
+	return mb.renderMapping(fmt.Sprintf("%s-%d.json", mappingType.String(), mb.EsVersion), templateOpts)
 }
 
 // GetSpanServiceMappings returns span and service mappings
 func (mb *MappingBuilder) GetSpanServiceMappings() (spanMapping string, serviceMapping string, err error) {
-	spanMapping, err = mb.GetMapping("jaeger-span")
+	spanMapping, err = mb.GetMapping(SpanMapping)
 	if err != nil {
 		return "", "", err
 	}
-	serviceMapping, err = mb.GetMapping("jaeger-service")
+	serviceMapping, err = mb.GetMapping(ServiceMapping)
 	if err != nil {
 		return "", "", err
 	}
@@ -89,12 +125,12 @@ func (mb *MappingBuilder) GetSpanServiceMappings() (spanMapping string, serviceM
 
 // GetDependenciesMappings returns dependencies mappings
 func (mb *MappingBuilder) GetDependenciesMappings() (string, error) {
-	return mb.GetMapping("jaeger-dependencies")
+	return mb.GetMapping(DependenciesMapping)
 }
 
 // GetSamplingMappings returns sampling mappings
 func (mb *MappingBuilder) GetSamplingMappings() (string, error) {
-	return mb.GetMapping("jaeger-sampling")
+	return mb.GetMapping(SamplingMapping)
 }
 
 func loadMapping(name string) string {

plugin/storage/es/mappings/mapping_test.go

@@ -26,27 +26,24 @@ import (
 var FIXTURES embed.FS
 
 func TestMappingBuilderGetMapping(t *testing.T) {
-	const (
-		jaegerSpan         = "jaeger-span"
-		jaegerService      = "jaeger-service"
-		jaegerDependencies = "jaeger-dependencies"
-	)
 	tests := []struct {
-		mapping   string
+		mapping   MappingType
 		esVersion uint
 	}{
-		{mapping: jaegerSpan, esVersion: 8},
-		{mapping: jaegerSpan, esVersion: 7},
-		{mapping: jaegerSpan, esVersion: 6},
-		{mapping: jaegerService, esVersion: 8},
-		{mapping: jaegerService, esVersion: 7},
-		{mapping: jaegerService, esVersion: 6},
-		{mapping: jaegerDependencies, esVersion: 8},
-		{mapping: jaegerDependencies, esVersion: 7},
-		{mapping: jaegerDependencies, esVersion: 6},
+		{mapping: SpanMapping, esVersion: 8},
+		{mapping: SpanMapping, esVersion: 7},
+		{mapping: SpanMapping, esVersion: 6},
+		{mapping: ServiceMapping, esVersion: 8},
+		{mapping: ServiceMapping, esVersion: 7},
+		{mapping: ServiceMapping, esVersion: 6},
+		{mapping: DependenciesMapping, esVersion: 8},
+		{mapping: DependenciesMapping, esVersion: 7},
+		{mapping: DependenciesMapping, esVersion: 6},
 	}
 	for _, tt := range tests {
-		t.Run(tt.mapping, func(t *testing.T) {
+		templateName := tt.mapping.String()
+
+		t.Run(templateName, func(t *testing.T) {
 			defaultOpts := func(p int64) config.IndexOptions {
 				return config.IndexOptions{
 					Shards:   3,
@@ -75,14 +72,41 @@ func TestMappingBuilderGetMapping(t *testing.T) {
 			require.NoError(t, err)
 			var wantbytes []byte
 			fileSuffix := fmt.Sprintf("-%d", tt.esVersion)
-			wantbytes, err = FIXTURES.ReadFile("fixtures/" + tt.mapping + fileSuffix + ".json")
+			wantbytes, err = FIXTURES.ReadFile("fixtures/" + templateName + fileSuffix + ".json")
 			require.NoError(t, err)
 			want := string(wantbytes)
 			assert.Equal(t, want, got)
 		})
 	}
 }
 
+func TestMappingTypeFromString(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected MappingType
+		hasError bool
+	}{
+		{"jaeger-span", SpanMapping, false},
+		{"jaeger-service", ServiceMapping, false},
+		{"jaeger-dependencies", DependenciesMapping, false},
+		{"jaeger-sampling", SamplingMapping, false},
+		{"invalid", MappingType(-1), true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			result, err := MappingTypeFromString(tt.input)
+			if tt.hasError {
+				require.Error(t, err)
+				assert.Equal(t, "unknown", result.String())
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, tt.expected, result)
+			}
+		})
+	}
+}
+
 func TestMappingBuilderLoadMapping(t *testing.T) {
 	tests := []struct {
 		name string
@@ -167,7 +191,7 @@ func TestMappingBuilderFixMapping(t *testing.T) {
 				UseILM:        true,
 				ILMPolicyName: "jaeger-test-policy",
 			}
-			_, err := mappingBuilder.renderMapping("test", mappingBuilder.getMappingTemplateOptions("test"))
+			_, err := mappingBuilder.renderMapping("test", mappingBuilder.getMappingTemplateOptions(SpanMapping))
 			if test.err != "" {
 				require.EqualError(t, err, test.err)
 			} else {