Skip to content

Commit

Permalink
add smaller interfaces local to packages using them
Browse files Browse the repository at this point in the history
  • Loading branch information
moh-osman3 committed Oct 27, 2022
1 parent 6a502f8 commit a2b20ef
Show file tree
Hide file tree
Showing 12 changed files with 36 additions and 62 deletions.
4 changes: 2 additions & 2 deletions apis/v1alpha1/opentelemetrycollector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 4 additions & 12 deletions cmd/otel-allocator/allocation/consistent_hashing.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type consistentHashingAllocator struct {

log logr.Logger

filterFunction prehook.Hook
filterFunction Filter
}

func newConsistentHashingAllocator(log logr.Logger, opts ...AllocOption) Allocator {
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down
16 changes: 4 additions & 12 deletions cmd/otel-allocator/allocation/least_weighted.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down
11 changes: 7 additions & 4 deletions cmd/otel-allocator/allocation/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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)
}
}

Expand All @@ -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{}
Expand Down
9 changes: 6 additions & 3 deletions cmd/otel-allocator/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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() {
Expand Down
10 changes: 5 additions & 5 deletions cmd/otel-allocator/prehook/prehook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down
13 changes: 0 additions & 13 deletions cmd/otel-allocator/prehook/relabel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -4546,7 +4546,7 @@ TargetAllocator indicates a value which determines whether to spawn a target all
<td><b>filterStrategy</b></td>
<td>string</td>
<td>
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<br/>
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.<br/>
</td>
<td>false</td>
</tr><tr>
Expand Down
3 changes: 1 addition & 2 deletions pkg/collector/reconcile/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions pkg/collector/reconcile/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"])
Expand Down

0 comments on commit a2b20ef

Please sign in to comment.