From 5eb2eca0bd02b3fa83491a3730401b1bda3656d1 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 27 Apr 2021 14:32:05 -0400 Subject: [PATCH 01/40] Original PR Signed-off-by: Joe Elliott Co-authored-by: Ashmita Bohara ashmita.bohara152@gmail.com --- cmd/all-in-one/main.go | 6 ++- cmd/collector/main.go | 6 ++- .../strategystore/adaptive/constants.go | 36 +++++++++++++++ .../strategystore/adaptive/factory.go | 13 ++++-- .../strategystore/adaptive/options.go | 2 +- .../strategystore/adaptive/processor.go | 44 ++++++++++++++++++- .../strategystore/adaptive/processor_test.go | 12 +++-- plugin/sampling/strategystore/factory.go | 19 ++++---- .../sampling/strategystore/factory_config.go | 23 ++++++---- .../strategystore/factory_config_test.go | 8 ++-- plugin/sampling/strategystore/factory_test.go | 14 +++--- 11 files changed, 140 insertions(+), 43 deletions(-) create mode 100644 plugin/sampling/strategystore/adaptive/constants.go diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 4433194cf47..afc27825101 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -66,7 +66,11 @@ func main() { if err != nil { log.Fatalf("Cannot initialize storage factory: %v", err) } - strategyStoreFactory, err := ss.NewFactory(ss.FactoryConfigFromEnv()) + strategyStoreFactoryConfig, err := ss.FactoryConfigFromEnv() + if err != nil { + log.Fatalf("Cannot initialize sampling strategy store factory config: %v", err) + } + strategyStoreFactory, err := ss.NewFactory(*strategyStoreFactoryConfig) if err != nil { log.Fatalf("Cannot initialize sampling strategy store factory: %v", err) } diff --git a/cmd/collector/main.go b/cmd/collector/main.go index 87d24ba5b9a..06b48618a75 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -50,7 +50,11 @@ func main() { if err != nil { log.Fatalf("Cannot initialize storage factory: %v", err) } - strategyStoreFactory, err := ss.NewFactory(ss.FactoryConfigFromEnv()) + strategyStoreFactoryConfig, err := ss.FactoryConfigFromEnv() + if err != nil { + log.Fatalf("Cannot initialize sampling strategy store factory config: %v", err) + } + strategyStoreFactory, err := ss.NewFactory(*strategyStoreFactoryConfig) if err != nil { log.Fatalf("Cannot initialize sampling strategy store factory: %v", err) } diff --git a/plugin/sampling/strategystore/adaptive/constants.go b/plugin/sampling/strategystore/adaptive/constants.go new file mode 100644 index 00000000000..474e3bc0ebe --- /dev/null +++ b/plugin/sampling/strategystore/adaptive/constants.go @@ -0,0 +1,36 @@ +// 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 adaptive + +import ( + "github.com/jaegertracing/jaeger/thrift-gen/sampling" +) + +const ( + // defaultSamplingProbability is the default sampling probability the + // Strategy Store will use if none is provided. + defaultSamplingProbability = 0.001 +) + +// defaultStrategy is the default sampling strategy the Strategy Store will return +// if none is provided. +func defaultStrategyResponse() *sampling.SamplingStrategyResponse { + return &sampling.SamplingStrategyResponse{ + StrategyType: sampling.SamplingStrategyType_PROBABILISTIC, + ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ + SamplingRate: defaultSamplingProbability, + }, + } +} diff --git a/plugin/sampling/strategystore/adaptive/factory.go b/plugin/sampling/strategystore/adaptive/factory.go index 70c967337fc..976dfd04e9d 100644 --- a/plugin/sampling/strategystore/adaptive/factory.go +++ b/plugin/sampling/strategystore/adaptive/factory.go @@ -26,7 +26,7 @@ import ( // Factory implements strategystore.Factory for an adaptive strategy store. type Factory struct { - options Options + options *Options logger *zap.Logger metricsFactory metrics.Factory } @@ -34,6 +34,7 @@ type Factory struct { // NewFactory creates a new Factory. func NewFactory() *Factory { return &Factory{ + options: &Options{}, logger: zap.NewNop(), metricsFactory: metrics.NullFactory, } @@ -46,7 +47,7 @@ func (f *Factory) AddFlags(flagSet *flag.FlagSet) { // InitFromViper implements plugin.Configurable func (f *Factory) InitFromViper(v *viper.Viper) { - f.options = Options{}.InitFromViper(v) + f.options.InitFromViper(v) } // Initialize implements strategystore.Factory @@ -58,6 +59,10 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) // CreateStrategyStore implements strategystore.Factory func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, error) { - // TODO - return nil, nil + p, err := NewStrategyStore(*f.options, f.logger) + if err != nil { + return nil, err + } + p.Start() + return p, nil } diff --git a/plugin/sampling/strategystore/adaptive/options.go b/plugin/sampling/strategystore/adaptive/options.go index ee5272950d4..f1bc2eddc6a 100644 --- a/plugin/sampling/strategystore/adaptive/options.go +++ b/plugin/sampling/strategystore/adaptive/options.go @@ -151,7 +151,7 @@ func AddFlags(flagSet *flag.FlagSet) { } // InitFromViper initializes Options with properties from viper -func (opts Options) InitFromViper(v *viper.Viper) Options { +func (opts *Options) InitFromViper(v *viper.Viper) *Options { opts.TargetSamplesPerSecond = v.GetFloat64(targetSamplesPerSecond) opts.DeltaTolerance = v.GetFloat64(deltaTolerance) opts.BucketsForCalculation = v.GetInt(bucketsForCalculation) diff --git a/plugin/sampling/strategystore/adaptive/processor.go b/plugin/sampling/strategystore/adaptive/processor.go index b644681b2c9..228cc51fad1 100644 --- a/plugin/sampling/strategystore/adaptive/processor.go +++ b/plugin/sampling/strategystore/adaptive/processor.go @@ -20,6 +20,7 @@ import ( "io" "math" "math/rand" + "os" "sync" "time" @@ -27,7 +28,6 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" - ss "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" "github.com/jaegertracing/jaeger/plugin/sampling/calculationstrategy" "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection" "github.com/jaegertracing/jaeger/storage/samplingstore" @@ -124,7 +124,7 @@ func NewProcessor( electionParticipant leaderelection.ElectionParticipant, metricsFactory metrics.Factory, logger *zap.Logger, -) (ss.StrategyStore, error) { +) (*processor, error) { if opts.CalculationInterval == 0 || opts.AggregationBuckets == 0 { return nil, errNonZero } @@ -517,3 +517,43 @@ func (p *processor) generateDefaultSamplingStrategyResponse() *sampling.Sampling }, } } + +type strategyStore struct { + logger *zap.Logger + processor *processor + ctx context.Context + cancelFunc context.CancelFunc +} + +type storedStrategies struct { + defaultStrategy *sampling.SamplingStrategyResponse + serviceStrategies map[string]*sampling.SamplingStrategyResponse +} + +// GetSamplingStrategy implements StrategyStore#GetSamplingStrategy. +func (h *strategyStore) GetSamplingStrategy(_ context.Context, serviceName string) (*sampling.SamplingStrategyResponse, error) { + serviceStrategies := h.processor.strategyResponses + if strategy, ok := serviceStrategies[serviceName]; ok { + return strategy, nil + } + h.logger.Debug("sampling strategy not found, using default", zap.String("service", serviceName)) + return defaultStrategyResponse(), nil +} + +// NewStrategyStore creates a strategy store that holds adaptive sampling strategies. +func NewStrategyStore(options Options, logger *zap.Logger) (*processor, error) { + // ctx, _ := context.WithCancel(context.Background()) + + hostname, err := os.Hostname() + if err != nil { + return nil, err + } + + // How to initialize storage, electionParticipant, metricsFactory for the NewProcessor ? + p, err := NewProcessor(options, hostname, nil, nil, nil, logger) + if err != nil { + return nil, err + } + + return p, nil +} diff --git a/plugin/sampling/strategystore/adaptive/processor_test.go b/plugin/sampling/strategystore/adaptive/processor_test.go index f6ebd10604a..cc24c66348e 100644 --- a/plugin/sampling/strategystore/adaptive/processor_test.go +++ b/plugin/sampling/strategystore/adaptive/processor_test.go @@ -17,7 +17,6 @@ package adaptive import ( "context" "errors" - "io" "testing" "time" @@ -347,7 +346,7 @@ func TestRunCalculationLoop(t *testing.T) { } p, err := NewProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, logger) require.NoError(t, err) - p.(*processor).Start() + p.Start() for i := 0; i < 1000; i++ { strategy, _ := p.GetSamplingStrategy(context.Background(), "svcA") @@ -356,7 +355,7 @@ func TestRunCalculationLoop(t *testing.T) { } time.Sleep(time.Millisecond) } - p.(*processor).Close() + p.Close() strategy, err := p.GetSamplingStrategy(context.Background(), "svcA") assert.NoError(t, err) @@ -378,9 +377,8 @@ func TestRunCalculationLoop_GetThroughputError(t *testing.T) { AggregationBuckets: 2, BucketsForCalculation: 10, } - proc, err := NewProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, logger) + p, err := NewProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, logger) require.NoError(t, err) - p := proc.(*processor) p.shutdown = make(chan struct{}) defer close(p.shutdown) go p.runCalculationLoop() @@ -470,7 +468,7 @@ func TestRealisticRunCalculationLoop(t *testing.T) { } p, err := NewProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, logger) require.NoError(t, err) - p.(*processor).Start() + p.Start() for i := 0; i < 100; i++ { strategy, _ := p.GetSamplingStrategy(context.Background(), "svcA") @@ -479,7 +477,7 @@ func TestRealisticRunCalculationLoop(t *testing.T) { } time.Sleep(250 * time.Millisecond) } - p.(io.Closer).Close() + p.Close() strategy, err := p.GetSamplingStrategy(context.Background(), "svcA") assert.NoError(t, err) diff --git a/plugin/sampling/strategystore/factory.go b/plugin/sampling/strategystore/factory.go index c2a0a9fa50c..782b0066cb5 100644 --- a/plugin/sampling/strategystore/factory.go +++ b/plugin/sampling/strategystore/factory.go @@ -24,29 +24,28 @@ import ( "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" "github.com/jaegertracing/jaeger/plugin" + "github.com/jaegertracing/jaeger/plugin/sampling/strategystore/adaptive" "github.com/jaegertracing/jaeger/plugin/sampling/strategystore/static" ) -const ( - staticStrategyStoreType = "static" -) +type StrategyStoreType string -var allSamplingTypes = []string{staticStrategyStoreType} // TODO support adaptive +var allSamplingTypes = []StrategyStoreType{"static", "adaptive"} // Factory implements strategystore.Factory interface as a meta-factory for strategy storage components. type Factory struct { FactoryConfig - factories map[string]strategystore.Factory + factories map[StrategyStoreType]strategystore.Factory } // NewFactory creates the meta-factory. func NewFactory(config FactoryConfig) (*Factory, error) { f := &Factory{FactoryConfig: config} - uniqueTypes := map[string]struct{}{ + uniqueTypes := map[StrategyStoreType]struct{}{ f.StrategyStoreType: {}, } - f.factories = make(map[string]strategystore.Factory) + f.factories = make(map[StrategyStoreType]strategystore.Factory) for t := range uniqueTypes { ff, err := f.getFactoryOfType(t) if err != nil { @@ -57,10 +56,12 @@ func NewFactory(config FactoryConfig) (*Factory, error) { return f, nil } -func (f *Factory) getFactoryOfType(factoryType string) (strategystore.Factory, error) { +func (f *Factory) getFactoryOfType(factoryType StrategyStoreType) (strategystore.Factory, error) { switch factoryType { - case staticStrategyStoreType: + case "static": return static.NewFactory(), nil + case "adaptive": + return adaptive.NewFactory(), nil default: return nil, fmt.Errorf("unknown sampling strategy store type %s. Valid types are %v", factoryType, allSamplingTypes) } diff --git a/plugin/sampling/strategystore/factory_config.go b/plugin/sampling/strategystore/factory_config.go index e04450848ed..a58f3240b8d 100644 --- a/plugin/sampling/strategystore/factory_config.go +++ b/plugin/sampling/strategystore/factory_config.go @@ -15,6 +15,7 @@ package strategystore import ( + "fmt" "os" ) @@ -25,18 +26,24 @@ const ( // FactoryConfig tells the Factory what sampling type it needs to create. type FactoryConfig struct { - StrategyStoreType string + StrategyStoreType StrategyStoreType } // FactoryConfigFromEnv reads the desired sampling type from the SAMPLING_TYPE environment variable. Allowed values: // * `static` - built-in -// * `adaptive` - built-in // TODO -func FactoryConfigFromEnv() FactoryConfig { - strategyStoreType := os.Getenv(SamplingTypeEnvVar) - if strategyStoreType == "" { - strategyStoreType = staticStrategyStoreType +// * `adaptive` - built-in +func FactoryConfigFromEnv() (*FactoryConfig, error) { + var ( + strategyStoreType string + ok bool + ) + if strategyStoreType, ok = os.LookupEnv(SamplingTypeEnvVar); !ok { + strategyStoreType = "static" } - return FactoryConfig{ - StrategyStoreType: strategyStoreType, + if strategyStoreType != "adaptive" && strategyStoreType != "static" { + return nil, fmt.Errorf("invalid sampling type: %s", strategyStoreType) } + return &FactoryConfig{ + StrategyStoreType: StrategyStoreType(strategyStoreType), + }, nil } diff --git a/plugin/sampling/strategystore/factory_config_test.go b/plugin/sampling/strategystore/factory_config_test.go index 409573d5626..2ef4559faf5 100644 --- a/plugin/sampling/strategystore/factory_config_test.go +++ b/plugin/sampling/strategystore/factory_config_test.go @@ -20,16 +20,18 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func clearEnv() { - os.Setenv(SamplingTypeEnvVar, "") + os.Setenv(SamplingTypeEnvVar, "static") } func TestFactoryConfigFromEnv(t *testing.T) { clearEnv() defer clearEnv() - f := FactoryConfigFromEnv() - assert.Equal(t, staticStrategyStoreType, f.StrategyStoreType) + f, err := FactoryConfigFromEnv() + require.NoError(t, err) + assert.Equal(t, StrategyStoreType("static"), f.StrategyStoreType) } diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index 9425ba337ce..e4976c1454e 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -34,14 +34,14 @@ var _ ss.Factory = new(Factory) var _ plugin.Configurable = new(Factory) func TestNewFactory(t *testing.T) { - f, err := NewFactory(FactoryConfig{StrategyStoreType: staticStrategyStoreType}) + f, err := NewFactory(FactoryConfig{StrategyStoreType: "static"}) require.NoError(t, err) assert.NotEmpty(t, f.factories) - assert.NotEmpty(t, f.factories[staticStrategyStoreType]) - assert.Equal(t, staticStrategyStoreType, f.StrategyStoreType) + assert.NotEmpty(t, f.factories["static"]) + assert.Equal(t, StrategyStoreType("static"), f.StrategyStoreType) mock := new(mockFactory) - f.factories[staticStrategyStoreType] = mock + f.factories["static"] = mock assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) _, err = f.CreateStrategyStore() @@ -66,13 +66,13 @@ func TestConfigurable(t *testing.T) { clearEnv() defer clearEnv() - f, err := NewFactory(FactoryConfig{StrategyStoreType: staticStrategyStoreType}) + f, err := NewFactory(FactoryConfig{StrategyStoreType: "static"}) require.NoError(t, err) assert.NotEmpty(t, f.factories) - assert.NotEmpty(t, f.factories[staticStrategyStoreType]) + assert.NotEmpty(t, f.factories["static"]) mock := new(mockFactory) - f.factories[staticStrategyStoreType] = mock + f.factories["static"] = mock fs := new(flag.FlagSet) v := viper.New() From 42aedd3fd7365b92dcad578f554c28349e397d5d Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 27 Apr 2021 14:56:41 -0400 Subject: [PATCH 02/40] cleaned up tests and config factory logic Signed-off-by: Joe Elliott --- .../sampling/strategystore/factory_config.go | 7 +-- .../strategystore/factory_config_test.go | 43 +++++++++++++++---- plugin/sampling/strategystore/factory_test.go | 5 +++ 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/plugin/sampling/strategystore/factory_config.go b/plugin/sampling/strategystore/factory_config.go index a58f3240b8d..18f7a8cfb1c 100644 --- a/plugin/sampling/strategystore/factory_config.go +++ b/plugin/sampling/strategystore/factory_config.go @@ -33,11 +33,8 @@ type FactoryConfig struct { // * `static` - built-in // * `adaptive` - built-in func FactoryConfigFromEnv() (*FactoryConfig, error) { - var ( - strategyStoreType string - ok bool - ) - if strategyStoreType, ok = os.LookupEnv(SamplingTypeEnvVar); !ok { + strategyStoreType := os.Getenv(SamplingTypeEnvVar) + if strategyStoreType == "" { strategyStoreType = "static" } if strategyStoreType != "adaptive" && strategyStoreType != "static" { diff --git a/plugin/sampling/strategystore/factory_config_test.go b/plugin/sampling/strategystore/factory_config_test.go index 2ef4559faf5..4462b46f5bd 100644 --- a/plugin/sampling/strategystore/factory_config_test.go +++ b/plugin/sampling/strategystore/factory_config_test.go @@ -23,15 +23,40 @@ import ( "github.com/stretchr/testify/require" ) -func clearEnv() { - os.Setenv(SamplingTypeEnvVar, "static") -} - func TestFactoryConfigFromEnv(t *testing.T) { - clearEnv() - defer clearEnv() + tests := []struct { + env string + expectedType StrategyStoreType + expectsError bool + }{ + { + expectedType: StrategyStoreType("static"), + }, + { + env: "static", + expectedType: StrategyStoreType("static"), + }, + { + env: "adaptive", + expectedType: StrategyStoreType("adaptive"), + }, + { + env: "??", + expectsError: true, + }, + } + + for _, tc := range tests { + err := os.Setenv(SamplingTypeEnvVar, tc.env) + require.NoError(t, err) + + f, err := FactoryConfigFromEnv() + if tc.expectsError { + assert.Error(t, err) + continue + } - f, err := FactoryConfigFromEnv() - require.NoError(t, err) - assert.Equal(t, StrategyStoreType("static"), f.StrategyStoreType) + require.NoError(t, err) + assert.Equal(t, tc.expectedType, f.StrategyStoreType) + } } diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index e4976c1454e..609a27b53fc 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -18,6 +18,7 @@ package strategystore import ( "errors" "flag" + "os" "testing" "github.com/spf13/viper" @@ -30,6 +31,10 @@ import ( "github.com/jaegertracing/jaeger/plugin" ) +func clearEnv() { + os.Setenv(SamplingTypeEnvVar, "static") +} + var _ ss.Factory = new(Factory) var _ plugin.Configurable = new(Factory) From 81136864d6e21f4651bb8b4fe38ac17980b0812e Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 28 Apr 2021 14:46:57 -0400 Subject: [PATCH 03/40] Wire up lock/samplingstore Signed-off-by: Joe Elliott --- cmd/all-in-one/main.go | 6 ++- .../app/sampling/strategystore/factory.go | 4 +- cmd/collector/main.go | 6 ++- cmd/query/app/querysvc/query_service_test.go | 5 +++ .../strategystore/adaptive/factory.go | 17 +++++++- .../strategystore/adaptive/factory_test.go | 31 +++++++++++++- .../strategystore/adaptive/processor.go | 12 +++--- plugin/sampling/strategystore/factory.go | 6 ++- plugin/sampling/strategystore/factory_test.go | 41 +++++++++++++++++-- .../sampling/strategystore/static/factory.go | 4 +- .../strategystore/static/factory_test.go | 2 +- plugin/storage/badger/factory.go | 8 ++++ plugin/storage/cassandra/factory.go | 13 ++++++ plugin/storage/es/factory.go | 7 ++++ plugin/storage/factory.go | 19 +++++++++ plugin/storage/factory_test.go | 6 +++ plugin/storage/grpc/factory.go | 7 ++++ plugin/storage/kafka/factory.go | 8 ++++ plugin/storage/memory/factory.go | 8 ++++ storage/factory.go | 9 ++++ storage/mocks/Factory.go | 25 ++++++++--- 21 files changed, 219 insertions(+), 25 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index afc27825101..e6a3bb5975f 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -110,9 +110,13 @@ by default uses only in-memory database.`, if err != nil { logger.Fatal("Failed to create dependency reader", zap.Error(err)) } + lock, samplingStore, err := storageFactory.CreateLockAndSamplingStore() + if err != nil { + logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) + } strategyStoreFactory.InitFromViper(v) - if err := strategyStoreFactory.Initialize(metricsFactory, logger); err != nil { + if err := strategyStoreFactory.Initialize(metricsFactory, logger, lock, samplingStore); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) } strategyStore, err := strategyStoreFactory.CreateStrategyStore() diff --git a/cmd/collector/app/sampling/strategystore/factory.go b/cmd/collector/app/sampling/strategystore/factory.go index 6edb90df13a..245028565b5 100644 --- a/cmd/collector/app/sampling/strategystore/factory.go +++ b/cmd/collector/app/sampling/strategystore/factory.go @@ -15,6 +15,8 @@ package strategystore import ( + "github.com/jaegertracing/jaeger/pkg/distributedlock" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" ) @@ -27,7 +29,7 @@ import ( // plugin.Configurable type Factory interface { // Initialize performs internal initialization of the factory. - Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error + Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error // CreateStrategyStore initializes the StrategyStore and returns it. CreateStrategyStore() (StrategyStore, error) diff --git a/cmd/collector/main.go b/cmd/collector/main.go index 06b48618a75..f063b98280d 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -82,9 +82,13 @@ func main() { if err != nil { logger.Fatal("Failed to create span writer", zap.Error(err)) } + lock, samplingStore, err := storageFactory.CreateLockAndSamplingStore() + if err != nil { + logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) + } strategyStoreFactory.InitFromViper(v) - if err := strategyStoreFactory.Initialize(metricsFactory, logger); err != nil { + if err := strategyStoreFactory.Initialize(metricsFactory, logger, lock, samplingStore); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) } strategyStore, err := strategyStoreFactory.CreateStrategyStore() diff --git a/cmd/query/app/querysvc/query_service_test.go b/cmd/query/app/querysvc/query_service_test.go index baeb08e4bb4..016376252f6 100644 --- a/cmd/query/app/querysvc/query_service_test.go +++ b/cmd/query/app/querysvc/query_service_test.go @@ -27,9 +27,11 @@ import ( "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/model/adjuster" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" depsmocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) @@ -281,6 +283,9 @@ func (*fakeStorageFactory1) Initialize(metricsFactory metrics.Factory, logger *z func (*fakeStorageFactory1) CreateSpanReader() (spanstore.Reader, error) { return nil, nil } func (*fakeStorageFactory1) CreateSpanWriter() (spanstore.Writer, error) { return nil, nil } func (*fakeStorageFactory1) CreateDependencyReader() (dependencystore.Reader, error) { return nil, nil } +func (*fakeStorageFactory1) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + return nil, nil, nil +} func (f *fakeStorageFactory2) CreateArchiveSpanReader() (spanstore.Reader, error) { return f.r, f.rErr } func (f *fakeStorageFactory2) CreateArchiveSpanWriter() (spanstore.Writer, error) { return f.w, f.wErr } diff --git a/plugin/sampling/strategystore/adaptive/factory.go b/plugin/sampling/strategystore/adaptive/factory.go index 976dfd04e9d..fdb4025e0b7 100644 --- a/plugin/sampling/strategystore/adaptive/factory.go +++ b/plugin/sampling/strategystore/adaptive/factory.go @@ -15,6 +15,7 @@ package adaptive import ( + "errors" "flag" "github.com/spf13/viper" @@ -22,6 +23,8 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" + "github.com/jaegertracing/jaeger/pkg/distributedlock" + "github.com/jaegertracing/jaeger/storage/samplingstore" ) // Factory implements strategystore.Factory for an adaptive strategy store. @@ -29,6 +32,8 @@ type Factory struct { options *Options logger *zap.Logger metricsFactory metrics.Factory + lock distributedlock.Lock + store samplingstore.Store } // NewFactory creates a new Factory. @@ -37,6 +42,8 @@ func NewFactory() *Factory { options: &Options{}, logger: zap.NewNop(), metricsFactory: metrics.NullFactory, + lock: nil, + store: nil, } } @@ -51,15 +58,21 @@ func (f *Factory) InitFromViper(v *viper.Viper) { } // Initialize implements strategystore.Factory -func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { +func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error { + if lock == nil || store == nil { + return errors.New("lock or samplingStore nil. adaptive sampling only supported with Cassandra backend") // todo(jpe): better check/error msg + } + f.logger = logger f.metricsFactory = metricsFactory + f.lock = lock + f.store = store return nil } // CreateStrategyStore implements strategystore.Factory func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, error) { - p, err := NewStrategyStore(*f.options, f.logger) + p, err := NewStrategyStore(*f.options, f.metricsFactory, f.logger, f.lock, f.store) if err != nil { return nil, err } diff --git a/plugin/sampling/strategystore/adaptive/factory_test.go b/plugin/sampling/strategystore/adaptive/factory_test.go index 72eac3b1ecf..ada2a736b0a 100644 --- a/plugin/sampling/strategystore/adaptive/factory_test.go +++ b/plugin/sampling/strategystore/adaptive/factory_test.go @@ -22,6 +22,7 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" ss "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" "github.com/jaegertracing/jaeger/pkg/config" "github.com/jaegertracing/jaeger/plugin" @@ -61,7 +62,35 @@ func TestFactory(t *testing.T) { assert.Equal(t, time.Second, f.options.LeaderLeaseRefreshInterval) assert.Equal(t, time.Second*2, f.options.FollowerLeaseRefreshInterval) - assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) + assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), &mockLock{}, &mockStore{})) _, err := f.CreateStrategyStore() assert.NoError(t, err) } + +type mockStore struct{} + +func (m *mockStore) InsertThroughput(throughput []*model.Throughput) error { + return nil +} +func (m *mockStore) InsertProbabilitiesAndQPS(hostname string, probabilities model.ServiceOperationProbabilities, qps model.ServiceOperationQPS) error { + return nil +} +func (m *mockStore) GetThroughput(start, end time.Time) ([]*model.Throughput, error) { + return nil, nil +} +func (m *mockStore) GetProbabilitiesAndQPS(start, end time.Time) (map[string][]model.ServiceOperationData, error) { + return nil, nil +} +func (m *mockStore) GetLatestProbabilities() (model.ServiceOperationProbabilities, error) { + return nil, nil +} + +type mockLock struct{} + +func (m *mockLock) Acquire(resource string, ttl time.Duration) (acquired bool, err error) { + return true, nil +} + +func (m *mockLock) Forfeit(resource string) (forfeited bool, err error) { + return true, nil +} diff --git a/plugin/sampling/strategystore/adaptive/processor.go b/plugin/sampling/strategystore/adaptive/processor.go index 228cc51fad1..6d4576f3d8c 100644 --- a/plugin/sampling/strategystore/adaptive/processor.go +++ b/plugin/sampling/strategystore/adaptive/processor.go @@ -28,6 +28,7 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/plugin/sampling/calculationstrategy" "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection" "github.com/jaegertracing/jaeger/storage/samplingstore" @@ -43,6 +44,8 @@ const ( // The number of past entries for samplingCache the leader keeps in memory serviceCacheSize = 25 + + defaultResourceName = "sampling_store_leader" ) var ( @@ -521,7 +524,6 @@ func (p *processor) generateDefaultSamplingStrategyResponse() *sampling.Sampling type strategyStore struct { logger *zap.Logger processor *processor - ctx context.Context cancelFunc context.CancelFunc } @@ -541,16 +543,14 @@ func (h *strategyStore) GetSamplingStrategy(_ context.Context, serviceName strin } // NewStrategyStore creates a strategy store that holds adaptive sampling strategies. -func NewStrategyStore(options Options, logger *zap.Logger) (*processor, error) { - // ctx, _ := context.WithCancel(context.Background()) - +func NewStrategyStore(options Options, metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) (*processor, error) { hostname, err := os.Hostname() if err != nil { return nil, err } - // How to initialize storage, electionParticipant, metricsFactory for the NewProcessor ? - p, err := NewProcessor(options, hostname, nil, nil, nil, logger) + participant := leaderelection.NewElectionParticipant(lock, defaultResourceName, leaderelection.ElectionParticipantOptions{}) // todo(jpe) : wire up options/resource name + p, err := NewProcessor(options, hostname, store, participant, metricsFactory, logger) if err != nil { return nil, err } diff --git a/plugin/sampling/strategystore/factory.go b/plugin/sampling/strategystore/factory.go index 782b0066cb5..06959bdbbcc 100644 --- a/plugin/sampling/strategystore/factory.go +++ b/plugin/sampling/strategystore/factory.go @@ -23,9 +23,11 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/plugin" "github.com/jaegertracing/jaeger/plugin/sampling/strategystore/adaptive" "github.com/jaegertracing/jaeger/plugin/sampling/strategystore/static" + "github.com/jaegertracing/jaeger/storage/samplingstore" ) type StrategyStoreType string @@ -86,9 +88,9 @@ func (f *Factory) InitFromViper(v *viper.Viper) { } // Initialize implements strategystore.Factory -func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { +func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error { for _, factory := range f.factories { - if err := factory.Initialize(metricsFactory, logger); err != nil { + if err := factory.Initialize(metricsFactory, logger, lock, store); err != nil { return err } } diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index 609a27b53fc..ce64d58a29f 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -20,6 +20,7 @@ import ( "flag" "os" "testing" + "time" "github.com/spf13/viper" "github.com/stretchr/testify/assert" @@ -27,8 +28,11 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" ss "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/plugin" + "github.com/jaegertracing/jaeger/storage/samplingstore" ) func clearEnv() { @@ -48,13 +52,16 @@ func TestNewFactory(t *testing.T) { mock := new(mockFactory) f.factories["static"] = mock - assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) + lock := &mockLock{} + store := &mockStore{} + + assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), lock, store)) _, err = f.CreateStrategyStore() assert.NoError(t, err) // force the mock to return errors mock.retError = true - assert.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop()), "error initializing store") + assert.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), lock, store), "error initializing store") _, err = f.CreateStrategyStore() assert.EqualError(t, err, "error creating store") @@ -110,9 +117,37 @@ func (f *mockFactory) CreateStrategyStore() (ss.StrategyStore, error) { return nil, nil } -func (f *mockFactory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { +func (f *mockFactory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error { if f.retError { return errors.New("error initializing store") } return nil } + +type mockStore struct{} + +func (m *mockStore) InsertThroughput(throughput []*model.Throughput) error { + return nil +} +func (m *mockStore) InsertProbabilitiesAndQPS(hostname string, probabilities model.ServiceOperationProbabilities, qps model.ServiceOperationQPS) error { + return nil +} +func (m *mockStore) GetThroughput(start, end time.Time) ([]*model.Throughput, error) { + return nil, nil +} +func (m *mockStore) GetProbabilitiesAndQPS(start, end time.Time) (map[string][]model.ServiceOperationData, error) { + return nil, nil +} +func (m *mockStore) GetLatestProbabilities() (model.ServiceOperationProbabilities, error) { + return nil, nil +} + +type mockLock struct{} + +func (m *mockLock) Acquire(resource string, ttl time.Duration) (acquired bool, err error) { + return true, nil +} + +func (m *mockLock) Forfeit(resource string) (forfeited bool, err error) { + return true, nil +} diff --git a/plugin/sampling/strategystore/static/factory.go b/plugin/sampling/strategystore/static/factory.go index ddc769e94af..104bf83a0fc 100644 --- a/plugin/sampling/strategystore/static/factory.go +++ b/plugin/sampling/strategystore/static/factory.go @@ -22,6 +22,8 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" + "github.com/jaegertracing/jaeger/pkg/distributedlock" + "github.com/jaegertracing/jaeger/storage/samplingstore" ) // Factory implements strategystore.Factory for a static strategy store. @@ -49,7 +51,7 @@ func (f *Factory) InitFromViper(v *viper.Viper) { } // Initialize implements strategystore.Factory -func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { +func (f *Factory) Initialize(_ metrics.Factory, logger *zap.Logger, _ distributedlock.Lock, _ samplingstore.Store) error { f.logger = logger return nil } diff --git a/plugin/sampling/strategystore/static/factory_test.go b/plugin/sampling/strategystore/static/factory_test.go index 205c35651ba..05a8593e29c 100644 --- a/plugin/sampling/strategystore/static/factory_test.go +++ b/plugin/sampling/strategystore/static/factory_test.go @@ -35,7 +35,7 @@ func TestFactory(t *testing.T) { command.ParseFlags([]string{"--sampling.strategies-file=fixtures/strategies.json"}) f.InitFromViper(v) - assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop())) + assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), nil, nil)) _, err := f.CreateStrategyStore() assert.NoError(t, err) } diff --git a/plugin/storage/badger/factory.go b/plugin/storage/badger/factory.go index 4d19f12c3e8..972579af0ab 100644 --- a/plugin/storage/badger/factory.go +++ b/plugin/storage/badger/factory.go @@ -28,9 +28,12 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/distributedlock" depStore "github.com/jaegertracing/jaeger/plugin/storage/badger/dependencystore" badgerStore "github.com/jaegertracing/jaeger/plugin/storage/badger/spanstore" + "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -167,6 +170,11 @@ func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { return depStore.NewDependencyStore(sr), nil } +// CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + return nil, nil, storage.ErrLockAndSamplingStoreNotSupported +} + // Close Implements io.Closer and closes the underlying storage func (f *Factory) Close() error { close(f.maintenanceDone) diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index d48d0d0896b..9e87c5b0573 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -26,17 +26,23 @@ import ( "github.com/jaegertracing/jaeger/pkg/cassandra" "github.com/jaegertracing/jaeger/pkg/cassandra/config" + "github.com/jaegertracing/jaeger/pkg/distributedlock" + cLock "github.com/jaegertracing/jaeger/plugin/pkg/distributedlock/cassandra" cDepStore "github.com/jaegertracing/jaeger/plugin/storage/cassandra/dependencystore" + cSamplingStore "github.com/jaegertracing/jaeger/plugin/storage/cassandra/samplingstore" cSpanStore "github.com/jaegertracing/jaeger/plugin/storage/cassandra/spanstore" "github.com/jaegertracing/jaeger/plugin/storage/cassandra/spanstore/dbmodel" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) const ( primaryStorageConfig = "cassandra" archiveStorageConfig = "cassandra-archive" + + defaultLockTenant = "jaeger" ) // Factory implements storage.Factory for Cassandra backend. @@ -147,6 +153,13 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { return cSpanStore.NewSpanWriter(f.archiveSession, f.Options.SpanStoreWriteCacheTTL, f.archiveMetricsFactory, f.logger, options...), nil } +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + store := cSamplingStore.New(f.primarySession, f.primaryMetricsFactory, f.logger) + lock := cLock.NewLock(f.primarySession, defaultLockTenant) + + return lock, store, nil +} + func writerOptions(opts *Options) ([]cSpanStore.Option, error) { var tagFilters []dbmodel.TagFilter diff --git a/plugin/storage/es/factory.go b/plugin/storage/es/factory.go index 3877840750c..7e16f68f39a 100644 --- a/plugin/storage/es/factory.go +++ b/plugin/storage/es/factory.go @@ -24,12 +24,14 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/es" "github.com/jaegertracing/jaeger/pkg/es/config" esDepStore "github.com/jaegertracing/jaeger/plugin/storage/es/dependencystore" "github.com/jaegertracing/jaeger/plugin/storage/es/mappings" esSpanStore "github.com/jaegertracing/jaeger/plugin/storage/es/spanstore" "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -130,6 +132,11 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { return createSpanWriter(f.metricsFactory, f.logger, f.archiveClient, f.archiveConfig, true) } +// CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + return nil, nil, nil +} + func createSpanReader( mFactory metrics.Factory, logger *zap.Logger, diff --git a/plugin/storage/factory.go b/plugin/storage/factory.go index 59d8a19e4aa..424c034d0ff 100644 --- a/plugin/storage/factory.go +++ b/plugin/storage/factory.go @@ -24,6 +24,7 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/multierror" "github.com/jaegertracing/jaeger/plugin" "github.com/jaegertracing/jaeger/plugin/storage/badger" @@ -34,6 +35,7 @@ import ( "github.com/jaegertracing/jaeger/plugin/storage/memory" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -158,6 +160,23 @@ func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { }), nil } +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + for _, factory := range f.factories { + lock, store, err := factory.CreateLockAndSamplingStore() + if err != nil && err != storage.ErrLockAndSamplingStoreNotSupported { + return nil, nil, err + } + // returns the first backend that can support adaptive sampling + if lock != nil && store != nil { + return lock, store, nil + } + } + + // returning nothing is valid here. it's quite possible that the user has no backend that can support adaptive sampling + // this is fine as long as adaptive sampling is also not configured + return nil, nil, nil +} + // CreateDependencyReader implements storage.Factory func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { factory, ok := f.factories[f.DependenciesStorageType] diff --git a/plugin/storage/factory_test.go b/plugin/storage/factory_test.go index 973862eca9e..3e0e471750b 100644 --- a/plugin/storage/factory_test.go +++ b/plugin/storage/factory_test.go @@ -34,10 +34,12 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/config" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" depStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" "github.com/jaegertracing/jaeger/storage/mocks" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" spanStoreMocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) @@ -413,6 +415,10 @@ func (e errorCloseFactory) CreateDependencyReader() (dependencystore.Reader, err panic("implement me") } +func (e errorCloseFactory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + panic("implement me") +} + func (e errorCloseFactory) Close() error { return e.closeErr } diff --git a/plugin/storage/grpc/factory.go b/plugin/storage/grpc/factory.go index e5ee5dc7855..b6fd6c4f69d 100644 --- a/plugin/storage/grpc/factory.go +++ b/plugin/storage/grpc/factory.go @@ -22,10 +22,12 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/plugin/storage/grpc/config" "github.com/jaegertracing/jaeger/plugin/storage/grpc/shared" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -124,3 +126,8 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { } return f.archiveStore.ArchiveSpanWriter(), nil } + +// CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + return nil, nil, storage.ErrLockAndSamplingStoreNotSupported +} diff --git a/plugin/storage/kafka/factory.go b/plugin/storage/kafka/factory.go index b11caf1983c..94018257904 100644 --- a/plugin/storage/kafka/factory.go +++ b/plugin/storage/kafka/factory.go @@ -24,8 +24,11 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/kafka/producer" + "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -100,6 +103,11 @@ func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { return nil, errors.New("kafka storage is write-only") } +// CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + return nil, nil, storage.ErrLockAndSamplingStoreNotSupported +} + var _ io.Closer = (*Factory)(nil) // Close closes the resources held by the factory diff --git a/plugin/storage/memory/factory.go b/plugin/storage/memory/factory.go index 48d3d6bdcb2..4bff2bc7476 100644 --- a/plugin/storage/memory/factory.go +++ b/plugin/storage/memory/factory.go @@ -22,7 +22,10 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/distributedlock" + "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -79,6 +82,11 @@ func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { return f.store, nil } +// CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + return nil, nil, storage.ErrLockAndSamplingStoreNotSupported +} + func (f *Factory) publishOpts() { internalFactory := f.metricsFactory.Namespace(metrics.NSOptions{Name: "internal"}) internalFactory.Gauge(metrics.Options{Name: limit}). diff --git a/storage/factory.go b/storage/factory.go index c8c2bbd996c..66299863c30 100644 --- a/storage/factory.go +++ b/storage/factory.go @@ -21,7 +21,9 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -44,6 +46,10 @@ type Factory interface { // CreateDependencyReader creates a dependencystore.Reader. CreateDependencyReader() (dependencystore.Reader, error) + + // CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store. + // These are used for adaptive sampling. + CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) } var ( @@ -52,6 +58,9 @@ var ( // ErrArchiveStorageNotSupported can be returned by the ArchiveFactory when the archive storage is not supported by the backend. ErrArchiveStorageNotSupported = errors.New("archive storage not supported") + + // ErrLockAndSamplingStoreNotSupported can be returned by the StorageFactory when the sampling storage is not supported by the backend. + ErrLockAndSamplingStoreNotSupported = errors.New("archive storage not supported") ) // ArchiveFactory is an additional interface that can be implemented by a factory to support trace archiving. diff --git a/storage/mocks/Factory.go b/storage/mocks/Factory.go index 9a7ace61700..8f22c37be38 100644 --- a/storage/mocks/Factory.go +++ b/storage/mocks/Factory.go @@ -14,12 +14,20 @@ package mocks -import dependencystore "github.com/jaegertracing/jaeger/storage/dependencystore" -import metrics "github.com/uber/jaeger-lib/metrics" -import mock "github.com/stretchr/testify/mock" -import spanstore "github.com/jaegertracing/jaeger/storage/spanstore" -import storage "github.com/jaegertracing/jaeger/storage" -import zap "go.uber.org/zap" +import ( + "github.com/jaegertracing/jaeger/pkg/distributedlock" + dependencystore "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" + metrics "github.com/uber/jaeger-lib/metrics" + + mock "github.com/stretchr/testify/mock" + + spanstore "github.com/jaegertracing/jaeger/storage/spanstore" + + storage "github.com/jaegertracing/jaeger/storage" + + zap "go.uber.org/zap" +) // Factory is an autogenerated mock type for the Factory type type Factory struct { @@ -95,6 +103,11 @@ func (_m *Factory) CreateSpanWriter() (spanstore.Writer, error) { return r0, r1 } +// CreateLockAndSamplingStore returns all nils ... for now +func (_m *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + return nil, nil, nil +} + // Initialize provides a mock function with given fields: metricsFactory, logger func (_m *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { ret := _m.Called(metricsFactory, logger) From 5130f11074764287394d6e7318fafd2a309b0258 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 28 Apr 2021 15:00:37 -0400 Subject: [PATCH 04/40] changelog Signed-off-by: Joe Elliott --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1277058c611..c064aaec7cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ Changes by Version ------------------- ### Backend Changes +#### New Features + +* Add support for adaptive sampling with a Cassandra backend. ([#2966](https://github.com/jaegertracing/jaeger/pull/2966), [@joe-elliott](https://github.com/joe-elliott)): + #### Breaking Changes * Remove unused `--es-archive.max-span-age` flag ([#2865](https://github.com/jaegertracing/jaeger/pull/2865), [@albertteoh](https://github.com/albertteoh)): From 418ee39e8ee0b08956bad1792aa96b8f5d0440d2 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 4 May 2021 14:32:31 -0400 Subject: [PATCH 05/40] lint Signed-off-by: Joe Elliott --- .../strategystore/adaptive/constants.go | 36 -------- .../strategystore/adaptive/processor.go | 91 ++++++------------- .../strategystore/adaptive/processor_test.go | 42 ++++----- .../strategystore/adaptive/strategy_store.go | 27 ++++++ plugin/sampling/strategystore/factory.go | 13 +-- .../sampling/strategystore/factory_config.go | 4 +- .../strategystore/factory_config_test.go | 8 +- plugin/sampling/strategystore/factory_test.go | 2 +- plugin/storage/cassandra/factory.go | 1 + plugin/storage/es/factory.go | 3 +- plugin/storage/factory.go | 1 + storage/mocks/Factory.go | 4 +- 12 files changed, 94 insertions(+), 138 deletions(-) delete mode 100644 plugin/sampling/strategystore/adaptive/constants.go create mode 100644 plugin/sampling/strategystore/adaptive/strategy_store.go diff --git a/plugin/sampling/strategystore/adaptive/constants.go b/plugin/sampling/strategystore/adaptive/constants.go deleted file mode 100644 index 474e3bc0ebe..00000000000 --- a/plugin/sampling/strategystore/adaptive/constants.go +++ /dev/null @@ -1,36 +0,0 @@ -// 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 adaptive - -import ( - "github.com/jaegertracing/jaeger/thrift-gen/sampling" -) - -const ( - // defaultSamplingProbability is the default sampling probability the - // Strategy Store will use if none is provided. - defaultSamplingProbability = 0.001 -) - -// defaultStrategy is the default sampling strategy the Strategy Store will return -// if none is provided. -func defaultStrategyResponse() *sampling.SamplingStrategyResponse { - return &sampling.SamplingStrategyResponse{ - StrategyType: sampling.SamplingStrategyType_PROBABILISTIC, - ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ - SamplingRate: defaultSamplingProbability, - }, - } -} diff --git a/plugin/sampling/strategystore/adaptive/processor.go b/plugin/sampling/strategystore/adaptive/processor.go index 6d4576f3d8c..70244aa5ec4 100644 --- a/plugin/sampling/strategystore/adaptive/processor.go +++ b/plugin/sampling/strategystore/adaptive/processor.go @@ -20,7 +20,6 @@ import ( "io" "math" "math/rand" - "os" "sync" "time" @@ -28,7 +27,6 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" - "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/plugin/sampling/calculationstrategy" "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection" "github.com/jaegertracing/jaeger/storage/samplingstore" @@ -74,11 +72,11 @@ type throughputBucket struct { endTime time.Time } -// processor retrieves service throughput over a look back interval and calculates sampling probabilities +// Processor retrieves service throughput over a look back interval and calculates sampling probabilities // per operation such that each operation is sampled at a specified target QPS. It achieves this by // retrieving discrete buckets of operation throughput and doing a weighted average of the throughput // and generating a probability to match the targetQPS. -type processor struct { +type Processor struct { sync.RWMutex Options @@ -119,15 +117,15 @@ type processor struct { calculateProbabilitiesLatency metrics.Timer } -// NewProcessor creates a new sampling processor that generates sampling rates for service operations. -func NewProcessor( +// newProcessor creates a new sampling processor that generates sampling rates for service operations. +func newProcessor( opts Options, hostname string, storage samplingstore.Store, electionParticipant leaderelection.ElectionParticipant, metricsFactory metrics.Factory, logger *zap.Logger, -) (*processor, error) { +) (*Processor, error) { if opts.CalculationInterval == 0 || opts.AggregationBuckets == 0 { return nil, errNonZero } @@ -135,7 +133,7 @@ func NewProcessor( return nil, errBucketsForCalculation } metricsFactory = metricsFactory.Namespace(metrics.NSOptions{Name: "adaptive_sampling_processor"}) - return &processor{ + return &Processor{ Options: opts, storage: storage, probabilities: make(model.ServiceOperationProbabilities), @@ -155,7 +153,7 @@ func NewProcessor( } // GetSamplingStrategy implements Thrift endpoint for retrieving sampling strategy for a service. -func (p *processor) GetSamplingStrategy(_ context.Context, service string) (*sampling.SamplingStrategyResponse, error) { +func (p *Processor) GetSamplingStrategy(_ context.Context, service string) (*sampling.SamplingStrategyResponse, error) { p.RLock() defer p.RUnlock() if strategy, ok := p.strategyResponses[service]; ok { @@ -165,7 +163,7 @@ func (p *processor) GetSamplingStrategy(_ context.Context, service string) (*sam } // Start initializes and starts the sampling processor which regularly calculates sampling probabilities. -func (p *processor) Start() error { +func (p *Processor) Start() error { p.logger.Info("starting adaptive sampling processor") p.shutdown = make(chan struct{}) p.loadProbabilities() @@ -176,7 +174,7 @@ func (p *processor) Start() error { } // Close stops the processor from calculating probabilities. -func (p *processor) Close() error { +func (p *Processor) Close() error { p.logger.Info("stopping adaptive sampling processor") if closer, ok := p.electionParticipant.(io.Closer); ok { closer.Close() @@ -185,7 +183,7 @@ func (p *processor) Close() error { return nil } -func (p *processor) loadProbabilities() { +func (p *Processor) loadProbabilities() { // TODO GetLatestProbabilities API can be changed to return the latest measured qps for initialization probabilities, err := p.storage.GetLatestProbabilities() if err != nil { @@ -199,7 +197,7 @@ func (p *processor) loadProbabilities() { // runUpdateProbabilitiesLoop is a loop that reads probabilities from storage. // The follower updates its local cache with the latest probabilities and serves them. -func (p *processor) runUpdateProbabilitiesLoop() { +func (p *Processor) runUpdateProbabilitiesLoop() { addJitter(p.followerRefreshInterval) ticker := time.NewTicker(p.followerRefreshInterval) defer ticker.Stop() @@ -217,7 +215,7 @@ func (p *processor) runUpdateProbabilitiesLoop() { } } -func (p *processor) isLeader() bool { +func (p *Processor) isLeader() bool { return p.electionParticipant.IsLeader() } @@ -230,7 +228,7 @@ func addJitter(jitterAmount time.Duration) { time.Sleep(delay) } -func (p *processor) runCalculationLoop() { +func (p *Processor) runCalculationLoop() { lastCheckedTime := time.Now().Add(p.Delay * -1) p.initializeThroughput(lastCheckedTime) // NB: the first tick will be slightly delayed by the initializeThroughput call. @@ -280,7 +278,7 @@ func (p *processor) runCalculationLoop() { } } -func (p *processor) saveProbabilitiesAndQPS() { +func (p *Processor) saveProbabilitiesAndQPS() { p.RLock() defer p.RUnlock() if err := p.storage.InsertProbabilitiesAndQPS(p.hostname, p.probabilities, p.qps); err != nil { @@ -288,7 +286,7 @@ func (p *processor) saveProbabilitiesAndQPS() { } } -func (p *processor) prependThroughputBucket(bucket *throughputBucket) { +func (p *Processor) prependThroughputBucket(bucket *throughputBucket) { p.throughputs = append([]*throughputBucket{bucket}, p.throughputs...) if len(p.throughputs) > p.AggregationBuckets { p.throughputs = p.throughputs[0:p.AggregationBuckets] @@ -298,7 +296,7 @@ func (p *processor) prependThroughputBucket(bucket *throughputBucket) { // aggregateThroughput aggregates operation throughput from different buckets into one. // All input buckets represent a single time range, but there are many of them because // they are all independently generated by different collector instances from inbound span traffic. -func (p *processor) aggregateThroughput(throughputs []*model.Throughput) serviceOperationThroughput { +func (p *Processor) aggregateThroughput(throughputs []*model.Throughput) serviceOperationThroughput { aggregatedThroughput := make(serviceOperationThroughput) for _, throughput := range throughputs { service := throughput.Service @@ -316,7 +314,7 @@ func (p *processor) aggregateThroughput(throughputs []*model.Throughput) service return aggregatedThroughput } -func (p *processor) initializeThroughput(endTime time.Time) { +func (p *Processor) initializeThroughput(endTime time.Time) { for i := 0; i < p.AggregationBuckets; i++ { startTime := endTime.Add(p.CalculationInterval * -1) throughput, err := p.storage.GetThroughput(startTime, endTime) @@ -338,7 +336,7 @@ func (p *processor) initializeThroughput(endTime time.Time) { } // throughputToQPS converts raw throughput counts for all accumulated buckets to QPS values. -func (p *processor) throughputToQPS() serviceOperationQPS { +func (p *Processor) throughputToQPS() serviceOperationQPS { // TODO previous qps buckets have already been calculated, just need to calculate latest batch // and append them where necessary and throw out the oldest batch. // Edge case #buckets < p.AggregationBuckets, then we shouldn't throw out @@ -366,7 +364,7 @@ func calculateQPS(count int64, interval time.Duration) float64 { // calculateWeightedQPS calculates the weighted qps of the slice allQPS where weights are biased // towards more recent qps. This function assumes that the most recent qps is at the head of the slice. -func (p *processor) calculateWeightedQPS(allQPS []float64) float64 { +func (p *Processor) calculateWeightedQPS(allQPS []float64) float64 { if len(allQPS) == 0 { return 0 } @@ -378,14 +376,14 @@ func (p *processor) calculateWeightedQPS(allQPS []float64) float64 { return qps } -func (p *processor) prependServiceCache() { +func (p *Processor) prependServiceCache() { p.serviceCache = append([]SamplingCache{make(SamplingCache)}, p.serviceCache...) if len(p.serviceCache) > serviceCacheSize { p.serviceCache = p.serviceCache[0:serviceCacheSize] } } -func (p *processor) calculateProbabilitiesAndQPS() (model.ServiceOperationProbabilities, model.ServiceOperationQPS) { +func (p *Processor) calculateProbabilitiesAndQPS() (model.ServiceOperationProbabilities, model.ServiceOperationQPS) { p.prependServiceCache() retProbabilities := make(model.ServiceOperationProbabilities) retQPS := make(model.ServiceOperationQPS) @@ -409,7 +407,7 @@ func (p *processor) calculateProbabilitiesAndQPS() (model.ServiceOperationProbab return retProbabilities, retQPS } -func (p *processor) calculateProbability(service, operation string, qps float64) float64 { +func (p *Processor) calculateProbability(service, operation string, qps float64) float64 { oldProbability := p.InitialSamplingProbability // TODO: is this loop overly expensive? p.RLock() @@ -444,7 +442,7 @@ func (p *processor) calculateProbability(service, operation string, qps float64) } // is actual value within p.DeltaTolerance percentage of expected value. -func (p *processor) withinTolerance(actual, expected float64) bool { +func (p *Processor) withinTolerance(actual, expected float64) bool { return math.Abs(actual-expected)/expected < p.DeltaTolerance } @@ -456,7 +454,7 @@ func merge(p1 map[string]struct{}, p2 map[string]struct{}) map[string]struct{} { return p1 } -func (p *processor) isUsingAdaptiveSampling( +func (p *Processor) isUsingAdaptiveSampling( probability float64, service string, operation string, @@ -485,7 +483,7 @@ func (p *processor) isUsingAdaptiveSampling( } // generateStrategyResponses generates and caches SamplingStrategyResponse from the calculated sampling probabilities. -func (p *processor) generateStrategyResponses() { +func (p *Processor) generateStrategyResponses() { p.RLock() strategies := make(map[string]*sampling.SamplingStrategyResponse) for svc, opProbabilities := range p.probabilities { @@ -511,7 +509,7 @@ func (p *processor) generateStrategyResponses() { p.strategyResponses = strategies } -func (p *processor) generateDefaultSamplingStrategyResponse() *sampling.SamplingStrategyResponse { +func (p *Processor) generateDefaultSamplingStrategyResponse() *sampling.SamplingStrategyResponse { return &sampling.SamplingStrategyResponse{ StrategyType: sampling.SamplingStrategyType_PROBABILISTIC, OperationSampling: &sampling.PerOperationSamplingStrategies{ @@ -520,40 +518,3 @@ func (p *processor) generateDefaultSamplingStrategyResponse() *sampling.Sampling }, } } - -type strategyStore struct { - logger *zap.Logger - processor *processor - cancelFunc context.CancelFunc -} - -type storedStrategies struct { - defaultStrategy *sampling.SamplingStrategyResponse - serviceStrategies map[string]*sampling.SamplingStrategyResponse -} - -// GetSamplingStrategy implements StrategyStore#GetSamplingStrategy. -func (h *strategyStore) GetSamplingStrategy(_ context.Context, serviceName string) (*sampling.SamplingStrategyResponse, error) { - serviceStrategies := h.processor.strategyResponses - if strategy, ok := serviceStrategies[serviceName]; ok { - return strategy, nil - } - h.logger.Debug("sampling strategy not found, using default", zap.String("service", serviceName)) - return defaultStrategyResponse(), nil -} - -// NewStrategyStore creates a strategy store that holds adaptive sampling strategies. -func NewStrategyStore(options Options, metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) (*processor, error) { - hostname, err := os.Hostname() - if err != nil { - return nil, err - } - - participant := leaderelection.NewElectionParticipant(lock, defaultResourceName, leaderelection.ElectionParticipantOptions{}) // todo(jpe) : wire up options/resource name - p, err := NewProcessor(options, hostname, store, participant, metricsFactory, logger) - if err != nil { - return nil, err - } - - return p, nil -} diff --git a/plugin/sampling/strategystore/adaptive/processor_test.go b/plugin/sampling/strategystore/adaptive/processor_test.go index cc24c66348e..148ac565f92 100644 --- a/plugin/sampling/strategystore/adaptive/processor_test.go +++ b/plugin/sampling/strategystore/adaptive/processor_test.go @@ -79,7 +79,7 @@ var ( ) func TestAggregateThroughput(t *testing.T) { - p := &processor{} + p := &Processor{} aggregatedThroughput := p.aggregateThroughput(testThroughputs) require.Len(t, aggregatedThroughput, 2) @@ -115,7 +115,7 @@ func TestInitializeThroughput(t *testing.T) { Return([]*model.Throughput{{Service: "svcA", Operation: "GET", Count: 7}}, nil) mockStorage.On("GetThroughput", time.Time{}.Add(time.Minute*17), time.Time{}.Add(time.Minute*18)). Return([]*model.Throughput{}, nil) - p := &processor{storage: mockStorage, Options: Options{CalculationInterval: time.Minute, AggregationBuckets: 3}} + p := &Processor{storage: mockStorage, Options: Options{CalculationInterval: time.Minute, AggregationBuckets: 3}} p.initializeThroughput(time.Time{}.Add(time.Minute * 20)) require.Len(t, p.throughputs, 2) @@ -131,7 +131,7 @@ func TestInitializeThroughputFailure(t *testing.T) { mockStorage := &smocks.Store{} mockStorage.On("GetThroughput", time.Time{}.Add(time.Minute*19), time.Time{}.Add(time.Minute*20)). Return(nil, errTestStorage) - p := &processor{storage: mockStorage, Options: Options{CalculationInterval: time.Minute, AggregationBuckets: 1}} + p := &Processor{storage: mockStorage, Options: Options{CalculationInterval: time.Minute, AggregationBuckets: 1}} p.initializeThroughput(time.Time{}.Add(time.Minute * 20)) assert.Len(t, p.throughputs, 0) @@ -146,7 +146,7 @@ func TestCalculateQPS(t *testing.T) { } func TestGenerateOperationQPS(t *testing.T) { - p := &processor{throughputs: testThroughputBuckets, Options: Options{BucketsForCalculation: 10, AggregationBuckets: 10}} + p := &Processor{throughputs: testThroughputBuckets, Options: Options{BucketsForCalculation: 10, AggregationBuckets: 10}} svcOpQPS := p.throughputToQPS() assert.Len(t, svcOpQPS, 2) @@ -194,7 +194,7 @@ func TestGenerateOperationQPS(t *testing.T) { } func TestGenerateOperationQPS_UseMostRecentBucketOnly(t *testing.T) { - p := &processor{throughputs: testThroughputBuckets, Options: Options{BucketsForCalculation: 1, AggregationBuckets: 10}} + p := &Processor{throughputs: testThroughputBuckets, Options: Options{BucketsForCalculation: 1, AggregationBuckets: 10}} svcOpQPS := p.throughputToQPS() assert.Len(t, svcOpQPS, 2) @@ -228,7 +228,7 @@ func TestGenerateOperationQPS_UseMostRecentBucketOnly(t *testing.T) { } func TestCalculateWeightedQPS(t *testing.T) { - p := processor{weightVectorCache: NewWeightVectorCache()} + p := Processor{weightVectorCache: NewWeightVectorCache()} assert.InDelta(t, 0.86735, p.calculateWeightedQPS([]float64{0.8, 1.2, 1.0}), 0.001) assert.InDelta(t, 0.95197, p.calculateWeightedQPS([]float64{1.0, 1.0, 0.0, 0.0}), 0.001) assert.Equal(t, 0.0, p.calculateWeightedQPS([]float64{})) @@ -255,7 +255,7 @@ func TestCalculateProbability(t *testing.T) { InitialSamplingProbability: 0.001, MinSamplingProbability: 0.00001, } - p := &processor{ + p := &Processor{ Options: cfg, probabilities: probabilities, probabilityCalculator: testCalculator, @@ -295,7 +295,7 @@ func TestCalculateProbabilitiesAndQPS(t *testing.T) { }, } mets := metricstest.NewFactory(0) - p := &processor{ + p := &Processor{ Options: Options{ TargetSamplesPerSecond: 1.0, DeltaTolerance: 0.2, @@ -344,7 +344,7 @@ func TestRunCalculationLoop(t *testing.T) { FollowerLeaseRefreshInterval: time.Second, BucketsForCalculation: 10, } - p, err := NewProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, logger) + p, err := newProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, logger) require.NoError(t, err) p.Start() @@ -377,7 +377,7 @@ func TestRunCalculationLoop_GetThroughputError(t *testing.T) { AggregationBuckets: 2, BucketsForCalculation: 10, } - p, err := NewProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, logger) + p, err := newProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, logger) require.NoError(t, err) p.shutdown = make(chan struct{}) defer close(p.shutdown) @@ -399,7 +399,7 @@ func TestLoadProbabilities(t *testing.T) { mockStorage := &smocks.Store{} mockStorage.On("GetLatestProbabilities").Return(make(model.ServiceOperationProbabilities), nil) - p := &processor{storage: mockStorage} + p := &Processor{storage: mockStorage} require.Nil(t, p.probabilities) p.loadProbabilities() require.NotNil(t, p.probabilities) @@ -413,7 +413,7 @@ func TestRunUpdateProbabilitiesLoop(t *testing.T) { mockEP.On("Close").Return(nil) mockEP.On("IsLeader").Return(false) - p := &processor{ + p := &Processor{ storage: mockStorage, shutdown: make(chan struct{}), followerRefreshInterval: time.Millisecond, @@ -466,7 +466,7 @@ func TestRealisticRunCalculationLoop(t *testing.T) { AggregationBuckets: 1, Delay: time.Second * 10, } - p, err := NewProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, logger) + p, err := newProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, logger) require.NoError(t, err) p.Start() @@ -503,7 +503,7 @@ func TestRealisticRunCalculationLoop(t *testing.T) { } func TestPrependBucket(t *testing.T) { - p := &processor{Options: Options{AggregationBuckets: 1}} + p := &Processor{Options: Options{AggregationBuckets: 1}} p.prependThroughputBucket(&throughputBucket{interval: time.Minute}) require.Len(t, p.throughputs, 1) assert.Equal(t, time.Minute, p.throughputs[0].interval) @@ -523,17 +523,17 @@ func TestConstructorFailure(t *testing.T) { CalculationInterval: time.Second * 5, AggregationBuckets: 0, } - _, err := NewProcessor(cfg, "host", nil, nil, metrics.NullFactory, logger) + _, err := newProcessor(cfg, "host", nil, nil, metrics.NullFactory, logger) assert.EqualError(t, err, "CalculationInterval and AggregationBuckets must be greater than 0") cfg.CalculationInterval = 0 - _, err = NewProcessor(cfg, "host", nil, nil, metrics.NullFactory, logger) + _, err = newProcessor(cfg, "host", nil, nil, metrics.NullFactory, logger) assert.EqualError(t, err, "CalculationInterval and AggregationBuckets must be greater than 0") cfg.CalculationInterval = time.Millisecond cfg.AggregationBuckets = 1 cfg.BucketsForCalculation = -1 - _, err = NewProcessor(cfg, "host", nil, nil, metrics.NullFactory, logger) + _, err = newProcessor(cfg, "host", nil, nil, metrics.NullFactory, logger) assert.EqualError(t, err, "BucketsForCalculation cannot be less than 1") } @@ -543,7 +543,7 @@ func TestGenerateStrategyResponses(t *testing.T) { "GET": 0.5, }, } - p := &processor{ + p := &Processor{ probabilities: probabilities, Options: Options{ InitialSamplingProbability: 0.001, @@ -572,7 +572,7 @@ func TestGenerateStrategyResponses(t *testing.T) { } func TestUsingAdaptiveSampling(t *testing.T) { - p := &processor{} + p := &Processor{} throughput := serviceOperationThroughput{ "svc": map[string]*model.Throughput{ "op": {Probabilities: map[string]struct{}{"0.010000": {}}}, @@ -597,7 +597,7 @@ func TestUsingAdaptiveSampling(t *testing.T) { } } func TestPrependServiceCache(t *testing.T) { - p := &processor{} + p := &Processor{} for i := 0; i < serviceCacheSize*2; i++ { p.prependServiceCache() } @@ -620,7 +620,7 @@ func TestCalculateProbabilitiesAndQPSMultiple(t *testing.T) { }, } - p := &processor{ + p := &Processor{ Options: Options{ TargetSamplesPerSecond: 1.0, DeltaTolerance: 0.002, diff --git a/plugin/sampling/strategystore/adaptive/strategy_store.go b/plugin/sampling/strategystore/adaptive/strategy_store.go new file mode 100644 index 00000000000..dfae25285c8 --- /dev/null +++ b/plugin/sampling/strategystore/adaptive/strategy_store.go @@ -0,0 +1,27 @@ +package adaptive + +import ( + "os" + + "github.com/jaegertracing/jaeger/pkg/distributedlock" + "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection" + "github.com/jaegertracing/jaeger/storage/samplingstore" + "github.com/uber/jaeger-lib/metrics" + "go.uber.org/zap" +) + +// NewStrategyStore creates a strategy store that holds adaptive sampling strategies. +func NewStrategyStore(options Options, metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) (*Processor, error) { + hostname, err := os.Hostname() + if err != nil { + return nil, err + } + + participant := leaderelection.NewElectionParticipant(lock, defaultResourceName, leaderelection.ElectionParticipantOptions{}) // todo(jpe) : wire up options/resource name + p, err := newProcessor(options, hostname, store, participant, metricsFactory, logger) + if err != nil { + return nil, err + } + + return p, nil +} diff --git a/plugin/sampling/strategystore/factory.go b/plugin/sampling/strategystore/factory.go index 06959bdbbcc..1dad28693c8 100644 --- a/plugin/sampling/strategystore/factory.go +++ b/plugin/sampling/strategystore/factory.go @@ -30,24 +30,25 @@ import ( "github.com/jaegertracing/jaeger/storage/samplingstore" ) -type StrategyStoreType string +// Kind is a datatype holding the type of strategy store. +type Kind string -var allSamplingTypes = []StrategyStoreType{"static", "adaptive"} +var allSamplingTypes = []Kind{"static", "adaptive"} // Factory implements strategystore.Factory interface as a meta-factory for strategy storage components. type Factory struct { FactoryConfig - factories map[StrategyStoreType]strategystore.Factory + factories map[Kind]strategystore.Factory } // NewFactory creates the meta-factory. func NewFactory(config FactoryConfig) (*Factory, error) { f := &Factory{FactoryConfig: config} - uniqueTypes := map[StrategyStoreType]struct{}{ + uniqueTypes := map[Kind]struct{}{ f.StrategyStoreType: {}, } - f.factories = make(map[StrategyStoreType]strategystore.Factory) + f.factories = make(map[Kind]strategystore.Factory) for t := range uniqueTypes { ff, err := f.getFactoryOfType(t) if err != nil { @@ -58,7 +59,7 @@ func NewFactory(config FactoryConfig) (*Factory, error) { return f, nil } -func (f *Factory) getFactoryOfType(factoryType StrategyStoreType) (strategystore.Factory, error) { +func (f *Factory) getFactoryOfType(factoryType Kind) (strategystore.Factory, error) { switch factoryType { case "static": return static.NewFactory(), nil diff --git a/plugin/sampling/strategystore/factory_config.go b/plugin/sampling/strategystore/factory_config.go index 18f7a8cfb1c..b97db12846e 100644 --- a/plugin/sampling/strategystore/factory_config.go +++ b/plugin/sampling/strategystore/factory_config.go @@ -26,7 +26,7 @@ const ( // FactoryConfig tells the Factory what sampling type it needs to create. type FactoryConfig struct { - StrategyStoreType StrategyStoreType + StrategyStoreType Kind } // FactoryConfigFromEnv reads the desired sampling type from the SAMPLING_TYPE environment variable. Allowed values: @@ -41,6 +41,6 @@ func FactoryConfigFromEnv() (*FactoryConfig, error) { return nil, fmt.Errorf("invalid sampling type: %s", strategyStoreType) } return &FactoryConfig{ - StrategyStoreType: StrategyStoreType(strategyStoreType), + StrategyStoreType: Kind(strategyStoreType), }, nil } diff --git a/plugin/sampling/strategystore/factory_config_test.go b/plugin/sampling/strategystore/factory_config_test.go index 4462b46f5bd..589f95de99b 100644 --- a/plugin/sampling/strategystore/factory_config_test.go +++ b/plugin/sampling/strategystore/factory_config_test.go @@ -26,19 +26,19 @@ import ( func TestFactoryConfigFromEnv(t *testing.T) { tests := []struct { env string - expectedType StrategyStoreType + expectedType Kind expectsError bool }{ { - expectedType: StrategyStoreType("static"), + expectedType: Kind("static"), }, { env: "static", - expectedType: StrategyStoreType("static"), + expectedType: Kind("static"), }, { env: "adaptive", - expectedType: StrategyStoreType("adaptive"), + expectedType: Kind("adaptive"), }, { env: "??", diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index ce64d58a29f..cc1696cd3aa 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -47,7 +47,7 @@ func TestNewFactory(t *testing.T) { require.NoError(t, err) assert.NotEmpty(t, f.factories) assert.NotEmpty(t, f.factories["static"]) - assert.Equal(t, StrategyStoreType("static"), f.StrategyStoreType) + assert.Equal(t, Kind("static"), f.StrategyStoreType) mock := new(mockFactory) f.factories["static"] = mock diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index 9e87c5b0573..4de12f3bce8 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -153,6 +153,7 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { return cSpanStore.NewSpanWriter(f.archiveSession, f.Options.SpanStoreWriteCacheTTL, f.archiveMetricsFactory, f.logger, options...), nil } +// CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { store := cSamplingStore.New(f.primarySession, f.primaryMetricsFactory, f.logger) lock := cLock.NewLock(f.primarySession, defaultLockTenant) diff --git a/plugin/storage/es/factory.go b/plugin/storage/es/factory.go index 7e16f68f39a..6fab62f6c3e 100644 --- a/plugin/storage/es/factory.go +++ b/plugin/storage/es/factory.go @@ -30,6 +30,7 @@ import ( esDepStore "github.com/jaegertracing/jaeger/plugin/storage/es/dependencystore" "github.com/jaegertracing/jaeger/plugin/storage/es/mappings" esSpanStore "github.com/jaegertracing/jaeger/plugin/storage/es/spanstore" + "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" @@ -134,7 +135,7 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { // CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, nil + return nil, nil, storage.ErrLockAndSamplingStoreNotSupported } func createSpanReader( diff --git a/plugin/storage/factory.go b/plugin/storage/factory.go index 424c034d0ff..622747fe95a 100644 --- a/plugin/storage/factory.go +++ b/plugin/storage/factory.go @@ -160,6 +160,7 @@ func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { }), nil } +// CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { for _, factory := range f.factories { lock, store, err := factory.CreateLockAndSamplingStore() diff --git a/storage/mocks/Factory.go b/storage/mocks/Factory.go index 8f22c37be38..db425cace25 100644 --- a/storage/mocks/Factory.go +++ b/storage/mocks/Factory.go @@ -103,9 +103,9 @@ func (_m *Factory) CreateSpanWriter() (spanstore.Writer, error) { return r0, r1 } -// CreateLockAndSamplingStore returns all nils ... for now +// CreateLockAndSamplingStore is not supproted func (_m *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, nil + return nil, nil, storage.ErrLockAndSamplingStoreNotSupported // todo(jpe): add tests that require this? } // Initialize provides a mock function with given fields: metricsFactory, logger From 3dbe31a675e6c53e8ba808f856b83d6e307cbc9d Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 4 May 2021 14:55:29 -0400 Subject: [PATCH 06/40] Added method for conditionally instantiating the lock and sampling store Signed-off-by: Joe Elliott --- cmd/all-in-one/main.go | 16 +++++++++++++--- .../app/sampling/strategystore/factory.go | 8 ++++++-- cmd/collector/main.go | 16 +++++++++++++--- .../strategystore/adaptive/factory.go | 5 +++++ .../strategystore/adaptive/strategy_store.go | 19 +++++++++++++++++-- plugin/sampling/strategystore/factory.go | 9 +++++++++ plugin/sampling/strategystore/factory_test.go | 7 +++++++ .../sampling/strategystore/static/factory.go | 5 +++++ storage/mocks/Factory.go | 11 ++++------- 9 files changed, 79 insertions(+), 17 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index e6a3bb5975f..9de2e04ec35 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -43,11 +43,13 @@ import ( "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/cmd/status" "github.com/jaegertracing/jaeger/pkg/config" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/version" ss "github.com/jaegertracing/jaeger/plugin/sampling/strategystore" "github.com/jaegertracing/jaeger/plugin/storage" "github.com/jaegertracing/jaeger/ports" "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" storageMetrics "github.com/jaegertracing/jaeger/storage/spanstore/metrics" ) @@ -110,11 +112,19 @@ by default uses only in-memory database.`, if err != nil { logger.Fatal("Failed to create dependency reader", zap.Error(err)) } - lock, samplingStore, err := storageFactory.CreateLockAndSamplingStore() + + requireLockAndSamplingStore, err := strategyStoreFactory.RequiresLockAndSamplingStore() if err != nil { - logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) + logger.Fatal("Failed to determine if lock and sampling store is required.", zap.Error(err)) + } + var lock distributedlock.Lock + var samplingStore samplingstore.Store + if requireLockAndSamplingStore { + lock, samplingStore, err = storageFactory.CreateLockAndSamplingStore() + if err != nil { + logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) + } } - strategyStoreFactory.InitFromViper(v) if err := strategyStoreFactory.Initialize(metricsFactory, logger, lock, samplingStore); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) diff --git a/cmd/collector/app/sampling/strategystore/factory.go b/cmd/collector/app/sampling/strategystore/factory.go index 245028565b5..2093c3015ec 100644 --- a/cmd/collector/app/sampling/strategystore/factory.go +++ b/cmd/collector/app/sampling/strategystore/factory.go @@ -15,10 +15,11 @@ package strategystore import ( - "github.com/jaegertracing/jaeger/pkg/distributedlock" - "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + + "github.com/jaegertracing/jaeger/pkg/distributedlock" + "github.com/jaegertracing/jaeger/storage/samplingstore" ) // Factory defines an interface for a factory that can create implementations of different strategy storage components. @@ -33,4 +34,7 @@ type Factory interface { // CreateStrategyStore initializes the StrategyStore and returns it. CreateStrategyStore() (StrategyStore, error) + + // RequiresLockAndSamplingStore indicates whether this strategy store requires a Lock and SamplingStore + RequiresLockAndSamplingStore() (bool, error) } diff --git a/cmd/collector/main.go b/cmd/collector/main.go index f063b98280d..04e3493b915 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -35,10 +35,12 @@ import ( "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/status" "github.com/jaegertracing/jaeger/pkg/config" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/version" ss "github.com/jaegertracing/jaeger/plugin/sampling/strategystore" "github.com/jaegertracing/jaeger/plugin/storage" "github.com/jaegertracing/jaeger/ports" + "github.com/jaegertracing/jaeger/storage/samplingstore" ) const serviceName = "jaeger-collector" @@ -82,11 +84,19 @@ func main() { if err != nil { logger.Fatal("Failed to create span writer", zap.Error(err)) } - lock, samplingStore, err := storageFactory.CreateLockAndSamplingStore() + + requireLockAndSamplingStore, err := strategyStoreFactory.RequiresLockAndSamplingStore() if err != nil { - logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) + logger.Fatal("Failed to determine if lock and sampling store is required.", zap.Error(err)) + } + var lock distributedlock.Lock + var samplingStore samplingstore.Store + if requireLockAndSamplingStore { + lock, samplingStore, err = storageFactory.CreateLockAndSamplingStore() + if err != nil { + logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) + } } - strategyStoreFactory.InitFromViper(v) if err := strategyStoreFactory.Initialize(metricsFactory, logger, lock, samplingStore); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) diff --git a/plugin/sampling/strategystore/adaptive/factory.go b/plugin/sampling/strategystore/adaptive/factory.go index fdb4025e0b7..a36c61f86af 100644 --- a/plugin/sampling/strategystore/adaptive/factory.go +++ b/plugin/sampling/strategystore/adaptive/factory.go @@ -79,3 +79,8 @@ func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, error) { p.Start() return p, nil } + +// RequiresLockAndSamplingStore implements strategystore.Factory +func (f *Factory) RequiresLockAndSamplingStore() (bool, error) { + return true, nil +} diff --git a/plugin/sampling/strategystore/adaptive/strategy_store.go b/plugin/sampling/strategystore/adaptive/strategy_store.go index dfae25285c8..e1fc6b16d91 100644 --- a/plugin/sampling/strategystore/adaptive/strategy_store.go +++ b/plugin/sampling/strategystore/adaptive/strategy_store.go @@ -1,13 +1,28 @@ +// Copyright (c) 2021 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 adaptive import ( "os" + "github.com/uber/jaeger-lib/metrics" + "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection" "github.com/jaegertracing/jaeger/storage/samplingstore" - "github.com/uber/jaeger-lib/metrics" - "go.uber.org/zap" ) // NewStrategyStore creates a strategy store that holds adaptive sampling strategies. diff --git a/plugin/sampling/strategystore/factory.go b/plugin/sampling/strategystore/factory.go index 1dad28693c8..7798727002a 100644 --- a/plugin/sampling/strategystore/factory.go +++ b/plugin/sampling/strategystore/factory.go @@ -106,3 +106,12 @@ func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, error) { } return factory.CreateStrategyStore() } + +// RequiresLockAndSamplingStore indicates whether or not the configured sampling strategy store +func (f *Factory) RequiresLockAndSamplingStore() (bool, error) { + factory, ok := f.factories[f.StrategyStoreType] + if !ok { + return false, fmt.Errorf("no %s strategy store registered", f.StrategyStoreType) + } + return factory.RequiresLockAndSamplingStore() +} diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index cc1696cd3aa..b4e0b0ccd76 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -124,6 +124,13 @@ func (f *mockFactory) Initialize(metricsFactory metrics.Factory, logger *zap.Log return nil } +func (f *mockFactory) RequiresLockAndSamplingStore() (bool, error) { + if f.retError { + return false, errors.New("error creating store") + } + return false, nil +} + type mockStore struct{} func (m *mockStore) InsertThroughput(throughput []*model.Throughput) error { diff --git a/plugin/sampling/strategystore/static/factory.go b/plugin/sampling/strategystore/static/factory.go index 104bf83a0fc..7b6c2b9f33d 100644 --- a/plugin/sampling/strategystore/static/factory.go +++ b/plugin/sampling/strategystore/static/factory.go @@ -60,3 +60,8 @@ func (f *Factory) Initialize(_ metrics.Factory, logger *zap.Logger, _ distribute func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, error) { return NewStrategyStore(*f.options, f.logger) } + +// RequiresLockAndSamplingStore implements strategystore.Factory +func (f *Factory) RequiresLockAndSamplingStore() (bool, error) { + return false, nil +} diff --git a/storage/mocks/Factory.go b/storage/mocks/Factory.go index db425cace25..3c84a93956c 100644 --- a/storage/mocks/Factory.go +++ b/storage/mocks/Factory.go @@ -15,18 +15,15 @@ package mocks import ( - "github.com/jaegertracing/jaeger/pkg/distributedlock" - dependencystore "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/samplingstore" metrics "github.com/uber/jaeger-lib/metrics" - mock "github.com/stretchr/testify/mock" + zap "go.uber.org/zap" + "github.com/jaegertracing/jaeger/pkg/distributedlock" + dependencystore "github.com/jaegertracing/jaeger/storage/dependencystore" + "github.com/jaegertracing/jaeger/storage/samplingstore" spanstore "github.com/jaegertracing/jaeger/storage/spanstore" - storage "github.com/jaegertracing/jaeger/storage" - - zap "go.uber.org/zap" ) // Factory is an autogenerated mock type for the Factory type From b852076233d0fad6f0c590578ec7120bfa8605a7 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 4 May 2021 15:28:44 -0400 Subject: [PATCH 07/40] Additional tests Signed-off-by: Joe Elliott --- .../strategystore/adaptive/factory.go | 5 -- .../strategystore/adaptive/factory_test.go | 7 +++ plugin/sampling/strategystore/factory.go | 2 +- plugin/sampling/strategystore/factory_test.go | 63 +++++++++++-------- .../strategystore/static/factory_test.go | 7 +++ plugin/storage/badger/factory_test.go | 4 ++ plugin/storage/cassandra/factory.go | 2 +- plugin/storage/es/factory_test.go | 3 + plugin/storage/factory.go | 2 +- plugin/storage/grpc/factory_test.go | 2 + plugin/storage/kafka/factory_test.go | 3 + plugin/storage/memory/factory_test.go | 2 + 12 files changed, 69 insertions(+), 33 deletions(-) diff --git a/plugin/sampling/strategystore/adaptive/factory.go b/plugin/sampling/strategystore/adaptive/factory.go index a36c61f86af..dd52ccfef89 100644 --- a/plugin/sampling/strategystore/adaptive/factory.go +++ b/plugin/sampling/strategystore/adaptive/factory.go @@ -15,7 +15,6 @@ package adaptive import ( - "errors" "flag" "github.com/spf13/viper" @@ -59,10 +58,6 @@ func (f *Factory) InitFromViper(v *viper.Viper) { // Initialize implements strategystore.Factory func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error { - if lock == nil || store == nil { - return errors.New("lock or samplingStore nil. adaptive sampling only supported with Cassandra backend") // todo(jpe): better check/error msg - } - f.logger = logger f.metricsFactory = metricsFactory f.lock = lock diff --git a/plugin/sampling/strategystore/adaptive/factory_test.go b/plugin/sampling/strategystore/adaptive/factory_test.go index ada2a736b0a..cff323c9d3c 100644 --- a/plugin/sampling/strategystore/adaptive/factory_test.go +++ b/plugin/sampling/strategystore/adaptive/factory_test.go @@ -67,6 +67,13 @@ func TestFactory(t *testing.T) { assert.NoError(t, err) } +func TestRequiresLockAndSamplingStore(t *testing.T) { + f := NewFactory() + required, err := f.RequiresLockAndSamplingStore() + assert.True(t, required) + assert.NoError(t, err) +} + type mockStore struct{} func (m *mockStore) InsertThroughput(throughput []*model.Throughput) error { diff --git a/plugin/sampling/strategystore/factory.go b/plugin/sampling/strategystore/factory.go index 7798727002a..4deeb6cee2c 100644 --- a/plugin/sampling/strategystore/factory.go +++ b/plugin/sampling/strategystore/factory.go @@ -107,7 +107,7 @@ func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, error) { return factory.CreateStrategyStore() } -// RequiresLockAndSamplingStore indicates whether or not the configured sampling strategy store +// RequiresLockAndSamplingStore implements strategystore.Factory func (f *Factory) RequiresLockAndSamplingStore() (bool, error) { factory, ok := f.factories[f.StrategyStoreType] if !ok { diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index b4e0b0ccd76..a6623a8fbcc 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -43,35 +43,48 @@ var _ ss.Factory = new(Factory) var _ plugin.Configurable = new(Factory) func TestNewFactory(t *testing.T) { - f, err := NewFactory(FactoryConfig{StrategyStoreType: "static"}) - require.NoError(t, err) - assert.NotEmpty(t, f.factories) - assert.NotEmpty(t, f.factories["static"]) - assert.Equal(t, Kind("static"), f.StrategyStoreType) - - mock := new(mockFactory) - f.factories["static"] = mock + tests := []struct { + strategyStoreType string + expectError bool + }{ + { + strategyStoreType: "static", + }, + { + strategyStoreType: "adaptive", + }, + { + strategyStoreType: "nonsense", + expectError: true, + }, + } lock := &mockLock{} store := &mockStore{} - assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), lock, store)) - _, err = f.CreateStrategyStore() - assert.NoError(t, err) - - // force the mock to return errors - mock.retError = true - assert.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), lock, store), "error initializing store") - _, err = f.CreateStrategyStore() - assert.EqualError(t, err, "error creating store") - - f.StrategyStoreType = "nonsense" - _, err = f.CreateStrategyStore() - assert.EqualError(t, err, "no nonsense strategy store registered") - - _, err = NewFactory(FactoryConfig{StrategyStoreType: "nonsense"}) - require.Error(t, err) - assert.Contains(t, err.Error(), "unknown sampling strategy store type") + for _, tc := range tests { + f, err := NewFactory(FactoryConfig{StrategyStoreType: Kind(tc.strategyStoreType)}) + if tc.expectError { + assert.Error(t, err) + continue + } + assert.NotEmpty(t, f.factories) + assert.NotEmpty(t, f.factories[Kind(tc.strategyStoreType)]) + assert.Equal(t, Kind(tc.strategyStoreType), f.StrategyStoreType) + + mock := new(mockFactory) + f.factories[Kind(tc.strategyStoreType)] = mock + + assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), lock, store)) + _, err = f.CreateStrategyStore() + require.NoError(t, err) + + // force the mock to return errors + mock.retError = true + assert.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), lock, store), "error initializing store") + _, err = f.CreateStrategyStore() + assert.EqualError(t, err, "error creating store") + } } func TestConfigurable(t *testing.T) { diff --git a/plugin/sampling/strategystore/static/factory_test.go b/plugin/sampling/strategystore/static/factory_test.go index 05a8593e29c..04911f59306 100644 --- a/plugin/sampling/strategystore/static/factory_test.go +++ b/plugin/sampling/strategystore/static/factory_test.go @@ -39,3 +39,10 @@ func TestFactory(t *testing.T) { _, err := f.CreateStrategyStore() assert.NoError(t, err) } + +func TestRequiresLockAndSamplingStore(t *testing.T) { + f := NewFactory() + required, err := f.RequiresLockAndSamplingStore() + assert.False(t, required) + assert.NoError(t, err) +} diff --git a/plugin/storage/badger/factory_test.go b/plugin/storage/badger/factory_test.go index d63de46e501..3f54c9f606d 100644 --- a/plugin/storage/badger/factory_test.go +++ b/plugin/storage/badger/factory_test.go @@ -28,6 +28,7 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/config" + "github.com/jaegertracing/jaeger/storage" ) func TestInitializationErrors(t *testing.T) { @@ -68,6 +69,9 @@ func TestForCodecov(t *testing.T) { _, err = f.CreateDependencyReader() assert.NoError(t, err) + _, _, err = f.CreateLockAndSamplingStore() + assert.Equal(t, storage.ErrLockAndSamplingStoreNotSupported, err) + // Now, remove the badger directories err = os.RemoveAll(f.tmpDir) assert.NoError(t, err) diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index 4de12f3bce8..ee7982a5467 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -154,7 +154,7 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { } // CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { // todo(jpe): test store := cSamplingStore.New(f.primarySession, f.primaryMetricsFactory, f.logger) lock := cLock.NewLock(f.primarySession, defaultLockTenant) diff --git a/plugin/storage/es/factory_test.go b/plugin/storage/es/factory_test.go index 728e203d339..631fa3f2c0d 100644 --- a/plugin/storage/es/factory_test.go +++ b/plugin/storage/es/factory_test.go @@ -87,6 +87,9 @@ func TestElasticsearchFactory(t *testing.T) { _, err = f.CreateArchiveSpanWriter() assert.NoError(t, err) assert.NoError(t, f.Close()) + + _, _, err = f.CreateLockAndSamplingStore() + assert.Equal(t, storage.ErrLockAndSamplingStoreNotSupported, err) } func TestElasticsearchTagsFileDoNotExist(t *testing.T) { diff --git a/plugin/storage/factory.go b/plugin/storage/factory.go index 622747fe95a..760db1ec3a7 100644 --- a/plugin/storage/factory.go +++ b/plugin/storage/factory.go @@ -161,7 +161,7 @@ func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { } // CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { // todo(jpe): test for _, factory := range f.factories { lock, store, err := factory.CreateLockAndSamplingStore() if err != nil && err != storage.ErrLockAndSamplingStoreNotSupported { diff --git a/plugin/storage/grpc/factory_test.go b/plugin/storage/grpc/factory_test.go index b8b295435fd..9ad046f732d 100644 --- a/plugin/storage/grpc/factory_test.go +++ b/plugin/storage/grpc/factory_test.go @@ -129,6 +129,8 @@ func TestGRPCStorageFactory(t *testing.T) { depReader, err := f.CreateDependencyReader() assert.NoError(t, err) assert.Equal(t, f.store.DependencyReader(), depReader) + _, _, err = f.CreateLockAndSamplingStore() + assert.Equal(t, storage.ErrLockAndSamplingStoreNotSupported, err) } func TestGRPCStorageFactory_Capabilities(t *testing.T) { diff --git a/plugin/storage/kafka/factory_test.go b/plugin/storage/kafka/factory_test.go index e6538f7c278..61f1da6be4d 100644 --- a/plugin/storage/kafka/factory_test.go +++ b/plugin/storage/kafka/factory_test.go @@ -73,6 +73,9 @@ func TestKafkaFactory(t *testing.T) { _, err = f.CreateDependencyReader() assert.Error(t, err) + _, _, err = f.CreateLockAndSamplingStore() + assert.Equal(t, storage.ErrLockAndSamplingStoreNotSupported, err) + assert.NoError(t, f.Close()) } diff --git a/plugin/storage/memory/factory_test.go b/plugin/storage/memory/factory_test.go index f024f2ff0a4..27f9710b3e8 100644 --- a/plugin/storage/memory/factory_test.go +++ b/plugin/storage/memory/factory_test.go @@ -44,6 +44,8 @@ func TestMemoryStorageFactory(t *testing.T) { depReader, err := f.CreateDependencyReader() assert.NoError(t, err) assert.Equal(t, f.store, depReader) + _, _, err = f.CreateLockAndSamplingStore() + assert.Equal(t, storage.ErrLockAndSamplingStoreNotSupported, err) } func TestWithConfiguration(t *testing.T) { From 006dbbf34e800cfad56ecf400850c6a5e18ccb8a Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 12 May 2021 15:50:27 -0400 Subject: [PATCH 08/40] Passed appropriate settings and added override hostname Signed-off-by: Joe Elliott --- .../strategystore/adaptive/factory_test.go | 2 ++ .../sampling/strategystore/adaptive/options.go | 10 ++++++++++ .../strategystore/adaptive/strategy_store.go | 17 +++++++++++++---- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/plugin/sampling/strategystore/adaptive/factory_test.go b/plugin/sampling/strategystore/adaptive/factory_test.go index cff323c9d3c..ac00f289d92 100644 --- a/plugin/sampling/strategystore/adaptive/factory_test.go +++ b/plugin/sampling/strategystore/adaptive/factory_test.go @@ -46,6 +46,7 @@ func TestFactory(t *testing.T) { "--sampling.min-samples-per-second=1", "--sampling.leader-lease-refresh-interval=1s", "--sampling.follower-lease-refresh-interval=2s", + "--sampling.override-hostname=blerg", }) f.InitFromViper(v) @@ -61,6 +62,7 @@ func TestFactory(t *testing.T) { assert.Equal(t, 1.0, f.options.MinSamplesPerSecond) assert.Equal(t, time.Second, f.options.LeaderLeaseRefreshInterval) assert.Equal(t, time.Second*2, f.options.FollowerLeaseRefreshInterval) + assert.Equal(t, "blerg", f.options.OverrideHostname) assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), &mockLock{}, &mockStore{})) _, err := f.CreateStrategyStore() diff --git a/plugin/sampling/strategystore/adaptive/options.go b/plugin/sampling/strategystore/adaptive/options.go index f1bc2eddc6a..bc6f2c396c6 100644 --- a/plugin/sampling/strategystore/adaptive/options.go +++ b/plugin/sampling/strategystore/adaptive/options.go @@ -33,6 +33,7 @@ const ( minSamplesPerSecond = "sampling.min-samples-per-second" leaderLeaseRefreshInterval = "sampling.leader-lease-refresh-interval" followerLeaseRefreshInterval = "sampling.follower-lease-refresh-interval" + overrideHostname = "sampling.override-hostname" defaultTargetSamplesPerSecond = 1 defaultDeltaTolerance = 0.3 @@ -45,6 +46,7 @@ const ( defaultMinSamplesPerSecond = 1.0 / float64(time.Minute/time.Second) // once every 1 minute defaultLeaderLeaseRefreshInterval = 5 * time.Second defaultFollowerLeaseRefreshInterval = 60 * time.Second + defaultOverrideHostname = "" ) // Options holds configuration for the adaptive sampling strategy store. @@ -111,6 +113,10 @@ type Options struct { // FollowerLeaseRefreshInterval is the duration to sleep if this processor is a follower // (ie. failed to gain the leader lock). FollowerLeaseRefreshInterval time.Duration + + // OverrideHostname overrides the hostname returned from os.Hostname for use in the adaptive sampling + // strategy processor + OverrideHostname string } // AddFlags adds flags for Options @@ -148,6 +154,9 @@ func AddFlags(flagSet *flag.FlagSet) { flagSet.Duration(followerLeaseRefreshInterval, defaultFollowerLeaseRefreshInterval, "The duration to sleep if this processor is a follower.", ) + flagSet.String(overrideHostname, defaultOverrideHostname, + "Overrides os.Hostname() for use in adaptive sampling.", + ) } // InitFromViper initializes Options with properties from viper @@ -163,5 +172,6 @@ func (opts *Options) InitFromViper(v *viper.Viper) *Options { opts.MinSamplesPerSecond = v.GetFloat64(minSamplesPerSecond) opts.LeaderLeaseRefreshInterval = v.GetDuration(leaderLeaseRefreshInterval) opts.FollowerLeaseRefreshInterval = v.GetDuration(followerLeaseRefreshInterval) + opts.OverrideHostname = v.GetString(overrideHostname) return opts } diff --git a/plugin/sampling/strategystore/adaptive/strategy_store.go b/plugin/sampling/strategystore/adaptive/strategy_store.go index e1fc6b16d91..605af2b71a2 100644 --- a/plugin/sampling/strategystore/adaptive/strategy_store.go +++ b/plugin/sampling/strategystore/adaptive/strategy_store.go @@ -27,12 +27,21 @@ import ( // NewStrategyStore creates a strategy store that holds adaptive sampling strategies. func NewStrategyStore(options Options, metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) (*Processor, error) { - hostname, err := os.Hostname() - if err != nil { - return nil, err + hostname := options.OverrideHostname + if len(hostname) == 0 { + var err error + hostname, err = os.Hostname() + if err != nil { + return nil, err + } + logger.Info("Adaptive sampling hostname retrieved from os.Hostname()", zap.String("hostname", hostname)) } - participant := leaderelection.NewElectionParticipant(lock, defaultResourceName, leaderelection.ElectionParticipantOptions{}) // todo(jpe) : wire up options/resource name + participant := leaderelection.NewElectionParticipant(lock, defaultResourceName, leaderelection.ElectionParticipantOptions{ + FollowerLeaseRefreshInterval: options.FollowerLeaseRefreshInterval, + LeaderLeaseRefreshInterval: options.LeaderLeaseRefreshInterval, + Logger: logger, + }) p, err := newProcessor(options, hostname, store, participant, metricsFactory, logger) if err != nil { return nil, err From 922eb5653530c14ba0eeec9d6f7a3b383a6adcf4 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Mon, 17 May 2021 15:11:29 -0400 Subject: [PATCH 09/40] pass hostname to lock Signed-off-by: Joe Elliott --- plugin/storage/cassandra/factory.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index ee7982a5467..7e6c0ed92e8 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -19,6 +19,7 @@ import ( "errors" "flag" "io" + "os" "github.com/spf13/viper" "github.com/uber/jaeger-lib/metrics" @@ -41,8 +42,6 @@ import ( const ( primaryStorageConfig = "cassandra" archiveStorageConfig = "cassandra-archive" - - defaultLockTenant = "jaeger" ) // Factory implements storage.Factory for Cassandra backend. @@ -155,8 +154,13 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { // CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { // todo(jpe): test + hostname, err := os.Hostname() // todo(jpe): wire up --sampling.override-hostname in ./plugin/sampling/strategystore/adaptive/options.go? + if err != nil { + return nil, nil, err + } + store := cSamplingStore.New(f.primarySession, f.primaryMetricsFactory, f.logger) - lock := cLock.NewLock(f.primarySession, defaultLockTenant) + lock := cLock.NewLock(f.primarySession, hostname) return lock, store, nil } From 7c90815e02d2a5014572416c1857c58713cb93ea Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Mon, 17 May 2021 15:43:46 -0400 Subject: [PATCH 10/40] First pass cassandra tables Signed-off-by: Joe Elliott --- plugin/storage/cassandra/schema/docker.sh | 2 +- plugin/storage/cassandra/schema/v003.cql.tmpl | 27 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/plugin/storage/cassandra/schema/docker.sh b/plugin/storage/cassandra/schema/docker.sh index ca812fb2011..150ea20259e 100755 --- a/plugin/storage/cassandra/schema/docker.sh +++ b/plugin/storage/cassandra/schema/docker.sh @@ -3,7 +3,7 @@ # This script is used in the Docker image jaegertracing/jaeger-cassandra-schema # that allows installing Jaeger keyspace and schema without installing cqlsh. -CQLSH=${CQLSH:-"/opt/cassandra/bin/cqlsh"} +CQLSH=${CQLSH:-"/usr/bin/cqlsh"} # todo(jpe) why did i have to change this? CQLSH_HOST=${CQLSH_HOST:-"cassandra"} CQLSH_PORT=${CQLSH_PORT:-"9042"} CQLSH_SSL=${CQLSH_SSL:-""} diff --git a/plugin/storage/cassandra/schema/v003.cql.tmpl b/plugin/storage/cassandra/schema/v003.cql.tmpl index 07b64437a93..09879f9cdad 100644 --- a/plugin/storage/cassandra/schema/v003.cql.tmpl +++ b/plugin/storage/cassandra/schema/v003.cql.tmpl @@ -201,4 +201,29 @@ CREATE TABLE IF NOT EXISTS ${keyspace}.dependencies_v2 ( 'max_threshold': '32', 'class': 'org.apache.cassandra.db.compaction.SizeTieredCompactionStrategy' } - AND default_time_to_live = ${dependencies_ttl}; \ No newline at end of file + AND default_time_to_live = ${dependencies_ttl}; + +-- adaptive sampling tables +-- ./plugin/storage/cassandra/samplingstore/storage.go +CREATE TABLE IF NOT EXISTS ${keyspace}.operation_throughput ( + bucket int, + ts timestamp, + throughput text, + PRIMARY KEY(bucket, ts) +) WITH CLUSTERING ORDER BY (ts desc); + +CREATE TABLE IF NOT EXISTS ${keyspace}.sampling_probabilities ( + bucket int, + ts timestamp, + hostname text, + probabilities text, + PRIMARY KEY(bucket, ts) +) WITH CLUSTERING ORDER BY (ts desc); + +-- distributed lock +-- ./plugin/pkg/distributedlock/cassandra/lock.go +CREATE TABLE IF NOT EXISTS ${keyspace}.leases ( + name text, + owner text, + PRIMARY KEY (name, owner) +); \ No newline at end of file From 9c1e1812177e7a78fe678943a81695379231ecdd Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Mon, 17 May 2021 16:30:47 -0400 Subject: [PATCH 11/40] schema cleanup. start electionParticipant Signed-off-by: Joe Elliott --- plugin/sampling/leaderelection/leader_election.go | 2 ++ plugin/sampling/strategystore/adaptive/processor.go | 8 +++++--- plugin/storage/cassandra/schema/v003.cql.tmpl | 6 +++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/plugin/sampling/leaderelection/leader_election.go b/plugin/sampling/leaderelection/leader_election.go index 56369451104..20963bdc9aa 100644 --- a/plugin/sampling/leaderelection/leader_election.go +++ b/plugin/sampling/leaderelection/leader_election.go @@ -31,6 +31,8 @@ const ( // ElectionParticipant partakes in leader election to become leader. type ElectionParticipant interface { IsLeader() bool + Start() error + Close() error } // DistributedElectionParticipant implements ElectionParticipant on top of a distributed lock. diff --git a/plugin/sampling/strategystore/adaptive/processor.go b/plugin/sampling/strategystore/adaptive/processor.go index 70244aa5ec4..be3b43f2126 100644 --- a/plugin/sampling/strategystore/adaptive/processor.go +++ b/plugin/sampling/strategystore/adaptive/processor.go @@ -17,7 +17,6 @@ package adaptive import ( "context" "errors" - "io" "math" "math/rand" "sync" @@ -165,6 +164,9 @@ func (p *Processor) GetSamplingStrategy(_ context.Context, service string) (*sam // Start initializes and starts the sampling processor which regularly calculates sampling probabilities. func (p *Processor) Start() error { p.logger.Info("starting adaptive sampling processor") + if err := p.electionParticipant.Start(); err != nil { + return err + } p.shutdown = make(chan struct{}) p.loadProbabilities() p.generateStrategyResponses() @@ -176,8 +178,8 @@ func (p *Processor) Start() error { // Close stops the processor from calculating probabilities. func (p *Processor) Close() error { p.logger.Info("stopping adaptive sampling processor") - if closer, ok := p.electionParticipant.(io.Closer); ok { - closer.Close() + if err := p.electionParticipant.Close(); err != nil { + return err } close(p.shutdown) return nil diff --git a/plugin/storage/cassandra/schema/v003.cql.tmpl b/plugin/storage/cassandra/schema/v003.cql.tmpl index 09879f9cdad..6ed90e9dee9 100644 --- a/plugin/storage/cassandra/schema/v003.cql.tmpl +++ b/plugin/storage/cassandra/schema/v003.cql.tmpl @@ -207,14 +207,14 @@ CREATE TABLE IF NOT EXISTS ${keyspace}.dependencies_v2 ( -- ./plugin/storage/cassandra/samplingstore/storage.go CREATE TABLE IF NOT EXISTS ${keyspace}.operation_throughput ( bucket int, - ts timestamp, + ts timeuuid, throughput text, PRIMARY KEY(bucket, ts) ) WITH CLUSTERING ORDER BY (ts desc); CREATE TABLE IF NOT EXISTS ${keyspace}.sampling_probabilities ( bucket int, - ts timestamp, + ts timeuuid, hostname text, probabilities text, PRIMARY KEY(bucket, ts) @@ -225,5 +225,5 @@ CREATE TABLE IF NOT EXISTS ${keyspace}.sampling_probabilities ( CREATE TABLE IF NOT EXISTS ${keyspace}.leases ( name text, owner text, - PRIMARY KEY (name, owner) + PRIMARY KEY (name) ); \ No newline at end of file From 1c08ab9df78df3b66763576f255e674713f4dd31 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Mon, 17 May 2021 16:38:01 -0400 Subject: [PATCH 12/40] revert cqlsh path Signed-off-by: Joe Elliott --- docker-compose/jaeger-docker-compose.yml | 2 ++ plugin/storage/cassandra/schema/docker.sh | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docker-compose/jaeger-docker-compose.yml b/docker-compose/jaeger-docker-compose.yml index 8cf98d03e54..ac73fb8ecd1 100644 --- a/docker-compose/jaeger-docker-compose.yml +++ b/docker-compose/jaeger-docker-compose.yml @@ -4,6 +4,8 @@ services: jaeger-collector: image: jaegertracing/jaeger-collector command: ["--cassandra.keyspace=jaeger_v1_dc1", "--cassandra.servers=cassandra", "--collector.zipkin.host-port=9411"] + environment: + - SAMPLING_TYPE=adaptive ports: - "14269" - "14268:14268" diff --git a/plugin/storage/cassandra/schema/docker.sh b/plugin/storage/cassandra/schema/docker.sh index 150ea20259e..ca812fb2011 100755 --- a/plugin/storage/cassandra/schema/docker.sh +++ b/plugin/storage/cassandra/schema/docker.sh @@ -3,7 +3,7 @@ # This script is used in the Docker image jaegertracing/jaeger-cassandra-schema # that allows installing Jaeger keyspace and schema without installing cqlsh. -CQLSH=${CQLSH:-"/usr/bin/cqlsh"} # todo(jpe) why did i have to change this? +CQLSH=${CQLSH:-"/opt/cassandra/bin/cqlsh"} CQLSH_HOST=${CQLSH_HOST:-"cassandra"} CQLSH_PORT=${CQLSH_PORT:-"9042"} CQLSH_SSL=${CQLSH_SSL:-""} From cf2338c9ee293480f5cbca9507392248698567be Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 9 Jun 2021 15:28:11 -0400 Subject: [PATCH 13/40] compose updates Signed-off-by: Joe Elliott --- docker-compose/jaeger-docker-compose.yml | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/docker-compose/jaeger-docker-compose.yml b/docker-compose/jaeger-docker-compose.yml index ac73fb8ecd1..49396f01c95 100644 --- a/docker-compose/jaeger-docker-compose.yml +++ b/docker-compose/jaeger-docker-compose.yml @@ -1,13 +1,31 @@ version: '2' services: + hotrod: + image: jaegertracing/example-hotrod:latest + ports: + - '8080:8080' + - '8083:8083' + command: ["-m","prometheus","all"] + environment: + - JAEGER_AGENT_HOST=jaeger-agent + - JAEGER_AGENT_PORT=6831 + - JAEGER_SAMPLING_ENDPOINT=http://jaeger-agent:5778/sampling + depends_on: + - jaeger-agent + jaeger-collector: image: jaegertracing/jaeger-collector - command: ["--cassandra.keyspace=jaeger_v1_dc1", "--cassandra.servers=cassandra", "--collector.zipkin.host-port=9411"] + command: + - "--cassandra.keyspace=jaeger_v1_dc1" + - "--cassandra.servers=cassandra" + - "--collector.zipkin.host-port=9411" + - "--sampling.initial-sampling-probability=.5" + - "--sampling.target-samples-per-second=.01" environment: - SAMPLING_TYPE=adaptive ports: - - "14269" + - "14269:14269" - "14268:14268" - "14250" - "9411:9411" From 044a5be1391c76e662980223a6c93c396ab9b5fc Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 15 Jun 2021 11:03:28 -0400 Subject: [PATCH 14/40] Pass in processspan Signed-off-by: Joe Elliott --- cmd/all-in-one/main.go | 21 ++++++--- cmd/collector/app/collector.go | 45 ++++++++++--------- .../app/sampling/strategystore/factory.go | 2 +- .../app/sampling/strategystore/interface.go | 12 +++++ cmd/collector/app/span_handler_builder.go | 4 +- cmd/collector/app/span_processor.go | 7 ++- cmd/collector/app/span_processor_test.go | 17 ++++--- cmd/collector/main.go | 20 ++++++--- .../strategystore/adaptive/aggregator.go | 15 +------ .../strategystore/adaptive/factory.go | 8 ++-- .../strategystore/adaptive/factory_test.go | 2 +- .../adaptive/root_span_handler.go | 3 +- plugin/sampling/strategystore/factory.go | 4 +- plugin/sampling/strategystore/factory_test.go | 10 ++--- .../sampling/strategystore/static/factory.go | 9 +++- .../strategystore/static/factory_test.go | 2 +- 16 files changed, 106 insertions(+), 75 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index cb4c5552d39..a33976e4483 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -36,6 +36,7 @@ import ( agentRep "github.com/jaegertracing/jaeger/cmd/agent/app/reporter" agentGrpcRep "github.com/jaegertracing/jaeger/cmd/agent/app/reporter/grpc" "github.com/jaegertracing/jaeger/cmd/all-in-one/setupcontext" + "github.com/jaegertracing/jaeger/cmd/collector/app" collectorApp "github.com/jaegertracing/jaeger/cmd/collector/app" "github.com/jaegertracing/jaeger/cmd/docs" "github.com/jaegertracing/jaeger/cmd/env" @@ -48,6 +49,7 @@ import ( "github.com/jaegertracing/jaeger/pkg/version" metricsPlugin "github.com/jaegertracing/jaeger/plugin/metrics" ss "github.com/jaegertracing/jaeger/plugin/sampling/strategystore" + "github.com/jaegertracing/jaeger/plugin/sampling/strategystore/adaptive" "github.com/jaegertracing/jaeger/plugin/storage" "github.com/jaegertracing/jaeger/ports" "github.com/jaegertracing/jaeger/storage/dependencystore" @@ -143,10 +145,14 @@ by default uses only in-memory database.`, if err := strategyStoreFactory.Initialize(metricsFactory, logger, lock, samplingStore); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) } - strategyStore, err := strategyStoreFactory.CreateStrategyStore() + strategyStore, aggregator, err := strategyStoreFactory.CreateStrategyStore() if err != nil { logger.Fatal("Failed to create sampling strategy store", zap.Error(err)) } + var additionalProcessors []app.ProcessSpan + if aggregator != nil { + additionalProcessors = append(additionalProcessors, adaptive.HandleRootSpan(aggregator, logger)) + } aOpts := new(agentApp.Builder).InitFromViper(v) repOpts := new(agentRep.Options).InitFromViper(v, logger) @@ -156,12 +162,13 @@ by default uses only in-memory database.`, // collector c := collectorApp.New(&collectorApp.CollectorParams{ - ServiceName: "jaeger-collector", - Logger: logger, - MetricsFactory: metricsFactory, - SpanWriter: spanWriter, - StrategyStore: strategyStore, - HealthCheck: svc.HC(), + ServiceName: "jaeger-collector", + Logger: logger, + MetricsFactory: metricsFactory, + SpanWriter: spanWriter, + StrategyStore: strategyStore, + HealthCheck: svc.HC(), + AdditionalProcessors: additionalProcessors, }) if err := c.Start(cOpts); err != nil { log.Fatal(err) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index f46f86a2f5d..3dc293bd747 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -35,14 +35,15 @@ import ( // Collector returns the collector as a manageable unit of work type Collector struct { // required to start a new collector - serviceName string - logger *zap.Logger - metricsFactory metrics.Factory - spanWriter spanstore.Writer - strategyStore strategystore.StrategyStore - hCheck *healthcheck.HealthCheck - spanProcessor processor.SpanProcessor - spanHandlers *SpanHandlers + serviceName string + logger *zap.Logger + metricsFactory metrics.Factory + spanWriter spanstore.Writer + strategyStore strategystore.StrategyStore + additionalProcessors []ProcessSpan + hCheck *healthcheck.HealthCheck + spanProcessor processor.SpanProcessor + spanHandlers *SpanHandlers // state, read only hServer *http.Server @@ -54,23 +55,25 @@ type Collector struct { // CollectorParams to construct a new Jaeger Collector. type CollectorParams struct { - ServiceName string - Logger *zap.Logger - MetricsFactory metrics.Factory - SpanWriter spanstore.Writer - StrategyStore strategystore.StrategyStore - HealthCheck *healthcheck.HealthCheck + ServiceName string + Logger *zap.Logger + MetricsFactory metrics.Factory + SpanWriter spanstore.Writer + StrategyStore strategystore.StrategyStore + AdditionalProcessors []ProcessSpan + HealthCheck *healthcheck.HealthCheck } // New constructs a new collector component, ready to be started func New(params *CollectorParams) *Collector { return &Collector{ - serviceName: params.ServiceName, - logger: params.Logger, - metricsFactory: params.MetricsFactory, - spanWriter: params.SpanWriter, - strategyStore: params.StrategyStore, - hCheck: params.HealthCheck, + serviceName: params.ServiceName, + logger: params.Logger, + metricsFactory: params.MetricsFactory, + spanWriter: params.SpanWriter, + strategyStore: params.StrategyStore, + additionalProcessors: params.AdditionalProcessors, + hCheck: params.HealthCheck, } } @@ -83,7 +86,7 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error { MetricsFactory: c.metricsFactory, } - c.spanProcessor = handlerBuilder.BuildSpanProcessor() + c.spanProcessor = handlerBuilder.BuildSpanProcessor(c.additionalProcessors...) c.spanHandlers = handlerBuilder.BuildHandlers(c.spanProcessor) grpcServer, err := server.StartGRPCServer(&server.GRPCServerParams{ diff --git a/cmd/collector/app/sampling/strategystore/factory.go b/cmd/collector/app/sampling/strategystore/factory.go index 2093c3015ec..2808a8f4474 100644 --- a/cmd/collector/app/sampling/strategystore/factory.go +++ b/cmd/collector/app/sampling/strategystore/factory.go @@ -33,7 +33,7 @@ type Factory interface { Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error // CreateStrategyStore initializes the StrategyStore and returns it. - CreateStrategyStore() (StrategyStore, error) + CreateStrategyStore() (StrategyStore, Aggregator, error) // RequiresLockAndSamplingStore indicates whether this strategy store requires a Lock and SamplingStore RequiresLockAndSamplingStore() (bool, error) diff --git a/cmd/collector/app/sampling/strategystore/interface.go b/cmd/collector/app/sampling/strategystore/interface.go index df6ae9de89a..5b0c238d30d 100644 --- a/cmd/collector/app/sampling/strategystore/interface.go +++ b/cmd/collector/app/sampling/strategystore/interface.go @@ -25,3 +25,15 @@ type StrategyStore interface { // GetSamplingStrategy retrieves the sampling strategy for the specified service. GetSamplingStrategy(ctx context.Context, serviceName string) (*sampling.SamplingStrategyResponse, error) } + +// Aggregator defines an interface used to aggregate operation throughput. +type Aggregator interface { + // RecordThroughput records throughput for an operation for aggregation. + RecordThroughput(service, operation, samplerType string, probability float64) + + // Start starts aggregating operation throughput. + Start() + + // Stop stops the aggregator from aggregating throughput. + Stop() +} diff --git a/cmd/collector/app/span_handler_builder.go b/cmd/collector/app/span_handler_builder.go index 46fa5728eb5..301cecfbf5d 100644 --- a/cmd/collector/app/span_handler_builder.go +++ b/cmd/collector/app/span_handler_builder.go @@ -44,13 +44,14 @@ type SpanHandlers struct { } // BuildSpanProcessor builds the span processor to be used with the handlers -func (b *SpanHandlerBuilder) BuildSpanProcessor() processor.SpanProcessor { +func (b *SpanHandlerBuilder) BuildSpanProcessor(additional ...ProcessSpan) processor.SpanProcessor { hostname, _ := os.Hostname() svcMetrics := b.metricsFactory() hostMetrics := svcMetrics.Namespace(metrics.NSOptions{Tags: map[string]string{"host": hostname}}) return NewSpanProcessor( b.SpanWriter, + additional, Options.ServiceMetrics(svcMetrics), Options.HostMetrics(hostMetrics), Options.Logger(b.logger()), @@ -61,7 +62,6 @@ func (b *SpanHandlerBuilder) BuildSpanProcessor() processor.SpanProcessor { Options.DynQueueSizeWarmup(uint(b.CollectorOpts.QueueSize)), // same as queue size for now Options.DynQueueSizeMemory(b.CollectorOpts.DynQueueSizeMemory), ) - } // BuildHandlers builds span handlers (Zipkin, Jaeger) diff --git a/cmd/collector/app/span_processor.go b/cmd/collector/app/span_processor.go index 119e66ece66..9b2e540e8c4 100644 --- a/cmd/collector/app/span_processor.go +++ b/cmd/collector/app/span_processor.go @@ -66,9 +66,10 @@ type queueItem struct { // NewSpanProcessor returns a SpanProcessor that preProcesses, filters, queues, sanitizes, and processes spans func NewSpanProcessor( spanWriter spanstore.Writer, + additional []ProcessSpan, opts ...Option, ) processor.SpanProcessor { - sp := newSpanProcessor(spanWriter, opts...) + sp := newSpanProcessor(spanWriter, additional, opts...) sp.queue.StartConsumers(sp.numWorkers, func(item interface{}) { value := item.(*queueItem) @@ -84,7 +85,7 @@ func NewSpanProcessor( return sp } -func newSpanProcessor(spanWriter spanstore.Writer, opts ...Option) *spanProcessor { +func newSpanProcessor(spanWriter spanstore.Writer, additional []ProcessSpan, opts ...Option) *spanProcessor { // todo(jpe): test additional options := Options.apply(opts...) handlerMetrics := NewSpanProcessorMetrics( options.serviceMetrics, @@ -122,6 +123,8 @@ func newSpanProcessor(spanWriter spanstore.Writer, opts ...Option) *spanProcesso processSpanFuncs = append(processSpanFuncs, sp.countSpan) } + processSpanFuncs = append(processSpanFuncs, additional...) + sp.processSpan = ChainedProcessSpan(processSpanFuncs...) return &sp } diff --git a/cmd/collector/app/span_processor_test.go b/cmd/collector/app/span_processor_test.go index 67f358745a7..009f95e1d79 100644 --- a/cmd/collector/app/span_processor_test.go +++ b/cmd/collector/app/span_processor_test.go @@ -84,6 +84,7 @@ func TestBySvcMetrics(t *testing.T) { hostMetrics := mb.Namespace(metrics.NSOptions{Name: "host", Tags: nil}) sp := newSpanProcessor( &fakeSpanWriter{}, + nil, Options.ServiceMetrics(serviceMetrics), Options.HostMetrics(hostMetrics), Options.Logger(logger), @@ -219,7 +220,7 @@ func makeJaegerSpan(service string, rootSpan bool, debugEnabled bool) (*jaeger.S func TestSpanProcessor(t *testing.T) { w := &fakeSpanWriter{} - p := NewSpanProcessor(w, Options.QueueSize(1)).(*spanProcessor) + p := NewSpanProcessor(w, nil, Options.QueueSize(1)).(*spanProcessor) res, err := p.ProcessSpans([]*model.Span{ { @@ -241,6 +242,7 @@ func TestSpanProcessorErrors(t *testing.T) { mb := metricstest.NewFactory(time.Hour) serviceMetrics := mb.Namespace(metrics.NSOptions{Name: "service", Tags: nil}) p := NewSpanProcessor(w, + nil, Options.Logger(logger), Options.ServiceMetrics(serviceMetrics), Options.QueueSize(1), @@ -283,6 +285,7 @@ func (w *blockingWriter) WriteSpan(ctx context.Context, span *model.Span) error func TestSpanProcessorBusy(t *testing.T) { w := &blockingWriter{} p := NewSpanProcessor(w, + nil, Options.NumWorkers(1), Options.QueueSize(1), Options.ReportBusy(true), @@ -321,7 +324,7 @@ func TestSpanProcessorWithNilProcess(t *testing.T) { serviceMetrics := mb.Namespace(metrics.NSOptions{Name: "service", Tags: nil}) w := &fakeSpanWriter{} - p := NewSpanProcessor(w, Options.ServiceMetrics(serviceMetrics)).(*spanProcessor) + p := NewSpanProcessor(w, nil, Options.ServiceMetrics(serviceMetrics)).(*spanProcessor) defer assert.NoError(t, p.Close()) p.saveSpan(&model.Span{}) @@ -340,7 +343,7 @@ func TestSpanProcessorWithCollectorTags(t *testing.T) { } w := &fakeSpanWriter{} - p := NewSpanProcessor(w, Options.CollectorTags(testCollectorTags)).(*spanProcessor) + p := NewSpanProcessor(w, nil, Options.CollectorTags(testCollectorTags)).(*spanProcessor) defer assert.NoError(t, p.Close()) span := &model.Span{ @@ -385,7 +388,7 @@ func TestSpanProcessorCountSpan(t *testing.T) { m := mb.Namespace(metrics.NSOptions{}) w := &fakeSpanWriter{} - p := NewSpanProcessor(w, Options.HostMetrics(m), Options.DynQueueSizeMemory(1000)).(*spanProcessor) + p := NewSpanProcessor(w, nil, Options.HostMetrics(m), Options.DynQueueSizeMemory(1000)).(*spanProcessor) p.background(10*time.Millisecond, p.updateGauges) p.processSpan(&model.Span{}) @@ -482,7 +485,7 @@ func TestUpdateDynQueueSize(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { w := &fakeSpanWriter{} - p := newSpanProcessor(w, Options.QueueSize(tt.initialCapacity), Options.DynQueueSizeWarmup(tt.warmup), Options.DynQueueSizeMemory(tt.sizeInBytes)) + p := newSpanProcessor(w, nil, Options.QueueSize(tt.initialCapacity), Options.DynQueueSizeWarmup(tt.warmup), Options.DynQueueSizeMemory(tt.sizeInBytes)) assert.EqualValues(t, tt.initialCapacity, p.queue.Capacity()) p.spansProcessed = atomic.NewUint64(tt.spansProcessed) @@ -496,14 +499,14 @@ func TestUpdateDynQueueSize(t *testing.T) { func TestUpdateQueueSizeNoActivityYet(t *testing.T) { w := &fakeSpanWriter{} - p := newSpanProcessor(w, Options.QueueSize(1), Options.DynQueueSizeWarmup(1), Options.DynQueueSizeMemory(1)) + p := newSpanProcessor(w, nil, Options.QueueSize(1), Options.DynQueueSizeWarmup(1), Options.DynQueueSizeMemory(1)) assert.NotPanics(t, p.updateQueueSize) } func TestStartDynQueueSizeUpdater(t *testing.T) { w := &fakeSpanWriter{} oneGiB := uint(1024 * 1024 * 1024) - p := newSpanProcessor(w, Options.QueueSize(100), Options.DynQueueSizeWarmup(1000), Options.DynQueueSizeMemory(oneGiB)) + p := newSpanProcessor(w, nil, Options.QueueSize(100), Options.DynQueueSizeWarmup(1000), Options.DynQueueSizeMemory(oneGiB)) assert.EqualValues(t, 100, p.queue.Capacity()) p.spansProcessed = atomic.NewUint64(1000) diff --git a/cmd/collector/main.go b/cmd/collector/main.go index 04e3493b915..03c909166df 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -38,6 +38,7 @@ import ( "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/version" ss "github.com/jaegertracing/jaeger/plugin/sampling/strategystore" + "github.com/jaegertracing/jaeger/plugin/sampling/strategystore/adaptive" "github.com/jaegertracing/jaeger/plugin/storage" "github.com/jaegertracing/jaeger/ports" "github.com/jaegertracing/jaeger/storage/samplingstore" @@ -101,18 +102,23 @@ func main() { if err := strategyStoreFactory.Initialize(metricsFactory, logger, lock, samplingStore); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) } - strategyStore, err := strategyStoreFactory.CreateStrategyStore() + strategyStore, aggregator, err := strategyStoreFactory.CreateStrategyStore() if err != nil { logger.Fatal("Failed to create sampling strategy store", zap.Error(err)) } + var additionalProcessors []app.ProcessSpan + if aggregator != nil { + additionalProcessors = append(additionalProcessors, adaptive.HandleRootSpan(aggregator, logger)) + } c := app.New(&app.CollectorParams{ - ServiceName: serviceName, - Logger: logger, - MetricsFactory: metricsFactory, - SpanWriter: spanWriter, - StrategyStore: strategyStore, - HealthCheck: svc.HC(), + ServiceName: serviceName, + Logger: logger, + MetricsFactory: metricsFactory, + SpanWriter: spanWriter, + StrategyStore: strategyStore, + HealthCheck: svc.HC(), + AdditionalProcessors: additionalProcessors, }) collectorOpts := new(app.CollectorOptions).InitFromViper(v) if err := c.Start(collectorOpts); err != nil { diff --git a/plugin/sampling/strategystore/adaptive/aggregator.go b/plugin/sampling/strategystore/adaptive/aggregator.go index bd674782d39..e055d2816cd 100644 --- a/plugin/sampling/strategystore/adaptive/aggregator.go +++ b/plugin/sampling/strategystore/adaptive/aggregator.go @@ -22,6 +22,7 @@ import ( "github.com/uber/jaeger-lib/metrics" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" + "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" "github.com/jaegertracing/jaeger/storage/samplingstore" ) @@ -29,18 +30,6 @@ const ( maxProbabilities = 10 ) -// Aggregator defines an interface used to aggregate operation throughput. -type Aggregator interface { - // RecordThroughput records throughput for an operation for aggregation. - RecordThroughput(service, operation, samplerType string, probability float64) - - // Start starts aggregating operation throughput. - Start() - - // Stop stops the aggregator from aggregating throughput. - Stop() -} - type aggregator struct { sync.Mutex @@ -54,7 +43,7 @@ type aggregator struct { // NewAggregator creates a throughput aggregator that simply emits metrics // about the number of operations seen over the aggregationInterval. -func NewAggregator(metricsFactory metrics.Factory, interval time.Duration, storage samplingstore.Store) Aggregator { +func NewAggregator(metricsFactory metrics.Factory, interval time.Duration, storage samplingstore.Store) strategystore.Aggregator { return &aggregator{ operationsCounter: metricsFactory.Counter(metrics.Options{Name: "sampling_operations"}), servicesCounter: metricsFactory.Counter(metrics.Options{Name: "sampling_services"}), diff --git a/plugin/sampling/strategystore/adaptive/factory.go b/plugin/sampling/strategystore/adaptive/factory.go index dd52ccfef89..b632959ad82 100644 --- a/plugin/sampling/strategystore/adaptive/factory.go +++ b/plugin/sampling/strategystore/adaptive/factory.go @@ -66,13 +66,15 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger, } // CreateStrategyStore implements strategystore.Factory -func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, error) { +func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, strategystore.Aggregator, error) { p, err := NewStrategyStore(*f.options, f.metricsFactory, f.logger, f.lock, f.store) if err != nil { - return nil, err + return nil, nil, err } p.Start() - return p, nil + a := NewAggregator(f.metricsFactory, f.options.CalculationInterval, f.store) // todo(jpe): is CalculationInterval correct? + a.Start() // todo(jpe): what calls stop? + return p, a, nil } // RequiresLockAndSamplingStore implements strategystore.Factory diff --git a/plugin/sampling/strategystore/adaptive/factory_test.go b/plugin/sampling/strategystore/adaptive/factory_test.go index ac00f289d92..19068fbfdbc 100644 --- a/plugin/sampling/strategystore/adaptive/factory_test.go +++ b/plugin/sampling/strategystore/adaptive/factory_test.go @@ -65,7 +65,7 @@ func TestFactory(t *testing.T) { assert.Equal(t, "blerg", f.options.OverrideHostname) assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), &mockLock{}, &mockStore{})) - _, err := f.CreateStrategyStore() + _, _, err := f.CreateStrategyStore() assert.NoError(t, err) } diff --git a/plugin/sampling/strategystore/adaptive/root_span_handler.go b/plugin/sampling/strategystore/adaptive/root_span_handler.go index ec91af6f661..d55d744fc30 100644 --- a/plugin/sampling/strategystore/adaptive/root_span_handler.go +++ b/plugin/sampling/strategystore/adaptive/root_span_handler.go @@ -18,11 +18,12 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app" + "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" "github.com/jaegertracing/jaeger/model" ) // HandleRootSpan returns a function that records throughput for root spans -func HandleRootSpan(aggregator Aggregator, logger *zap.Logger) app.ProcessSpan { +func HandleRootSpan(aggregator strategystore.Aggregator, logger *zap.Logger) app.ProcessSpan { return func(span *model.Span) { // TODO simply checking parentId to determine if a span is a root span is not sufficient. However, // we can be sure that only a root span will have sampler tags. diff --git a/plugin/sampling/strategystore/factory.go b/plugin/sampling/strategystore/factory.go index 4deeb6cee2c..2a3734630c9 100644 --- a/plugin/sampling/strategystore/factory.go +++ b/plugin/sampling/strategystore/factory.go @@ -99,10 +99,10 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger, } // CreateStrategyStore implements strategystore.Factory -func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, error) { +func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, strategystore.Aggregator, error) { factory, ok := f.factories[f.StrategyStoreType] if !ok { - return nil, fmt.Errorf("no %s strategy store registered", f.StrategyStoreType) + return nil, nil, fmt.Errorf("no %s strategy store registered", f.StrategyStoreType) } return factory.CreateStrategyStore() } diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index a6623a8fbcc..3e2f35d8cc6 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -76,13 +76,13 @@ func TestNewFactory(t *testing.T) { f.factories[Kind(tc.strategyStoreType)] = mock assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), lock, store)) - _, err = f.CreateStrategyStore() + _, _, err = f.CreateStrategyStore() require.NoError(t, err) // force the mock to return errors mock.retError = true assert.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), lock, store), "error initializing store") - _, err = f.CreateStrategyStore() + _, _, err = f.CreateStrategyStore() assert.EqualError(t, err, "error creating store") } } @@ -123,11 +123,11 @@ func (f *mockFactory) InitFromViper(v *viper.Viper) { f.viper = v } -func (f *mockFactory) CreateStrategyStore() (ss.StrategyStore, error) { +func (f *mockFactory) CreateStrategyStore() (ss.StrategyStore, ss.Aggregator, error) { if f.retError { - return nil, errors.New("error creating store") + return nil, nil, errors.New("error creating store") } - return nil, nil + return nil, nil, nil } func (f *mockFactory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error { diff --git a/plugin/sampling/strategystore/static/factory.go b/plugin/sampling/strategystore/static/factory.go index 7b6c2b9f33d..cc02b4d825f 100644 --- a/plugin/sampling/strategystore/static/factory.go +++ b/plugin/sampling/strategystore/static/factory.go @@ -57,8 +57,13 @@ func (f *Factory) Initialize(_ metrics.Factory, logger *zap.Logger, _ distribute } // CreateStrategyStore implements strategystore.Factory -func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, error) { - return NewStrategyStore(*f.options, f.logger) +func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, strategystore.Aggregator, error) { + s, err := NewStrategyStore(*f.options, f.logger) + if err != nil { + return nil, nil, err + } + + return s, nil, nil } // RequiresLockAndSamplingStore implements strategystore.Factory diff --git a/plugin/sampling/strategystore/static/factory_test.go b/plugin/sampling/strategystore/static/factory_test.go index 04911f59306..d109cb946cd 100644 --- a/plugin/sampling/strategystore/static/factory_test.go +++ b/plugin/sampling/strategystore/static/factory_test.go @@ -36,7 +36,7 @@ func TestFactory(t *testing.T) { f.InitFromViper(v) assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), nil, nil)) - _, err := f.CreateStrategyStore() + _, _, err := f.CreateStrategyStore() assert.NoError(t, err) } From a0bb1c0de5c588bd8bcfcc777c53b8ccd0a539d6 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 15 Jun 2021 12:21:35 -0400 Subject: [PATCH 15/40] Rearranged hotrod startup to allow env override Signed-off-by: Joe Elliott --- docker-compose/jaeger-docker-compose.yml | 1 + examples/hotrod/pkg/tracing/init.go | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docker-compose/jaeger-docker-compose.yml b/docker-compose/jaeger-docker-compose.yml index 49396f01c95..6adb9156cc0 100644 --- a/docker-compose/jaeger-docker-compose.yml +++ b/docker-compose/jaeger-docker-compose.yml @@ -10,6 +10,7 @@ services: environment: - JAEGER_AGENT_HOST=jaeger-agent - JAEGER_AGENT_PORT=6831 + - JAEGER_SAMPLER_TYPE=remote - JAEGER_SAMPLING_ENDPOINT=http://jaeger-agent:5778/sampling depends_on: - jaeger-agent diff --git a/examples/hotrod/pkg/tracing/init.go b/examples/hotrod/pkg/tracing/init.go index e22f35373b4..ac7eea460dc 100644 --- a/examples/hotrod/pkg/tracing/init.go +++ b/examples/hotrod/pkg/tracing/init.go @@ -30,14 +30,18 @@ import ( // Init creates a new instance of Jaeger tracer. func Init(serviceName string, metricsFactory metrics.Factory, logger log.Factory) opentracing.Tracer { - cfg, err := config.FromEnv() - if err != nil { - logger.Bg().Fatal("cannot parse Jaeger env vars", zap.Error(err)) + cfg := &config.Configuration{ + Sampler: &config.SamplerConfig{}, } cfg.ServiceName = serviceName cfg.Sampler.Type = "const" cfg.Sampler.Param = 1 + _, err := cfg.FromEnv() + if err != nil { + logger.Bg().Fatal("cannot parse Jaeger env vars", zap.Error(err)) + } + // TODO(ys) a quick hack to ensure random generators get different seeds, which are based on current time. time.Sleep(100 * time.Millisecond) jaegerLogger := jaegerLoggerAdapter{logger.Bg()} From 2635b43308a5a460e97e9cf286f5dd9c60fe912b Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 15 Jun 2021 13:33:40 -0400 Subject: [PATCH 16/40] call aggregator.Stop() Signed-off-by: Joe Elliott --- cmd/collector/main.go | 1 + plugin/sampling/strategystore/adaptive/factory.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/collector/main.go b/cmd/collector/main.go index 03c909166df..9a48edaabc8 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -106,6 +106,7 @@ func main() { if err != nil { logger.Fatal("Failed to create sampling strategy store", zap.Error(err)) } + defer aggregator.Stop() var additionalProcessors []app.ProcessSpan if aggregator != nil { additionalProcessors = append(additionalProcessors, adaptive.HandleRootSpan(aggregator, logger)) diff --git a/plugin/sampling/strategystore/adaptive/factory.go b/plugin/sampling/strategystore/adaptive/factory.go index b632959ad82..33f771f6da8 100644 --- a/plugin/sampling/strategystore/adaptive/factory.go +++ b/plugin/sampling/strategystore/adaptive/factory.go @@ -72,8 +72,8 @@ func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, strategyst return nil, nil, err } p.Start() - a := NewAggregator(f.metricsFactory, f.options.CalculationInterval, f.store) // todo(jpe): is CalculationInterval correct? - a.Start() // todo(jpe): what calls stop? + a := NewAggregator(f.metricsFactory, f.options.CalculationInterval, f.store) + a.Start() return p, a, nil } From 6f44e1d71247072bd92079d3d984653ceeecf381 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 15 Jun 2021 13:53:28 -0400 Subject: [PATCH 17/40] Made aggregator make sense Signed-off-by: Joe Elliott --- cmd/all-in-one/main.go | 20 +-- cmd/collector/app/collector.go | 58 +++++--- .../collector/app}/root_span_handler.go | 9 +- .../collector/app}/root_span_handler_test.go | 4 +- cmd/collector/main.go | 21 +-- model/span.go | 48 ++++++ model/span_test.go | 113 +++++++++++++++ .../sampling/strategystore/adaptive/span.go | 70 --------- .../strategystore/adaptive/span_test.go | 137 ------------------ 9 files changed, 215 insertions(+), 265 deletions(-) rename {plugin/sampling/strategystore/adaptive => cmd/collector/app}/root_span_handler.go (81%) rename {plugin/sampling/strategystore/adaptive => cmd/collector/app}/root_span_handler_test.go (96%) delete mode 100644 plugin/sampling/strategystore/adaptive/span.go delete mode 100644 plugin/sampling/strategystore/adaptive/span_test.go diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index a33976e4483..edb897579b0 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -36,7 +36,6 @@ import ( agentRep "github.com/jaegertracing/jaeger/cmd/agent/app/reporter" agentGrpcRep "github.com/jaegertracing/jaeger/cmd/agent/app/reporter/grpc" "github.com/jaegertracing/jaeger/cmd/all-in-one/setupcontext" - "github.com/jaegertracing/jaeger/cmd/collector/app" collectorApp "github.com/jaegertracing/jaeger/cmd/collector/app" "github.com/jaegertracing/jaeger/cmd/docs" "github.com/jaegertracing/jaeger/cmd/env" @@ -49,7 +48,6 @@ import ( "github.com/jaegertracing/jaeger/pkg/version" metricsPlugin "github.com/jaegertracing/jaeger/plugin/metrics" ss "github.com/jaegertracing/jaeger/plugin/sampling/strategystore" - "github.com/jaegertracing/jaeger/plugin/sampling/strategystore/adaptive" "github.com/jaegertracing/jaeger/plugin/storage" "github.com/jaegertracing/jaeger/ports" "github.com/jaegertracing/jaeger/storage/dependencystore" @@ -149,10 +147,6 @@ by default uses only in-memory database.`, if err != nil { logger.Fatal("Failed to create sampling strategy store", zap.Error(err)) } - var additionalProcessors []app.ProcessSpan - if aggregator != nil { - additionalProcessors = append(additionalProcessors, adaptive.HandleRootSpan(aggregator, logger)) - } aOpts := new(agentApp.Builder).InitFromViper(v) repOpts := new(agentRep.Options).InitFromViper(v, logger) @@ -162,13 +156,13 @@ by default uses only in-memory database.`, // collector c := collectorApp.New(&collectorApp.CollectorParams{ - ServiceName: "jaeger-collector", - Logger: logger, - MetricsFactory: metricsFactory, - SpanWriter: spanWriter, - StrategyStore: strategyStore, - HealthCheck: svc.HC(), - AdditionalProcessors: additionalProcessors, + ServiceName: "jaeger-collector", + Logger: logger, + MetricsFactory: metricsFactory, + SpanWriter: spanWriter, + StrategyStore: strategyStore, + Aggregator: aggregator, + HealthCheck: svc.HC(), }) if err := c.Start(cOpts); err != nil { log.Fatal(err) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index 3dc293bd747..5fa5d8c0341 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -35,15 +35,15 @@ import ( // Collector returns the collector as a manageable unit of work type Collector struct { // required to start a new collector - serviceName string - logger *zap.Logger - metricsFactory metrics.Factory - spanWriter spanstore.Writer - strategyStore strategystore.StrategyStore - additionalProcessors []ProcessSpan - hCheck *healthcheck.HealthCheck - spanProcessor processor.SpanProcessor - spanHandlers *SpanHandlers + serviceName string + logger *zap.Logger + metricsFactory metrics.Factory + spanWriter spanstore.Writer + strategyStore strategystore.StrategyStore + aggregator strategystore.Aggregator + hCheck *healthcheck.HealthCheck + spanProcessor processor.SpanProcessor + spanHandlers *SpanHandlers // state, read only hServer *http.Server @@ -55,25 +55,25 @@ type Collector struct { // CollectorParams to construct a new Jaeger Collector. type CollectorParams struct { - ServiceName string - Logger *zap.Logger - MetricsFactory metrics.Factory - SpanWriter spanstore.Writer - StrategyStore strategystore.StrategyStore - AdditionalProcessors []ProcessSpan - HealthCheck *healthcheck.HealthCheck + ServiceName string + Logger *zap.Logger + MetricsFactory metrics.Factory + SpanWriter spanstore.Writer + StrategyStore strategystore.StrategyStore + Aggregator strategystore.Aggregator + HealthCheck *healthcheck.HealthCheck } // New constructs a new collector component, ready to be started func New(params *CollectorParams) *Collector { return &Collector{ - serviceName: params.ServiceName, - logger: params.Logger, - metricsFactory: params.MetricsFactory, - spanWriter: params.SpanWriter, - strategyStore: params.StrategyStore, - additionalProcessors: params.AdditionalProcessors, - hCheck: params.HealthCheck, + serviceName: params.ServiceName, + logger: params.Logger, + metricsFactory: params.MetricsFactory, + spanWriter: params.SpanWriter, + strategyStore: params.StrategyStore, + aggregator: params.Aggregator, + hCheck: params.HealthCheck, } } @@ -86,7 +86,12 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error { MetricsFactory: c.metricsFactory, } - c.spanProcessor = handlerBuilder.BuildSpanProcessor(c.additionalProcessors...) + var additionalProcessors []ProcessSpan + if c.aggregator != nil { + additionalProcessors = append(additionalProcessors, handleRootSpan(c.aggregator, c.logger)) + } + + c.spanProcessor = handlerBuilder.BuildSpanProcessor(additionalProcessors...) c.spanHandlers = handlerBuilder.BuildHandlers(c.spanProcessor) grpcServer, err := server.StartGRPCServer(&server.GRPCServerParams{ @@ -171,6 +176,11 @@ func (c *Collector) Close() error { c.logger.Error("failed to close span processor.", zap.Error(err)) } + // aggregator does not exist for all strategy stores. only Stop() if exists. + if c.aggregator != nil { + c.aggregator.Stop() + } + // watchers actually never return errors from Close _ = c.tlsGRPCCertWatcherCloser.Close() _ = c.tlsHTTPCertWatcherCloser.Close() diff --git a/plugin/sampling/strategystore/adaptive/root_span_handler.go b/cmd/collector/app/root_span_handler.go similarity index 81% rename from plugin/sampling/strategystore/adaptive/root_span_handler.go rename to cmd/collector/app/root_span_handler.go index d55d744fc30..5a2cbdb336e 100644 --- a/plugin/sampling/strategystore/adaptive/root_span_handler.go +++ b/cmd/collector/app/root_span_handler.go @@ -12,18 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -package adaptive +package app import ( "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" "github.com/jaegertracing/jaeger/model" ) -// HandleRootSpan returns a function that records throughput for root spans -func HandleRootSpan(aggregator strategystore.Aggregator, logger *zap.Logger) app.ProcessSpan { +// handleRootSpan returns a function that records throughput for root spans +func handleRootSpan(aggregator strategystore.Aggregator, logger *zap.Logger) ProcessSpan { return func(span *model.Span) { // TODO simply checking parentId to determine if a span is a root span is not sufficient. However, // we can be sure that only a root span will have sampler tags. @@ -34,7 +33,7 @@ func HandleRootSpan(aggregator strategystore.Aggregator, logger *zap.Logger) app if service == "" || span.OperationName == "" { return } - samplerType, samplerParam := GetSamplerParams(span, logger) + samplerType, samplerParam := span.GetSamplerParams(logger) if samplerType == "" { return } diff --git a/plugin/sampling/strategystore/adaptive/root_span_handler_test.go b/cmd/collector/app/root_span_handler_test.go similarity index 96% rename from plugin/sampling/strategystore/adaptive/root_span_handler_test.go rename to cmd/collector/app/root_span_handler_test.go index 8fc8d388291..8daf459dc13 100644 --- a/plugin/sampling/strategystore/adaptive/root_span_handler_test.go +++ b/cmd/collector/app/root_span_handler_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package adaptive +package app import ( "testing" @@ -35,7 +35,7 @@ func (t *mockAggregator) Stop() {} func TestHandleRootSpan(t *testing.T) { aggregator := &mockAggregator{} - processor := HandleRootSpan(aggregator, zap.NewNop()) + processor := handleRootSpan(aggregator, zap.NewNop()) // Testing non-root span span := &model.Span{References: []model.SpanRef{{SpanID: model.NewSpanID(1), RefType: model.ChildOf}}} diff --git a/cmd/collector/main.go b/cmd/collector/main.go index 9a48edaabc8..0cd95b6e709 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -38,7 +38,6 @@ import ( "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/version" ss "github.com/jaegertracing/jaeger/plugin/sampling/strategystore" - "github.com/jaegertracing/jaeger/plugin/sampling/strategystore/adaptive" "github.com/jaegertracing/jaeger/plugin/storage" "github.com/jaegertracing/jaeger/ports" "github.com/jaegertracing/jaeger/storage/samplingstore" @@ -106,20 +105,14 @@ func main() { if err != nil { logger.Fatal("Failed to create sampling strategy store", zap.Error(err)) } - defer aggregator.Stop() - var additionalProcessors []app.ProcessSpan - if aggregator != nil { - additionalProcessors = append(additionalProcessors, adaptive.HandleRootSpan(aggregator, logger)) - } - c := app.New(&app.CollectorParams{ - ServiceName: serviceName, - Logger: logger, - MetricsFactory: metricsFactory, - SpanWriter: spanWriter, - StrategyStore: strategyStore, - HealthCheck: svc.HC(), - AdditionalProcessors: additionalProcessors, + ServiceName: serviceName, + Logger: logger, + MetricsFactory: metricsFactory, + SpanWriter: spanWriter, + StrategyStore: strategyStore, + Aggregator: aggregator, + HealthCheck: svc.HC(), }) collectorOpts := new(app.CollectorOptions).InitFromViper(v) if err := c.Start(collectorOpts); err != nil { diff --git a/model/span.go b/model/span.go index 8650396e09f..8b7d133726f 100644 --- a/model/span.go +++ b/model/span.go @@ -18,8 +18,11 @@ package model import ( "encoding/gob" "io" + "strconv" "github.com/opentracing/opentracing-go/ext" + "github.com/uber/jaeger-client-go" + "go.uber.org/zap" ) const ( @@ -118,6 +121,39 @@ func (s *Span) ReplaceParentID(newParentID SpanID) { s.References = MaybeAddParentSpanID(s.TraceID, newParentID, s.References) } +// GetSamplerParams returns the sampler.type and sampler.param value if they are valid. +func (s *Span) GetSamplerParams(logger *zap.Logger) (string, float64) { + tag, ok := KeyValues(s.Tags).FindByKey(jaeger.SamplerTypeTagKey) + if !ok { + return "", 0 + } + if tag.VType != StringType { + logger. + With(zap.String("traceID", s.TraceID.String())). + With(zap.String("spanID", s.SpanID.String())). + Warn("sampler.type tag is not a string", zap.Any("tag", tag)) + return "", 0 + } + samplerType := tag.AsString() + if samplerType != jaeger.SamplerTypeProbabilistic && samplerType != jaeger.SamplerTypeLowerBound && + samplerType != jaeger.SamplerTypeRateLimiting { + return "", 0 + } + tag, ok = KeyValues(s.Tags).FindByKey(jaeger.SamplerParamTagKey) + if !ok { + return "", 0 + } + samplerParam, err := getParam(tag) + if err != nil { + logger. + With(zap.String("traceID", s.TraceID.String())). + With(zap.String("spanID", s.SpanID.String())). + Warn("sampler.param tag is not a number", zap.Any("tag", tag)) + return "", 0 + } + return samplerType, samplerParam +} + // ------- Flags ------- // SetSampled sets the Flags as sampled @@ -159,3 +195,15 @@ func (f Flags) IsFirehoseEnabled() bool { func (f Flags) checkFlags(bit Flags) bool { return f&bit == bit } + +func getParam(samplerParamTag KeyValue) (float64, error) { + // The param could be represented as a string, an int, or a float + switch samplerParamTag.VType { + case Float64Type: + return samplerParamTag.Float64(), nil + case Int64Type: + return float64(samplerParamTag.Int64()), nil + default: + return strconv.ParseFloat(samplerParamTag.AsString(), 64) + } +} diff --git a/model/span_test.go b/model/span_test.go index 6c2099bd9eb..4ddd7944d4b 100644 --- a/model/span_test.go +++ b/model/span_test.go @@ -26,6 +26,7 @@ import ( "github.com/opentracing/opentracing-go/ext" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap" "github.com/jaegertracing/jaeger/model" ) @@ -383,3 +384,115 @@ func BenchmarkBatchSerialization(b *testing.B) { } }) } + +func TestGetSamplerParams(t *testing.T) { + logger := zap.NewNop() + tests := []struct { + tags model.KeyValues + expectedType string + expectedParam float64 + }{ + { + tags: model.KeyValues{ + model.String("sampler.type", "probabilistic"), + model.String("sampler.param", "1e-05"), + }, + expectedType: "probabilistic", + expectedParam: 0.00001, + }, + { + tags: model.KeyValues{ + model.String("sampler.type", "probabilistic"), + model.Float64("sampler.param", 0.10404450002098709), + }, + expectedType: "probabilistic", + expectedParam: 0.10404450002098709, + }, + { + tags: model.KeyValues{ + model.String("sampler.type", "probabilistic"), + model.String("sampler.param", "0.10404450002098709"), + }, + expectedType: "probabilistic", + expectedParam: 0.10404450002098709, + }, + { + tags: model.KeyValues{ + model.String("sampler.type", "probabilistic"), + model.Int64("sampler.param", 1), + }, + expectedType: "probabilistic", + expectedParam: 1.0, + }, + { + tags: model.KeyValues{ + model.String("sampler.type", "ratelimiting"), + model.String("sampler.param", "1"), + }, + expectedType: "ratelimiting", + expectedParam: 1, + }, + { + tags: model.KeyValues{ + model.Float64("sampler.type", 1.5), + }, + expectedType: "", + expectedParam: 0, + }, + { + tags: model.KeyValues{ + model.String("sampler.type", "probabilistic"), + }, + expectedType: "", + expectedParam: 0, + }, + { + tags: model.KeyValues{}, + expectedType: "", + expectedParam: 0, + }, + { + tags: model.KeyValues{ + model.String("sampler.type", "lowerbound"), + model.String("sampler.param", "1"), + }, + expectedType: "lowerbound", + expectedParam: 1, + }, + { + tags: model.KeyValues{ + model.String("sampler.type", "lowerbound"), + model.Int64("sampler.param", 1), + }, + expectedType: "lowerbound", + expectedParam: 1, + }, + { + tags: model.KeyValues{ + model.String("sampler.type", "lowerbound"), + model.Float64("sampler.param", 0.5), + }, + expectedType: "lowerbound", + expectedParam: 0.5, + }, + { + tags: model.KeyValues{ + model.String("sampler.type", "lowerbound"), + model.String("sampler.param", "not_a_number"), + }, + expectedType: "", + expectedParam: 0, + }, + } + + for i, test := range tests { + tt := test + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + span := &model.Span{} + span.Tags = tt.tags + actualType, actualParam := span.GetSamplerParams(logger) + assert.Equal(t, tt.expectedType, actualType) + assert.Equal(t, tt.expectedParam, actualParam) + }) + } +} diff --git a/plugin/sampling/strategystore/adaptive/span.go b/plugin/sampling/strategystore/adaptive/span.go deleted file mode 100644 index 71a38258cfb..00000000000 --- a/plugin/sampling/strategystore/adaptive/span.go +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright (c) 2021 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 adaptive - -import ( - "strconv" - - "github.com/uber/jaeger-client-go" - "go.uber.org/zap" - - "github.com/jaegertracing/jaeger/model" -) - -// GetSamplerParams returns the sampler.type and sampler.param value if they are valid. -func GetSamplerParams(span *model.Span, logger *zap.Logger) (string, float64) { - // TODO move this into model.Span - tag, ok := model.KeyValues(span.Tags).FindByKey(jaeger.SamplerTypeTagKey) - if !ok { - return "", 0 - } - if tag.VType != model.StringType { - logger. - With(zap.String("traceID", span.TraceID.String())). - With(zap.String("spanID", span.SpanID.String())). - Warn("sampler.type tag is not a string", zap.Any("tag", tag)) - return "", 0 - } - samplerType := tag.AsString() - if samplerType != jaeger.SamplerTypeProbabilistic && samplerType != jaeger.SamplerTypeLowerBound && - samplerType != jaeger.SamplerTypeRateLimiting { - return "", 0 - } - tag, ok = model.KeyValues(span.Tags).FindByKey(jaeger.SamplerParamTagKey) - if !ok { - return "", 0 - } - samplerParam, err := getParam(tag) - if err != nil { - logger. - With(zap.String("traceID", span.TraceID.String())). - With(zap.String("spanID", span.SpanID.String())). - Warn("sampler.param tag is not a number", zap.Any("tag", tag)) - return "", 0 - } - return samplerType, samplerParam -} - -func getParam(samplerParamTag model.KeyValue) (float64, error) { - // The param could be represented as a string, an int, or a float - switch samplerParamTag.VType { - case model.Float64Type: - return samplerParamTag.Float64(), nil - case model.Int64Type: - return float64(samplerParamTag.Int64()), nil - default: - return strconv.ParseFloat(samplerParamTag.AsString(), 64) - } -} diff --git a/plugin/sampling/strategystore/adaptive/span_test.go b/plugin/sampling/strategystore/adaptive/span_test.go deleted file mode 100644 index 02f4c1b8a71..00000000000 --- a/plugin/sampling/strategystore/adaptive/span_test.go +++ /dev/null @@ -1,137 +0,0 @@ -// Copyright (c) 2021 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 adaptive - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - "go.uber.org/zap" - - "github.com/jaegertracing/jaeger/model" -) - -func TestGetSamplerParams(t *testing.T) { - logger := zap.NewNop() - tests := []struct { - tags model.KeyValues - expectedType string - expectedParam float64 - }{ - { - tags: model.KeyValues{ - model.String("sampler.type", "probabilistic"), - model.String("sampler.param", "1e-05"), - }, - expectedType: "probabilistic", - expectedParam: 0.00001, - }, - { - tags: model.KeyValues{ - model.String("sampler.type", "probabilistic"), - model.Float64("sampler.param", 0.10404450002098709), - }, - expectedType: "probabilistic", - expectedParam: 0.10404450002098709, - }, - { - tags: model.KeyValues{ - model.String("sampler.type", "probabilistic"), - model.String("sampler.param", "0.10404450002098709"), - }, - expectedType: "probabilistic", - expectedParam: 0.10404450002098709, - }, - { - tags: model.KeyValues{ - model.String("sampler.type", "probabilistic"), - model.Int64("sampler.param", 1), - }, - expectedType: "probabilistic", - expectedParam: 1.0, - }, - { - tags: model.KeyValues{ - model.String("sampler.type", "ratelimiting"), - model.String("sampler.param", "1"), - }, - expectedType: "ratelimiting", - expectedParam: 1, - }, - { - tags: model.KeyValues{ - model.Float64("sampler.type", 1.5), - }, - expectedType: "", - expectedParam: 0, - }, - { - tags: model.KeyValues{ - model.String("sampler.type", "probabilistic"), - }, - expectedType: "", - expectedParam: 0, - }, - { - tags: model.KeyValues{}, - expectedType: "", - expectedParam: 0, - }, - { - tags: model.KeyValues{ - model.String("sampler.type", "lowerbound"), - model.String("sampler.param", "1"), - }, - expectedType: "lowerbound", - expectedParam: 1, - }, - { - tags: model.KeyValues{ - model.String("sampler.type", "lowerbound"), - model.Int64("sampler.param", 1), - }, - expectedType: "lowerbound", - expectedParam: 1, - }, - { - tags: model.KeyValues{ - model.String("sampler.type", "lowerbound"), - model.Float64("sampler.param", 0.5), - }, - expectedType: "lowerbound", - expectedParam: 0.5, - }, - { - tags: model.KeyValues{ - model.String("sampler.type", "lowerbound"), - model.String("sampler.param", "not_a_number"), - }, - expectedType: "", - expectedParam: 0, - }, - } - - for i, test := range tests { - tt := test - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - span := &model.Span{} - span.Tags = tt.tags - actualType, actualParam := GetSamplerParams(span, logger) - assert.Equal(t, tt.expectedType, actualType) - assert.Equal(t, tt.expectedParam, actualParam) - }) - } -} From 0629d93b08a2a04aeeff2e42daa2cd1012548a55 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 15 Jun 2021 14:10:08 -0400 Subject: [PATCH 18/40] Added additional processors test Signed-off-by: Joe Elliott --- cmd/collector/app/span_processor.go | 2 +- cmd/collector/app/span_processor_test.go | 35 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/cmd/collector/app/span_processor.go b/cmd/collector/app/span_processor.go index 9b2e540e8c4..906b3930aca 100644 --- a/cmd/collector/app/span_processor.go +++ b/cmd/collector/app/span_processor.go @@ -85,7 +85,7 @@ func NewSpanProcessor( return sp } -func newSpanProcessor(spanWriter spanstore.Writer, additional []ProcessSpan, opts ...Option) *spanProcessor { // todo(jpe): test additional +func newSpanProcessor(spanWriter spanstore.Writer, additional []ProcessSpan, opts ...Option) *spanProcessor { options := Options.apply(opts...) handlerMetrics := NewSpanProcessorMetrics( options.serviceMetrics, diff --git a/cmd/collector/app/span_processor_test.go b/cmd/collector/app/span_processor_test.go index 009f95e1d79..38f500b55dd 100644 --- a/cmd/collector/app/span_processor_test.go +++ b/cmd/collector/app/span_processor_test.go @@ -527,3 +527,38 @@ func TestStartDynQueueSizeUpdater(t *testing.T) { assert.EqualValues(t, 104857, p.queue.Capacity()) } + +func TestAdditionalProcessors(t *testing.T) { + w := &fakeSpanWriter{} + + // nil doesn't fail + p := NewSpanProcessor(w, nil, Options.QueueSize(1)) + res, err := p.ProcessSpans([]*model.Span{ + { + Process: &model.Process{ + ServiceName: "x", + }, + }, + }, processor.SpansOptions{SpanFormat: processor.JaegerSpanFormat}) + assert.NoError(t, err) + assert.Equal(t, []bool{true}, res) + assert.NoError(t, p.Close()) + + // additional processor is called + count := 0 + f := func(s *model.Span) { + count++ + } + p = NewSpanProcessor(w, []ProcessSpan{f}, Options.QueueSize(1)) + res, err = p.ProcessSpans([]*model.Span{ + { + Process: &model.Process{ + ServiceName: "x", + }, + }, + }, processor.SpansOptions{SpanFormat: processor.JaegerSpanFormat}) + assert.NoError(t, err) + assert.Equal(t, []bool{true}, res) + assert.NoError(t, p.Close()) + assert.Equal(t, 1, count) +} From cc478ce85c82ba07101337e420cb22eafab05cad Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 15 Jun 2021 14:52:34 -0400 Subject: [PATCH 19/40] Added CreateLockAndSamplingStore tests Signed-off-by: Joe Elliott --- plugin/storage/badger/factory_test.go | 8 +++ plugin/storage/cassandra/factory.go | 2 +- plugin/storage/cassandra/factory_test.go | 3 ++ plugin/storage/es/factory_test.go | 8 +++ plugin/storage/factory.go | 2 +- plugin/storage/factory_test.go | 67 +++++++++++++++++++----- plugin/storage/grpc/factory_test.go | 8 +++ plugin/storage/kafka/factory_test.go | 8 +++ plugin/storage/memory/factory_test.go | 8 +++ storage/mocks/Factory.go | 6 +-- 10 files changed, 103 insertions(+), 17 deletions(-) diff --git a/plugin/storage/badger/factory_test.go b/plugin/storage/badger/factory_test.go index 3f54c9f606d..738981fb78f 100644 --- a/plugin/storage/badger/factory_test.go +++ b/plugin/storage/badger/factory_test.go @@ -199,3 +199,11 @@ func TestInitFromOptions(t *testing.T) { f.InitFromOptions(opts) assert.Equal(t, &opts, f.Options) } + +func TestCreateLockAndSamplingStore(t *testing.T) { + f := NewFactory() + lock, ss, err := f.CreateLockAndSamplingStore() + assert.Nil(t, lock) + assert.Nil(t, ss) + assert.Equal(t, err, storage.ErrLockAndSamplingStoreNotSupported) +} diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index 7e6c0ed92e8..1a7682247ad 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -153,7 +153,7 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { } // CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { // todo(jpe): test +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { hostname, err := os.Hostname() // todo(jpe): wire up --sampling.override-hostname in ./plugin/sampling/strategystore/adaptive/options.go? if err != nil { return nil, nil, err diff --git a/plugin/storage/cassandra/factory_test.go b/plugin/storage/cassandra/factory_test.go index cc1b2541252..b52674a5787 100644 --- a/plugin/storage/cassandra/factory_test.go +++ b/plugin/storage/cassandra/factory_test.go @@ -100,6 +100,9 @@ func TestCassandraFactory(t *testing.T) { _, err = f.CreateArchiveSpanWriter() assert.NoError(t, err) + _, _, err = f.CreateLockAndSamplingStore() + assert.NoError(t, err) + assert.NoError(t, f.Close()) } diff --git a/plugin/storage/es/factory_test.go b/plugin/storage/es/factory_test.go index 631fa3f2c0d..081ebf49938 100644 --- a/plugin/storage/es/factory_test.go +++ b/plugin/storage/es/factory_test.go @@ -221,3 +221,11 @@ func TestInitFromOptions(t *testing.T) { assert.Equal(t, o.GetPrimary(), f.primaryConfig) assert.Equal(t, o.Get(archiveNamespace), f.archiveConfig) } + +func TestCreateLockAndSamplingStore(t *testing.T) { + f := NewFactory() + lock, ss, err := f.CreateLockAndSamplingStore() + assert.Nil(t, lock) + assert.Nil(t, ss) + assert.Equal(t, err, storage.ErrLockAndSamplingStoreNotSupported) +} diff --git a/plugin/storage/factory.go b/plugin/storage/factory.go index 760db1ec3a7..622747fe95a 100644 --- a/plugin/storage/factory.go +++ b/plugin/storage/factory.go @@ -161,7 +161,7 @@ func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { } // CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { // todo(jpe): test +func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { for _, factory := range f.factories { lock, store, err := factory.CreateLockAndSamplingStore() if err != nil && err != storage.ErrLockAndSamplingStoreNotSupported { diff --git a/plugin/storage/factory_test.go b/plugin/storage/factory_test.go index 3e0e471750b..c61d2ee1d7d 100644 --- a/plugin/storage/factory_test.go +++ b/plugin/storage/factory_test.go @@ -94,7 +94,7 @@ func TestClose(t *testing.T) { err := fmt.Errorf("some error") f := Factory{ factories: map[string]storage.Factory{ - storageType: &errorCloseFactory{closeErr: err}, + storageType: &errorFactory{closeErr: err}, }, FactoryConfig: FactoryConfig{SpanWriterTypes: []string{storageType}}, } @@ -296,6 +296,13 @@ func TestCreateError(t *testing.T) { assert.Nil(t, w) assert.EqualError(t, err, expectedErr) } + + { + l, ss, err := f.CreateLockAndSamplingStore() + assert.Nil(t, l) + assert.Nil(t, ss) + assert.NoError(t, err) // no supporting backend is valid + } } type configurable struct { @@ -392,33 +399,69 @@ func TestPublishOpts(t *testing.T) { }) } -type errorCloseFactory struct { - closeErr error +func TestCreateLockAndSamplingStore(t *testing.T) { + supports := &errorFactory{} + doesNotSupport := &errorFactory{ + lockAndSamplingStore: storage.ErrLockAndSamplingStoreNotSupported, + } + breaks := &errorFactory{ + lockAndSamplingStore: errors.New("wups"), + } + + // test succeeds (supported or not) + f := Factory{ + factories: map[string]storage.Factory{ + "supports": supports, + "does-not-support": doesNotSupport, + }, + } + + _, _, err := f.CreateLockAndSamplingStore() + assert.NoError(t, err) + + delete(f.factories, "supports") + _, _, err = f.CreateLockAndSamplingStore() + assert.NoError(t, err) + + // test breaks + f = Factory{ + factories: map[string]storage.Factory{ + "breaks": breaks, + "does-not-support": doesNotSupport, + }, + } + _, _, err = f.CreateLockAndSamplingStore() + assert.Error(t, err) +} + +type errorFactory struct { + closeErr error + lockAndSamplingStore error } -var _ storage.Factory = (*errorCloseFactory)(nil) -var _ io.Closer = (*errorCloseFactory)(nil) +var _ storage.Factory = (*errorFactory)(nil) +var _ io.Closer = (*errorFactory)(nil) -func (e errorCloseFactory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { +func (e errorFactory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { panic("implement me") } -func (e errorCloseFactory) CreateSpanReader() (spanstore.Reader, error) { +func (e errorFactory) CreateSpanReader() (spanstore.Reader, error) { panic("implement me") } -func (e errorCloseFactory) CreateSpanWriter() (spanstore.Writer, error) { +func (e errorFactory) CreateSpanWriter() (spanstore.Writer, error) { panic("implement me") } -func (e errorCloseFactory) CreateDependencyReader() (dependencystore.Reader, error) { +func (e errorFactory) CreateDependencyReader() (dependencystore.Reader, error) { panic("implement me") } -func (e errorCloseFactory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - panic("implement me") +func (e errorFactory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { + return nil, nil, e.lockAndSamplingStore } -func (e errorCloseFactory) Close() error { +func (e errorFactory) Close() error { return e.closeErr } diff --git a/plugin/storage/grpc/factory_test.go b/plugin/storage/grpc/factory_test.go index 9ad046f732d..64686fda64d 100644 --- a/plugin/storage/grpc/factory_test.go +++ b/plugin/storage/grpc/factory_test.go @@ -269,3 +269,11 @@ func TestInitFromOptions(t *testing.T) { assert.Equal(t, o, f.options) assert.Equal(t, &o.Configuration, f.builder) } + +func TestCreateLockAndSamplingStore(t *testing.T) { + f := NewFactory() + lock, ss, err := f.CreateLockAndSamplingStore() + assert.Nil(t, lock) + assert.Nil(t, ss) + assert.Equal(t, err, storage.ErrLockAndSamplingStoreNotSupported) +} diff --git a/plugin/storage/kafka/factory_test.go b/plugin/storage/kafka/factory_test.go index 61f1da6be4d..18567597679 100644 --- a/plugin/storage/kafka/factory_test.go +++ b/plugin/storage/kafka/factory_test.go @@ -171,3 +171,11 @@ func TestInitFromOptions(t *testing.T) { assert.Equal(t, o, f.options) assert.Equal(t, &o.Config, f.Builder) } + +func TestCreateLockAndSamplingStore(t *testing.T) { + f := NewFactory() + lock, ss, err := f.CreateLockAndSamplingStore() + assert.Nil(t, lock) + assert.Nil(t, ss) + assert.Equal(t, err, storage.ErrLockAndSamplingStoreNotSupported) +} diff --git a/plugin/storage/memory/factory_test.go b/plugin/storage/memory/factory_test.go index 27f9710b3e8..91c7f37f756 100644 --- a/plugin/storage/memory/factory_test.go +++ b/plugin/storage/memory/factory_test.go @@ -79,3 +79,11 @@ func TestPublishOpts(t *testing.T) { Value: f.options.Configuration.MaxTraces, }) } + +func TestCreateLockAndSamplingStore(t *testing.T) { + f := NewFactory() + lock, ss, err := f.CreateLockAndSamplingStore() + assert.Nil(t, lock) + assert.Nil(t, ss) + assert.Equal(t, err, storage.ErrLockAndSamplingStoreNotSupported) +} diff --git a/storage/mocks/Factory.go b/storage/mocks/Factory.go index 3c84a93956c..0b05495d1a9 100644 --- a/storage/mocks/Factory.go +++ b/storage/mocks/Factory.go @@ -15,15 +15,15 @@ package mocks import ( - metrics "github.com/uber/jaeger-lib/metrics" mock "github.com/stretchr/testify/mock" + metrics "github.com/uber/jaeger-lib/metrics" zap "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/distributedlock" + storage "github.com/jaegertracing/jaeger/storage" dependencystore "github.com/jaegertracing/jaeger/storage/dependencystore" "github.com/jaegertracing/jaeger/storage/samplingstore" spanstore "github.com/jaegertracing/jaeger/storage/spanstore" - storage "github.com/jaegertracing/jaeger/storage" ) // Factory is an autogenerated mock type for the Factory type @@ -102,7 +102,7 @@ func (_m *Factory) CreateSpanWriter() (spanstore.Writer, error) { // CreateLockAndSamplingStore is not supproted func (_m *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, storage.ErrLockAndSamplingStoreNotSupported // todo(jpe): add tests that require this? + return nil, nil, storage.ErrLockAndSamplingStoreNotSupported } // Initialize provides a mock function with given fields: metricsFactory, logger From 8524c9de48ed40013b1bdb885707b7ee07469f58 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Fri, 25 Jun 2021 16:21:10 -0400 Subject: [PATCH 20/40] remove overrideHostname in favor of a short random postfix Signed-off-by: Joe Elliott --- .../strategystore/adaptive/factory_test.go | 2 -- plugin/sampling/strategystore/adaptive/options.go | 10 ---------- .../strategystore/adaptive/strategy_store.go | 15 +++++---------- plugin/storage/cassandra/factory.go | 5 +++-- 4 files changed, 8 insertions(+), 24 deletions(-) diff --git a/plugin/sampling/strategystore/adaptive/factory_test.go b/plugin/sampling/strategystore/adaptive/factory_test.go index 05e1676c641..804e591a5d5 100644 --- a/plugin/sampling/strategystore/adaptive/factory_test.go +++ b/plugin/sampling/strategystore/adaptive/factory_test.go @@ -46,7 +46,6 @@ func TestFactory(t *testing.T) { "--sampling.min-samples-per-second=1", "--sampling.leader-lease-refresh-interval=1s", "--sampling.follower-lease-refresh-interval=2s", - "--sampling.override-hostname=blerg", }) f.InitFromViper(v, zap.NewNop()) @@ -62,7 +61,6 @@ func TestFactory(t *testing.T) { assert.Equal(t, 1.0, f.options.MinSamplesPerSecond) assert.Equal(t, time.Second, f.options.LeaderLeaseRefreshInterval) assert.Equal(t, time.Second*2, f.options.FollowerLeaseRefreshInterval) - assert.Equal(t, "blerg", f.options.OverrideHostname) assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), &mockLock{}, &mockStore{})) _, _, err := f.CreateStrategyStore() diff --git a/plugin/sampling/strategystore/adaptive/options.go b/plugin/sampling/strategystore/adaptive/options.go index bc6f2c396c6..f1bc2eddc6a 100644 --- a/plugin/sampling/strategystore/adaptive/options.go +++ b/plugin/sampling/strategystore/adaptive/options.go @@ -33,7 +33,6 @@ const ( minSamplesPerSecond = "sampling.min-samples-per-second" leaderLeaseRefreshInterval = "sampling.leader-lease-refresh-interval" followerLeaseRefreshInterval = "sampling.follower-lease-refresh-interval" - overrideHostname = "sampling.override-hostname" defaultTargetSamplesPerSecond = 1 defaultDeltaTolerance = 0.3 @@ -46,7 +45,6 @@ const ( defaultMinSamplesPerSecond = 1.0 / float64(time.Minute/time.Second) // once every 1 minute defaultLeaderLeaseRefreshInterval = 5 * time.Second defaultFollowerLeaseRefreshInterval = 60 * time.Second - defaultOverrideHostname = "" ) // Options holds configuration for the adaptive sampling strategy store. @@ -113,10 +111,6 @@ type Options struct { // FollowerLeaseRefreshInterval is the duration to sleep if this processor is a follower // (ie. failed to gain the leader lock). FollowerLeaseRefreshInterval time.Duration - - // OverrideHostname overrides the hostname returned from os.Hostname for use in the adaptive sampling - // strategy processor - OverrideHostname string } // AddFlags adds flags for Options @@ -154,9 +148,6 @@ func AddFlags(flagSet *flag.FlagSet) { flagSet.Duration(followerLeaseRefreshInterval, defaultFollowerLeaseRefreshInterval, "The duration to sleep if this processor is a follower.", ) - flagSet.String(overrideHostname, defaultOverrideHostname, - "Overrides os.Hostname() for use in adaptive sampling.", - ) } // InitFromViper initializes Options with properties from viper @@ -172,6 +163,5 @@ func (opts *Options) InitFromViper(v *viper.Viper) *Options { opts.MinSamplesPerSecond = v.GetFloat64(minSamplesPerSecond) opts.LeaderLeaseRefreshInterval = v.GetDuration(leaderLeaseRefreshInterval) opts.FollowerLeaseRefreshInterval = v.GetDuration(followerLeaseRefreshInterval) - opts.OverrideHostname = v.GetString(overrideHostname) return opts } diff --git a/plugin/sampling/strategystore/adaptive/strategy_store.go b/plugin/sampling/strategystore/adaptive/strategy_store.go index 605af2b71a2..cf5cafd2c4f 100644 --- a/plugin/sampling/strategystore/adaptive/strategy_store.go +++ b/plugin/sampling/strategystore/adaptive/strategy_store.go @@ -15,27 +15,22 @@ package adaptive import ( - "os" - "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/distributedlock" + "github.com/jaegertracing/jaeger/pkg/hostname" "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection" "github.com/jaegertracing/jaeger/storage/samplingstore" ) // NewStrategyStore creates a strategy store that holds adaptive sampling strategies. func NewStrategyStore(options Options, metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) (*Processor, error) { - hostname := options.OverrideHostname - if len(hostname) == 0 { - var err error - hostname, err = os.Hostname() - if err != nil { - return nil, err - } - logger.Info("Adaptive sampling hostname retrieved from os.Hostname()", zap.String("hostname", hostname)) + hostname, err := hostname.AsIdentifier() + if err != nil { + return nil, err } + logger.Info("Retrieved unique hostname from the OS for use in adaptive sampling", zap.String("hostname", hostname)) participant := leaderelection.NewElectionParticipant(lock, defaultResourceName, leaderelection.ElectionParticipantOptions{ FollowerLeaseRefreshInterval: options.FollowerLeaseRefreshInterval, diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index 5581329bdb7..2b79a9652c5 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -19,7 +19,6 @@ import ( "errors" "flag" "io" - "os" "github.com/spf13/viper" "github.com/uber/jaeger-lib/metrics" @@ -28,6 +27,7 @@ import ( "github.com/jaegertracing/jaeger/pkg/cassandra" "github.com/jaegertracing/jaeger/pkg/cassandra/config" "github.com/jaegertracing/jaeger/pkg/distributedlock" + "github.com/jaegertracing/jaeger/pkg/hostname" cLock "github.com/jaegertracing/jaeger/plugin/pkg/distributedlock/cassandra" cDepStore "github.com/jaegertracing/jaeger/plugin/storage/cassandra/dependencystore" cSamplingStore "github.com/jaegertracing/jaeger/plugin/storage/cassandra/samplingstore" @@ -154,10 +154,11 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { // CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - hostname, err := os.Hostname() // todo(jpe): wire up --sampling.override-hostname in ./plugin/sampling/strategystore/adaptive/options.go? + hostname, err := hostname.AsIdentifier() if err != nil { return nil, nil, err } + f.logger.Info("Retrieved unique hostname from the OS for use in the distributed lock", zap.String("hostname", hostname)) store := cSamplingStore.New(f.primarySession, f.primaryMetricsFactory, f.logger) lock := cLock.NewLock(f.primarySession, hostname) From b72f4704b4dac191d147fa34b92f23dbf5804280 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Fri, 25 Jun 2021 16:21:59 -0400 Subject: [PATCH 21/40] added hostname Signed-off-by: Joe Elliott --- pkg/hostname/hostname.go | 24 ++++++++++++++++++++++++ pkg/hostname/hostname_test.go | 23 +++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 pkg/hostname/hostname.go create mode 100644 pkg/hostname/hostname_test.go diff --git a/pkg/hostname/hostname.go b/pkg/hostname/hostname.go new file mode 100644 index 00000000000..ef43b81270d --- /dev/null +++ b/pkg/hostname/hostname.go @@ -0,0 +1,24 @@ +package hostname + +import ( + "fmt" + "math/rand" + "os" +) + +// AsIdentifier uses the hostname of the os and postfixes a short random string to guarantee uniqueness +// The returned value is appropriate to use as a convenient unique identifier. +func AsIdentifier() (string, error) { + hostname, err := os.Hostname() + if err != nil { + return "", err + } + + buff := make([]byte, 8) + _, err = rand.Read(buff) + if err != nil { + return "", err + } + + return hostname + "-" + fmt.Sprintf("%2x", buff), nil +} diff --git a/pkg/hostname/hostname_test.go b/pkg/hostname/hostname_test.go new file mode 100644 index 00000000000..9a18f9b0798 --- /dev/null +++ b/pkg/hostname/hostname_test.go @@ -0,0 +1,23 @@ +package hostname + +import ( + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAsIdentifier(t *testing.T) { + hostname1, err := AsIdentifier() + require.NoError(t, err) + hostname2, err := AsIdentifier() + require.NoError(t, err) + + actualHostname, _ := os.Hostname() + + assert.NotEqual(t, hostname1, hostname2) + assert.True(t, strings.HasPrefix(hostname1, actualHostname)) + assert.True(t, strings.HasPrefix(hostname2, actualHostname)) +} From 4570cb8cabc57c6caffeb6340d368c3b5d06fca3 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 6 Jul 2021 15:16:15 -0400 Subject: [PATCH 22/40] Update storage/factory.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Juraci Paixão Kröhling Signed-off-by: Joe Elliott --- storage/factory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/factory.go b/storage/factory.go index 27f0442c133..700d19154e8 100644 --- a/storage/factory.go +++ b/storage/factory.go @@ -61,7 +61,7 @@ var ( ErrArchiveStorageNotSupported = errors.New("archive storage not supported") // ErrLockAndSamplingStoreNotSupported can be returned by the StorageFactory when the sampling storage is not supported by the backend. - ErrLockAndSamplingStoreNotSupported = errors.New("archive storage not supported") + ErrLockAndSamplingStoreNotSupported = errors.New("lock/sampling storage not supported") ) // ArchiveFactory is an additional interface that can be implemented by a factory to support trace archiving. From c1e6b04869ef7379503fb2bfcb3797f08b5267fc Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 6 Jul 2021 14:26:08 -0400 Subject: [PATCH 23/40] lint really wants crypto/rand Signed-off-by: Joe Elliott --- pkg/hostname/hostname.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/hostname/hostname.go b/pkg/hostname/hostname.go index ef43b81270d..867bcb840ce 100644 --- a/pkg/hostname/hostname.go +++ b/pkg/hostname/hostname.go @@ -1,8 +1,8 @@ package hostname import ( + "crypto/rand" "fmt" - "math/rand" "os" ) From f10304cc9b53c0abaa7b3020e4e43c8f8f1eec25 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 6 Jul 2021 14:35:44 -0400 Subject: [PATCH 24/40] logger to last param Signed-off-by: Joe Elliott --- cmd/all-in-one/main.go | 2 +- cmd/collector/app/sampling/strategystore/factory.go | 2 +- cmd/collector/main.go | 2 +- plugin/sampling/strategystore/adaptive/factory.go | 2 +- plugin/sampling/strategystore/adaptive/factory_test.go | 2 +- plugin/sampling/strategystore/factory.go | 4 ++-- plugin/sampling/strategystore/factory_test.go | 6 +++--- plugin/sampling/strategystore/static/factory.go | 2 +- plugin/sampling/strategystore/static/factory_test.go | 2 +- 9 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 77cd9675d81..63fe3db8ef2 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -141,7 +141,7 @@ by default uses only in-memory database.`, } strategyStoreFactory.InitFromViper(v, logger) - if err := strategyStoreFactory.Initialize(metricsFactory, logger, lock, samplingStore); err != nil { + if err := strategyStoreFactory.Initialize(metricsFactory, lock, samplingStore, logger); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) } strategyStore, aggregator, err := strategyStoreFactory.CreateStrategyStore() diff --git a/cmd/collector/app/sampling/strategystore/factory.go b/cmd/collector/app/sampling/strategystore/factory.go index 2808a8f4474..c4882060c65 100644 --- a/cmd/collector/app/sampling/strategystore/factory.go +++ b/cmd/collector/app/sampling/strategystore/factory.go @@ -30,7 +30,7 @@ import ( // plugin.Configurable type Factory interface { // Initialize performs internal initialization of the factory. - Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error + Initialize(metricsFactory metrics.Factory, lock distributedlock.Lock, store samplingstore.Store, logger *zap.Logger) error // CreateStrategyStore initializes the StrategyStore and returns it. CreateStrategyStore() (StrategyStore, Aggregator, error) diff --git a/cmd/collector/main.go b/cmd/collector/main.go index 3e8c267033e..973825e87cb 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -99,7 +99,7 @@ func main() { } } strategyStoreFactory.InitFromViper(v, logger) - if err := strategyStoreFactory.Initialize(metricsFactory, logger, lock, samplingStore); err != nil { + if err := strategyStoreFactory.Initialize(metricsFactory, lock, samplingStore, logger); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) } strategyStore, aggregator, err := strategyStoreFactory.CreateStrategyStore() diff --git a/plugin/sampling/strategystore/adaptive/factory.go b/plugin/sampling/strategystore/adaptive/factory.go index 90d2f748bff..0924b61cfc3 100644 --- a/plugin/sampling/strategystore/adaptive/factory.go +++ b/plugin/sampling/strategystore/adaptive/factory.go @@ -57,7 +57,7 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { } // Initialize implements strategystore.Factory -func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error { +func (f *Factory) Initialize(metricsFactory metrics.Factory, lock distributedlock.Lock, store samplingstore.Store, logger *zap.Logger) error { f.logger = logger f.metricsFactory = metricsFactory f.lock = lock diff --git a/plugin/sampling/strategystore/adaptive/factory_test.go b/plugin/sampling/strategystore/adaptive/factory_test.go index 804e591a5d5..f3745f6d633 100644 --- a/plugin/sampling/strategystore/adaptive/factory_test.go +++ b/plugin/sampling/strategystore/adaptive/factory_test.go @@ -62,7 +62,7 @@ func TestFactory(t *testing.T) { assert.Equal(t, time.Second, f.options.LeaderLeaseRefreshInterval) assert.Equal(t, time.Second*2, f.options.FollowerLeaseRefreshInterval) - assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), &mockLock{}, &mockStore{})) + assert.NoError(t, f.Initialize(metrics.NullFactory, &mockLock{}, &mockStore{}, zap.NewNop())) _, _, err := f.CreateStrategyStore() assert.NoError(t, err) } diff --git a/plugin/sampling/strategystore/factory.go b/plugin/sampling/strategystore/factory.go index 47608f881b9..61aa1145c37 100644 --- a/plugin/sampling/strategystore/factory.go +++ b/plugin/sampling/strategystore/factory.go @@ -89,9 +89,9 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { } // Initialize implements strategystore.Factory -func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error { +func (f *Factory) Initialize(metricsFactory metrics.Factory, lock distributedlock.Lock, store samplingstore.Store, logger *zap.Logger) error { for _, factory := range f.factories { - if err := factory.Initialize(metricsFactory, logger, lock, store); err != nil { + if err := factory.Initialize(metricsFactory, lock, store, logger); err != nil { return err } } diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index 2213450f983..1a66f409dc0 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -75,13 +75,13 @@ func TestNewFactory(t *testing.T) { mock := new(mockFactory) f.factories[Kind(tc.strategyStoreType)] = mock - assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), lock, store)) + assert.NoError(t, f.Initialize(metrics.NullFactory, lock, store, zap.NewNop())) _, _, err = f.CreateStrategyStore() require.NoError(t, err) // force the mock to return errors mock.retError = true - assert.EqualError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), lock, store), "error initializing store") + assert.EqualError(t, f.Initialize(metrics.NullFactory, lock, store, zap.NewNop()), "error initializing store") _, _, err = f.CreateStrategyStore() assert.EqualError(t, err, "error creating store") } @@ -132,7 +132,7 @@ func (f *mockFactory) CreateStrategyStore() (ss.StrategyStore, ss.Aggregator, er return nil, nil, nil } -func (f *mockFactory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger, lock distributedlock.Lock, store samplingstore.Store) error { +func (f *mockFactory) Initialize(metricsFactory metrics.Factory, lock distributedlock.Lock, store samplingstore.Store, logger *zap.Logger) error { if f.retError { return errors.New("error initializing store") } diff --git a/plugin/sampling/strategystore/static/factory.go b/plugin/sampling/strategystore/static/factory.go index f296401b12c..65519b6d6c4 100644 --- a/plugin/sampling/strategystore/static/factory.go +++ b/plugin/sampling/strategystore/static/factory.go @@ -51,7 +51,7 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { } // Initialize implements strategystore.Factory -func (f *Factory) Initialize(_ metrics.Factory, logger *zap.Logger, _ distributedlock.Lock, _ samplingstore.Store) error { +func (f *Factory) Initialize(_ metrics.Factory, _ distributedlock.Lock, _ samplingstore.Store, logger *zap.Logger) error { f.logger = logger return nil } diff --git a/plugin/sampling/strategystore/static/factory_test.go b/plugin/sampling/strategystore/static/factory_test.go index ff23e52e9e6..4f96f108556 100644 --- a/plugin/sampling/strategystore/static/factory_test.go +++ b/plugin/sampling/strategystore/static/factory_test.go @@ -35,7 +35,7 @@ func TestFactory(t *testing.T) { command.ParseFlags([]string{"--sampling.strategies-file=fixtures/strategies.json"}) f.InitFromViper(v, zap.NewNop()) - assert.NoError(t, f.Initialize(metrics.NullFactory, zap.NewNop(), nil, nil)) + assert.NoError(t, f.Initialize(metrics.NullFactory, nil, nil, zap.NewNop())) _, _, err := f.CreateStrategyStore() assert.NoError(t, err) } From c0b0b3a03304210d32840e131dc37614bdebe91e Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 6 Jul 2021 14:42:31 -0400 Subject: [PATCH 25/40] io.Closer Signed-off-by: Joe Elliott --- cmd/collector/app/collector.go | 2 +- cmd/collector/app/root_span_handler_test.go | 4 ++-- cmd/collector/app/sampling/strategystore/interface.go | 7 ++++--- plugin/sampling/leaderelection/leader_election.go | 4 +++- plugin/sampling/strategystore/adaptive/aggregator.go | 3 ++- plugin/sampling/strategystore/adaptive/aggregator_test.go | 2 +- 6 files changed, 13 insertions(+), 9 deletions(-) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index 5fa5d8c0341..9ee129dca3d 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -178,7 +178,7 @@ func (c *Collector) Close() error { // aggregator does not exist for all strategy stores. only Stop() if exists. if c.aggregator != nil { - c.aggregator.Stop() + c.aggregator.Close() } // watchers actually never return errors from Close diff --git a/cmd/collector/app/root_span_handler_test.go b/cmd/collector/app/root_span_handler_test.go index 8daf459dc13..155fa199577 100644 --- a/cmd/collector/app/root_span_handler_test.go +++ b/cmd/collector/app/root_span_handler_test.go @@ -30,8 +30,8 @@ type mockAggregator struct { func (t *mockAggregator) RecordThroughput(service, operation, samplerType string, probability float64) { t.callCount++ } -func (t *mockAggregator) Start() {} -func (t *mockAggregator) Stop() {} +func (t *mockAggregator) Start() {} +func (t *mockAggregator) Close() error { return nil } func TestHandleRootSpan(t *testing.T) { aggregator := &mockAggregator{} diff --git a/cmd/collector/app/sampling/strategystore/interface.go b/cmd/collector/app/sampling/strategystore/interface.go index 5b0c238d30d..f3d3541187e 100644 --- a/cmd/collector/app/sampling/strategystore/interface.go +++ b/cmd/collector/app/sampling/strategystore/interface.go @@ -16,6 +16,7 @@ package strategystore import ( "context" + "io" "github.com/jaegertracing/jaeger/thrift-gen/sampling" ) @@ -28,12 +29,12 @@ type StrategyStore interface { // Aggregator defines an interface used to aggregate operation throughput. type Aggregator interface { + // Close() from io.Closer stops the aggregator from aggregating throughput. + io.Closer + // RecordThroughput records throughput for an operation for aggregation. RecordThroughput(service, operation, samplerType string, probability float64) // Start starts aggregating operation throughput. Start() - - // Stop stops the aggregator from aggregating throughput. - Stop() } diff --git a/plugin/sampling/leaderelection/leader_election.go b/plugin/sampling/leaderelection/leader_election.go index 20963bdc9aa..e800263b136 100644 --- a/plugin/sampling/leaderelection/leader_election.go +++ b/plugin/sampling/leaderelection/leader_election.go @@ -15,6 +15,7 @@ package leaderelection import ( + "io" "sync" "time" @@ -30,9 +31,10 @@ const ( // ElectionParticipant partakes in leader election to become leader. type ElectionParticipant interface { + io.Closer + IsLeader() bool Start() error - Close() error } // DistributedElectionParticipant implements ElectionParticipant on top of a distributed lock. diff --git a/plugin/sampling/strategystore/adaptive/aggregator.go b/plugin/sampling/strategystore/adaptive/aggregator.go index e055d2816cd..26cc3c5e60b 100644 --- a/plugin/sampling/strategystore/adaptive/aggregator.go +++ b/plugin/sampling/strategystore/adaptive/aggregator.go @@ -115,6 +115,7 @@ func (a *aggregator) Start() { go a.runAggregationLoop() } -func (a *aggregator) Stop() { +func (a *aggregator) Close() error { close(a.stop) + return nil } diff --git a/plugin/sampling/strategystore/adaptive/aggregator_test.go b/plugin/sampling/strategystore/adaptive/aggregator_test.go index 3f5fba57f03..cc7fe0176fa 100644 --- a/plugin/sampling/strategystore/adaptive/aggregator_test.go +++ b/plugin/sampling/strategystore/adaptive/aggregator_test.go @@ -46,7 +46,7 @@ func TestAggregator(t *testing.T) { a.RecordThroughput("A", "GET", lowerbound, 0.001) a.Start() - defer a.Stop() + defer a.Close() for i := 0; i < 10000; i++ { counters, _ := metricsFactory.Snapshot() if _, ok := counters["sampling_operations"]; ok { From 3701832e6f16a9fe47b7f896b79d1dd32ba623f5 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 6 Jul 2021 14:45:17 -0400 Subject: [PATCH 26/40] log error on aggregator close Signed-off-by: Joe Elliott --- cmd/collector/app/collector.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/collector/app/collector.go b/cmd/collector/app/collector.go index 9ee129dca3d..b6fbb80eb8f 100644 --- a/cmd/collector/app/collector.go +++ b/cmd/collector/app/collector.go @@ -176,9 +176,11 @@ func (c *Collector) Close() error { c.logger.Error("failed to close span processor.", zap.Error(err)) } - // aggregator does not exist for all strategy stores. only Stop() if exists. + // aggregator does not exist for all strategy stores. only Close() if exists. if c.aggregator != nil { - c.aggregator.Close() + if err := c.aggregator.Close(); err != nil { + c.logger.Error("failed to close aggregator.", zap.Error(err)) + } } // watchers actually never return errors from Close From 51a6afbd22cb15991684dab1ee4f62e56a2fefad Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 6 Jul 2021 15:15:38 -0400 Subject: [PATCH 27/40] Added mocks to test that the right values are returned Signed-off-by: Joe Elliott --- plugin/storage/factory_test.go | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/plugin/storage/factory_test.go b/plugin/storage/factory_test.go index a94232b2e14..5d72e3db0e9 100644 --- a/plugin/storage/factory_test.go +++ b/plugin/storage/factory_test.go @@ -35,11 +35,13 @@ import ( "github.com/jaegertracing/jaeger/pkg/config" "github.com/jaegertracing/jaeger/pkg/distributedlock" + lmocks "github.com/jaegertracing/jaeger/pkg/distributedlock/mocks" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" depStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" "github.com/jaegertracing/jaeger/storage/mocks" "github.com/jaegertracing/jaeger/storage/samplingstore" + ssmocks "github.com/jaegertracing/jaeger/storage/samplingstore/mocks" "github.com/jaegertracing/jaeger/storage/spanstore" spanStoreMocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) @@ -402,7 +404,10 @@ func TestPublishOpts(t *testing.T) { } func TestCreateLockAndSamplingStore(t *testing.T) { - supports := &errorFactory{} + supports := &errorFactory{ + lock: &lmocks.Lock{}, + store: &ssmocks.Store{}, + } doesNotSupport := &errorFactory{ lockAndSamplingStore: storage.ErrLockAndSamplingStoreNotSupported, } @@ -410,7 +415,7 @@ func TestCreateLockAndSamplingStore(t *testing.T) { lockAndSamplingStore: errors.New("wups"), } - // test succeeds (supported or not) + // test succeeds f := Factory{ factories: map[string]storage.Factory{ "supports": supports, @@ -418,12 +423,18 @@ func TestCreateLockAndSamplingStore(t *testing.T) { }, } - _, _, err := f.CreateLockAndSamplingStore() + // finds factory that supports + lock, samplingStore, err := f.CreateLockAndSamplingStore() assert.NoError(t, err) + assert.Equal(t, supports.lock, lock) + assert.Equal(t, supports.store, samplingStore) + // does not find factory that supports delete(f.factories, "supports") - _, _, err = f.CreateLockAndSamplingStore() + lock, samplingStore, err = f.CreateLockAndSamplingStore() assert.NoError(t, err) + assert.Nil(t, lock) + assert.Nil(t, samplingStore) // test breaks f = Factory{ @@ -439,6 +450,9 @@ func TestCreateLockAndSamplingStore(t *testing.T) { type errorFactory struct { closeErr error lockAndSamplingStore error + + lock distributedlock.Lock + store samplingstore.Store } var _ storage.Factory = (*errorFactory)(nil) @@ -461,7 +475,7 @@ func (e errorFactory) CreateDependencyReader() (dependencystore.Reader, error) { } func (e errorFactory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, e.lockAndSamplingStore + return e.lock, e.store, e.lockAndSamplingStore } func (e errorFactory) Close() error { From 73dfc994c013295ee26449a3d4afd61e989799db Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 6 Jul 2021 15:20:44 -0400 Subject: [PATCH 28/40] Improved unique name logging Signed-off-by: Joe Elliott --- plugin/sampling/strategystore/adaptive/strategy_store.go | 2 +- plugin/storage/cassandra/factory.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/sampling/strategystore/adaptive/strategy_store.go b/plugin/sampling/strategystore/adaptive/strategy_store.go index cf5cafd2c4f..c47e01aaf12 100644 --- a/plugin/sampling/strategystore/adaptive/strategy_store.go +++ b/plugin/sampling/strategystore/adaptive/strategy_store.go @@ -30,7 +30,7 @@ func NewStrategyStore(options Options, metricsFactory metrics.Factory, logger *z if err != nil { return nil, err } - logger.Info("Retrieved unique hostname from the OS for use in adaptive sampling", zap.String("hostname", hostname)) + logger.Info("Using unique participantName in adaptive sampling", zap.String("participantName", hostname)) participant := leaderelection.NewElectionParticipant(lock, defaultResourceName, leaderelection.ElectionParticipantOptions{ FollowerLeaseRefreshInterval: options.FollowerLeaseRefreshInterval, diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index 2b79a9652c5..570fc74ab93 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -158,7 +158,7 @@ func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingst if err != nil { return nil, nil, err } - f.logger.Info("Retrieved unique hostname from the OS for use in the distributed lock", zap.String("hostname", hostname)) + f.logger.Info("Using unique participantName in the distributed lock", zap.String("participantName", hostname)) store := cSamplingStore.New(f.primarySession, f.primaryMetricsFactory, f.logger) lock := cLock.NewLock(f.primarySession, hostname) From 469b23537dddb6b2cd9ae0c3ecd38620076f456b Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 6 Jul 2021 15:41:54 -0400 Subject: [PATCH 29/40] make fmt Signed-off-by: Joe Elliott --- pkg/hostname/hostname.go | 14 ++++++++++++++ pkg/hostname/hostname_test.go | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/pkg/hostname/hostname.go b/pkg/hostname/hostname.go index 867bcb840ce..721aafe0129 100644 --- a/pkg/hostname/hostname.go +++ b/pkg/hostname/hostname.go @@ -1,3 +1,17 @@ +// Copyright (c) 2021 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 hostname import ( diff --git a/pkg/hostname/hostname_test.go b/pkg/hostname/hostname_test.go index 9a18f9b0798..58a2301c175 100644 --- a/pkg/hostname/hostname_test.go +++ b/pkg/hostname/hostname_test.go @@ -1,3 +1,17 @@ +// Copyright (c) 2021 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 hostname import ( From 1d540425c65d25d2062363ac6860eb9379d7b0ea Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 14 Jul 2021 10:55:46 -0400 Subject: [PATCH 30/40] Additional testing Signed-off-by: Joe Elliott --- cmd/collector/app/collector_test.go | 59 +++++++++++++++++++ cmd/collector/app/root_span_handler_test.go | 10 +++- model/span_test.go | 8 +++ pkg/hostname/hostname.go | 40 +++++++++---- pkg/hostname/hostname_test.go | 27 ++++++--- .../strategystore/adaptive/processor_test.go | 38 ++++++++++++ 6 files changed, 159 insertions(+), 23 deletions(-) diff --git a/cmd/collector/app/collector_test.go b/cmd/collector/app/collector_test.go index 9686177f997..d5e369e172b 100644 --- a/cmd/collector/app/collector_test.go +++ b/cmd/collector/app/collector_test.go @@ -25,6 +25,8 @@ import ( "github.com/uber/jaeger-lib/metrics/metricstest" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/cmd/collector/app/processor" + "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/healthcheck" "github.com/jaegertracing/jaeger/thrift-gen/sampling" ) @@ -98,3 +100,60 @@ func TestCollector_PublishOpts(t *testing.T) { Value: 42, }) } + +func TestAggregator(t *testing.T) { + // prepare + hc := healthcheck.New() + logger := zap.NewNop() + baseMetrics := metricstest.NewFactory(time.Hour) + spanWriter := &fakeSpanWriter{} + strategyStore := &mockStrategyStore{} + agg := &mockAggregator{} + + c := New(&CollectorParams{ + ServiceName: "collector", + Logger: logger, + MetricsFactory: baseMetrics, + SpanWriter: spanWriter, + StrategyStore: strategyStore, + HealthCheck: hc, + Aggregator: agg, + }) + collectorOpts := &CollectorOptions{ + QueueSize: 10, + NumWorkers: 10, + } + + // test + c.Start(collectorOpts) + + // assert that aggregator was added to the collector + _, err := c.spanProcessor.ProcessSpans([]*model.Span{ + { + OperationName: "y", + Process: &model.Process{ + ServiceName: "x", + }, + Tags: []model.KeyValue{ + { + Key: "sampler.type", + VStr: "probabilistic", + }, + { + Key: "sampler.param", + VStr: "1", + }, + }, + }, + }, processor.SpansOptions{SpanFormat: processor.JaegerSpanFormat}) + assert.NoError(t, err) + + // verify + assert.NoError(t, c.Close()) + + // assert that aggregator was used + assert.Equal(t, 1, agg.callCount) + + // assert that aggregator close was called + assert.Equal(t, 1, agg.closeCount) +} diff --git a/cmd/collector/app/root_span_handler_test.go b/cmd/collector/app/root_span_handler_test.go index 155fa199577..d646888aa48 100644 --- a/cmd/collector/app/root_span_handler_test.go +++ b/cmd/collector/app/root_span_handler_test.go @@ -24,14 +24,18 @@ import ( ) type mockAggregator struct { - callCount int + callCount int + closeCount int } func (t *mockAggregator) RecordThroughput(service, operation, samplerType string, probability float64) { t.callCount++ } -func (t *mockAggregator) Start() {} -func (t *mockAggregator) Close() error { return nil } +func (t *mockAggregator) Start() {} +func (t *mockAggregator) Close() error { + t.closeCount++ + return nil +} func TestHandleRootSpan(t *testing.T) { aggregator := &mockAggregator{} diff --git a/model/span_test.go b/model/span_test.go index 4ddd7944d4b..28eedb2824b 100644 --- a/model/span_test.go +++ b/model/span_test.go @@ -483,6 +483,14 @@ func TestGetSamplerParams(t *testing.T) { expectedType: "", expectedParam: 0, }, + { + tags: model.KeyValues{ + model.String("sampler.type", "not_a_type"), + model.String("sampler.param", "not_a_number"), + }, + expectedType: "", + expectedParam: 0, + }, } for i, test := range tests { diff --git a/pkg/hostname/hostname.go b/pkg/hostname/hostname.go index 721aafe0129..5e36b0a3b1f 100644 --- a/pkg/hostname/hostname.go +++ b/pkg/hostname/hostname.go @@ -18,21 +18,35 @@ import ( "crypto/rand" "fmt" "os" + "sync" ) +type hostname struct { + once sync.Once + err error + hostname string +} + +var h hostname + // AsIdentifier uses the hostname of the os and postfixes a short random string to guarantee uniqueness -// The returned value is appropriate to use as a convenient unique identifier. +// The returned value is appropriate to use as a convenient unique identifier and will always be equal +// when called from within the same process. func AsIdentifier() (string, error) { - hostname, err := os.Hostname() - if err != nil { - return "", err - } - - buff := make([]byte, 8) - _, err = rand.Read(buff) - if err != nil { - return "", err - } - - return hostname + "-" + fmt.Sprintf("%2x", buff), nil + h.once.Do(func() { + h.hostname, h.err = os.Hostname() + if h.err != nil { + return + } + + buff := make([]byte, 8) + _, h.err = rand.Read(buff) + if h.err != nil { + return + } + + h.hostname = h.hostname + "-" + fmt.Sprintf("%2x", buff) + }) + + return h.hostname, h.err } diff --git a/pkg/hostname/hostname_test.go b/pkg/hostname/hostname_test.go index 58a2301c175..34e61bb35b1 100644 --- a/pkg/hostname/hostname_test.go +++ b/pkg/hostname/hostname_test.go @@ -17,6 +17,7 @@ package hostname import ( "os" "strings" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -24,14 +25,26 @@ import ( ) func TestAsIdentifier(t *testing.T) { - hostname1, err := AsIdentifier() - require.NoError(t, err) - hostname2, err := AsIdentifier() - require.NoError(t, err) + var hostname1 string + var hostname2 string - actualHostname, _ := os.Hostname() + wg := sync.WaitGroup{} + wg.Add(2) + go func() { + var err error + hostname1, err = AsIdentifier() + require.NoError(t, err) + wg.Done() + }() + go func() { + var err error + hostname2, err = AsIdentifier() + require.NoError(t, err) + wg.Done() + }() + wg.Wait() - assert.NotEqual(t, hostname1, hostname2) + actualHostname, _ := os.Hostname() + assert.Equal(t, hostname1, hostname2) assert.True(t, strings.HasPrefix(hostname1, actualHostname)) - assert.True(t, strings.HasPrefix(hostname2, actualHostname)) } diff --git a/plugin/sampling/strategystore/adaptive/processor_test.go b/plugin/sampling/strategystore/adaptive/processor_test.go index 148ac565f92..680c3fd5a84 100644 --- a/plugin/sampling/strategystore/adaptive/processor_test.go +++ b/plugin/sampling/strategystore/adaptive/processor_test.go @@ -837,3 +837,41 @@ func TestCalculateProbabilitiesAndQPSMultiple(t *testing.T) { p.probabilities = probabilities p.qps = qps } + +func TestErrors(t *testing.T) { + mockStorage := &smocks.Store{} + mockStorage.On("GetLatestProbabilities").Return(model.ServiceOperationProbabilities{}, errTestStorage) + + cfg := Options{ + TargetSamplesPerSecond: 1.0, + DeltaTolerance: 0.1, + InitialSamplingProbability: 0.001, + CalculationInterval: time.Millisecond * 5, + AggregationBuckets: 2, + Delay: time.Millisecond * 5, + LeaderLeaseRefreshInterval: time.Millisecond, + FollowerLeaseRefreshInterval: time.Second, + BucketsForCalculation: 10, + } + + // start errors + mockEP := &epmocks.ElectionParticipant{} + mockEP.On("Start").Return(errors.New("?")) + mockEP.On("Close").Return(errors.New("!")) + mockEP.On("IsLeader").Return(false) + + p, err := newProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, zap.NewNop()) + require.NoError(t, err) + assert.Error(t, p.Start()) + + // close errors + mockEP = &epmocks.ElectionParticipant{} + mockEP.On("Start").Return(nil) + mockEP.On("Close").Return(errors.New("!")) + mockEP.On("IsLeader").Return(false) + + p, err = newProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, zap.NewNop()) + require.NoError(t, err) + assert.NoError(t, p.Start()) + assert.Error(t, p.Close()) +} From fe31e5a1b1b3ce610a477b6af77c044ce1960d28 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 14 Jul 2021 11:28:49 -0400 Subject: [PATCH 31/40] lint Signed-off-by: Joe Elliott --- plugin/sampling/strategystore/adaptive/processor_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugin/sampling/strategystore/adaptive/processor_test.go b/plugin/sampling/strategystore/adaptive/processor_test.go index 680c3fd5a84..8107e49888c 100644 --- a/plugin/sampling/strategystore/adaptive/processor_test.go +++ b/plugin/sampling/strategystore/adaptive/processor_test.go @@ -856,8 +856,8 @@ func TestErrors(t *testing.T) { // start errors mockEP := &epmocks.ElectionParticipant{} - mockEP.On("Start").Return(errors.New("?")) - mockEP.On("Close").Return(errors.New("!")) + mockEP.On("Start").Return(errors.New("bad")) + mockEP.On("Close").Return(errors.New("also bad")) mockEP.On("IsLeader").Return(false) p, err := newProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, zap.NewNop()) @@ -867,7 +867,7 @@ func TestErrors(t *testing.T) { // close errors mockEP = &epmocks.ElectionParticipant{} mockEP.On("Start").Return(nil) - mockEP.On("Close").Return(errors.New("!")) + mockEP.On("Close").Return(errors.New("still bad")) mockEP.On("IsLeader").Return(false) p, err = newProcessor(cfg, "host", mockStorage, mockEP, metrics.NullFactory, zap.NewNop()) From ceeff9a57d30625fbc482c3ae6d622472b48357b Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 14 Jul 2021 13:01:37 -0400 Subject: [PATCH 32/40] fix test Signed-off-by: Joe Elliott --- plugin/sampling/strategystore/adaptive/processor_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugin/sampling/strategystore/adaptive/processor_test.go b/plugin/sampling/strategystore/adaptive/processor_test.go index 8107e49888c..466479263c7 100644 --- a/plugin/sampling/strategystore/adaptive/processor_test.go +++ b/plugin/sampling/strategystore/adaptive/processor_test.go @@ -841,6 +841,8 @@ func TestCalculateProbabilitiesAndQPSMultiple(t *testing.T) { func TestErrors(t *testing.T) { mockStorage := &smocks.Store{} mockStorage.On("GetLatestProbabilities").Return(model.ServiceOperationProbabilities{}, errTestStorage) + mockStorage.On("GetThroughput", mock.AnythingOfType("time.Time"), mock.AnythingOfType("time.Time")). + Return(nil, nil) cfg := Options{ TargetSamplesPerSecond: 1.0, From e1e1ea36d19780947ce6a94a1c06f5db2f905cbb Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Thu, 15 Jul 2021 09:33:35 -0400 Subject: [PATCH 33/40] tests Signed-off-by: Joe Elliott --- .../strategystore/adaptive/factory_test.go | 22 +++++++++++++++++++ plugin/sampling/strategystore/factory_test.go | 14 +++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/plugin/sampling/strategystore/adaptive/factory_test.go b/plugin/sampling/strategystore/adaptive/factory_test.go index f3745f6d633..4fcd2592ee4 100644 --- a/plugin/sampling/strategystore/adaptive/factory_test.go +++ b/plugin/sampling/strategystore/adaptive/factory_test.go @@ -67,6 +67,28 @@ func TestFactory(t *testing.T) { assert.NoError(t, err) } +func TestBadConfigFail(t *testing.T) { + tests := []string{ + "--sampling.aggregation-buckets=0", + "--sampling.calculation-interval=0", + "--sampling.buckets-for-calculation=0", + } + + for _, tc := range tests { + f := NewFactory() + v, command := config.Viperize(f.AddFlags) + command.ParseFlags([]string{ + tc, + }) + + f.InitFromViper(v, zap.NewNop()) + + assert.NoError(t, f.Initialize(metrics.NullFactory, &mockLock{}, &mockStore{}, zap.NewNop())) + _, _, err := f.CreateStrategyStore() + assert.Error(t, err) + } +} + func TestRequiresLockAndSamplingStore(t *testing.T) { f := NewFactory() required, err := f.RequiresLockAndSamplingStore() diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index 1a66f409dc0..5487db7e6cf 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -78,12 +78,24 @@ func TestNewFactory(t *testing.T) { assert.NoError(t, f.Initialize(metrics.NullFactory, lock, store, zap.NewNop())) _, _, err = f.CreateStrategyStore() require.NoError(t, err) + r, err := f.RequiresLockAndSamplingStore() + require.NoError(t, err) + require.False(t, r) // mock returns false // force the mock to return errors mock.retError = true assert.EqualError(t, f.Initialize(metrics.NullFactory, lock, store, zap.NewNop()), "error initializing store") _, _, err = f.CreateStrategyStore() assert.EqualError(t, err, "error creating store") + _, err = f.RequiresLockAndSamplingStore() + assert.EqualError(t, err, "error calling require lock and sampling store") + + // request something that doesn't exist + f.StrategyStoreType = "doesntexist" + _, _, err = f.CreateStrategyStore() + assert.EqualError(t, err, "no doesntexist strategy store registered") + _, err = f.RequiresLockAndSamplingStore() + assert.EqualError(t, err, "no doesntexist strategy store registered") } } @@ -141,7 +153,7 @@ func (f *mockFactory) Initialize(metricsFactory metrics.Factory, lock distribute func (f *mockFactory) RequiresLockAndSamplingStore() (bool, error) { if f.retError { - return false, errors.New("error creating store") + return false, errors.New("error calling require lock and sampling store") } return false, nil } From 73648717407ffa2349c601e5f4ec3edcbffc9b5c Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Thu, 15 Jul 2021 14:16:52 -0400 Subject: [PATCH 34/40] getParam => samplerParamToFloat Signed-off-by: Joe Elliott --- model/span.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/model/span.go b/model/span.go index 8b7d133726f..5136e06b3ea 100644 --- a/model/span.go +++ b/model/span.go @@ -143,7 +143,7 @@ func (s *Span) GetSamplerParams(logger *zap.Logger) (string, float64) { if !ok { return "", 0 } - samplerParam, err := getParam(tag) + samplerParam, err := samplerParamToFloat(tag) if err != nil { logger. With(zap.String("traceID", s.TraceID.String())). @@ -196,7 +196,7 @@ func (f Flags) checkFlags(bit Flags) bool { return f&bit == bit } -func getParam(samplerParamTag KeyValue) (float64, error) { +func samplerParamToFloat(samplerParamTag KeyValue) (float64, error) { // The param could be represented as a string, an int, or a float switch samplerParamTag.VType { case Float64Type: From ff455453918bd285856120f817e17c482132f5d3 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Thu, 15 Jul 2021 14:32:32 -0400 Subject: [PATCH 35/40] Removed requirelockandsamplingstore Signed-off-by: Joe Elliott --- cmd/all-in-one/main.go | 14 ++------------ .../app/sampling/strategystore/factory.go | 3 --- cmd/collector/main.go | 15 +++------------ plugin/sampling/strategystore/adaptive/factory.go | 10 +++++----- .../strategystore/adaptive/factory_test.go | 8 ++++---- plugin/sampling/strategystore/factory.go | 9 --------- plugin/sampling/strategystore/factory_test.go | 14 -------------- plugin/sampling/strategystore/static/factory.go | 5 ----- .../sampling/strategystore/static/factory_test.go | 7 ------- 9 files changed, 14 insertions(+), 71 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 63fe3db8ef2..8d917328f60 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -44,14 +44,12 @@ import ( "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/cmd/status" "github.com/jaegertracing/jaeger/pkg/config" - "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/version" metricsPlugin "github.com/jaegertracing/jaeger/plugin/metrics" ss "github.com/jaegertracing/jaeger/plugin/sampling/strategystore" "github.com/jaegertracing/jaeger/plugin/storage" "github.com/jaegertracing/jaeger/ports" "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" storageMetrics "github.com/jaegertracing/jaeger/storage/spanstore/metrics" ) @@ -127,17 +125,9 @@ by default uses only in-memory database.`, logger.Fatal("Failed to create metrics reader", zap.Error(err)) } - requireLockAndSamplingStore, err := strategyStoreFactory.RequiresLockAndSamplingStore() + lock, samplingStore, err := storageFactory.CreateLockAndSamplingStore() if err != nil { - logger.Fatal("Failed to determine if lock and sampling store is required.", zap.Error(err)) - } - var lock distributedlock.Lock - var samplingStore samplingstore.Store - if requireLockAndSamplingStore { - lock, samplingStore, err = storageFactory.CreateLockAndSamplingStore() - if err != nil { - logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) - } + logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) } strategyStoreFactory.InitFromViper(v, logger) diff --git a/cmd/collector/app/sampling/strategystore/factory.go b/cmd/collector/app/sampling/strategystore/factory.go index c4882060c65..1cf24f489c2 100644 --- a/cmd/collector/app/sampling/strategystore/factory.go +++ b/cmd/collector/app/sampling/strategystore/factory.go @@ -34,7 +34,4 @@ type Factory interface { // CreateStrategyStore initializes the StrategyStore and returns it. CreateStrategyStore() (StrategyStore, Aggregator, error) - - // RequiresLockAndSamplingStore indicates whether this strategy store requires a Lock and SamplingStore - RequiresLockAndSamplingStore() (bool, error) } diff --git a/cmd/collector/main.go b/cmd/collector/main.go index 973825e87cb..b47d26e356b 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -35,12 +35,10 @@ import ( "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/status" "github.com/jaegertracing/jaeger/pkg/config" - "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/version" ss "github.com/jaegertracing/jaeger/plugin/sampling/strategystore" "github.com/jaegertracing/jaeger/plugin/storage" "github.com/jaegertracing/jaeger/ports" - "github.com/jaegertracing/jaeger/storage/samplingstore" ) const serviceName = "jaeger-collector" @@ -86,18 +84,11 @@ func main() { logger.Fatal("Failed to create span writer", zap.Error(err)) } - requireLockAndSamplingStore, err := strategyStoreFactory.RequiresLockAndSamplingStore() + lock, samplingStore, err := storageFactory.CreateLockAndSamplingStore() if err != nil { - logger.Fatal("Failed to determine if lock and sampling store is required.", zap.Error(err)) - } - var lock distributedlock.Lock - var samplingStore samplingstore.Store - if requireLockAndSamplingStore { - lock, samplingStore, err = storageFactory.CreateLockAndSamplingStore() - if err != nil { - logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) - } + logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) } + strategyStoreFactory.InitFromViper(v, logger) if err := strategyStoreFactory.Initialize(metricsFactory, lock, samplingStore, logger); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) diff --git a/plugin/sampling/strategystore/adaptive/factory.go b/plugin/sampling/strategystore/adaptive/factory.go index 0924b61cfc3..979009e30d1 100644 --- a/plugin/sampling/strategystore/adaptive/factory.go +++ b/plugin/sampling/strategystore/adaptive/factory.go @@ -15,6 +15,7 @@ package adaptive import ( + "errors" "flag" "github.com/spf13/viper" @@ -58,6 +59,10 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { // Initialize implements strategystore.Factory func (f *Factory) Initialize(metricsFactory metrics.Factory, lock distributedlock.Lock, store samplingstore.Store, logger *zap.Logger) error { + if lock == nil || store == nil { + return errors.New("lock or SamplingStore nil. Please configure a backend that supports adaptive sampling") + } + f.logger = logger f.metricsFactory = metricsFactory f.lock = lock @@ -76,8 +81,3 @@ func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, strategyst a.Start() return p, a, nil } - -// RequiresLockAndSamplingStore implements strategystore.Factory -func (f *Factory) RequiresLockAndSamplingStore() (bool, error) { - return true, nil -} diff --git a/plugin/sampling/strategystore/adaptive/factory_test.go b/plugin/sampling/strategystore/adaptive/factory_test.go index 4fcd2592ee4..46cec55b5ff 100644 --- a/plugin/sampling/strategystore/adaptive/factory_test.go +++ b/plugin/sampling/strategystore/adaptive/factory_test.go @@ -89,11 +89,11 @@ func TestBadConfigFail(t *testing.T) { } } -func TestRequiresLockAndSamplingStore(t *testing.T) { +func TestNilLockAndSamplingFails(t *testing.T) { f := NewFactory() - required, err := f.RequiresLockAndSamplingStore() - assert.True(t, required) - assert.NoError(t, err) + assert.Error(t, f.Initialize(metrics.NullFactory, nil, &mockStore{}, zap.NewNop())) + assert.Error(t, f.Initialize(metrics.NullFactory, &mockLock{}, nil, zap.NewNop())) + assert.Error(t, f.Initialize(metrics.NullFactory, nil, nil, zap.NewNop())) } type mockStore struct{} diff --git a/plugin/sampling/strategystore/factory.go b/plugin/sampling/strategystore/factory.go index 61aa1145c37..c37caca2330 100644 --- a/plugin/sampling/strategystore/factory.go +++ b/plugin/sampling/strategystore/factory.go @@ -106,12 +106,3 @@ func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, strategyst } return factory.CreateStrategyStore() } - -// RequiresLockAndSamplingStore implements strategystore.Factory -func (f *Factory) RequiresLockAndSamplingStore() (bool, error) { - factory, ok := f.factories[f.StrategyStoreType] - if !ok { - return false, fmt.Errorf("no %s strategy store registered", f.StrategyStoreType) - } - return factory.RequiresLockAndSamplingStore() -} diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index 5487db7e6cf..d571584f271 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -78,24 +78,17 @@ func TestNewFactory(t *testing.T) { assert.NoError(t, f.Initialize(metrics.NullFactory, lock, store, zap.NewNop())) _, _, err = f.CreateStrategyStore() require.NoError(t, err) - r, err := f.RequiresLockAndSamplingStore() - require.NoError(t, err) - require.False(t, r) // mock returns false // force the mock to return errors mock.retError = true assert.EqualError(t, f.Initialize(metrics.NullFactory, lock, store, zap.NewNop()), "error initializing store") _, _, err = f.CreateStrategyStore() assert.EqualError(t, err, "error creating store") - _, err = f.RequiresLockAndSamplingStore() - assert.EqualError(t, err, "error calling require lock and sampling store") // request something that doesn't exist f.StrategyStoreType = "doesntexist" _, _, err = f.CreateStrategyStore() assert.EqualError(t, err, "no doesntexist strategy store registered") - _, err = f.RequiresLockAndSamplingStore() - assert.EqualError(t, err, "no doesntexist strategy store registered") } } @@ -151,13 +144,6 @@ func (f *mockFactory) Initialize(metricsFactory metrics.Factory, lock distribute return nil } -func (f *mockFactory) RequiresLockAndSamplingStore() (bool, error) { - if f.retError { - return false, errors.New("error calling require lock and sampling store") - } - return false, nil -} - type mockStore struct{} func (m *mockStore) InsertThroughput(throughput []*model.Throughput) error { diff --git a/plugin/sampling/strategystore/static/factory.go b/plugin/sampling/strategystore/static/factory.go index 65519b6d6c4..956129e1b9c 100644 --- a/plugin/sampling/strategystore/static/factory.go +++ b/plugin/sampling/strategystore/static/factory.go @@ -65,8 +65,3 @@ func (f *Factory) CreateStrategyStore() (strategystore.StrategyStore, strategyst return s, nil, nil } - -// RequiresLockAndSamplingStore implements strategystore.Factory -func (f *Factory) RequiresLockAndSamplingStore() (bool, error) { - return false, nil -} diff --git a/plugin/sampling/strategystore/static/factory_test.go b/plugin/sampling/strategystore/static/factory_test.go index 4f96f108556..8578a972186 100644 --- a/plugin/sampling/strategystore/static/factory_test.go +++ b/plugin/sampling/strategystore/static/factory_test.go @@ -39,10 +39,3 @@ func TestFactory(t *testing.T) { _, _, err := f.CreateStrategyStore() assert.NoError(t, err) } - -func TestRequiresLockAndSamplingStore(t *testing.T) { - f := NewFactory() - required, err := f.RequiresLockAndSamplingStore() - assert.False(t, required) - assert.NoError(t, err) -} From 027faf120154201a38172a8a3796c84464d86e53 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 20 Jul 2021 10:54:27 -0400 Subject: [PATCH 36/40] use constants for constants Signed-off-by: Joe Elliott --- plugin/sampling/strategystore/factory.go | 11 ++++++++--- plugin/sampling/strategystore/factory_config.go | 7 ++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/plugin/sampling/strategystore/factory.go b/plugin/sampling/strategystore/factory.go index c37caca2330..3a3f363fd7b 100644 --- a/plugin/sampling/strategystore/factory.go +++ b/plugin/sampling/strategystore/factory.go @@ -33,7 +33,12 @@ import ( // Kind is a datatype holding the type of strategy store. type Kind string -var allSamplingTypes = []Kind{"static", "adaptive"} +const ( + samplingTypeAdaptive = "adaptive" + samplingTypeStatic = "static" +) + +var allSamplingTypes = []Kind{samplingTypeStatic, samplingTypeAdaptive} // Factory implements strategystore.Factory interface as a meta-factory for strategy storage components. type Factory struct { @@ -61,9 +66,9 @@ func NewFactory(config FactoryConfig) (*Factory, error) { func (f *Factory) getFactoryOfType(factoryType Kind) (strategystore.Factory, error) { switch factoryType { - case "static": + case samplingTypeStatic: return static.NewFactory(), nil - case "adaptive": + case samplingTypeAdaptive: return adaptive.NewFactory(), nil default: return nil, fmt.Errorf("unknown sampling strategy store type %s. Valid types are %v", factoryType, allSamplingTypes) diff --git a/plugin/sampling/strategystore/factory_config.go b/plugin/sampling/strategystore/factory_config.go index b97db12846e..fe22a8990bb 100644 --- a/plugin/sampling/strategystore/factory_config.go +++ b/plugin/sampling/strategystore/factory_config.go @@ -35,10 +35,11 @@ type FactoryConfig struct { func FactoryConfigFromEnv() (*FactoryConfig, error) { strategyStoreType := os.Getenv(SamplingTypeEnvVar) if strategyStoreType == "" { - strategyStoreType = "static" + strategyStoreType = samplingTypeStatic } - if strategyStoreType != "adaptive" && strategyStoreType != "static" { - return nil, fmt.Errorf("invalid sampling type: %s", strategyStoreType) + + if strategyStoreType != samplingTypeAdaptive && strategyStoreType != samplingTypeStatic { + return nil, fmt.Errorf("invalid sampling type: %s. . Valid types are %v", strategyStoreType, allSamplingTypes) } return &FactoryConfig{ StrategyStoreType: Kind(strategyStoreType), From e18530580811c369529694644851eb2b3c0f107e Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 20 Jul 2021 14:43:20 -0400 Subject: [PATCH 37/40] Split out sampling store interface Signed-off-by: Joe Elliott --- cmd/all-in-one/main.go | 6 +- .../app/sampling/strategystore/factory.go | 5 +- cmd/collector/main.go | 6 +- cmd/query/app/querysvc/query_service_test.go | 5 -- .../strategystore/adaptive/factory.go | 16 ++++-- .../strategystore/adaptive/factory_test.go | 48 +++++++--------- plugin/sampling/strategystore/factory.go | 7 +-- plugin/sampling/strategystore/factory_test.go | 37 +++--------- .../sampling/strategystore/static/factory.go | 5 +- .../strategystore/static/factory_test.go | 2 +- plugin/storage/badger/factory.go | 8 --- plugin/storage/badger/factory_test.go | 12 ---- plugin/storage/cassandra/factory.go | 14 +++-- plugin/storage/cassandra/factory_test.go | 5 +- plugin/storage/es/factory.go | 8 --- plugin/storage/es/factory_test.go | 11 ---- plugin/storage/factory.go | 16 ++---- plugin/storage/factory_test.go | 57 ------------------- plugin/storage/grpc/factory.go | 7 --- plugin/storage/grpc/factory_test.go | 10 ---- plugin/storage/kafka/factory.go | 8 --- plugin/storage/kafka/factory_test.go | 11 ---- plugin/storage/memory/factory.go | 8 --- plugin/storage/memory/factory_test.go | 10 ---- storage/factory.go | 14 +++-- storage/mocks/Factory.go | 7 --- 26 files changed, 79 insertions(+), 264 deletions(-) diff --git a/cmd/all-in-one/main.go b/cmd/all-in-one/main.go index 8d917328f60..5203a104bfd 100644 --- a/cmd/all-in-one/main.go +++ b/cmd/all-in-one/main.go @@ -125,13 +125,13 @@ by default uses only in-memory database.`, logger.Fatal("Failed to create metrics reader", zap.Error(err)) } - lock, samplingStore, err := storageFactory.CreateLockAndSamplingStore() + ssFactory, err := storageFactory.CreateSamplingStoreFactory() if err != nil { - logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) + logger.Fatal("Failed to create sampling store factory", zap.Error(err)) } strategyStoreFactory.InitFromViper(v, logger) - if err := strategyStoreFactory.Initialize(metricsFactory, lock, samplingStore, logger); err != nil { + if err := strategyStoreFactory.Initialize(metricsFactory, ssFactory, logger); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) } strategyStore, aggregator, err := strategyStoreFactory.CreateStrategyStore() diff --git a/cmd/collector/app/sampling/strategystore/factory.go b/cmd/collector/app/sampling/strategystore/factory.go index 1cf24f489c2..bd9f33b724e 100644 --- a/cmd/collector/app/sampling/strategystore/factory.go +++ b/cmd/collector/app/sampling/strategystore/factory.go @@ -18,8 +18,7 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/distributedlock" - "github.com/jaegertracing/jaeger/storage/samplingstore" + "github.com/jaegertracing/jaeger/storage" ) // Factory defines an interface for a factory that can create implementations of different strategy storage components. @@ -30,7 +29,7 @@ import ( // plugin.Configurable type Factory interface { // Initialize performs internal initialization of the factory. - Initialize(metricsFactory metrics.Factory, lock distributedlock.Lock, store samplingstore.Store, logger *zap.Logger) error + Initialize(metricsFactory metrics.Factory, ssFactory storage.SamplingStoreFactory, logger *zap.Logger) error // CreateStrategyStore initializes the StrategyStore and returns it. CreateStrategyStore() (StrategyStore, Aggregator, error) diff --git a/cmd/collector/main.go b/cmd/collector/main.go index b47d26e356b..aacf58968ff 100644 --- a/cmd/collector/main.go +++ b/cmd/collector/main.go @@ -84,13 +84,13 @@ func main() { logger.Fatal("Failed to create span writer", zap.Error(err)) } - lock, samplingStore, err := storageFactory.CreateLockAndSamplingStore() + ssFactory, err := storageFactory.CreateSamplingStoreFactory() if err != nil { - logger.Fatal("Failed to create lock and sampling store for adaptive sampling", zap.Error(err)) + logger.Fatal("Failed to create sampling store factory", zap.Error(err)) } strategyStoreFactory.InitFromViper(v, logger) - if err := strategyStoreFactory.Initialize(metricsFactory, lock, samplingStore, logger); err != nil { + if err := strategyStoreFactory.Initialize(metricsFactory, ssFactory, logger); err != nil { logger.Fatal("Failed to init sampling strategy store factory", zap.Error(err)) } strategyStore, aggregator, err := strategyStoreFactory.CreateStrategyStore() diff --git a/cmd/query/app/querysvc/query_service_test.go b/cmd/query/app/querysvc/query_service_test.go index 4f9a7bd9332..0c3445d5774 100644 --- a/cmd/query/app/querysvc/query_service_test.go +++ b/cmd/query/app/querysvc/query_service_test.go @@ -27,11 +27,9 @@ import ( "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/model/adjuster" - "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" depsmocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" - "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) @@ -305,9 +303,6 @@ func (*fakeStorageFactory1) Initialize(metricsFactory metrics.Factory, logger *z func (*fakeStorageFactory1) CreateSpanReader() (spanstore.Reader, error) { return nil, nil } func (*fakeStorageFactory1) CreateSpanWriter() (spanstore.Writer, error) { return nil, nil } func (*fakeStorageFactory1) CreateDependencyReader() (dependencystore.Reader, error) { return nil, nil } -func (*fakeStorageFactory1) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, nil -} func (f *fakeStorageFactory2) CreateArchiveSpanReader() (spanstore.Reader, error) { return f.r, f.rErr } func (f *fakeStorageFactory2) CreateArchiveSpanWriter() (spanstore.Writer, error) { return f.w, f.wErr } diff --git a/plugin/sampling/strategystore/adaptive/factory.go b/plugin/sampling/strategystore/adaptive/factory.go index 979009e30d1..4adfdc5214e 100644 --- a/plugin/sampling/strategystore/adaptive/factory.go +++ b/plugin/sampling/strategystore/adaptive/factory.go @@ -24,6 +24,7 @@ import ( "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" "github.com/jaegertracing/jaeger/pkg/distributedlock" + "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/samplingstore" ) @@ -58,15 +59,22 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { } // Initialize implements strategystore.Factory -func (f *Factory) Initialize(metricsFactory metrics.Factory, lock distributedlock.Lock, store samplingstore.Store, logger *zap.Logger) error { - if lock == nil || store == nil { +func (f *Factory) Initialize(metricsFactory metrics.Factory, ssFactory storage.SamplingStoreFactory, logger *zap.Logger) error { + if ssFactory == nil { return errors.New("lock or SamplingStore nil. Please configure a backend that supports adaptive sampling") } + var err error f.logger = logger f.metricsFactory = metricsFactory - f.lock = lock - f.store = store + f.lock, err = ssFactory.CreateLock() + if err != nil { + return err + } + f.store, err = ssFactory.CreateSamplingStore() + if err != nil { + return err + } return nil } diff --git a/plugin/sampling/strategystore/adaptive/factory_test.go b/plugin/sampling/strategystore/adaptive/factory_test.go index 46cec55b5ff..008d9516bf2 100644 --- a/plugin/sampling/strategystore/adaptive/factory_test.go +++ b/plugin/sampling/strategystore/adaptive/factory_test.go @@ -19,13 +19,18 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" ss "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" "github.com/jaegertracing/jaeger/pkg/config" + "github.com/jaegertracing/jaeger/pkg/distributedlock" + lmocks "github.com/jaegertracing/jaeger/pkg/distributedlock/mocks" "github.com/jaegertracing/jaeger/plugin" + "github.com/jaegertracing/jaeger/storage/samplingstore" + smocks "github.com/jaegertracing/jaeger/storage/samplingstore/mocks" ) var _ ss.Factory = new(Factory) @@ -62,7 +67,7 @@ func TestFactory(t *testing.T) { assert.Equal(t, time.Second, f.options.LeaderLeaseRefreshInterval) assert.Equal(t, time.Second*2, f.options.FollowerLeaseRefreshInterval) - assert.NoError(t, f.Initialize(metrics.NullFactory, &mockLock{}, &mockStore{}, zap.NewNop())) + assert.NoError(t, f.Initialize(metrics.NullFactory, &mockSamplingStoreFactory{}, zap.NewNop())) _, _, err := f.CreateStrategyStore() assert.NoError(t, err) } @@ -83,43 +88,30 @@ func TestBadConfigFail(t *testing.T) { f.InitFromViper(v, zap.NewNop()) - assert.NoError(t, f.Initialize(metrics.NullFactory, &mockLock{}, &mockStore{}, zap.NewNop())) + assert.NoError(t, f.Initialize(metrics.NullFactory, &mockSamplingStoreFactory{}, zap.NewNop())) _, _, err := f.CreateStrategyStore() assert.Error(t, err) } } -func TestNilLockAndSamplingFails(t *testing.T) { +func TestNilSamplingStoreFactoryFails(t *testing.T) { f := NewFactory() - assert.Error(t, f.Initialize(metrics.NullFactory, nil, &mockStore{}, zap.NewNop())) - assert.Error(t, f.Initialize(metrics.NullFactory, &mockLock{}, nil, zap.NewNop())) - assert.Error(t, f.Initialize(metrics.NullFactory, nil, nil, zap.NewNop())) + assert.Error(t, f.Initialize(metrics.NullFactory, nil, zap.NewNop())) } -type mockStore struct{} +type mockSamplingStoreFactory struct{} -func (m *mockStore) InsertThroughput(throughput []*model.Throughput) error { - return nil -} -func (m *mockStore) InsertProbabilitiesAndQPS(hostname string, probabilities model.ServiceOperationProbabilities, qps model.ServiceOperationQPS) error { - return nil -} -func (m *mockStore) GetThroughput(start, end time.Time) ([]*model.Throughput, error) { - return nil, nil -} -func (m *mockStore) GetProbabilitiesAndQPS(start, end time.Time) (map[string][]model.ServiceOperationData, error) { - return nil, nil -} -func (m *mockStore) GetLatestProbabilities() (model.ServiceOperationProbabilities, error) { - return nil, nil -} - -type mockLock struct{} +func (m *mockSamplingStoreFactory) CreateLock() (distributedlock.Lock, error) { + mockLock := &lmocks.Lock{} + mockLock.On("Acquire", mock.Anything, mock.Anything).Return(true, nil) -func (m *mockLock) Acquire(resource string, ttl time.Duration) (acquired bool, err error) { - return true, nil + return mockLock, nil } +func (m *mockSamplingStoreFactory) CreateSamplingStore() (samplingstore.Store, error) { + mockStorage := &smocks.Store{} + mockStorage.On("GetLatestProbabilities").Return(make(model.ServiceOperationProbabilities), nil) + mockStorage.On("GetThroughput", mock.AnythingOfType("time.Time"), mock.AnythingOfType("time.Time")). + Return([]*model.Throughput{}, nil) -func (m *mockLock) Forfeit(resource string) (forfeited bool, err error) { - return true, nil + return mockStorage, nil } diff --git a/plugin/sampling/strategystore/factory.go b/plugin/sampling/strategystore/factory.go index 3a3f363fd7b..c6ca3717374 100644 --- a/plugin/sampling/strategystore/factory.go +++ b/plugin/sampling/strategystore/factory.go @@ -23,11 +23,10 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" - "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/plugin" "github.com/jaegertracing/jaeger/plugin/sampling/strategystore/adaptive" "github.com/jaegertracing/jaeger/plugin/sampling/strategystore/static" - "github.com/jaegertracing/jaeger/storage/samplingstore" + "github.com/jaegertracing/jaeger/storage" ) // Kind is a datatype holding the type of strategy store. @@ -94,9 +93,9 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { } // Initialize implements strategystore.Factory -func (f *Factory) Initialize(metricsFactory metrics.Factory, lock distributedlock.Lock, store samplingstore.Store, logger *zap.Logger) error { +func (f *Factory) Initialize(metricsFactory metrics.Factory, ssFactory storage.SamplingStoreFactory, logger *zap.Logger) error { for _, factory := range f.factories { - if err := factory.Initialize(metricsFactory, lock, store, logger); err != nil { + if err := factory.Initialize(metricsFactory, ssFactory, logger); err != nil { return err } } diff --git a/plugin/sampling/strategystore/factory_test.go b/plugin/sampling/strategystore/factory_test.go index d571584f271..0458f39ca16 100644 --- a/plugin/sampling/strategystore/factory_test.go +++ b/plugin/sampling/strategystore/factory_test.go @@ -20,7 +20,6 @@ import ( "flag" "os" "testing" - "time" "github.com/spf13/viper" "github.com/stretchr/testify/assert" @@ -28,10 +27,10 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" ss "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/plugin" + "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/samplingstore" ) @@ -59,8 +58,7 @@ func TestNewFactory(t *testing.T) { }, } - lock := &mockLock{} - store := &mockStore{} + mockSSFactory := &mockSamplingStoreFactory{} for _, tc := range tests { f, err := NewFactory(FactoryConfig{StrategyStoreType: Kind(tc.strategyStoreType)}) @@ -75,13 +73,13 @@ func TestNewFactory(t *testing.T) { mock := new(mockFactory) f.factories[Kind(tc.strategyStoreType)] = mock - assert.NoError(t, f.Initialize(metrics.NullFactory, lock, store, zap.NewNop())) + assert.NoError(t, f.Initialize(metrics.NullFactory, mockSSFactory, zap.NewNop())) _, _, err = f.CreateStrategyStore() require.NoError(t, err) // force the mock to return errors mock.retError = true - assert.EqualError(t, f.Initialize(metrics.NullFactory, lock, store, zap.NewNop()), "error initializing store") + assert.EqualError(t, f.Initialize(metrics.NullFactory, mockSSFactory, zap.NewNop()), "error initializing store") _, _, err = f.CreateStrategyStore() assert.EqualError(t, err, "error creating store") @@ -137,37 +135,18 @@ func (f *mockFactory) CreateStrategyStore() (ss.StrategyStore, ss.Aggregator, er return nil, nil, nil } -func (f *mockFactory) Initialize(metricsFactory metrics.Factory, lock distributedlock.Lock, store samplingstore.Store, logger *zap.Logger) error { +func (f *mockFactory) Initialize(metricsFactory metrics.Factory, ssFactory storage.SamplingStoreFactory, logger *zap.Logger) error { if f.retError { return errors.New("error initializing store") } return nil } -type mockStore struct{} +type mockSamplingStoreFactory struct{} -func (m *mockStore) InsertThroughput(throughput []*model.Throughput) error { - return nil -} -func (m *mockStore) InsertProbabilitiesAndQPS(hostname string, probabilities model.ServiceOperationProbabilities, qps model.ServiceOperationQPS) error { - return nil -} -func (m *mockStore) GetThroughput(start, end time.Time) ([]*model.Throughput, error) { - return nil, nil -} -func (m *mockStore) GetProbabilitiesAndQPS(start, end time.Time) (map[string][]model.ServiceOperationData, error) { +func (m *mockSamplingStoreFactory) CreateLock() (distributedlock.Lock, error) { return nil, nil } -func (m *mockStore) GetLatestProbabilities() (model.ServiceOperationProbabilities, error) { +func (m *mockSamplingStoreFactory) CreateSamplingStore() (samplingstore.Store, error) { return nil, nil } - -type mockLock struct{} - -func (m *mockLock) Acquire(resource string, ttl time.Duration) (acquired bool, err error) { - return true, nil -} - -func (m *mockLock) Forfeit(resource string) (forfeited bool, err error) { - return true, nil -} diff --git a/plugin/sampling/strategystore/static/factory.go b/plugin/sampling/strategystore/static/factory.go index 956129e1b9c..ec7603efb8f 100644 --- a/plugin/sampling/strategystore/static/factory.go +++ b/plugin/sampling/strategystore/static/factory.go @@ -22,8 +22,7 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" - "github.com/jaegertracing/jaeger/pkg/distributedlock" - "github.com/jaegertracing/jaeger/storage/samplingstore" + "github.com/jaegertracing/jaeger/storage" ) // Factory implements strategystore.Factory for a static strategy store. @@ -51,7 +50,7 @@ func (f *Factory) InitFromViper(v *viper.Viper, logger *zap.Logger) { } // Initialize implements strategystore.Factory -func (f *Factory) Initialize(_ metrics.Factory, _ distributedlock.Lock, _ samplingstore.Store, logger *zap.Logger) error { +func (f *Factory) Initialize(_ metrics.Factory, _ storage.SamplingStoreFactory, logger *zap.Logger) error { f.logger = logger return nil } diff --git a/plugin/sampling/strategystore/static/factory_test.go b/plugin/sampling/strategystore/static/factory_test.go index 8578a972186..f2b47291b80 100644 --- a/plugin/sampling/strategystore/static/factory_test.go +++ b/plugin/sampling/strategystore/static/factory_test.go @@ -35,7 +35,7 @@ func TestFactory(t *testing.T) { command.ParseFlags([]string{"--sampling.strategies-file=fixtures/strategies.json"}) f.InitFromViper(v, zap.NewNop()) - assert.NoError(t, f.Initialize(metrics.NullFactory, nil, nil, zap.NewNop())) + assert.NoError(t, f.Initialize(metrics.NullFactory, nil, zap.NewNop())) _, _, err := f.CreateStrategyStore() assert.NoError(t, err) } diff --git a/plugin/storage/badger/factory.go b/plugin/storage/badger/factory.go index 7528bc4e68f..98e6a10e603 100644 --- a/plugin/storage/badger/factory.go +++ b/plugin/storage/badger/factory.go @@ -27,12 +27,9 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/distributedlock" depStore "github.com/jaegertracing/jaeger/plugin/storage/badger/dependencystore" badgerStore "github.com/jaegertracing/jaeger/plugin/storage/badger/spanstore" - "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -167,11 +164,6 @@ func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { return depStore.NewDependencyStore(sr), nil } -// CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, storage.ErrLockAndSamplingStoreNotSupported -} - // Close Implements io.Closer and closes the underlying storage func (f *Factory) Close() error { close(f.maintenanceDone) diff --git a/plugin/storage/badger/factory_test.go b/plugin/storage/badger/factory_test.go index 194086174ad..503919ef562 100644 --- a/plugin/storage/badger/factory_test.go +++ b/plugin/storage/badger/factory_test.go @@ -28,7 +28,6 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/config" - "github.com/jaegertracing/jaeger/storage" ) func TestInitializationErrors(t *testing.T) { @@ -69,9 +68,6 @@ func TestForCodecov(t *testing.T) { _, err = f.CreateDependencyReader() assert.NoError(t, err) - _, _, err = f.CreateLockAndSamplingStore() - assert.Equal(t, storage.ErrLockAndSamplingStoreNotSupported, err) - // Now, remove the badger directories err = os.RemoveAll(f.tmpDir) assert.NoError(t, err) @@ -199,11 +195,3 @@ func TestInitFromOptions(t *testing.T) { f.InitFromOptions(opts) assert.Equal(t, &opts, f.Options) } - -func TestCreateLockAndSamplingStore(t *testing.T) { - f := NewFactory() - lock, ss, err := f.CreateLockAndSamplingStore() - assert.Nil(t, lock) - assert.Nil(t, ss) - assert.Equal(t, err, storage.ErrLockAndSamplingStoreNotSupported) -} diff --git a/plugin/storage/cassandra/factory.go b/plugin/storage/cassandra/factory.go index 570fc74ab93..7cca3b13c7c 100644 --- a/plugin/storage/cassandra/factory.go +++ b/plugin/storage/cassandra/factory.go @@ -152,18 +152,20 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { return cSpanStore.NewSpanWriter(f.archiveSession, f.Options.SpanStoreWriteCacheTTL, f.archiveMetricsFactory, f.logger, options...), nil } -// CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { +// CreateLock implements storage.SamplingStoreFactory +func (f *Factory) CreateLock() (distributedlock.Lock, error) { hostname, err := hostname.AsIdentifier() if err != nil { - return nil, nil, err + return nil, err } f.logger.Info("Using unique participantName in the distributed lock", zap.String("participantName", hostname)) - store := cSamplingStore.New(f.primarySession, f.primaryMetricsFactory, f.logger) - lock := cLock.NewLock(f.primarySession, hostname) + return cLock.NewLock(f.primarySession, hostname), nil +} - return lock, store, nil +// CreateSamplingStore implements storage.SamplingStoreFactory +func (f *Factory) CreateSamplingStore() (samplingstore.Store, error) { + return cSamplingStore.New(f.primarySession, f.primaryMetricsFactory, f.logger), nil } func writerOptions(opts *Options) ([]cSpanStore.Option, error) { diff --git a/plugin/storage/cassandra/factory_test.go b/plugin/storage/cassandra/factory_test.go index 1426cd72098..bba78f9e2f6 100644 --- a/plugin/storage/cassandra/factory_test.go +++ b/plugin/storage/cassandra/factory_test.go @@ -100,7 +100,10 @@ func TestCassandraFactory(t *testing.T) { _, err = f.CreateArchiveSpanWriter() assert.NoError(t, err) - _, _, err = f.CreateLockAndSamplingStore() + _, err = f.CreateLock() + assert.NoError(t, err) + + _, err = f.CreateSamplingStore() assert.NoError(t, err) assert.NoError(t, f.Close()) diff --git a/plugin/storage/es/factory.go b/plugin/storage/es/factory.go index ae970deab2d..6ce1f25d34f 100644 --- a/plugin/storage/es/factory.go +++ b/plugin/storage/es/factory.go @@ -24,15 +24,12 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/es" "github.com/jaegertracing/jaeger/pkg/es/config" esDepStore "github.com/jaegertracing/jaeger/plugin/storage/es/dependencystore" "github.com/jaegertracing/jaeger/plugin/storage/es/mappings" esSpanStore "github.com/jaegertracing/jaeger/plugin/storage/es/spanstore" - "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -133,11 +130,6 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { return createSpanWriter(f.metricsFactory, f.logger, f.archiveClient, f.archiveConfig, true) } -// CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, storage.ErrLockAndSamplingStoreNotSupported -} - func createSpanReader( mFactory metrics.Factory, logger *zap.Logger, diff --git a/plugin/storage/es/factory_test.go b/plugin/storage/es/factory_test.go index bec51916bec..525dc9a29dc 100644 --- a/plugin/storage/es/factory_test.go +++ b/plugin/storage/es/factory_test.go @@ -87,9 +87,6 @@ func TestElasticsearchFactory(t *testing.T) { _, err = f.CreateArchiveSpanWriter() assert.NoError(t, err) assert.NoError(t, f.Close()) - - _, _, err = f.CreateLockAndSamplingStore() - assert.Equal(t, storage.ErrLockAndSamplingStoreNotSupported, err) } func TestElasticsearchTagsFileDoNotExist(t *testing.T) { @@ -221,11 +218,3 @@ func TestInitFromOptions(t *testing.T) { assert.Equal(t, o.GetPrimary(), f.primaryConfig) assert.Equal(t, o.Get(archiveNamespace), f.archiveConfig) } - -func TestCreateLockAndSamplingStore(t *testing.T) { - f := NewFactory() - lock, ss, err := f.CreateLockAndSamplingStore() - assert.Nil(t, lock) - assert.Nil(t, ss) - assert.Equal(t, err, storage.ErrLockAndSamplingStoreNotSupported) -} diff --git a/plugin/storage/factory.go b/plugin/storage/factory.go index 90ba2f339a0..61175401f3a 100644 --- a/plugin/storage/factory.go +++ b/plugin/storage/factory.go @@ -24,7 +24,6 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/multierror" "github.com/jaegertracing/jaeger/plugin" "github.com/jaegertracing/jaeger/plugin/storage/badger" @@ -35,7 +34,6 @@ import ( "github.com/jaegertracing/jaeger/plugin/storage/memory" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -161,21 +159,17 @@ func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { } // CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { +func (f *Factory) CreateSamplingStoreFactory() (storage.SamplingStoreFactory, error) { for _, factory := range f.factories { - lock, store, err := factory.CreateLockAndSamplingStore() - if err != nil && err != storage.ErrLockAndSamplingStoreNotSupported { - return nil, nil, err - } - // returns the first backend that can support adaptive sampling - if lock != nil && store != nil { - return lock, store, nil + ss, ok := factory.(storage.SamplingStoreFactory) + if ok { + return ss, nil } } // returning nothing is valid here. it's quite possible that the user has no backend that can support adaptive sampling // this is fine as long as adaptive sampling is also not configured - return nil, nil, nil + return nil, nil } // CreateDependencyReader implements storage.Factory diff --git a/plugin/storage/factory_test.go b/plugin/storage/factory_test.go index 5d72e3db0e9..b21b19be5df 100644 --- a/plugin/storage/factory_test.go +++ b/plugin/storage/factory_test.go @@ -35,13 +35,11 @@ import ( "github.com/jaegertracing/jaeger/pkg/config" "github.com/jaegertracing/jaeger/pkg/distributedlock" - lmocks "github.com/jaegertracing/jaeger/pkg/distributedlock/mocks" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" depStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" "github.com/jaegertracing/jaeger/storage/mocks" "github.com/jaegertracing/jaeger/storage/samplingstore" - ssmocks "github.com/jaegertracing/jaeger/storage/samplingstore/mocks" "github.com/jaegertracing/jaeger/storage/spanstore" spanStoreMocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) @@ -298,13 +296,6 @@ func TestCreateError(t *testing.T) { assert.Nil(t, w) assert.EqualError(t, err, expectedErr) } - - { - l, ss, err := f.CreateLockAndSamplingStore() - assert.Nil(t, l) - assert.Nil(t, ss) - assert.NoError(t, err) // no supporting backend is valid - } } type configurable struct { @@ -403,50 +394,6 @@ func TestPublishOpts(t *testing.T) { }) } -func TestCreateLockAndSamplingStore(t *testing.T) { - supports := &errorFactory{ - lock: &lmocks.Lock{}, - store: &ssmocks.Store{}, - } - doesNotSupport := &errorFactory{ - lockAndSamplingStore: storage.ErrLockAndSamplingStoreNotSupported, - } - breaks := &errorFactory{ - lockAndSamplingStore: errors.New("wups"), - } - - // test succeeds - f := Factory{ - factories: map[string]storage.Factory{ - "supports": supports, - "does-not-support": doesNotSupport, - }, - } - - // finds factory that supports - lock, samplingStore, err := f.CreateLockAndSamplingStore() - assert.NoError(t, err) - assert.Equal(t, supports.lock, lock) - assert.Equal(t, supports.store, samplingStore) - - // does not find factory that supports - delete(f.factories, "supports") - lock, samplingStore, err = f.CreateLockAndSamplingStore() - assert.NoError(t, err) - assert.Nil(t, lock) - assert.Nil(t, samplingStore) - - // test breaks - f = Factory{ - factories: map[string]storage.Factory{ - "breaks": breaks, - "does-not-support": doesNotSupport, - }, - } - _, _, err = f.CreateLockAndSamplingStore() - assert.Error(t, err) -} - type errorFactory struct { closeErr error lockAndSamplingStore error @@ -474,10 +421,6 @@ func (e errorFactory) CreateDependencyReader() (dependencystore.Reader, error) { panic("implement me") } -func (e errorFactory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return e.lock, e.store, e.lockAndSamplingStore -} - func (e errorFactory) Close() error { return e.closeErr } diff --git a/plugin/storage/grpc/factory.go b/plugin/storage/grpc/factory.go index 40f24c65e4d..bb5e3a89e3a 100644 --- a/plugin/storage/grpc/factory.go +++ b/plugin/storage/grpc/factory.go @@ -22,12 +22,10 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/plugin/storage/grpc/config" "github.com/jaegertracing/jaeger/plugin/storage/grpc/shared" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -126,8 +124,3 @@ func (f *Factory) CreateArchiveSpanWriter() (spanstore.Writer, error) { } return f.archiveStore.ArchiveSpanWriter(), nil } - -// CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, storage.ErrLockAndSamplingStoreNotSupported -} diff --git a/plugin/storage/grpc/factory_test.go b/plugin/storage/grpc/factory_test.go index bb88a36f901..5ed7a6b9e2e 100644 --- a/plugin/storage/grpc/factory_test.go +++ b/plugin/storage/grpc/factory_test.go @@ -129,8 +129,6 @@ func TestGRPCStorageFactory(t *testing.T) { depReader, err := f.CreateDependencyReader() assert.NoError(t, err) assert.Equal(t, f.store.DependencyReader(), depReader) - _, _, err = f.CreateLockAndSamplingStore() - assert.Equal(t, storage.ErrLockAndSamplingStoreNotSupported, err) } func TestGRPCStorageFactory_Capabilities(t *testing.T) { @@ -269,11 +267,3 @@ func TestInitFromOptions(t *testing.T) { assert.Equal(t, o, f.options) assert.Equal(t, &o.Configuration, f.builder) } - -func TestCreateLockAndSamplingStore(t *testing.T) { - f := NewFactory() - lock, ss, err := f.CreateLockAndSamplingStore() - assert.Nil(t, lock) - assert.Nil(t, ss) - assert.Equal(t, err, storage.ErrLockAndSamplingStoreNotSupported) -} diff --git a/plugin/storage/kafka/factory.go b/plugin/storage/kafka/factory.go index 2a5488a566a..6fd037c1c08 100644 --- a/plugin/storage/kafka/factory.go +++ b/plugin/storage/kafka/factory.go @@ -24,11 +24,8 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/pkg/kafka/producer" - "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -103,11 +100,6 @@ func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { return nil, errors.New("kafka storage is write-only") } -// CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, storage.ErrLockAndSamplingStoreNotSupported -} - var _ io.Closer = (*Factory)(nil) // Close closes the resources held by the factory diff --git a/plugin/storage/kafka/factory_test.go b/plugin/storage/kafka/factory_test.go index dc7139b0dd0..edb356cb665 100644 --- a/plugin/storage/kafka/factory_test.go +++ b/plugin/storage/kafka/factory_test.go @@ -73,9 +73,6 @@ func TestKafkaFactory(t *testing.T) { _, err = f.CreateDependencyReader() assert.Error(t, err) - _, _, err = f.CreateLockAndSamplingStore() - assert.Equal(t, storage.ErrLockAndSamplingStoreNotSupported, err) - assert.NoError(t, f.Close()) } @@ -171,11 +168,3 @@ func TestInitFromOptions(t *testing.T) { assert.Equal(t, o, f.options) assert.Equal(t, &o.Config, f.Builder) } - -func TestCreateLockAndSamplingStore(t *testing.T) { - f := NewFactory() - lock, ss, err := f.CreateLockAndSamplingStore() - assert.Nil(t, lock) - assert.Nil(t, ss) - assert.Equal(t, err, storage.ErrLockAndSamplingStoreNotSupported) -} diff --git a/plugin/storage/memory/factory.go b/plugin/storage/memory/factory.go index 30a4dad069f..c94fbfa6c50 100644 --- a/plugin/storage/memory/factory.go +++ b/plugin/storage/memory/factory.go @@ -22,10 +22,7 @@ import ( "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/distributedlock" - "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -82,11 +79,6 @@ func (f *Factory) CreateDependencyReader() (dependencystore.Reader, error) { return f.store, nil } -// CreateLockAndSamplingStore implements storage.ArchiveFactory. It is not supported by this storage plugin. -func (f *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, storage.ErrLockAndSamplingStoreNotSupported -} - func (f *Factory) publishOpts() { internalFactory := f.metricsFactory.Namespace(metrics.NSOptions{Name: "internal"}) internalFactory.Gauge(metrics.Options{Name: limit}). diff --git a/plugin/storage/memory/factory_test.go b/plugin/storage/memory/factory_test.go index f1713025a84..729c05ff0fd 100644 --- a/plugin/storage/memory/factory_test.go +++ b/plugin/storage/memory/factory_test.go @@ -44,8 +44,6 @@ func TestMemoryStorageFactory(t *testing.T) { depReader, err := f.CreateDependencyReader() assert.NoError(t, err) assert.Equal(t, f.store, depReader) - _, _, err = f.CreateLockAndSamplingStore() - assert.Equal(t, storage.ErrLockAndSamplingStoreNotSupported, err) } func TestWithConfiguration(t *testing.T) { @@ -79,11 +77,3 @@ func TestPublishOpts(t *testing.T) { Value: f.options.Configuration.MaxTraces, }) } - -func TestCreateLockAndSamplingStore(t *testing.T) { - f := NewFactory() - lock, ss, err := f.CreateLockAndSamplingStore() - assert.Nil(t, lock) - assert.Nil(t, ss) - assert.Equal(t, err, storage.ErrLockAndSamplingStoreNotSupported) -} diff --git a/storage/factory.go b/storage/factory.go index 700d19154e8..8c415aea48f 100644 --- a/storage/factory.go +++ b/storage/factory.go @@ -47,10 +47,15 @@ type Factory interface { // CreateDependencyReader creates a dependencystore.Reader. CreateDependencyReader() (dependencystore.Reader, error) +} - // CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store. - // These are used for adaptive sampling. - CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) +// SamplingStoreFactory defines an interface that is capable of returning the necessary backends for +// adaptive sampling. +type SamplingStoreFactory interface { + // CreateLock creates a distributed lock. + CreateLock() (distributedlock.Lock, error) + // CreateSamplingStore creates a sampling store. + CreateSamplingStore() (samplingstore.Store, error) } var ( @@ -59,9 +64,6 @@ var ( // ErrArchiveStorageNotSupported can be returned by the ArchiveFactory when the archive storage is not supported by the backend. ErrArchiveStorageNotSupported = errors.New("archive storage not supported") - - // ErrLockAndSamplingStoreNotSupported can be returned by the StorageFactory when the sampling storage is not supported by the backend. - ErrLockAndSamplingStoreNotSupported = errors.New("lock/sampling storage not supported") ) // ArchiveFactory is an additional interface that can be implemented by a factory to support trace archiving. diff --git a/storage/mocks/Factory.go b/storage/mocks/Factory.go index 0b05495d1a9..901f754626d 100644 --- a/storage/mocks/Factory.go +++ b/storage/mocks/Factory.go @@ -19,10 +19,8 @@ import ( metrics "github.com/uber/jaeger-lib/metrics" zap "go.uber.org/zap" - "github.com/jaegertracing/jaeger/pkg/distributedlock" storage "github.com/jaegertracing/jaeger/storage" dependencystore "github.com/jaegertracing/jaeger/storage/dependencystore" - "github.com/jaegertracing/jaeger/storage/samplingstore" spanstore "github.com/jaegertracing/jaeger/storage/spanstore" ) @@ -100,11 +98,6 @@ func (_m *Factory) CreateSpanWriter() (spanstore.Writer, error) { return r0, r1 } -// CreateLockAndSamplingStore is not supproted -func (_m *Factory) CreateLockAndSamplingStore() (distributedlock.Lock, samplingstore.Store, error) { - return nil, nil, storage.ErrLockAndSamplingStoreNotSupported -} - // Initialize provides a mock function with given fields: metricsFactory, logger func (_m *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { ret := _m.Called(metricsFactory, logger) From 7fcacad8915ce8a8cf0ec3ada6da002901027296 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 20 Jul 2021 15:02:27 -0400 Subject: [PATCH 38/40] lint Signed-off-by: Joe Elliott --- plugin/storage/factory.go | 2 +- plugin/storage/factory_test.go | 8 +------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/plugin/storage/factory.go b/plugin/storage/factory.go index 61175401f3a..8d00e409059 100644 --- a/plugin/storage/factory.go +++ b/plugin/storage/factory.go @@ -158,7 +158,7 @@ func (f *Factory) CreateSpanWriter() (spanstore.Writer, error) { }), nil } -// CreateLockAndSamplingStore creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling +// CreateSamplingStoreFactory creates a distributedlock.Lock and samplingstore.Store for use with adaptive sampling func (f *Factory) CreateSamplingStoreFactory() (storage.SamplingStoreFactory, error) { for _, factory := range f.factories { ss, ok := factory.(storage.SamplingStoreFactory) diff --git a/plugin/storage/factory_test.go b/plugin/storage/factory_test.go index b21b19be5df..0c77fac1cdf 100644 --- a/plugin/storage/factory_test.go +++ b/plugin/storage/factory_test.go @@ -34,12 +34,10 @@ import ( "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/config" - "github.com/jaegertracing/jaeger/pkg/distributedlock" "github.com/jaegertracing/jaeger/storage" "github.com/jaegertracing/jaeger/storage/dependencystore" depStoreMocks "github.com/jaegertracing/jaeger/storage/dependencystore/mocks" "github.com/jaegertracing/jaeger/storage/mocks" - "github.com/jaegertracing/jaeger/storage/samplingstore" "github.com/jaegertracing/jaeger/storage/spanstore" spanStoreMocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) @@ -395,11 +393,7 @@ func TestPublishOpts(t *testing.T) { } type errorFactory struct { - closeErr error - lockAndSamplingStore error - - lock distributedlock.Lock - store samplingstore.Store + closeErr error } var _ storage.Factory = (*errorFactory)(nil) From ffb4f84ba809008cdead9e964fc0cefd30b52d3d Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 20 Jul 2021 16:51:28 -0400 Subject: [PATCH 39/40] tests Signed-off-by: Joe Elliott --- .../strategystore/adaptive/factory_test.go | 24 +++++++++++++++++-- plugin/storage/factory_test.go | 16 +++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/plugin/sampling/strategystore/adaptive/factory_test.go b/plugin/sampling/strategystore/adaptive/factory_test.go index 008d9516bf2..bf757e13aaf 100644 --- a/plugin/sampling/strategystore/adaptive/factory_test.go +++ b/plugin/sampling/strategystore/adaptive/factory_test.go @@ -15,6 +15,7 @@ package adaptive import ( + "errors" "testing" "time" @@ -94,20 +95,39 @@ func TestBadConfigFail(t *testing.T) { } } -func TestNilSamplingStoreFactoryFails(t *testing.T) { +func TestSamplingStoreFactoryFails(t *testing.T) { f := NewFactory() + + // nil fails assert.Error(t, f.Initialize(metrics.NullFactory, nil, zap.NewNop())) + + // fail if lock fails + assert.Error(t, f.Initialize(metrics.NullFactory, &mockSamplingStoreFactory{lockFailsWith: errors.New("fail")}, zap.NewNop())) + + // fail if store fails + assert.Error(t, f.Initialize(metrics.NullFactory, &mockSamplingStoreFactory{storeFailsWith: errors.New("fail")}, zap.NewNop())) } -type mockSamplingStoreFactory struct{} +type mockSamplingStoreFactory struct { + lockFailsWith error + storeFailsWith error +} func (m *mockSamplingStoreFactory) CreateLock() (distributedlock.Lock, error) { + if m.lockFailsWith != nil { + return nil, m.lockFailsWith + } + mockLock := &lmocks.Lock{} mockLock.On("Acquire", mock.Anything, mock.Anything).Return(true, nil) return mockLock, nil } func (m *mockSamplingStoreFactory) CreateSamplingStore() (samplingstore.Store, error) { + if m.storeFailsWith != nil { + return nil, m.storeFailsWith + } + mockStorage := &smocks.Store{} mockStorage.On("GetLatestProbabilities").Return(make(model.ServiceOperationProbabilities), nil) mockStorage.On("GetThroughput", mock.AnythingOfType("time.Time"), mock.AnythingOfType("time.Time")). diff --git a/plugin/storage/factory_test.go b/plugin/storage/factory_test.go index 0c77fac1cdf..b2762d7c1a0 100644 --- a/plugin/storage/factory_test.go +++ b/plugin/storage/factory_test.go @@ -296,6 +296,22 @@ func TestCreateError(t *testing.T) { } } +func CreateSamplingStoreFactory(t *testing.T) { + f, err := NewFactory(defaultCfg()) + require.NoError(t, err) + assert.NotEmpty(t, f.factories) + assert.NotEmpty(t, f.factories[cassandraStorageType]) + + ssFactory, err := f.CreateSamplingStoreFactory() + assert.Equal(t, f.factories[cassandraStorageType], ssFactory) + assert.NoError(t, err) + + delete(f.factories, cassandraStorageType) + ssFactory, err = f.CreateSamplingStoreFactory() + assert.Nil(t, ssFactory) + assert.NoError(t, err) +} + type configurable struct { mocks.Factory flagSet *flag.FlagSet From 86f42d860193e3ed16d142961068ac416b9ec761 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Wed, 8 Sep 2021 11:47:55 -0400 Subject: [PATCH 40/40] Move changelog entry Signed-off-by: Yuri Shkuro --- CHANGELOG.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f348803057e..5afdc42985b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ Changes by Version ================== +next release +------------------- +### Backend Changes +#### New Features +* Add support for adaptive sampling with a Cassandra backend. ([#2966](https://github.com/jaegertracing/jaeger/pull/2966), [@joe-elliott](https://github.com/joe-elliott)) + + 1.26.0 (2021-09-06) ------------------- ### Backend Changes @@ -72,8 +79,6 @@ Changes by Version #### New Features -* Add support for adaptive sampling with a Cassandra backend. ([#2966](https://github.com/jaegertracing/jaeger/pull/2966), [@joe-elliott](https://github.com/joe-elliott)): - #### Breaking Changes * Remove unused `--es-archive.max-span-age` flag ([#2865](https://github.com/jaegertracing/jaeger/pull/2865), [@albertteoh](https://github.com/albertteoh)):