Skip to content

Commit

Permalink
Use an Options struct instead of function param
Browse files Browse the repository at this point in the history
Signed-off-by: Louis-Etienne Dorval <louis-etienne.dorval@ticketmaster.com>
  • Loading branch information
Louis-Etienne Dorval committed Dec 10, 2018
1 parent 4da81ba commit 80b6a82
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 20 deletions.
4 changes: 3 additions & 1 deletion cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ import (
zc "github.com/jaegertracing/jaeger/thrift-gen/zipkincore"
)

var unsupportedTypes = []string{storage.KafkaStorageType}

// all-in-one/main is a standalone full-stack jaeger backend, backed by a memory store
func main() {
var signalsChannel = make(chan os.Signal)
Expand All @@ -77,7 +79,7 @@ func main() {
os.Setenv(storage.SpanStorageTypeEnvVar, "memory") // other storage types default to SpanStorage
}
storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, os.Stderr),
[]string{storage.KafkaStorageType})
storage.Options{UnsupportedTypes: unsupportedTypes})
if err != nil {
log.Fatalf("Cannot initialize storage factory: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/collector/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func main() {
var signalsChannel = make(chan os.Signal)
signal.Notify(signalsChannel, os.Interrupt, syscall.SIGTERM)

storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, os.Stderr), nil)
storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, os.Stderr), storage.Options{})
if err != nil {
log.Fatalf("Cannot initialize storage factory: %v", err)
}
Expand Down
4 changes: 3 additions & 1 deletion cmd/ingester/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,14 @@ import (
"github.com/jaegertracing/jaeger/plugin/storage"
)

var unsupportedTypes = []string{storage.KafkaStorageType}

func main() {
var signalsChannel = make(chan os.Signal)
signal.Notify(signalsChannel, os.Interrupt, syscall.SIGTERM)

storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, os.Stderr),
[]string{storage.KafkaStorageType})
storage.Options{UnsupportedTypes: unsupportedTypes})
if err != nil {
log.Fatalf("Cannot initialize storage factory: %v", err)
}
Expand Down
4 changes: 3 additions & 1 deletion cmd/query/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@ import (
storageMetrics "github.com/jaegertracing/jaeger/storage/spanstore/metrics"
)

var unsupportedTypes = []string{storage.KafkaStorageType}

