Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove downsample flags from the query service #2782

Merged
merged 3 commits into from
Feb 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion cmd/collector/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func main() {
command,
svc.AddFlags,
app.AddFlags,
storageFactory.AddFlags,
storageFactory.AddPipelineFlags,
strategyStoreFactory.AddFlags,
)

Expand Down
2 changes: 1 addition & 1 deletion cmd/ingester/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func main() {
v,
command,
svc.AddFlags,
storageFactory.AddFlags,
storageFactory.AddPipelineFlags,
app.AddFlags,
)

Expand Down
25 changes: 21 additions & 4 deletions plugin/storage/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down
17 changes: 16 additions & 1 deletion plugin/storage/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
Expand All @@ -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)
Expand Down