From f60bd09754f835f39d2577f2138b6b3a82bf493e Mon Sep 17 00:00:00 2001 From: Aryan Goyal <137564277+ary82@users.noreply.github.com> Date: Mon, 13 Jan 2025 03:20:57 +0530 Subject: [PATCH] [refactor] Move cmd/collector/app/sampling/model types to storage (#6531) ## Which problem is this PR solving? - Towards #6411 ## Description of the changes - Involves refactoring the `model` package in `cmd/collector/app/sampling` - Move specific `plugin/storage/cassandra/samplingstore` types to that package as internal types - Move the other types to `storage/samplingstore/model` ## How was this change tested? - Covered by existing ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` Signed-off-by: Aryan Goyal <137564277+ary82@users.noreply.github.com> Co-authored-by: Yuri Shkuro --- .../strategyprovider/adaptive/aggregator.go | 2 +- .../strategyprovider/adaptive/factory_test.go | 2 +- .../adaptive/post_aggregator.go | 2 +- .../adaptive/post_aggregator_test.go | 2 +- .../strategyprovider/adaptive/provider.go | 2 +- .../adaptive/provider_test.go | 2 +- .../storage/badger/samplingstore/storage.go | 2 +- .../badger/samplingstore/storage_test.go | 2 +- .../cassandra/samplingstore/storage.go | 22 ++++++++++++++----- .../cassandra/samplingstore/storage_test.go | 6 ++--- .../es/samplingstore/dbmodel/converter.go | 2 +- .../samplingstore/dbmodel/converter_test.go | 2 +- plugin/storage/es/samplingstore/storage.go | 2 +- .../storage/es/samplingstore/storage_test.go | 2 +- plugin/storage/integration/integration.go | 2 +- plugin/storage/memory/sampling.go | 2 +- plugin/storage/memory/sampling_test.go | 2 +- storage/samplingstore/interface.go | 2 +- storage/samplingstore/mocks/Store.go | 2 +- .../samplingstore}/model/empty_test.go | 0 .../samplingstore}/model/sampling.go | 10 --------- 21 files changed, 36 insertions(+), 36 deletions(-) rename {cmd/collector/app/sampling => storage/samplingstore}/model/empty_test.go (100%) rename {cmd/collector/app/sampling => storage/samplingstore}/model/sampling.go (64%) diff --git a/plugin/sampling/strategyprovider/adaptive/aggregator.go b/plugin/sampling/strategyprovider/adaptive/aggregator.go index e08ce275017..2389ec9254e 100644 --- a/plugin/sampling/strategyprovider/adaptive/aggregator.go +++ b/plugin/sampling/strategyprovider/adaptive/aggregator.go @@ -9,13 +9,13 @@ import ( "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/samplingstrategy" span_model "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/hostname" "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection" "github.com/jaegertracing/jaeger/storage/samplingstore" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) const ( diff --git a/plugin/sampling/strategyprovider/adaptive/factory_test.go b/plugin/sampling/strategyprovider/adaptive/factory_test.go index f6f795913de..b008cc5fabb 100644 --- a/plugin/sampling/strategyprovider/adaptive/factory_test.go +++ b/plugin/sampling/strategyprovider/adaptive/factory_test.go @@ -13,7 +13,6 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" ss "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/samplingstrategy" "github.com/jaegertracing/jaeger/pkg/config" "github.com/jaegertracing/jaeger/pkg/distributedlock" @@ -22,6 +21,7 @@ import ( "github.com/jaegertracing/jaeger/plugin" "github.com/jaegertracing/jaeger/storage/samplingstore" smocks "github.com/jaegertracing/jaeger/storage/samplingstore/mocks" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) var ( diff --git a/plugin/sampling/strategyprovider/adaptive/post_aggregator.go b/plugin/sampling/strategyprovider/adaptive/post_aggregator.go index 407704892b2..048b3367a95 100644 --- a/plugin/sampling/strategyprovider/adaptive/post_aggregator.go +++ b/plugin/sampling/strategyprovider/adaptive/post_aggregator.go @@ -12,11 +12,11 @@ import ( "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection" "github.com/jaegertracing/jaeger/plugin/sampling/strategyprovider/adaptive/calculationstrategy" "github.com/jaegertracing/jaeger/storage/samplingstore" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) const ( diff --git a/plugin/sampling/strategyprovider/adaptive/post_aggregator_test.go b/plugin/sampling/strategyprovider/adaptive/post_aggregator_test.go index fd0da089ac6..1b83ceea27f 100644 --- a/plugin/sampling/strategyprovider/adaptive/post_aggregator_test.go +++ b/plugin/sampling/strategyprovider/adaptive/post_aggregator_test.go @@ -14,13 +14,13 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/internal/metricstest" "github.com/jaegertracing/jaeger/pkg/metrics" "github.com/jaegertracing/jaeger/pkg/testutils" epmocks "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection/mocks" "github.com/jaegertracing/jaeger/plugin/sampling/strategyprovider/adaptive/calculationstrategy" smocks "github.com/jaegertracing/jaeger/storage/samplingstore/mocks" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) func testThroughputs() []*model.Throughput { diff --git a/plugin/sampling/strategyprovider/adaptive/provider.go b/plugin/sampling/strategyprovider/adaptive/provider.go index dfc7ba29b18..bc04184f083 100644 --- a/plugin/sampling/strategyprovider/adaptive/provider.go +++ b/plugin/sampling/strategyprovider/adaptive/provider.go @@ -10,10 +10,10 @@ import ( "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection" "github.com/jaegertracing/jaeger/proto-gen/api_v2" "github.com/jaegertracing/jaeger/storage/samplingstore" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) const defaultFollowerProbabilityInterval = 20 * time.Second diff --git a/plugin/sampling/strategyprovider/adaptive/provider_test.go b/plugin/sampling/strategyprovider/adaptive/provider_test.go index 1342cd21790..8f0d83a640d 100644 --- a/plugin/sampling/strategyprovider/adaptive/provider_test.go +++ b/plugin/sampling/strategyprovider/adaptive/provider_test.go @@ -14,10 +14,10 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" epmocks "github.com/jaegertracing/jaeger/plugin/sampling/leaderelection/mocks" "github.com/jaegertracing/jaeger/proto-gen/api_v2" smocks "github.com/jaegertracing/jaeger/storage/samplingstore/mocks" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) func TestProviderLoadProbabilities(t *testing.T) { diff --git a/plugin/storage/badger/samplingstore/storage.go b/plugin/storage/badger/samplingstore/storage.go index 50c8f19f412..d0ef85b0a6e 100644 --- a/plugin/storage/badger/samplingstore/storage.go +++ b/plugin/storage/badger/samplingstore/storage.go @@ -11,8 +11,8 @@ import ( "github.com/dgraph-io/badger/v4" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" jaegermodel "github.com/jaegertracing/jaeger/model" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) const ( diff --git a/plugin/storage/badger/samplingstore/storage_test.go b/plugin/storage/badger/samplingstore/storage_test.go index 5a02dc0e802..cbb125d4daa 100644 --- a/plugin/storage/badger/samplingstore/storage_test.go +++ b/plugin/storage/badger/samplingstore/storage_test.go @@ -12,8 +12,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - samplemodel "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/pkg/testutils" + samplemodel "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) func newTestSamplingStore(db *badger.DB) *SamplingStore { diff --git a/plugin/storage/cassandra/samplingstore/storage.go b/plugin/storage/cassandra/samplingstore/storage.go index d3abeb31fdb..55434ba3ff0 100644 --- a/plugin/storage/cassandra/samplingstore/storage.go +++ b/plugin/storage/cassandra/samplingstore/storage.go @@ -17,10 +17,10 @@ import ( "github.com/gocql/gocql" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/pkg/cassandra" casMetrics "github.com/jaegertracing/jaeger/pkg/cassandra/metrics" "github.com/jaegertracing/jaeger/pkg/metrics" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) const ( @@ -34,6 +34,16 @@ const ( getLatestProbabilities = `SELECT probabilities FROM sampling_probabilities WHERE bucket = ` + constBucketStr + ` LIMIT 1` ) +// probabilityAndQPS contains the sampling probability and measured qps for an operation. +type probabilityAndQPS struct { + Probability float64 + QPS float64 +} + +// serviceOperationData contains the sampling probabilities and measured qps for all operations in a service. +// ie [service][operation] = ProbabilityAndQPS +type serviceOperationData map[string]map[string]*probabilityAndQPS + type samplingStoreMetrics struct { operationThroughput *casMetrics.Table probabilities *casMetrics.Table @@ -127,8 +137,8 @@ func probabilitiesAndQPSToString(probabilities model.ServiceOperationProbabiliti return buf.String() } -func (s *SamplingStore) stringToProbabilitiesAndQPS(probabilitiesAndQPSStr string) model.ServiceOperationData { - probabilitiesAndQPS := make(model.ServiceOperationData) +func (s *SamplingStore) stringToProbabilitiesAndQPS(probabilitiesAndQPSStr string) serviceOperationData { + probabilitiesAndQPS := make(serviceOperationData) appendFunc := s.appendProbabilityAndQPS(probabilitiesAndQPS) s.parseString(probabilitiesAndQPSStr, 4, appendFunc) return probabilitiesAndQPS @@ -167,7 +177,7 @@ func (s *SamplingStore) stringToThroughput(throughputStr string) []*model.Throug return throughput } -func (s *SamplingStore) appendProbabilityAndQPS(svcOpData model.ServiceOperationData) func(csvFields []string) { +func (s *SamplingStore) appendProbabilityAndQPS(svcOpData serviceOperationData) func(csvFields []string) { return func(csvFields []string) { probability, err := strconv.ParseFloat(csvFields[2], 64) if err != nil { @@ -182,9 +192,9 @@ func (s *SamplingStore) appendProbabilityAndQPS(svcOpData model.ServiceOperation service := csvFields[0] operation := csvFields[1] if _, ok := svcOpData[service]; !ok { - svcOpData[service] = make(map[string]*model.ProbabilityAndQPS) + svcOpData[service] = make(map[string]*probabilityAndQPS) } - svcOpData[service][operation] = &model.ProbabilityAndQPS{ + svcOpData[service][operation] = &probabilityAndQPS{ Probability: probability, QPS: qps, } diff --git a/plugin/storage/cassandra/samplingstore/storage_test.go b/plugin/storage/cassandra/samplingstore/storage_test.go index 774ffeb5422..094e3eaef82 100644 --- a/plugin/storage/cassandra/samplingstore/storage_test.go +++ b/plugin/storage/cassandra/samplingstore/storage_test.go @@ -16,11 +16,11 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/internal/metricstest" "github.com/jaegertracing/jaeger/pkg/cassandra/mocks" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/storage/samplingstore" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) var testTime = time.Date(2017, time.January, 24, 11, 15, 17, 12345, time.UTC) @@ -348,7 +348,7 @@ func TestStringToProbabilitiesAndQPS(t *testing.T) { probabilities := s.stringToProbabilitiesAndQPS(testStr) assert.Len(t, probabilities, 2) - assert.Equal(t, map[string]*model.ProbabilityAndQPS{ + assert.Equal(t, map[string]*probabilityAndQPS{ http.MethodGet: { Probability: 0.001, QPS: 63.2, @@ -358,7 +358,7 @@ func TestStringToProbabilitiesAndQPS(t *testing.T) { QPS: 0.0, }, }, probabilities["svc1"]) - assert.Equal(t, map[string]*model.ProbabilityAndQPS{ + assert.Equal(t, map[string]*probabilityAndQPS{ http.MethodGet: { Probability: 0.5, QPS: 34.2, diff --git a/plugin/storage/es/samplingstore/dbmodel/converter.go b/plugin/storage/es/samplingstore/dbmodel/converter.go index e5166e45c61..4eccc8b2ba3 100644 --- a/plugin/storage/es/samplingstore/dbmodel/converter.go +++ b/plugin/storage/es/samplingstore/dbmodel/converter.go @@ -4,7 +4,7 @@ package dbmodel import ( - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) func FromThroughputs(throughputs []*model.Throughput) []Throughput { diff --git a/plugin/storage/es/samplingstore/dbmodel/converter_test.go b/plugin/storage/es/samplingstore/dbmodel/converter_test.go index 82e84eacc4a..86043e1c577 100644 --- a/plugin/storage/es/samplingstore/dbmodel/converter_test.go +++ b/plugin/storage/es/samplingstore/dbmodel/converter_test.go @@ -9,8 +9,8 @@ import ( "github.com/stretchr/testify/assert" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/pkg/testutils" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) func TestConvertDependencies(t *testing.T) { diff --git a/plugin/storage/es/samplingstore/storage.go b/plugin/storage/es/samplingstore/storage.go index 0fd2d0db0c2..db6c960c69c 100644 --- a/plugin/storage/es/samplingstore/storage.go +++ b/plugin/storage/es/samplingstore/storage.go @@ -13,10 +13,10 @@ import ( "github.com/olivere/elastic" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/pkg/es" "github.com/jaegertracing/jaeger/pkg/es/config" "github.com/jaegertracing/jaeger/plugin/storage/es/samplingstore/dbmodel" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) const ( diff --git a/plugin/storage/es/samplingstore/storage_test.go b/plugin/storage/es/samplingstore/storage_test.go index 0d2764bb2cd..be821db11cc 100644 --- a/plugin/storage/es/samplingstore/storage_test.go +++ b/plugin/storage/es/samplingstore/storage_test.go @@ -16,12 +16,12 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" - samplemodel "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/pkg/es" "github.com/jaegertracing/jaeger/pkg/es/config" "github.com/jaegertracing/jaeger/pkg/es/mocks" "github.com/jaegertracing/jaeger/pkg/testutils" "github.com/jaegertracing/jaeger/plugin/storage/es/samplingstore/dbmodel" + samplemodel "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) const defaultMaxDocCount = 10_000 diff --git a/plugin/storage/integration/integration.go b/plugin/storage/integration/integration.go index f3132c41e21..d5998ac806f 100644 --- a/plugin/storage/integration/integration.go +++ b/plugin/storage/integration/integration.go @@ -22,10 +22,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - samplemodel "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/storage/dependencystore" "github.com/jaegertracing/jaeger/storage/samplingstore" + samplemodel "github.com/jaegertracing/jaeger/storage/samplingstore/model" "github.com/jaegertracing/jaeger/storage/spanstore" "github.com/jaegertracing/jaeger/storage_v2/depstore" "github.com/jaegertracing/jaeger/storage_v2/tracestore" diff --git a/plugin/storage/memory/sampling.go b/plugin/storage/memory/sampling.go index db603ecbc1a..b58e5c9487a 100644 --- a/plugin/storage/memory/sampling.go +++ b/plugin/storage/memory/sampling.go @@ -7,7 +7,7 @@ import ( "sync" "time" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) // SamplingStore is an in-memory store for sampling data diff --git a/plugin/storage/memory/sampling_test.go b/plugin/storage/memory/sampling_test.go index c1914109741..825e23ebad9 100644 --- a/plugin/storage/memory/sampling_test.go +++ b/plugin/storage/memory/sampling_test.go @@ -11,7 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) func withPopulatedSamplingStore(f func(samplingStore *SamplingStore)) { diff --git a/storage/samplingstore/interface.go b/storage/samplingstore/interface.go index ba2dbdf3cd8..130ad0bf2b8 100644 --- a/storage/samplingstore/interface.go +++ b/storage/samplingstore/interface.go @@ -7,7 +7,7 @@ package samplingstore import ( "time" - "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" + "github.com/jaegertracing/jaeger/storage/samplingstore/model" ) // Store writes and retrieves sampling data to and from storage. diff --git a/storage/samplingstore/mocks/Store.go b/storage/samplingstore/mocks/Store.go index 120782e1e81..b6194c2099b 100644 --- a/storage/samplingstore/mocks/Store.go +++ b/storage/samplingstore/mocks/Store.go @@ -8,7 +8,7 @@ package mocks import ( - model "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/model" + model "github.com/jaegertracing/jaeger/storage/samplingstore/model" mock "github.com/stretchr/testify/mock" time "time" diff --git a/cmd/collector/app/sampling/model/empty_test.go b/storage/samplingstore/model/empty_test.go similarity index 100% rename from cmd/collector/app/sampling/model/empty_test.go rename to storage/samplingstore/model/empty_test.go diff --git a/cmd/collector/app/sampling/model/sampling.go b/storage/samplingstore/model/sampling.go similarity index 64% rename from cmd/collector/app/sampling/model/sampling.go rename to storage/samplingstore/model/sampling.go index 9504506756c..77def29e945 100644 --- a/cmd/collector/app/sampling/model/sampling.go +++ b/storage/samplingstore/model/sampling.go @@ -19,13 +19,3 @@ type ServiceOperationProbabilities map[string]map[string]float64 // ServiceOperationQPS contains the qps for all operations in a service. // ie [service][operation] = qps type ServiceOperationQPS map[string]map[string]float64 - -// ProbabilityAndQPS contains the sampling probability and measured qps for an operation. -type ProbabilityAndQPS struct { - Probability float64 - QPS float64 -} - -// ServiceOperationData contains the sampling probabilities and measured qps for all operations in a service. -// ie [service][operation] = ProbabilityAndQPS -type ServiceOperationData map[string]map[string]*ProbabilityAndQPS