From a2b20efcd8c10a4987ca7c65a9d6ff25cbe6098e Mon Sep 17 00:00:00 2001 From: Mohamed Osman Date: Wed, 26 Oct 2022 12:20:01 -0400 Subject: [PATCH] add smaller interfaces local to packages using them --- apis/v1alpha1/opentelemetrycollector_types.go | 4 ++-- ...opentelemetry.io_opentelemetrycollectors.yaml | 6 +++--- .../allocation/consistent_hashing.go | 16 ++++------------ cmd/otel-allocator/allocation/least_weighted.go | 16 ++++------------ cmd/otel-allocator/allocation/strategy.go | 11 +++++++---- cmd/otel-allocator/discovery/discovery.go | 9 ++++++--- cmd/otel-allocator/prehook/prehook.go | 10 +++++----- cmd/otel-allocator/prehook/relabel.go | 13 ------------- ...opentelemetry.io_opentelemetrycollectors.yaml | 6 +++--- docs/api.md | 2 +- pkg/collector/reconcile/configmap.go | 3 +-- pkg/collector/reconcile/configmap_test.go | 2 -- 12 files changed, 36 insertions(+), 62 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 400687ef34..d16abab40f 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -160,8 +160,8 @@ type OpenTelemetryTargetAllocator struct { // +optional AllocationStrategy string `json:"allocationStrategy,omitempty"` // FilterStrategy determines how to filter targets before allocating them among the collectors. - // The current options are no-op (no filtering) and relabel-config (drops targets based on prom relabel_config) - // The default is no-op + // The only current option is relabel-config (drops targets based on prom relabel_config). + // Filtering is disabled by default. // +optional FilterStrategy string `json:"filterStrategy,omitempty"` // ServiceAccount indicates the name of an existing service account to use with this instance. diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index af3b9afe97..fcdfed60de 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -1703,9 +1703,9 @@ spec: type: boolean filterStrategy: description: FilterStrategy determines how to filter targets before - allocating them among the collectors. The current options are - no-op (no filtering) and relabel-config (drops targets based - on prom relabel_config) The default is no-op + allocating them among the collectors. The only current option + is relabel-config (drops targets based on prom relabel_config). + Filtering is disabled by default. type: string image: description: Image indicates the container image to use for the diff --git a/cmd/otel-allocator/allocation/consistent_hashing.go b/cmd/otel-allocator/allocation/consistent_hashing.go index e5977f5264..94c1ce4ea5 100644 --- a/cmd/otel-allocator/allocation/consistent_hashing.go +++ b/cmd/otel-allocator/allocation/consistent_hashing.go @@ -53,7 +53,7 @@ type consistentHashingAllocator struct { log logr.Logger - filterFunction prehook.Hook + filterFunction Filter } func newConsistentHashingAllocator(log logr.Logger, opts ...AllocOption) Allocator { @@ -78,8 +78,8 @@ func newConsistentHashingAllocator(log logr.Logger, opts ...AllocOption) Allocat } // SetHook sets the filtering hook to use. -func (c *consistentHashingAllocator) SetHook(hook prehook.Hook) { - c.filterFunction = hook +func (c *consistentHashingAllocator) SetFilter(filterFunction Filter) { + c.filterFunction = filterFunction } // addTargetToTargetItems assigns a target to the collector based on its hash and adds it to the allocator's targetItems @@ -163,16 +163,8 @@ func (c *consistentHashingAllocator) SetTargets(targets map[string]*target.Targe if c.filterFunction != nil { targets = c.filterFunction.Apply(targets) - } else { - // If no filterFunction is set, then filtering will be a no-op. - // Add metrics for targets kept and dropped in the no-op case. - targetsPerJob := prehook.GetTargetsPerJob(targets) - - for jName, numTargets := range targetsPerJob { - prehook.TargetsDropped.WithLabelValues(jName).Set(0) - prehook.TargetsKept.WithLabelValues(jName).Set(numTargets) - } } + prehook.RecordTargetsKeptPerJob(targets) c.m.Lock() defer c.m.Unlock() diff --git a/cmd/otel-allocator/allocation/least_weighted.go b/cmd/otel-allocator/allocation/least_weighted.go index b41e4e2efc..924e9154ea 100644 --- a/cmd/otel-allocator/allocation/least_weighted.go +++ b/cmd/otel-allocator/allocation/least_weighted.go @@ -52,12 +52,12 @@ type leastWeightedAllocator struct { log logr.Logger - filterFunction prehook.Hook + filterFunction Filter } // SetHook sets the filtering hook to use. -func (allocator *leastWeightedAllocator) SetHook(hook prehook.Hook) { - allocator.filterFunction = hook +func (allocator *leastWeightedAllocator) SetFilter(filterFunction Filter) { + allocator.filterFunction = filterFunction } // TargetItems returns a shallow copy of the targetItems map. @@ -175,16 +175,8 @@ func (allocator *leastWeightedAllocator) SetTargets(targets map[string]*target.T if allocator.filterFunction != nil { targets = allocator.filterFunction.Apply(targets) - } else { - // if no filterFunction is set, then filtering will be a no-op. - // add metrics for targets kept and dropped in the no-op case. - targetsPerJob := prehook.GetTargetsPerJob(targets) - - for jName, numTargets := range targetsPerJob { - prehook.TargetsDropped.WithLabelValues(jName).Set(0) - prehook.TargetsKept.WithLabelValues(jName).Set(numTargets) - } } + prehook.RecordTargetsKeptPerJob(targets) allocator.m.Lock() defer allocator.m.Unlock() diff --git a/cmd/otel-allocator/allocation/strategy.go b/cmd/otel-allocator/allocation/strategy.go index 71daccdf01..4a9ba1b506 100644 --- a/cmd/otel-allocator/allocation/strategy.go +++ b/cmd/otel-allocator/allocation/strategy.go @@ -23,7 +23,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/prehook" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/target" ) @@ -50,9 +49,13 @@ var ( type AllocOption func(Allocator) -func WithFilter(hook prehook.Hook) AllocOption { +type Filter interface { + Apply(map[string]*target.TargetItem) map[string]*target.TargetItem +} + +func WithFilter(filterFunction Filter) AllocOption { return func(allocator Allocator) { - allocator.SetHook(hook) + allocator.SetFilter(filterFunction) } } @@ -76,7 +79,7 @@ type Allocator interface { SetTargets(targets map[string]*target.TargetItem) TargetItems() map[string]*target.TargetItem Collectors() map[string]*Collector - SetHook(hook prehook.Hook) + SetFilter(filterFunction Filter) } var _ consistent.Member = Collector{} diff --git a/cmd/otel-allocator/discovery/discovery.go b/cmd/otel-allocator/discovery/discovery.go index b7548b645d..04831b03d4 100644 --- a/cmd/otel-allocator/discovery/discovery.go +++ b/cmd/otel-allocator/discovery/discovery.go @@ -26,7 +26,6 @@ import ( "github.com/prometheus/prometheus/discovery" "github.com/prometheus/prometheus/model/relabel" - "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/prehook" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/target" allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" ) @@ -44,10 +43,14 @@ type Manager struct { logger log.Logger close chan struct{} configsMap map[allocatorWatcher.EventSource]*config.Config - hook prehook.Hook + hook discoveryHook } -func NewManager(log logr.Logger, ctx context.Context, logger log.Logger, hook prehook.Hook, options ...func(*discovery.Manager)) *Manager { +type discoveryHook interface { + SetConfig(map[string][]*relabel.Config) +} + +func NewManager(log logr.Logger, ctx context.Context, logger log.Logger, hook discoveryHook, options ...func(*discovery.Manager)) *Manager { manager := discovery.NewManager(ctx, logger, options...) go func() { diff --git a/cmd/otel-allocator/prehook/prehook.go b/cmd/otel-allocator/prehook/prehook.go index ed64090bbf..f38f5f1713 100644 --- a/cmd/otel-allocator/prehook/prehook.go +++ b/cmd/otel-allocator/prehook/prehook.go @@ -35,10 +35,6 @@ var ( Name: "opentelemetry_allocator_targets_kept", Help: "Number of targets kept after filtering.", }, []string{"job_name"}) - TargetsDropped = promauto.NewGaugeVec(prometheus.GaugeOpts{ - Name: "opentelemetry_allocator_targets_dropped", - Help: "Number of targets dropped after filtering.", - }, []string{"job_name"}) ) type Hook interface { @@ -53,13 +49,17 @@ var ( registry = map[string]HookProvider{} ) -func GetTargetsPerJob(targets map[string]*target.TargetItem) map[string]float64 { +func RecordTargetsKeptPerJob(targets map[string]*target.TargetItem) map[string]float64 { targetsPerJob := make(map[string]float64) for _, tItem := range targets { targetsPerJob[tItem.JobName] += 1 } + for jName, numTargets := range targetsPerJob { + TargetsKept.WithLabelValues(jName).Set(numTargets) + } + return targetsPerJob } diff --git a/cmd/otel-allocator/prehook/relabel.go b/cmd/otel-allocator/prehook/relabel.go index dad19ed61b..7047d7a97a 100644 --- a/cmd/otel-allocator/prehook/relabel.go +++ b/cmd/otel-allocator/prehook/relabel.go @@ -49,16 +49,12 @@ func convertLabelToPromLabelSet(lbls model.LabelSet) []labels.Label { func (tf *RelabelConfigTargetFilter) Apply(targets map[string]*target.TargetItem) map[string]*target.TargetItem { numTargets := len(targets) - var droppedTargetsPerJob, targetsPerJob map[string]float64 // need to wait until relabelCfg is set if len(tf.relabelCfg) == 0 { return targets } - droppedTargetsPerJob = make(map[string]float64) - targetsPerJob = GetTargetsPerJob(targets) - // Note: jobNameKey != tItem.JobName (jobNameKey is hashed) for jobNameKey, tItem := range targets { keepTarget := true @@ -74,16 +70,7 @@ func (tf *RelabelConfigTargetFilter) Apply(targets map[string]*target.TargetItem if !keepTarget { delete(targets, jobNameKey) - droppedTargetsPerJob[tItem.JobName] += 1 } - - } - - // add metrics for number of targets kept and dropped per job. - for jName, numTargets := range targetsPerJob { - numDropped := droppedTargetsPerJob[jName] - TargetsDropped.WithLabelValues(jName).Set(numDropped) - TargetsKept.WithLabelValues(jName).Set(numTargets-numDropped) } tf.log.V(2).Info("Filtering complete", "seen", numTargets, "kept", len(targets)) diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 0c4cded5b4..1196232386 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -1701,9 +1701,9 @@ spec: type: boolean filterStrategy: description: FilterStrategy determines how to filter targets before - allocating them among the collectors. The current options are - no-op (no filtering) and relabel-config (drops targets based - on prom relabel_config) The default is no-op + allocating them among the collectors. The only current option + is relabel-config (drops targets based on prom relabel_config). + Filtering is disabled by default. type: string image: description: Image indicates the container image to use for the diff --git a/docs/api.md b/docs/api.md index 48af368269..015c87490a 100644 --- a/docs/api.md +++ b/docs/api.md @@ -4546,7 +4546,7 @@ TargetAllocator indicates a value which determines whether to spawn a target all filterStrategy string - FilterStrategy determines how to filter targets before allocating them among the collectors. The current options are no-op (no filtering) and relabel-config (drops targets based on prom relabel_config) The default is no-op
+ FilterStrategy determines how to filter targets before allocating them among the collectors. The only current option is relabel-config (drops targets based on prom relabel_config). Filtering is disabled by default.
false diff --git a/pkg/collector/reconcile/configmap.go b/pkg/collector/reconcile/configmap.go index f5a1cfd899..dce74a4f2c 100644 --- a/pkg/collector/reconcile/configmap.go +++ b/pkg/collector/reconcile/configmap.go @@ -122,9 +122,8 @@ func desiredTAConfigMap(params Params) (corev1.ConfigMap, error) { if len(params.Instance.Spec.TargetAllocator.FilterStrategy) > 0 { taConfig["filter_strategy"] = params.Instance.Spec.TargetAllocator.FilterStrategy - } else { - taConfig["filter_strategy"] = "no-op" } + taConfigYAML, err := yaml.Marshal(taConfig) if err != nil { return corev1.ConfigMap{}, err diff --git a/pkg/collector/reconcile/configmap_test.go b/pkg/collector/reconcile/configmap_test.go index e6c2300f27..bdc13d79ca 100644 --- a/pkg/collector/reconcile/configmap_test.go +++ b/pkg/collector/reconcile/configmap_test.go @@ -194,7 +194,6 @@ config: - targets: - 0.0.0.0:8888 - 0.0.0.0:9999 -filter_strategy: no-op label_selector: app.kubernetes.io/component: opentelemetry-collector app.kubernetes.io/instance: default.test @@ -327,7 +326,6 @@ func TestExpectedConfigMap(t *testing.T) { } taConfig["config"] = parmConfig taConfig["allocation_strategy"] = "least-weighted" - taConfig["filter_strategy"] = "no-op" taConfigYAML, _ := yaml.Marshal(taConfig) assert.Equal(t, string(taConfigYAML), actual.Data["targetallocator.yaml"])