Moved tag filters out of cassandra connection config

Signed-off-by: Joe Elliott <number101010@gmail.com>

pkg/cassandra/config/config.go

@@ -45,8 +45,6 @@ type Configuration struct {
 	DisableAutoDiscovery bool          `yaml:"disable_auto_discovery"`
 	EnableDependenciesV2 bool          `yaml:"enable_dependencies_v2"`
 	TLS                  TLS
-	TagIndexWhitelist    []string `yaml:"tag-index-whitelist"`
-	TagIndexBlacklist    []string `yaml:"tag-index-blacklist"`
 }
 
 // Authenticator holds the authentication properties needed to connect to a Cassandra cluster

plugin/storage/cassandra/factory.go

@@ -104,7 +104,7 @@ func (f *Factory) CreateSpanReader() (spanstore.Reader, error) {
 
 // CreateSpanWriter implements storage.Factory
 func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) {
-	options, err := writerOptions(f.Options.GetPrimary())
+	options, err := writerOptions(f.Options)
 	if err != nil {
 		return nil, err
 	}
@@ -130,23 +130,26 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) {
 	if f.archiveSession == nil {
 		return nil, storage.ErrArchiveStorageNotConfigured
 	}
-	options, err := writerOptions(f.Options.Get(archiveStorageConfig))
+	options, err := writerOptions(f.Options)
 	if err != nil {
 		return nil, err
 	}
 	return cSpanStore.NewSpanWriter(f.archiveSession, f.Options.SpanStoreWriteCacheTTL, f.archiveMetricsFactory, f.logger, options...), nil
 }
 