func main() {
var serverChannel = make(chan os.Signal)
signal.Notify(serverChannel, os.Interrupt, syscall.SIGTERM)

storageFactory, err := storage.NewFactory(storage.FactoryConfigFromEnvAndCLI(os.Args, os.Stderr),
[]string{storage.KafkaStorageType})
storage.Options{UnsupportedTypes: unsupportedTypes})
if err != nil {
log.Fatalf("Cannot initialize storage factory: %v", err)
}
Expand Down
11 changes: 5 additions & 6 deletions plugin/storage/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,13 @@ var allStorageTypes = []string{CassandraStorageType, ElasticsearchStorageType, M
// Factory implements storage.Factory interface as a meta-factory for storage components.
type Factory struct {
FactoryConfig

unsupportedTypes []string
factories map[string]storage.Factory
Options
factories map[string]storage.Factory
}

// NewFactory creates the meta-factory.
func NewFactory(config FactoryConfig, unsupportedTypes []string) (*Factory, error) {
f := &Factory{FactoryConfig: config, unsupportedTypes: unsupportedTypes}
func NewFactory(config FactoryConfig, opts Options) (*Factory, error) {
f := &Factory{FactoryConfig: config, Options: opts}
uniqueTypes := map[string]struct{}{
f.SpanReaderType: {},
f.DependenciesStorageType: {},
Expand All @@ -78,7 +77,7 @@ func NewFactory(config FactoryConfig, unsupportedTypes []string) (*Factory, erro
}

func (f *Factory) isUnsupportedType(factoryType string) bool {
for _, t := range f.unsupportedTypes {
for _, t := range f.Options.UnsupportedTypes {
if t == factoryType {
return true
}
Expand Down
20 changes: 10 additions & 10 deletions plugin/storage/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func defaultCfg() FactoryConfig {
}

func TestNewFactory(t *testing.T) {
f, err := NewFactory(defaultCfg(), nil)
f, err := NewFactory(defaultCfg(), Options{})
require.NoError(t, err)
assert.NotEmpty(t, f.factories)
assert.NotEmpty(t, f.factories[CassandraStorageType])
Expand All @@ -56,7 +56,7 @@ func TestNewFactory(t *testing.T) {
SpanWriterTypes: []string{CassandraStorageType, KafkaStorageType},
SpanReaderType: ElasticsearchStorageType,
DependenciesStorageType: MemoryStorageType,
}, nil)
}, Options{})
require.NoError(t, err)
assert.NotEmpty(t, f.factories)
assert.NotEmpty(t, f.factories[CassandraStorageType])
Expand All @@ -67,20 +67,20 @@ func TestNewFactory(t *testing.T) {
assert.Equal(t, ElasticsearchStorageType, f.SpanReaderType)
assert.Equal(t, MemoryStorageType, f.DependenciesStorageType)

f, err = NewFactory(FactoryConfig{SpanWriterTypes: []string{"x"}, DependenciesStorageType: "y", SpanReaderType: "z"}, nil)
f, err = NewFactory(FactoryConfig{SpanWriterTypes: []string{"x"}, DependenciesStorageType: "y", SpanReaderType: "z"}, Options{})
require.Error(t, err)
expected := "Unknown storage type" // could be 'x' or 'y' since code iterates through map.
assert.Equal(t, expected, err.Error()[0:len(expected)])
}

func TestNewFactoryWithUnsupportedType(t *testing.T) {
_, err := NewFactory(defaultCfg(), []string{CassandraStorageType})
_, err := NewFactory(defaultCfg(), Options{UnsupportedTypes: []string{CassandraStorageType}})

assert.EqualError(t, err, "The cassandra storage type is unsupported by this command")
}

func TestInitialize(t *testing.T) {
f, err := NewFactory(defaultCfg(), nil)
f, err := NewFactory(defaultCfg(), Options{})
require.NoError(t, err)
assert.NotEmpty(t, f.factories)
assert.NotEmpty(t, f.factories[CassandraStorageType])
Expand All @@ -100,7 +100,7 @@ func TestInitialize(t *testing.T) {
}

func TestCreate(t *testing.T) {
f, err := NewFactory(defaultCfg(), nil)
f, err := NewFactory(defaultCfg(), Options{})
require.NoError(t, err)
assert.NotEmpty(t, f.factories)
assert.NotEmpty(t, f.factories[CassandraStorageType])
Expand Down Expand Up @@ -143,7 +143,7 @@ func TestCreate(t *testing.T) {
func TestCreateMulti(t *testing.T) {
cfg := defaultCfg()
cfg.SpanWriterTypes = append(cfg.SpanWriterTypes, ElasticsearchStorageType)
f, err := NewFactory(cfg, nil)
f, err := NewFactory(cfg, Options{})
require.NoError(t, err)

mock := new(mocks.Factory)
Expand All @@ -168,7 +168,7 @@ func TestCreateMulti(t *testing.T) {
}

func TestCreateArchive(t *testing.T) {
f, err := NewFactory(defaultCfg(), nil)
f, err := NewFactory(defaultCfg(), Options{})
require.NoError(t, err)
assert.NotEmpty(t, f.factories[CassandraStorageType])

Expand All @@ -194,7 +194,7 @@ func TestCreateArchive(t *testing.T) {
}

func TestCreateError(t *testing.T) {
f, err := NewFactory(defaultCfg(), nil)
f, err := NewFactory(defaultCfg(), Options{})
require.NoError(t, err)
assert.NotEmpty(t, f.factories)
assert.NotEmpty(t, f.factories[CassandraStorageType])
Expand Down Expand Up @@ -253,7 +253,7 @@ func TestConfigurable(t *testing.T) {
clearEnv()
defer clearEnv()

f, err := NewFactory(defaultCfg(), nil)
f, err := NewFactory(defaultCfg(), Options{})
require.NoError(t, err)
assert.NotEmpty(t, f.factories)
assert.NotEmpty(t, f.factories[CassandraStorageType])
Expand Down
21 changes: 21 additions & 0 deletions plugin/storage/options.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) 2018 The Jaeger Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package storage

// Options allow the different commands to control certain behavior of storage.Factory
type Options struct {
// UnsupportedTypes prevents the usage of the provided storage types
UnsupportedTypes []string
}

0 comments on commit 80b6a82

Please sign in to comment.