From 2b648a684d5abd468fe212be126ae0bdff240bb3 Mon Sep 17 00:00:00 2001 From: Kristina Pathak Date: Mon, 12 Sep 2022 18:31:51 -0700 Subject: [PATCH 1/3] fix lint issues in target allocator files squash of all current commits on this branch --- .github/workflows/continuous-integration.yaml | 4 ++ .golangci.yaml | 2 - Makefile | 1 + cmd/otel-allocator/.golangci.yaml | 26 +++++++++ .../allocation/consistent_hashing.go | 26 ++++++--- .../allocation/consistent_hashing_test.go | 26 +++++++-- cmd/otel-allocator/allocation/http.go | 14 +++++ cmd/otel-allocator/allocation/http_test.go | 14 +++++ .../allocation/least_weighted.go | 30 +++++++--- .../allocation/least_weighted_test.go | 40 ++++++++----- cmd/otel-allocator/allocation/strategy.go | 24 ++++++-- cmd/otel-allocator/collector/collector.go | 16 +++++- .../collector/collector_test.go | 14 +++++ cmd/otel-allocator/config/config.go | 20 +++++-- cmd/otel-allocator/config/config_test.go | 14 +++++ cmd/otel-allocator/diff/diff.go | 14 +++++ cmd/otel-allocator/diff/diff_test.go | 14 +++++ cmd/otel-allocator/discovery/discovery.go | 16 +++++- .../discovery/discovery_test.go | 14 +++++ cmd/otel-allocator/header.txt | 13 +++++ cmd/otel-allocator/main.go | 57 ++++++++++++------- cmd/otel-allocator/watcher/file.go | 14 +++++ cmd/otel-allocator/watcher/main.go | 16 +++++- cmd/otel-allocator/watcher/promOperator.go | 19 ++++++- 24 files changed, 374 insertions(+), 74 deletions(-) create mode 100644 cmd/otel-allocator/.golangci.yaml create mode 100644 cmd/otel-allocator/header.txt diff --git a/.github/workflows/continuous-integration.yaml b/.github/workflows/continuous-integration.yaml index 6deff2320d..2b5a9437b0 100644 --- a/.github/workflows/continuous-integration.yaml +++ b/.github/workflows/continuous-integration.yaml @@ -31,6 +31,9 @@ jobs: lint: name: Code standards (linting) runs-on: ubuntu-20.04 + strategy: + matrix: + workdir: [".", "./cmd/otel-allocator"] steps: - name: Set up Go uses: actions/setup-go@v3 @@ -44,6 +47,7 @@ jobs: with: args: -v version: v1.48 + working-directory: ${{ matrix.workdir }} security: name: Security diff --git a/.golangci.yaml b/.golangci.yaml index 879cfc6199..f1a7317c86 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -13,8 +13,6 @@ linters-settings: locale: US govet: disable-all: true - enable: - - fieldalignment linters: enable: diff --git a/Makefile b/Makefile index 09ce4b790a..d3602da554 100644 --- a/Makefile +++ b/Makefile @@ -146,6 +146,7 @@ vet: .PHONY: lint lint: golangci-lint run + cd cmd/otel-allocator && golangci-lint run # Generate code .PHONY: generate diff --git a/cmd/otel-allocator/.golangci.yaml b/cmd/otel-allocator/.golangci.yaml new file mode 100644 index 0000000000..d00dba1286 --- /dev/null +++ b/cmd/otel-allocator/.golangci.yaml @@ -0,0 +1,26 @@ +run: + timeout: 5m + +# all available settings of specific linters +linters-settings: + goheader: + template-path: header.txt + goimports: + local-prefixes: github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator + maligned: + suggest-new: true + misspell: + locale: US + govet: + disable-all: true + +linters: + enable: + - goheader + - goimports + - misspell + - gosec + - govet + - exhaustive + - godot + - unparam diff --git a/cmd/otel-allocator/allocation/consistent_hashing.go b/cmd/otel-allocator/allocation/consistent_hashing.go index 31a78e35ab..41f31a4e92 100644 --- a/cmd/otel-allocator/allocation/consistent_hashing.go +++ b/cmd/otel-allocator/allocation/consistent_hashing.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 allocation import ( @@ -56,8 +70,8 @@ func newConsistentHashingAllocator(log logr.Logger) Allocator { // addTargetToTargetItems assigns a target to the collector based on its hash and adds it to the allocator's targetItems // This method is called from within SetTargets and SetCollectors, which acquire the needed lock. -// This is only called after the collectors are cleared or when a new target has been found in the tempTargetMap -// INVARIANT: c.collectors must have at least 1 collector set +// This is only called after the collectors are cleared or when a new target has been found in the tempTargetMap. +// INVARIANT: c.collectors must have at least 1 collector set. func (c *consistentHashingAllocator) addTargetToTargetItems(target *TargetItem) { // Check if this is a reassignment, if so, decrement the previous collector's NumTargets if previousColName, ok := c.collectors[target.CollectorName]; ok { @@ -78,8 +92,8 @@ func (c *consistentHashingAllocator) addTargetToTargetItems(target *TargetItem) } // handleTargets receives the new and removed targets and reconciles the current state. -// Any removals are removed from the allocator's targetItems and unassigned from the corresponding collector -// Any net-new additions are assigned to the next available collector +// Any removals are removed from the allocator's targetItems and unassigned from the corresponding collector. +// Any net-new additions are assigned to the next available collector. func (c *consistentHashingAllocator) handleTargets(diff diff.Changes[*TargetItem]) { // Check for removals for k, target := range c.targetItems { @@ -105,7 +119,7 @@ func (c *consistentHashingAllocator) handleTargets(diff diff.Changes[*TargetItem } // handleCollectors receives the new and removed collectors and reconciles the current state. -// Any removals are removed from the allocator's collectors. New collectors are added to the allocator's collector map +// Any removals are removed from the allocator's collectors. New collectors are added to the allocator's collector map. // Finally, update all targets' collectors to match the consistent hashing. func (c *consistentHashingAllocator) handleCollectors(diff diff.Changes[*Collector]) { // Clear removed collectors @@ -146,7 +160,6 @@ func (c *consistentHashingAllocator) SetTargets(targets map[string]*TargetItem) if len(targetsDiff.Additions()) != 0 || len(targetsDiff.Removals()) != 0 { c.handleTargets(targetsDiff) } - return } // SetCollectors sets the set of collectors with key=collectorName, value=Collector object. @@ -170,7 +183,6 @@ func (c *consistentHashingAllocator) SetCollectors(collectors map[string]*Collec if len(collectorsDiff.Additions()) != 0 || len(collectorsDiff.Removals()) != 0 { c.handleCollectors(collectorsDiff) } - return } // TargetItems returns a shallow copy of the targetItems map. diff --git a/cmd/otel-allocator/allocation/consistent_hashing_test.go b/cmd/otel-allocator/allocation/consistent_hashing_test.go index 8793af53be..31ac9c2007 100644 --- a/cmd/otel-allocator/allocation/consistent_hashing_test.go +++ b/cmd/otel-allocator/allocation/consistent_hashing_test.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 allocation import ( @@ -7,7 +21,7 @@ import ( ) func TestCanSetSingleTarget(t *testing.T) { - cols := makeNCollectors(3, 0, 0) + cols := makeNCollectors(3, 0) c := newConsistentHashingAllocator(logger) c.SetCollectors(cols) c.SetTargets(makeNNewTargets(1, 3, 0)) @@ -21,7 +35,7 @@ func TestCanSetSingleTarget(t *testing.T) { func TestRelativelyEvenDistribution(t *testing.T) { numCols := 15 numItems := 10000 - cols := makeNCollectors(numCols, 0, 0) + cols := makeNCollectors(numCols, 0) var expectedPerCollector = float64(numItems / numCols) expectedDelta := (expectedPerCollector * 1.5) - expectedPerCollector c := newConsistentHashingAllocator(logger) @@ -38,7 +52,7 @@ func TestRelativelyEvenDistribution(t *testing.T) { } func TestFullReallocation(t *testing.T) { - cols := makeNCollectors(10, 0, 0) + cols := makeNCollectors(10, 0) c := newConsistentHashingAllocator(logger) c.SetCollectors(cols) c.SetTargets(makeNNewTargets(10000, 10, 0)) @@ -46,7 +60,7 @@ func TestFullReallocation(t *testing.T) { assert.Len(t, actualTargetItems, 10000) actualCollectors := c.Collectors() assert.Len(t, actualCollectors, 10) - newCols := makeNCollectors(10, 0, 10) + newCols := makeNCollectors(10, 10) c.SetCollectors(newCols) updatedTargetItems := c.TargetItems() assert.Len(t, updatedTargetItems, 10000) @@ -63,7 +77,7 @@ func TestNumRemapped(t *testing.T) { numInitialCols := 15 numFinalCols := 16 expectedDelta := float64((numFinalCols - numInitialCols) * (numItems / numFinalCols)) - cols := makeNCollectors(numInitialCols, 0, 0) + cols := makeNCollectors(numInitialCols, 0) c := newConsistentHashingAllocator(logger) c.SetCollectors(cols) c.SetTargets(makeNNewTargets(numItems, numInitialCols, 0)) @@ -71,7 +85,7 @@ func TestNumRemapped(t *testing.T) { assert.Len(t, actualTargetItems, numItems) actualCollectors := c.Collectors() assert.Len(t, actualCollectors, numInitialCols) - newCols := makeNCollectors(numFinalCols, 0, 0) + newCols := makeNCollectors(numFinalCols, 0) c.SetCollectors(newCols) updatedTargetItems := c.TargetItems() assert.Len(t, updatedTargetItems, numItems) diff --git a/cmd/otel-allocator/allocation/http.go b/cmd/otel-allocator/allocation/http.go index 37d08769fb..ee60f31609 100644 --- a/cmd/otel-allocator/allocation/http.go +++ b/cmd/otel-allocator/allocation/http.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 allocation import ( diff --git a/cmd/otel-allocator/allocation/http_test.go b/cmd/otel-allocator/allocation/http_test.go index c54a7a4dcc..e4e7fb7b61 100644 --- a/cmd/otel-allocator/allocation/http_test.go +++ b/cmd/otel-allocator/allocation/http_test.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 allocation import ( diff --git a/cmd/otel-allocator/allocation/least_weighted.go b/cmd/otel-allocator/allocation/least_weighted.go index 3824d1a1d6..bed1dbd51f 100644 --- a/cmd/otel-allocator/allocation/least_weighted.go +++ b/cmd/otel-allocator/allocation/least_weighted.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 allocation import ( @@ -61,8 +75,8 @@ func (allocator *leastWeightedAllocator) Collectors() map[string]*Collector { // findNextCollector finds the next collector with fewer number of targets. // This method is called from within SetTargets and SetCollectors, whose caller -// acquires the needed lock. This method assumes there are is at least 1 collector set -// INVARIANT: allocator.collectors must have at least 1 collector set +// acquires the needed lock. This method assumes there are is at least 1 collector set. +// INVARIANT: allocator.collectors must have at least 1 collector set. func (allocator *leastWeightedAllocator) findNextCollector() *Collector { var col *Collector for _, v := range allocator.collectors { @@ -78,8 +92,8 @@ func (allocator *leastWeightedAllocator) findNextCollector() *Collector { // addTargetToTargetItems assigns a target to the next available collector and adds it to the allocator's targetItems // This method is called from within SetTargets and SetCollectors, which acquire the needed lock. -// This is only called after the collectors are cleared or when a new target has been found in the tempTargetMap -// INVARIANT: allocator.collectors must have at least 1 collector set +// This is only called after the collectors are cleared or when a new target has been found in the tempTargetMap. +// INVARIANT: allocator.collectors must have at least 1 collector set. func (allocator *leastWeightedAllocator) addTargetToTargetItems(target *TargetItem) { chosenCollector := allocator.findNextCollector() targetItem := &TargetItem{ @@ -95,8 +109,8 @@ func (allocator *leastWeightedAllocator) addTargetToTargetItems(target *TargetIt } // handleTargets receives the new and removed targets and reconciles the current state. -// Any removals are removed from the allocator's targetItems and unassigned from the corresponding collector -// Any net-new additions are assigned to the next available collector +// Any removals are removed from the allocator's targetItems and unassigned from the corresponding collector. +// Any net-new additions are assigned to the next available collector. func (allocator *leastWeightedAllocator) handleTargets(diff diff.Changes[*TargetItem]) { // Check for removals for k, target := range allocator.targetItems { @@ -122,7 +136,7 @@ func (allocator *leastWeightedAllocator) handleTargets(diff diff.Changes[*Target } // handleCollectors receives the new and removed collectors and reconciles the current state. -// Any removals are removed from the allocator's collectors. New collectors are added to the allocator's collector map +// Any removals are removed from the allocator's collectors. New collectors are added to the allocator's collector map. // Finally, any targets of removed collectors are reallocated to the next available collector. func (allocator *leastWeightedAllocator) handleCollectors(diff diff.Changes[*Collector]) { // Clear removed collectors @@ -163,7 +177,6 @@ func (allocator *leastWeightedAllocator) SetTargets(targets map[string]*TargetIt if len(targetsDiff.Additions()) != 0 || len(targetsDiff.Removals()) != 0 { allocator.handleTargets(targetsDiff) } - return } // SetCollectors sets the set of collectors with key=collectorName, value=Collector object. @@ -187,7 +200,6 @@ func (allocator *leastWeightedAllocator) SetCollectors(collectors map[string]*Co if len(collectorsDiff.Additions()) != 0 || len(collectorsDiff.Removals()) != 0 { allocator.handleCollectors(collectorsDiff) } - return } func newLeastWeightedAllocator(log logr.Logger) Allocator { diff --git a/cmd/otel-allocator/allocation/least_weighted_test.go b/cmd/otel-allocator/allocation/least_weighted_test.go index d6b98b3d09..3b6209ddd5 100644 --- a/cmd/otel-allocator/allocation/least_weighted_test.go +++ b/cmd/otel-allocator/allocation/least_weighted_test.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 allocation import ( @@ -36,13 +50,13 @@ func makeNNewTargets(n int, numCollectors int, startingIndex int) map[string]*Ta return toReturn } -func makeNCollectors(n int, targetsForEach int, startingIndex int) map[string]*Collector { +func makeNCollectors(n int, startingIndex int) map[string]*Collector { toReturn := map[string]*Collector{} for i := startingIndex; i < n+startingIndex; i++ { collector := fmt.Sprintf("collector-%d", i) toReturn[collector] = &Collector{ Name: collector, - NumTargets: targetsForEach, + NumTargets: 0, } } return toReturn @@ -51,7 +65,7 @@ func makeNCollectors(n int, targetsForEach int, startingIndex int) map[string]*C func TestSetCollectors(t *testing.T) { s, _ := New("least-weighted", logger) - cols := makeNCollectors(3, 0, 0) + cols := makeNCollectors(3, 0) s.SetCollectors(cols) expectedColLen := len(cols) @@ -67,7 +81,7 @@ func TestAddingAndRemovingTargets(t *testing.T) { // prepare allocator with initial targets and collectors s, _ := New("least-weighted", logger) - cols := makeNCollectors(3, 0, 0) + cols := makeNCollectors(3, 0) s.SetCollectors(cols) initTargets := makeNNewTargets(6, 3, 0) @@ -97,12 +111,12 @@ func TestAddingAndRemovingTargets(t *testing.T) { } } -// Tests that two targets with the same target url and job name but different label set are both added +// Tests that two targets with the same target url and job name but different label set are both added. func TestAllocationCollision(t *testing.T) { // prepare allocator with initial targets and collectors s, _ := New("least-weighted", logger) - cols := makeNCollectors(3, 0, 0) + cols := makeNCollectors(3, 0) s.SetCollectors(cols) firstLabels := model.LabelSet{ "test": "test1", @@ -136,7 +150,7 @@ func TestAllocationCollision(t *testing.T) { func TestNoCollectorReassignment(t *testing.T) { s, _ := New("least-weighted", logger) - cols := makeNCollectors(3, 0, 0) + cols := makeNCollectors(3, 0) s.SetCollectors(cols) expectedColLen := len(cols) @@ -156,7 +170,7 @@ func TestNoCollectorReassignment(t *testing.T) { assert.Len(t, targetItems, expectedTargetLen) // assign new set of collectors with the same names - newCols := makeNCollectors(3, 0, 0) + newCols := makeNCollectors(3, 0) s.SetCollectors(newCols) newTargetItems := s.TargetItems() @@ -167,7 +181,7 @@ func TestNoCollectorReassignment(t *testing.T) { func TestSmartCollectorReassignment(t *testing.T) { s, _ := New("least-weighted", logger) - cols := makeNCollectors(4, 0, 0) + cols := makeNCollectors(4, 0) s.SetCollectors(cols) expectedColLen := len(cols) @@ -212,13 +226,13 @@ func TestSmartCollectorReassignment(t *testing.T) { } } -// Tests that the delta in number of targets per collector is less than 15% of an even distribution +// Tests that the delta in number of targets per collector is less than 15% of an even distribution. func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) { // prepare allocator with 3 collectors and 'random' amount of targets s, _ := New("least-weighted", logger) - cols := makeNCollectors(3, 0, 0) + cols := makeNCollectors(3, 0) s.SetCollectors(cols) targets := makeNNewTargets(27, 3, 0) @@ -241,8 +255,8 @@ func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) { // Remove half of targets randomly toDelete := len(targets) / 2 counter := 0 - for index, _ := range targets { - shouldDelete := rand.Intn(toDelete) + for index := range targets { + shouldDelete := rand.Intn(toDelete) //nolint:gosec if counter < shouldDelete { delete(targets, index) } diff --git a/cmd/otel-allocator/allocation/strategy.go b/cmd/otel-allocator/allocation/strategy.go index 949a397fd4..2808c32b5d 100644 --- a/cmd/otel-allocator/allocation/strategy.go +++ b/cmd/otel-allocator/allocation/strategy.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 allocation import ( @@ -17,7 +31,7 @@ type AllocatorProvider func(log logr.Logger) Allocator var ( registry = map[string]AllocatorProvider{} - // TargetsPerCollector records how many targets have been assigned to each collector + // TargetsPerCollector records how many targets have been assigned to each collector. // It is currently the responsibility of the strategy to track this information. TargetsPerCollector = promauto.NewGaugeVec(prometheus.GaugeOpts{ Name: "opentelemetry_allocator_targets_per_collector", @@ -37,7 +51,7 @@ func New(name string, log logr.Logger) (Allocator, error) { if p, ok := registry[name]; ok { return p(log), nil } - return nil, errors.New(fmt.Sprintf("unregistered strategy: %s", name)) + return nil, fmt.Errorf("unregistered strategy: %s", name) } func Register(name string, provider AllocatorProvider) error { @@ -79,9 +93,9 @@ func (t TargetItem) Hash() string { var _ consistent.Member = Collector{} -// Collector Creates a struct that holds Collector information -// This struct will be parsed into endpoint with Collector and jobs info -// This struct can be extended with information like annotations and labels in the future +// Collector Creates a struct that holds Collector information. +// This struct will be parsed into endpoint with Collector and jobs info. +// This struct can be extended with information like annotations and labels in the future. type Collector struct { Name string NumTargets int diff --git a/cmd/otel-allocator/collector/collector.go b/cmd/otel-allocator/collector/collector.go index e1cf66ec1f..f6e7f0ff89 100644 --- a/cmd/otel-allocator/collector/collector.go +++ b/cmd/otel-allocator/collector/collector.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 collector import ( @@ -108,7 +122,7 @@ func runWatch(ctx context.Context, k *Client, c <-chan watch.Event, collectorMap return "no event" } - switch event.Type { + switch event.Type { //nolint:exhaustive case watch.Added: collectorMap[pod.Name] = allocation.NewCollector(pod.Name) case watch.Deleted: diff --git a/cmd/otel-allocator/collector/collector_test.go b/cmd/otel-allocator/collector/collector_test.go index 059e616702..d4bd1680dc 100644 --- a/cmd/otel-allocator/collector/collector_test.go +++ b/cmd/otel-allocator/collector/collector_test.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 collector import ( diff --git a/cmd/otel-allocator/config/config.go b/cmd/otel-allocator/config/config.go index 05d0e38405..0fa8879f0e 100644 --- a/cmd/otel-allocator/config/config.go +++ b/cmd/otel-allocator/config/config.go @@ -1,7 +1,20 @@ +// Copyright The OpenTelemetry 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 config import ( - "errors" "flag" "fmt" "io/fs" @@ -22,11 +35,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" ) -// ErrInvalidYAML represents an error in the format of the original YAML configuration file. -var ( - ErrInvalidYAML = errors.New("couldn't parse the loadbalancer configuration") -) - const DefaultResyncTime = 5 * time.Minute const DefaultConfigFilePath string = "/conf/targetallocator.yaml" diff --git a/cmd/otel-allocator/config/config_test.go b/cmd/otel-allocator/config/config_test.go index 09ed11a27d..3c45ffefe4 100644 --- a/cmd/otel-allocator/config/config_test.go +++ b/cmd/otel-allocator/config/config_test.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 config import ( diff --git a/cmd/otel-allocator/diff/diff.go b/cmd/otel-allocator/diff/diff.go index 44ec55e040..4bb201cfc0 100644 --- a/cmd/otel-allocator/diff/diff.go +++ b/cmd/otel-allocator/diff/diff.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 diff // Changes is the result of the difference between two maps – items that are added and items that are removed diff --git a/cmd/otel-allocator/diff/diff_test.go b/cmd/otel-allocator/diff/diff_test.go index bcae530d55..9dec103b79 100644 --- a/cmd/otel-allocator/diff/diff_test.go +++ b/cmd/otel-allocator/diff/diff_test.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 diff import ( diff --git a/cmd/otel-allocator/discovery/discovery.go b/cmd/otel-allocator/discovery/discovery.go index 2c0393f619..b6e45f9915 100644 --- a/cmd/otel-allocator/discovery/discovery.go +++ b/cmd/otel-allocator/discovery/discovery.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 discovery import ( @@ -35,7 +49,7 @@ func NewManager(log logr.Logger, ctx context.Context, logger log.Logger, options go func() { if err := manager.Run(); err != nil { - logger.Log("Discovery manager failed", err) + log.Error(err, "Discovery manager failed") } }() return &Manager{ diff --git a/cmd/otel-allocator/discovery/discovery_test.go b/cmd/otel-allocator/discovery/discovery_test.go index 7edacde407..afea787176 100644 --- a/cmd/otel-allocator/discovery/discovery_test.go +++ b/cmd/otel-allocator/discovery/discovery_test.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 discovery import ( diff --git a/cmd/otel-allocator/header.txt b/cmd/otel-allocator/header.txt new file mode 100644 index 0000000000..3881885f31 --- /dev/null +++ b/cmd/otel-allocator/header.txt @@ -0,0 +1,13 @@ +Copyright The OpenTelemetry 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. \ No newline at end of file diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index 80d8ec4743..7c830322ea 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 main import ( @@ -8,6 +22,7 @@ import ( "os" "os/signal" "syscall" + "time" gokitlog "github.com/go-kit/log" "github.com/go-logr/logr" @@ -64,7 +79,12 @@ func main() { setupLog.Error(err, "Can't start the watchers") os.Exit(1) } - defer watcher.Close() + defer func() { + err := watcher.Close() + if err != nil { + log.Error(err, "failed to close watcher") + } + }() // creates a new discovery manager discoveryManager := lbdiscovery.NewManager(log, ctx, gokitlog.NewNopLogger()) @@ -77,10 +97,7 @@ func main() { os.Exit(1) } - srv, err := newServer(log, allocator, discoveryManager, k8sclient, cliConf.ListenAddr) - if err != nil { - setupLog.Error(err, "Can't start the server") - } + srv := newServer(log, allocator, discoveryManager, k8sclient, cliConf.ListenAddr) interrupts := make(chan os.Signal, 1) signal.Notify(interrupts, os.Interrupt, syscall.SIGTERM) @@ -108,10 +125,7 @@ func main() { if err := srv.Shutdown(ctx); err != nil { setupLog.Error(err, "Cannot shutdown the server") } - srv, err = newServer(log, allocator, discoveryManager, k8sclient, cliConf.ListenAddr) - if err != nil { - setupLog.Error(err, "Error restarting the server with new config") - } + srv = newServer(log, allocator, discoveryManager, k8sclient, cliConf.ListenAddr) go func() { if err := srv.Start(); err != http.ErrServerClosed { setupLog.Error(err, "Can't restart the server") @@ -143,7 +157,7 @@ type server struct { server *http.Server } -func newServer(log logr.Logger, allocator allocation.Allocator, discoveryManager *lbdiscovery.Manager, k8sclient *collector.Client, listenAddr *string) (*server, error) { +func newServer(log logr.Logger, allocator allocation.Allocator, discoveryManager *lbdiscovery.Manager, k8sclient *collector.Client, listenAddr *string) *server { s := &server{ logger: log, allocator: allocator, @@ -155,8 +169,8 @@ func newServer(log logr.Logger, allocator allocation.Allocator, discoveryManager router.HandleFunc("/jobs", s.JobHandler).Methods("GET") router.HandleFunc("/jobs/{job_id}/targets", s.TargetsHandler).Methods("GET") router.Path("/metrics").Handler(promhttp.Handler()) - s.server = &http.Server{Addr: *listenAddr, Handler: router} - return s, nil + s.server = &http.Server{Addr: *listenAddr, Handler: router, ReadHeaderTimeout: 90 * time.Second} + return s } func configureFileDiscovery(log logr.Logger, allocator allocation.Allocator, discoveryManager *lbdiscovery.Manager, ctx context.Context, cliConfig config.CLIConfig) (*collector.Client, error) { @@ -195,7 +209,7 @@ func (s *server) JobHandler(w http.ResponseWriter, r *http.Request) { for _, v := range s.allocator.TargetItems() { displayData[v.JobName] = allocation.LinkJSON{Link: v.Link.Link} } - jsonHandler(w, r, displayData) + jsonHandler(s.logger, w, displayData) } // PrometheusMiddleware implements mux.MiddlewareFunc. @@ -219,30 +233,33 @@ func (s *server) TargetsHandler(w http.ResponseWriter, r *http.Request) { params := mux.Vars(r) jobId, err := url.QueryUnescape(params["job_id"]) if err != nil { - errorHandler(err, w, r) + errorHandler(w) return } if len(q) == 0 { displayData := allocation.GetAllTargetsByJob(jobId, compareMap, s.allocator) - jsonHandler(w, r, displayData) + jsonHandler(s.logger, w, displayData) } else { tgs := allocation.GetAllTargetsByCollectorAndJob(q[0], jobId, compareMap, s.allocator) // Displays empty list if nothing matches if len(tgs) == 0 { - jsonHandler(w, r, []interface{}{}) + jsonHandler(s.logger, w, []interface{}{}) return } - jsonHandler(w, r, tgs) + jsonHandler(s.logger, w, tgs) } } -func errorHandler(err error, w http.ResponseWriter, r *http.Request) { +func errorHandler(w http.ResponseWriter) { w.WriteHeader(500) } -func jsonHandler(w http.ResponseWriter, r *http.Request, data interface{}) { +func jsonHandler(logger logr.Logger, w http.ResponseWriter, data interface{}) { w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(data) + err := json.NewEncoder(w).Encode(data) + if err != nil { + logger.Error(err, "failed to encode data for http response") + } } diff --git a/cmd/otel-allocator/watcher/file.go b/cmd/otel-allocator/watcher/file.go index 6adc3c33e9..a2a7f9fd92 100644 --- a/cmd/otel-allocator/watcher/file.go +++ b/cmd/otel-allocator/watcher/file.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 watcher import ( diff --git a/cmd/otel-allocator/watcher/main.go b/cmd/otel-allocator/watcher/main.go index dac48d5fb2..8dbd71036f 100644 --- a/cmd/otel-allocator/watcher/main.go +++ b/cmd/otel-allocator/watcher/main.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 watcher import ( @@ -59,7 +73,7 @@ func NewWatcher(logger logr.Logger, config config.CLIConfig, allocator allocatio watcher.watchers = append(watcher.watchers, &fileWatcher) if *config.PromCRWatcherConf.Enabled { - promWatcher, err := newCRDMonitorWatcher(logger, config) + promWatcher, err := newCRDMonitorWatcher(config) if err != nil { return nil, err } diff --git a/cmd/otel-allocator/watcher/promOperator.go b/cmd/otel-allocator/watcher/promOperator.go index 382b2f217e..09eee6f50f 100644 --- a/cmd/otel-allocator/watcher/promOperator.go +++ b/cmd/otel-allocator/watcher/promOperator.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 watcher import ( @@ -6,7 +20,6 @@ import ( allocatorconfig "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config" "github.com/go-kit/log" - "github.com/go-logr/logr" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" "github.com/prometheus-operator/prometheus-operator/pkg/assets" monitoringclient "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned" @@ -20,7 +33,7 @@ import ( "k8s.io/client-go/tools/cache" ) -func newCRDMonitorWatcher(logger logr.Logger, config allocatorconfig.CLIConfig) (*PrometheusCRWatcher, error) { +func newCRDMonitorWatcher(config allocatorconfig.CLIConfig) (*PrometheusCRWatcher, error) { mClient, err := monitoringclient.NewForConfig(config.ClusterConfig) if err != nil { return nil, err @@ -63,7 +76,7 @@ type PrometheusCRWatcher struct { configGenerator *prometheus.ConfigGenerator } -// Start wrapped informers and wait for an initial sync +// Start wrapped informers and wait for an initial sync. func (w *PrometheusCRWatcher) Start(upstreamEvents chan Event, upstreamErrors chan error) error { watcher := Watcher(w) event := Event{ From 427399099f9bd916ef734361affe3d974b6e6841 Mon Sep 17 00:00:00 2001 From: Kristina Pathak Date: Mon, 26 Sep 2022 18:04:32 -0700 Subject: [PATCH 2/3] remove addtl golangci config --- cmd/otel-allocator/.golangci.yaml | 26 -------------------------- 1 file changed, 26 deletions(-) delete mode 100644 cmd/otel-allocator/.golangci.yaml diff --git a/cmd/otel-allocator/.golangci.yaml b/cmd/otel-allocator/.golangci.yaml deleted file mode 100644 index d00dba1286..0000000000 --- a/cmd/otel-allocator/.golangci.yaml +++ /dev/null @@ -1,26 +0,0 @@ -run: - timeout: 5m - -# all available settings of specific linters -linters-settings: - goheader: - template-path: header.txt - goimports: - local-prefixes: github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator - maligned: - suggest-new: true - misspell: - locale: US - govet: - disable-all: true - -linters: - enable: - - goheader - - goimports - - misspell - - gosec - - govet - - exhaustive - - godot - - unparam From 72609d5e5bb5f94a2221e0e8de44bc27ff4d8438 Mon Sep 17 00:00:00 2001 From: Kristina Pathak Date: Tue, 27 Sep 2022 10:46:18 -0700 Subject: [PATCH 3/3] remove version.txt --- cmd/otel-allocator/version.txt | 1 - 1 file changed, 1 deletion(-) delete mode 100644 cmd/otel-allocator/version.txt diff --git a/cmd/otel-allocator/version.txt b/cmd/otel-allocator/version.txt deleted file mode 100644 index 6e8bf73aa5..0000000000 --- a/cmd/otel-allocator/version.txt +++ /dev/null @@ -1 +0,0 @@ -0.1.0