Remove downsample flags from the query service (#2782)

* First pass fix

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

* Added tests

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

* Use bool to track downsampling flags state

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

cmd/all-in-one/main.go

@@ -189,7 +189,7 @@ by default uses only in-memory database.`,
 		v,
 		command,
 		svc.AddFlags,
-		storageFactory.AddFlags,
+		storageFactory.AddPipelineFlags,
 		agentApp.AddFlags,
 		agentRep.AddFlags,
 		agentGrpcRep.AddFlags,

cmd/collector/main.go

@@ -130,7 +130,7 @@ func main() {
 		command,
 		svc.AddFlags,
 		app.AddFlags,
-		storageFactory.AddFlags,
+		storageFactory.AddPipelineFlags,
 		strategyStoreFactory.AddFlags,
 	)
 

cmd/ingester/main.go

@@ -106,7 +106,7 @@ func main() {
 		v,
 		command,
 		svc.AddFlags,
-		storageFactory.AddFlags,
+		storageFactory.AddPipelineFlags,
 		app.AddFlags,
 	)
 

plugin/storage/factory.go

@@ -60,8 +60,9 @@ var AllStorageTypes = []string{cassandraStorageType, elasticsearchStorageType, m
 // Factory implements storage.Factory interface as a meta-factory for storage components.
 type Factory struct {
 	FactoryConfig
-	metricsFactory metrics.Factory
-	factories      map[string]storage.Factory
+	metricsFactory         metrics.Factory
+	factories              map[string]storage.Factory
+	downsamplingFlagsAdded bool
 }
 
 // NewFactory creates the meta-factory.
@@ -173,11 +174,19 @@ func (f *Factory) AddFlags(flagSet *flag.FlagSet) {
 			conf.AddFlags(flagSet)
 		}
 	}
-	addDownsamplingFlags(flagSet)
+}
+
+// AddPipelineFlags adds all the standard flags as well as the downsampling
+//  flags.  This is intended to be used in Jaeger pipeline services such as
+//  the collector or ingester.
+func (f *Factory) AddPipelineFlags(flagSet *flag.FlagSet) {
+	f.AddFlags(flagSet)
+	f.addDownsamplingFlags(flagSet)
 }
 
 // addDownsamplingFlags add flags for Downsampling params
-func addDownsamplingFlags(flagSet *flag.FlagSet) {
+func (f *Factory) addDownsamplingFlags(flagSet *flag.FlagSet) {
+	f.downsamplingFlagsAdded = true
 	flagSet.Float64(
 		downsamplingRatio,
 		defaultDownsamplingRatio,
@@ -201,6 +210,14 @@ func (f *Factory) InitFromViper(v *viper.Viper) {
 }
 
 func (f *Factory) initDownsamplingFromViper(v *viper.Viper) {
+	// if the downsampling flag isn't set then this component used the standard "AddFlags" method
+	// and has no use for downsampling.  the default settings effectively disable downsampling
+	if !f.downsamplingFlagsAdded {
+		f.FactoryConfig.DownsamplingRatio = defaultDownsamplingRatio
+		f.FactoryConfig.DownsamplingHashSalt = defaultDownsamplingHashSalt
+		return
+	}
+
 	f.FactoryConfig.DownsamplingRatio = v.GetFloat64(downsamplingRatio)
 	if f.FactoryConfig.DownsamplingRatio < 0 || f.FactoryConfig.DownsamplingRatio > 1 {
 		// Values not in the range of 0 ~ 1.0 will be set to default.

plugin/storage/factory_test.go

@@ -336,7 +336,7 @@ func TestConfigurable(t *testing.T) {
 
 func TestParsingDownsamplingRatio(t *testing.T) {
 	f := Factory{}
-	v, command := config.Viperize(addDownsamplingFlags)
+	v, command := config.Viperize(f.AddPipelineFlags)
 	err := command.ParseFlags([]string{
 		"--downsampling.ratio=1.5",
 		"--downsampling.hashsalt=jaeger"})
@@ -353,6 +353,21 @@ func TestParsingDownsamplingRatio(t *testing.T) {
 	assert.Equal(t, f.FactoryConfig.DownsamplingRatio, 0.5)
 }
 
+func TestDefaultDownsamplingWithAddFlags(t *testing.T) {
+	f := Factory{}
+	v, command := config.Viperize(f.AddFlags)
+	err := command.ParseFlags([]string{})
+	assert.NoError(t, err)
+	f.InitFromViper(v)
+
+	assert.Equal(t, f.FactoryConfig.DownsamplingRatio, defaultDownsamplingRatio)
+	assert.Equal(t, f.FactoryConfig.DownsamplingHashSalt, defaultDownsamplingHashSalt)
+
+	err = command.ParseFlags([]string{
+		"--downsampling.ratio=0.5"})
+	assert.Error(t, err)
+}
+
 func TestPublishOpts(t *testing.T) {
 	f, err := NewFactory(defaultCfg())
 	require.NoError(t, err)