-func writerOptions(config *config.Configuration) ([]cSpanStore.Option, error) {
-	if len(config.TagIndexBlacklist) > 0 && len(config.TagIndexWhitelist) > 0 {
+func writerOptions(opts *Options) ([]cSpanStore.Option, error) {
+	tagIndexBlacklist := opts.GetTagIndexBlacklist()
+	tagIndexWhitelist := opts.GetTagIndexWhitelist()
+
+	if len(tagIndexBlacklist) > 0 && len(tagIndexWhitelist) > 0 {
 		return nil, errors.New("only one of TagIndexBlacklist and TagIndexWhitelist can be specified")
 	}
 
 	var options []cSpanStore.Option
-	if len(config.TagIndexBlacklist) > 0 {
-		options = append(options, cSpanStore.TagFilter(dbmodel.NewBlacklistTagFilter(config.TagIndexBlacklist)))
-	} else if len(config.TagIndexWhitelist) > 0 {
-		options = append(options, cSpanStore.TagFilter(dbmodel.NewWhitelistTagFilter(config.TagIndexWhitelist)))
+	if len(tagIndexBlacklist) > 0 {
+		options = append(options, cSpanStore.TagFilter(dbmodel.NewBlacklistTagFilter(tagIndexBlacklist)))
+	} else if len(tagIndexWhitelist) > 0 {
+		options = append(options, cSpanStore.TagFilter(dbmodel.NewWhitelistTagFilter(tagIndexWhitelist)))
 	}
 	return options, nil
 }

plugin/storage/cassandra/factory_test.go

@@ -146,22 +146,22 @@ func TestWriterOptions(t *testing.T) {
 	command.ParseFlags([]string{"--cassandra.tag-index-whitelist=a,b,c"})
 	opts.InitFromViper(v)
 
-	options, _ := writerOptions(opts.GetPrimary())
+	options, _ := writerOptions(opts)
 	assert.Len(t, options, 1)
 
 	opts = NewOptions("cassandra")
 	v, command = config.Viperize(opts.AddFlags)
 	command.ParseFlags([]string{"--cassandra.tag-index-blacklist=a,b,c"})
 	opts.InitFromViper(v)
 
-	options, _ = writerOptions(opts.GetPrimary())
+	options, _ = writerOptions(opts)
 	assert.Len(t, options, 1)
 
 	opts = NewOptions("cassandra")
 	v, command = config.Viperize(opts.AddFlags)
 	command.ParseFlags([]string{""})
 	opts.InitFromViper(v)
 
-	options, _ = writerOptions(opts.GetPrimary())
+	options, _ = writerOptions(opts)
 	assert.Len(t, options, 0)
 }

plugin/storage/cassandra/options.go

@@ -50,11 +50,11 @@ const (
 	suffixServerName           = ".tls.server-name"
 	suffixVerifyHost           = ".tls.verify-host"
 	suffixEnableDependenciesV2 = ".enable-dependencies-v2"
-	suffixTagIndexBlacklist    = ".tag-index-blacklist"
-	suffixTagIndexWhitelist    = ".tag-index-whitelist"
 
 	// common storage settings
 	suffixSpanStoreWriteCacheTTL = ".span-store-write-cache-ttl"
+	suffixTagIndexBlacklist      = ".tag-index-blacklist"
+	suffixTagIndexWhitelist      = ".tag-index-whitelist"
 )
 
 // Options contains various type of Cassandra configs and provides the ability
@@ -64,19 +64,19 @@ type Options struct {
 	primary                *namespaceConfig
 	others                 map[string]*namespaceConfig
 	SpanStoreWriteCacheTTL time.Duration
+	tagIndexBlacklist      string
+	tagIndexWhitelist      string
 }
 
 // the Servers field in config.Configuration is a list, which we cannot represent with flags.
 // This struct adds a plain string field that can be bound to flags and is then parsed when
 // preparing the actual config.Configuration.
 type namespaceConfig struct {
 	config.Configuration
-	servers           string
-	tagIndexBlacklist string
-	tagIndexWhitelist string
-	namespace         string
-	primary           bool
-	Enabled           bool
+	servers   string
+	namespace string
+	primary   bool
+	Enabled   bool
 }
 
 // NewOptions creates a new Options struct.
@@ -120,6 +120,15 @@ func (opt *Options) AddFlags(flagSet *flag.FlagSet) {
 	flagSet.Duration(opt.primary.namespace+suffixSpanStoreWriteCacheTTL,
 		opt.SpanStoreWriteCacheTTL,
 		"The duration to wait before rewriting an existing service or operation name")
+	flagSet.String(
+		opt.primary.namespace+suffixTagIndexBlacklist,
+		opt.tagIndexBlacklist,
+		"The comma-separated list of tags to blacklist")
+	flagSet.String(
+		opt.primary.namespace+suffixTagIndexWhitelist,
+		opt.tagIndexWhitelist,
+		"The comma-separated list of tags to whitelist")
+
 }
 
 func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) {
@@ -217,14 +226,6 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) {
 		nsConfig.namespace+suffixEnableDependenciesV2,
 		nsConfig.EnableDependenciesV2,
 		"(deprecated) Jaeger will automatically detect the version of the dependencies table")
-	flagSet.String(
-		nsConfig.namespace+suffixTagIndexBlacklist,
-		nsConfig.tagIndexWhitelist,
-		"The comma-separated list of tags to blacklist")
-	flagSet.String(
-		nsConfig.namespace+suffixTagIndexWhitelist,
-		nsConfig.tagIndexBlacklist,
-		"The comma-separated list of tags to whitelist")
 }
 
 // InitFromViper initializes Options with properties from viper
@@ -234,6 +235,9 @@ func (opt *Options) InitFromViper(v *viper.Viper) {
 		cfg.initFromViper(v)
 	}
 	opt.SpanStoreWriteCacheTTL = v.GetDuration(opt.primary.namespace + suffixSpanStoreWriteCacheTTL)
+	opt.tagIndexBlacklist = stripWhiteSpace(v.GetString(opt.primary.namespace + suffixTagIndexBlacklist))
+	opt.tagIndexWhitelist = stripWhiteSpace(v.GetString(opt.primary.namespace + suffixTagIndexWhitelist))
+
 }
 
 func (cfg *namespaceConfig) initFromViper(v *viper.Viper) {
@@ -262,19 +266,11 @@ func (cfg *namespaceConfig) initFromViper(v *viper.Viper) {
 	cfg.TLS.EnableHostVerification = v.GetBool(cfg.namespace + suffixVerifyHost)
 	cfg.EnableDependenciesV2 = v.GetBool(cfg.namespace + suffixEnableDependenciesV2)
 	cfg.DisableCompression = v.GetBool(cfg.namespace + suffixDisableCompression)
-	cfg.tagIndexBlacklist = stripWhiteSpace(v.GetString(cfg.namespace + suffixTagIndexBlacklist))
-	cfg.tagIndexWhitelist = stripWhiteSpace(v.GetString(cfg.namespace + suffixTagIndexWhitelist))
 }
 
 // GetPrimary returns primary configuration.
 func (opt *Options) GetPrimary() *config.Configuration {
 	opt.primary.Servers = strings.Split(opt.primary.servers, ",")
-	if len(opt.primary.tagIndexBlacklist) > 0 {
-		opt.primary.TagIndexBlacklist = strings.Split(opt.primary.tagIndexBlacklist, ",")
-	}
-	if len(opt.primary.tagIndexWhitelist) > 0 {
-		opt.primary.TagIndexWhitelist = strings.Split(opt.primary.tagIndexWhitelist, ",")
-	}
 	return &opt.primary.Configuration
 }
 
@@ -293,19 +289,25 @@ func (opt *Options) Get(namespace string) *config.Configuration {
 		nsCfg.servers = opt.primary.servers
 	}
 	nsCfg.Servers = strings.Split(nsCfg.servers, ",")
-	if nsCfg.tagIndexBlacklist == "" {
-		nsCfg.tagIndexBlacklist = opt.primary.tagIndexBlacklist
-	}
-	if len(nsCfg.tagIndexBlacklist) > 0 {
-		nsCfg.TagIndexBlacklist = strings.Split(nsCfg.tagIndexBlacklist, ",")
-	}
-	if nsCfg.tagIndexWhitelist == "" {
-		nsCfg.tagIndexWhitelist = opt.primary.tagIndexWhitelist
+	return &nsCfg.Configuration
+}
+
+// GetTagIndexBlacklist returns the list of blacklisted tags
+func (opt *Options) GetTagIndexBlacklist() []string {
+	if len(opt.tagIndexBlacklist) > 0 {
+		return strings.Split(opt.tagIndexBlacklist, ",")
 	}
-	if len(nsCfg.tagIndexWhitelist) > 0 {
-		nsCfg.TagIndexWhitelist = strings.Split(nsCfg.tagIndexWhitelist, ",")
+
+	return []string{}
+}
+
+// GetTagIndexWhitelist returns the list of whitelisted tags
+func (opt *Options) GetTagIndexWhitelist() []string {
+	if len(opt.tagIndexWhitelist) > 0 {
+		return strings.Split(opt.tagIndexWhitelist, ",")
 	}
-	return &nsCfg.Configuration
+
+	return []string{}
 }
 
 // stripWhiteSpace removes all whitespace characters from a string

plugin/storage/cassandra/options_test.go

@@ -43,8 +43,6 @@ func TestOptions(t *testing.T) {
 	assert.Equal(t, primary.Servers, aux.Servers)
 	assert.Equal(t, primary.ConnectionsPerHost, aux.ConnectionsPerHost)
 	assert.Equal(t, primary.ReconnectInterval, aux.ReconnectInterval)
-	assert.Equal(t, primary.TagIndexBlacklist, aux.TagIndexBlacklist)
-	assert.Equal(t, primary.TagIndexWhitelist, aux.TagIndexWhitelist)
 }
 
 func TestOptionsWithFlags(t *testing.T) {
@@ -63,7 +61,7 @@ func TestOptionsWithFlags(t *testing.T) {
 		"--cas.proto-version=3",
 		"--cas.socket-keep-alive=42s",
 		"--cas.tag-index-blacklist=blerg, blarg,blorg ",
-		"--cas.tag-index-whitelist=blerg, blarg,blorg ",
+		"--cas.tag-index-whitelist=flerg, flarg,florg ",
 		// enable aux with a couple overrides
 		"--cas-aux.enabled=true",
 		"--cas-aux.keyspace=jaeger-archive",
@@ -79,8 +77,8 @@ func TestOptionsWithFlags(t *testing.T) {
 	assert.Equal(t, []string{"1.1.1.1", "2.2.2.2"}, primary.Servers)
 	assert.Equal(t, "ONE", primary.Consistency)
 	assert.Equal(t, false, primary.EnableDependenciesV2)
-	assert.Equal(t, []string{"blerg", "blarg", "blorg"}, primary.TagIndexBlacklist)
-	assert.Equal(t, []string{"blerg", "blarg", "blorg"}, primary.TagIndexWhitelist)
+	assert.Equal(t, []string{"blerg", "blarg", "blorg"}, opts.GetTagIndexBlacklist())
+	assert.Equal(t, []string{"flerg", "flarg", "florg"}, opts.GetTagIndexWhitelist())
 
 	aux := opts.Get("cas-aux")
 	require.NotNil(t, aux)
@@ -95,6 +93,14 @@ func TestOptionsWithFlags(t *testing.T) {
 	assert.Equal(t, 3, aux.ProtoVersion)
 	assert.Equal(t, 42*time.Second, aux.SocketKeepAlive)
 	assert.Equal(t, true, aux.EnableDependenciesV2)
-	assert.Equal(t, []string{"blerg", "blarg", "blorg"}, aux.TagIndexBlacklist)
-	assert.Equal(t, []string{"foo", "bar"}, aux.TagIndexWhitelist)
+}
+
+func TestEmptyBlackWhiteLists(t *testing.T) {
+	opts := NewOptions("cas")
+	v, command := config.Viperize(opts.AddFlags)
+	command.ParseFlags([]string{})
+	opts.InitFromViper(v)
+
+	assert.Equal(t, []string{}, opts.GetTagIndexBlacklist())
+	assert.Equal(t, []string{}, opts.GetTagIndexWhitelist())
 }