[es] Move all mapping related code to mappings package (#2822)

* 2813 - Move all mapping related code to mappings package & implement @yurishkuro's feedback on refactoring Signed-off-by: santosh <[email protected]> * 2813 - Fix name of the index templates Signed-off-by: santosh <[email protected]> * 2813 - Rename ESPrefix to IndexPrefix for clarity & Implement @yurishkuro's feedback Signed-off-by: santosh <[email protected]> * 2813 - Fix fmt issue Signed-off-by: santosh <[email protected]> * 2813 - Fix issue with adding hyphen to index prefix Signed-off-by: santosh <[email protected]> * 2813 - Used golang embed package instead of esc to embed index templates & implement feedback on tests Signed-off-by: santosh <[email protected]>
jaegertracing · Feb 26, 2021 · e4cacd0 · e4cacd0
1 parent 0d2fd6b
commit e4cacd0
Show file tree

Hide file tree

Showing 26 changed files with 937 additions and 727 deletions.
diff --git a/Makefile b/Makefile
@@ -193,10 +193,6 @@ go-lint:
 	@$(GOLINT) $(ALL_PKGS) | grep -v _nolint.go >> $(LINT_LOG) || true;
 	@[ ! -s "$(LINT_LOG)" ] || (echo "Lint Failures" | cat - $(LINT_LOG) && false)
 
-.PHONY: elasticsearch-mappings
-elasticsearch-mappings:
-	esc -pkg mappings -o plugin/storage/es/mappings/gen_assets.go -ignore assets -prefix plugin/storage/es/mappings plugin/storage/es/mappings
-
 .PHONY: build-examples
 build-examples:
 	$(GOBUILD) -o ./examples/hotrod/hotrod-$(GOOS)-$(GOARCH) ./examples/hotrod/main.go
@@ -244,7 +240,7 @@ build-all-in-one-debug build-agent-debug build-query-debug build-collector-debug
 build-all-in-one-debug build-agent-debug build-query-debug build-collector-debug build-ingester-debug: SUFFIX = -debug
 
 .PHONY: build-all-in-one build-all-in-one-debug
-build-all-in-one build-all-in-one-debug: build-ui elasticsearch-mappings
+build-all-in-one build-all-in-one-debug: build-ui
 	$(GOBUILD) $(DISABLE_OPTIMIZATIONS) -tags ui -o ./cmd/all-in-one/all-in-one$(SUFFIX)-$(GOOS)-$(GOARCH) $(BUILD_INFO) ./cmd/all-in-one/main.go
 
 .PHONY: build-agent build-agent-debug
@@ -256,7 +252,7 @@ build-query build-query-debug: build-ui
 	$(GOBUILD) $(DISABLE_OPTIMIZATIONS) -tags ui -o ./cmd/query/query$(SUFFIX)-$(GOOS)-$(GOARCH) $(BUILD_INFO) ./cmd/query/main.go
 
 .PHONY: build-collector build-collector-debug
-build-collector build-collector-debug: elasticsearch-mappings
+build-collector build-collector-debug:
 	$(GOBUILD) $(DISABLE_OPTIMIZATIONS) -o ./cmd/collector/collector$(SUFFIX)-$(GOOS)-$(GOARCH) $(BUILD_INFO) ./cmd/collector/main.go
 
 .PHONY: build-ingester build-ingester-debug

diff --git a/cmd/esmapping-generator/app/flags.go b/cmd/esmapping-generator/app/flags.go
@@ -20,21 +20,21 @@ import (
 
 // Options represent configurable parameters for jaeger-esmapping-generator
 type Options struct {
-	Mapping   string
-	EsVersion int64
-	Shards    int64
-	Replicas  int64
-	EsPrefix  string
-	UseILM    string // using string as util is being used in python and using bool leads to type issues.
+	Mapping     string
+	EsVersion   uint
+	Shards      int64
+	Replicas    int64
+	IndexPrefix string
+	UseILM      string // using string as util is being used in python and using bool leads to type issues.
 }
 
 const (
-	mappingFlag   = "mapping"
-	esVersionFlag = "es-version"
-	shardsFlag    = "shards"
-	replicasFlag  = "replicas"
-	esPrefixFlag  = "es-prefix"
-	useILMFlag    = "use-ilm"
+	mappingFlag     = "mapping"
+	esVersionFlag   = "es-version"
+	shardsFlag      = "shards"
+	replicasFlag    = "replicas"
+	indexPrefixFlag = "index-prefix"
+	useILMFlag      = "use-ilm"
 )
 
 // AddFlags adds flags for esmapping-generator main program
@@ -44,7 +44,7 @@ func (o *Options) AddFlags(command *cobra.Command) {
 		mappingFlag,
 		"",
 		"The index mapping the template will be applied to. Pass either jaeger-span or jaeger-service")
-	command.Flags().Int64Var(
+	command.Flags().UintVar(
 		&o.EsVersion,
 		esVersionFlag,
 		7,
@@ -60,8 +60,8 @@ func (o *Options) AddFlags(command *cobra.Command) {
 		1,
 		"The number of replicas per index in Elasticsearch")
 	command.Flags().StringVar(
-		&o.EsPrefix,
-		esPrefixFlag,
+		&o.IndexPrefix,
+		indexPrefixFlag,
 		"",
 		"Specifies index prefix")
 	command.Flags().StringVar(

diff --git a/cmd/esmapping-generator/app/flags_test.go b/cmd/esmapping-generator/app/flags_test.go
@@ -28,10 +28,10 @@ func TestOptionsWithDefaultFlags(t *testing.T) {
 	o.AddFlags(&c)
 
 	assert.Equal(t, "", o.Mapping)
-	assert.Equal(t, int64(7), o.EsVersion)
+	assert.Equal(t, uint(7), o.EsVersion)
 	assert.Equal(t, int64(5), o.Shards)
 	assert.Equal(t, int64(1), o.Replicas)
-	assert.Equal(t, "", o.EsPrefix)
+	assert.Equal(t, "", o.IndexPrefix)
 	assert.Equal(t, "false", o.UseILM)
 }
 
@@ -45,14 +45,14 @@ func TestOptionsWithFlags(t *testing.T) {
 		"--es-version=6",
 		"--shards=5",
 		"--replicas=1",
-		"--es-prefix=test",
+		"--index-prefix=test",
 		"--use-ilm=true",
 	})
 	require.NoError(t, err)
 	assert.Equal(t, "jaeger-span", o.Mapping)
-	assert.Equal(t, int64(6), o.EsVersion)
+	assert.Equal(t, uint(6), o.EsVersion)
 	assert.Equal(t, int64(5), o.Shards)
 	assert.Equal(t, int64(1), o.Replicas)
-	assert.Equal(t, "test", o.EsPrefix)
+	assert.Equal(t, "test", o.IndexPrefix)
 	assert.Equal(t, "true", o.UseILM)
 }
diff --git a/cmd/esmapping-generator/app/renderer/render.go b/cmd/esmapping-generator/app/renderer/render.go
@@ -18,8 +18,8 @@ import (
 	"strconv"
 
 	"github.com/jaegertracing/jaeger/cmd/esmapping-generator/app"
-	estemplate "github.com/jaegertracing/jaeger/pkg/es"
-	"github.com/jaegertracing/jaeger/plugin/storage/es"
+	"github.com/jaegertracing/jaeger/pkg/es"
+	"github.com/jaegertracing/jaeger/plugin/storage/es/mappings"
 )
 
 var supportedMappings = map[string]struct{}{
@@ -28,15 +28,22 @@ var supportedMappings = map[string]struct{}{
 }
 
 // GetMappingAsString returns rendered index templates as string
-func GetMappingAsString(builder estemplate.TemplateBuilder, opt *app.Options) (string, error) {
-	if opt.EsVersion == 7 {
-		enableILM, err := strconv.ParseBool(opt.UseILM)
-		if err != nil {
-			return "", err
-		}
-		return es.FixMapping(builder, es.LoadMapping("/"+opt.Mapping+"-7.json"), opt.Shards, opt.Replicas, opt.EsPrefix, enableILM)
+func GetMappingAsString(builder es.TemplateBuilder, opt *app.Options) (string, error) {
+
+	enableILM, err := strconv.ParseBool(opt.UseILM)
+	if err != nil {
+		return "", err
+	}
+
+	mappingBuilder := mappings.MappingBuilder{
+		TemplateBuilder: builder,
+		Shards:          opt.Shards,
+		Replicas:        opt.Replicas,
+		EsVersion:       opt.EsVersion,
+		IndexPrefix:     opt.IndexPrefix,
+		UseILM:          enableILM,
 	}
-	return es.FixMapping(builder, es.LoadMapping("/"+opt.Mapping+".json"), opt.Shards, opt.Replicas, "", false)
+	return mappingBuilder.GetMapping(opt.Mapping)
 }
 
 // IsValidOption checks if passed option is a valid index template.

diff --git a/cmd/esmapping-generator/app/renderer/render_test.go b/cmd/esmapping-generator/app/renderer/render_test.go
@@ -50,23 +50,23 @@ func Test_getMappingAsString(t *testing.T) {
 		wantErr error
 	}{
 		{
-			name: "ES version 7", args: app.Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: 1, EsPrefix: "test", UseILM: "true"},
+			name: "ES version 7", args: app.Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: 1, IndexPrefix: "test", UseILM: "true"},
 			want: "ES version 7",
 		},
 		{
-			name: "ES version 6", args: app.Options{Mapping: "jaeger-span", EsVersion: 6, Shards: 5, Replicas: 1, EsPrefix: "test", UseILM: "false"},
+			name: "ES version 6", args: app.Options{Mapping: "jaeger-span", EsVersion: 6, Shards: 5, Replicas: 1, IndexPrefix: "test", UseILM: "false"},
 			want: "ES version 6",
 		},
 		{
-			name: "Parse Error version 6", args: app.Options{Mapping: "jaeger-span", EsVersion: 6, Shards: 5, Replicas: 1, EsPrefix: "test", UseILM: "false"},
+			name: "Parse Error version 6", args: app.Options{Mapping: "jaeger-span", EsVersion: 6, Shards: 5, Replicas: 1, IndexPrefix: "test", UseILM: "false"},
 			wantErr: errors.New("parse error"),
 		},
 		{
-			name: "Parse Error version 7", args: app.Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: 1, EsPrefix: "test", UseILM: "true"},
+			name: "Parse Error version 7", args: app.Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: 1, IndexPrefix: "test", UseILM: "true"},
 			wantErr: errors.New("parse error"),
 		},
 		{
-			name: "Parse bool error", args: app.Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: 1, EsPrefix: "test", UseILM: "foo"},
+			name: "Parse bool error", args: app.Options{Mapping: "jaeger-span", EsVersion: 7, Shards: 5, Replicas: 1, IndexPrefix: "test", UseILM: "foo"},
 			wantErr: errors.New("strconv.ParseBool: parsing \"foo\": invalid syntax"),
 		},
 	}

diff --git a/plugin/storage/es/dependencystore/storage.go b/plugin/storage/es/dependencystore/storage.go
@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/olivere/elastic"
@@ -47,7 +48,7 @@ type DependencyStore struct {
 // NewDependencyStore returns a DependencyStore
 func NewDependencyStore(client es.Client, logger *zap.Logger, indexPrefix, indexDateLayout string, maxDocCount int) *DependencyStore {
 	var prefix string
-	if indexPrefix != "" {
+	if indexPrefix != "" && !strings.HasSuffix(indexPrefix, "-") {
 		prefix = indexPrefix + "-"
 	}
 	return &DependencyStore{

diff --git a/plugin/storage/es/esRollover.py b/plugin/storage/es/esRollover.py
@@ -194,10 +194,10 @@ def str2bool(v):
     return v.lower() in ('true', '1')
 
 
-def fix_mapping(template_name, esVersion, shards, replicas, esprefix, use_ilm):
+def fix_mapping(template_name, esVersion, shards, replicas, indexPrefix, use_ilm):
     output = subprocess.Popen(['esmapping-generator', '--mapping', template_name, '--es-version', str(esVersion),
                                '--shards', str(shards), '--replicas',
-                               str(replicas), '--es-prefix', esprefix, '--use-ilm', str(use_ilm)],
+                               str(replicas), '--index-prefix', indexPrefix, '--use-ilm', str(use_ilm)],
                               stdout=subprocess.PIPE,
                               stderr=subprocess.STDOUT)
     mapping, stderr = output.communicate()

diff --git a/plugin/storage/es/factory.go b/plugin/storage/es/factory.go
@@ -16,7 +16,6 @@
 package es
 
 import (
-	"bytes"
 	"flag"
 	"fmt"
 	"io"
@@ -172,7 +171,16 @@ func createSpanWriter(
 		return nil, err
 	}
 
-	spanMapping, serviceMapping, err := GetSpanServiceMappings(es.TextTemplateBuilder{}, cfg.GetNumShards(), cfg.GetNumReplicas(), client.GetVersion(), cfg.GetIndexPrefix(), cfg.GetUseILM())
+	mappingBuilder := mappings.MappingBuilder{
+		TemplateBuilder: es.TextTemplateBuilder{},
+		Shards:          cfg.GetNumShards(),
+		Replicas:        cfg.GetNumReplicas(),
+		EsVersion:       cfg.GetVersion(),
+		IndexPrefix:     cfg.GetIndexPrefix(),
+		UseILM:          cfg.GetUseILM(),
+	}
+
+	spanMapping, serviceMapping, err := mappingBuilder.GetSpanServiceMappings()
 	if err != nil {
 		return nil, err
 	}
@@ -197,70 +205,6 @@ func createSpanWriter(
 	return writer, nil
 }
 
-// GetSpanServiceMappings returns span and service mappings
-func GetSpanServiceMappings(tb es.TemplateBuilder, shards, replicas int64, esVersion uint, esPrefix string, useILM bool) (string, string, error) {
-	if esVersion == 7 {
-		spanMapping, err := FixMapping(tb, LoadMapping("/jaeger-span-7.json"), shards, replicas, esPrefix, useILM)
-		if err != nil {
-			return "", "", err
-		}
-		serviceMapping, err := FixMapping(tb, LoadMapping("/jaeger-service-7.json"), shards, replicas, esPrefix, useILM)
-		if err != nil {
-			return "", "", err
-		}
-		return spanMapping, serviceMapping, nil
-	}
-	spanMapping, err := FixMapping(tb, LoadMapping("/jaeger-span.json"), shards, replicas, "", false)
-	if err != nil {
-		return "", "", err
-	}
-	serviceMapping, err := FixMapping(tb, LoadMapping("/jaeger-service.json"), shards, replicas, "", false)
-	if err != nil {
-		return "", "", err
-	}
-	return spanMapping, serviceMapping, nil
-}
-
-// GetDependenciesMappings returns dependencies mappings
-func GetDependenciesMappings(tb es.TemplateBuilder, shards, replicas int64, esVersion uint) (string, error) {
-	if esVersion == 7 {
-		return FixMapping(tb, LoadMapping("/jaeger-dependencies-7.json"), shards, replicas, "", false)
-	}
-	return FixMapping(tb, LoadMapping("/jaeger-dependencies.json"), shards, replicas, "", false)
-}
-
-// LoadMapping returns index mappings from go assets as strings
-func LoadMapping(name string) string {
-	s, _ := mappings.FSString(false, name)
-	return s
-}
-
-// FixMapping parses the index mappings with given values and returns parsed template as string
-func FixMapping(tb es.TemplateBuilder, mapping string, shards, replicas int64, esPrefix string, useILM bool) (string, error) {
-
-	tmpl, err := tb.Parse(mapping)
-	if err != nil {
-		return "", err
-	}
-	writer := new(bytes.Buffer)
-
-	if esPrefix != "" {
-		esPrefix += "-"
-	}
-	values := struct {
-		NumberOfShards   int64
-		NumberOfReplicas int64
-		ESPrefix         string
-		UseILM           bool
-	}{shards, replicas, esPrefix, useILM}
-
-	if err := tmpl.Execute(writer, values); err != nil {
-		return "", err
-	}
-
-	return writer.String(), nil
-}
-
 var _ io.Closer = (*Factory)(nil)
 
 // Close closes the resources held by the factory