Fix es-archive namespace default values (#2865)

* Fix es-archive namespace default values Signed-off-by: albertteoh <albert.teoh@logz.io> * Add breaking change to changelog Signed-off-by: albertteoh <albert.teoh@logz.io> * Fix missing Version default setter Signed-off-by: albertteoh <albert.teoh@logz.io> * Disable other namespaces by default Signed-off-by: albertteoh <albert.teoh@logz.io> * Revert setting version default Signed-off-by: albertteoh <albert.teoh@logz.io>
jaegertracing · Mar 10, 2021 · 6072ef2 · 6072ef2
1 parent 70e9df7
commit 6072ef2
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,11 @@ Changes by Version
 
 1.23.0 (unreleased)
 -------------------
+### Backend Changes
+
+#### Breaking Changes
+
+* Remove unused `--es-archive.max-span-age` flag ([#2865](https://github.com/jaegertracing/jaeger/pull/2865), [@albertteoh](https://github.com/albertteoh)):
 
 1.22.0 (2021-02-23)
 -------------------

diff --git a/plugin/storage/es/options.go b/plugin/storage/es/options.go
@@ -83,35 +83,41 @@ type namespaceConfig struct {
 // NewOptions creates a new Options struct.
 func NewOptions(primaryNamespace string, otherNamespaces ...string) *Options {
 	// TODO all default values should be defined via cobra flags
+	defaultConfig := config.Configuration{
+		Username:          "",
+		Password:          "",
+		Sniffer:           false,
+		MaxSpanAge:        72 * time.Hour,
+		NumShards:         5,
+		NumReplicas:       1,
+		BulkSize:          5 * 1000 * 1000,
+		BulkWorkers:       1,
+		BulkActions:       1000,
+		BulkFlushInterval: time.Millisecond * 200,
+		Tags: config.TagsAsFields{
+			DotReplacement: "@",
+		},
+		Enabled:              true,
+		CreateIndexTemplates: true,
+		Version:              0,
+		Servers:              []string{defaultServerURL},
+		MaxDocCount:          defaultMaxDocCount,
+	}
 	options := &Options{
 		Primary: namespaceConfig{
-			Configuration: config.Configuration{
-				Username:          "",
-				Password:          "",
-				Sniffer:           false,
-				MaxSpanAge:        72 * time.Hour,
-				NumShards:         5,
-				NumReplicas:       1,
-				BulkSize:          5 * 1000 * 1000,
-				BulkWorkers:       1,
-				BulkActions:       1000,
-				BulkFlushInterval: time.Millisecond * 200,
-				Tags: config.TagsAsFields{
-					DotReplacement: "@",
-				},
-				Enabled:              true,
-				CreateIndexTemplates: true,
-				Version:              0,
-				Servers:              []string{defaultServerURL},
-				MaxDocCount:          defaultMaxDocCount,
-			},
-			namespace: primaryNamespace,
+			Configuration: defaultConfig,
+			namespace:     primaryNamespace,
 		},
 		others: make(map[string]*namespaceConfig, len(otherNamespaces)),
 	}
 
+	// Other namespaces need to be explicitly enabled.
+	defaultConfig.Enabled = false
 	for _, namespace := range otherNamespaces {
-		options.others[namespace] = &namespaceConfig{namespace: namespace}
+		options.others[namespace] = &namespaceConfig{
+			Configuration: defaultConfig,
+			namespace:     namespace,
+		}
 	}
 
 	return options
@@ -158,10 +164,6 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) {
 		nsConfig.namespace+suffixTimeout,
 		nsConfig.Timeout,
 		"Timeout used for queries. A Timeout of zero means no timeout")
-	flagSet.Duration(
-		nsConfig.namespace+suffixMaxSpanAge,
-		nsConfig.MaxSpanAge,
-		"The maximum lookback for spans in Elasticsearch")
 	flagSet.Int64(
 		nsConfig.namespace+suffixNumShards,
 		nsConfig.NumShards,
@@ -215,7 +217,7 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) {
 		nsConfig.UseReadWriteAliases,
 		"Use read and write aliases for indices. Use this option with Elasticsearch rollover "+
 			"API. It requires an external component to create aliases before startup and then performing its management. "+
-			"Note that "+nsConfig.namespace+suffixMaxSpanAge+" will influence trace search window start times.")
+			"Note that es"+suffixMaxSpanAge+" will influence trace search window start times.")
 	flagSet.Bool(
 		nsConfig.namespace+suffixUseILM,
 		nsConfig.UseILM,
@@ -238,11 +240,19 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) {
 		nsConfig.namespace+suffixMaxDocCount,
 		nsConfig.MaxDocCount,
 		"The maximum document count to return from an Elasticsearch query. This will also apply to aggregations.")
+
 	if nsConfig.namespace == archiveNamespace {
 		flagSet.Bool(
 			nsConfig.namespace+suffixEnabled,
 			nsConfig.Enabled,
 			"Enable extra storage")
+	} else {
+		// MaxSpanAge is only relevant when searching for unarchived traces.
+		// Archived traces are searched with no look-back limit.
+		flagSet.Duration(
+			nsConfig.namespace+suffixMaxSpanAge,
+			nsConfig.MaxSpanAge,
+			"The maximum lookback for spans in Elasticsearch")
 	}
 	nsConfig.getTLSFlagsConfig().AddFlags(flagSet)
 }

diff --git a/plugin/storage/es/options_test.go b/plugin/storage/es/options_test.go
@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/jaegertracing/jaeger/pkg/config"
 )
@@ -45,7 +46,7 @@ func TestOptions(t *testing.T) {
 func TestOptionsWithFlags(t *testing.T) {
 	opts := NewOptions("es", "es.aux")
 	v, command := config.Viperize(opts.AddFlags)
-	command.ParseFlags([]string{
+	err := command.ParseFlags([]string{
 		"--es.server-urls=1.1.1.1, 2.2.2.2",
 		"--es.username=hello",
 		"--es.password=world",
@@ -69,6 +70,7 @@ func TestOptionsWithFlags(t *testing.T) {
 		"--es.tags-as-fields.dot-replacement=!",
 		"--es.use-ilm=true",
 	})
+	require.NoError(t, err)
 	opts.InitFromViper(v)
 
 	primary := opts.GetPrimary()
@@ -89,18 +91,26 @@ func TestOptionsWithFlags(t *testing.T) {
 	assert.Equal(t, []string{"3.3.3.3", "4.4.4.4"}, aux.Servers)
 	assert.Equal(t, "hello", aux.Username)
 	assert.Equal(t, "world", aux.Password)
-	assert.Equal(t, int64(20), aux.NumShards)
+	assert.Equal(t, int64(5), aux.NumShards)
 	assert.Equal(t, int64(10), aux.NumReplicas)
 	assert.Equal(t, 24*time.Hour, aux.MaxSpanAge)
 	assert.True(t, aux.Sniffer)
 	assert.True(t, aux.Tags.AllAsFields)
-	assert.Equal(t, "!", aux.Tags.DotReplacement)
+	assert.Equal(t, "@", aux.Tags.DotReplacement)
 	assert.Equal(t, "./file.txt", aux.Tags.File)
 	assert.Equal(t, "test,tags", aux.Tags.Include)
 	assert.Equal(t, "2006.01.02", aux.IndexDateLayout)
 	assert.True(t, primary.UseILM)
 }
 
+func TestMaxSpanAgeSetErrorInArchiveMode(t *testing.T) {
+	opts := NewOptions("es", archiveNamespace)
+	_, command := config.Viperize(opts.AddFlags)
+	flags := []string{"--es-archive.max-span-age=24h"}
+	err := command.ParseFlags(flags)
+	assert.EqualError(t, err, "unknown flag: --es-archive.max-span-age")
+}
+
 func TestMaxDocCount(t *testing.T) {
 	testCases := []struct {
 		name            string