Skip to content

Commit

Permalink
fix gocritic issue for extension observer (#11066)
Browse files Browse the repository at this point in the history
* fix gocritic issue for extension observer

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>

* Apply suggestions from code review

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>

Co-authored-by: Daniel Jaglowski <jaglows3@gmail.com>
  • Loading branch information
fatsheep9146 and djaglowski authored Jun 20, 2022
1 parent 04ce2b4 commit 0721367
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 16 deletions.
7 changes: 2 additions & 5 deletions extension/observer/dockerobserver/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// nolint:gocritic
package dockerobserver // import "github.com/open-telemetry/opentelemetry-collector-contrib/extension/observer/dockerobserver"

import (
Expand Down Expand Up @@ -217,13 +216,11 @@ func (d *dockerObserver) updateEndpointsByContainerID(listener observer.Notify,
// If it does not exist already, it is a new endpoint. Add it.
if existingEndpoint, ok := existingEndpointsMap[e.ID]; !ok {
addedEndpoints = append(addedEndpoints, e)
} else {
} else if !reflect.DeepEqual(existingEndpoint, e) {
// if it already exists, see if it's equal.
// if it's not equal, update it
// if its equal, no-op.
if !reflect.DeepEqual(existingEndpoint, e) {
updatedEndpoints = append(updatedEndpoints, e)
}
updatedEndpoints = append(updatedEndpoints, e)
}
}

Expand Down
9 changes: 3 additions & 6 deletions extension/observer/endpointswatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// nolint:gocritic
package observer // import "github.com/open-telemetry/opentelemetry-collector-contrib/extension/observer"

import (
Expand Down Expand Up @@ -73,12 +72,10 @@ func (ew *EndpointsWatcher) refreshEndpoints(listener Notify) {
if existingEndpoint, ok := ew.existingEndpoints[e.ID]; !ok {
ew.existingEndpoints[e.ID] = e
addedEndpoints = append(addedEndpoints, e)
} else {
} else if !reflect.DeepEqual(existingEndpoint, e) {
// Collect updated endpoints.
if !reflect.DeepEqual(existingEndpoint, e) {
ew.existingEndpoints[e.ID] = e
updatedEndpoints = append(updatedEndpoints, e)
}
ew.existingEndpoints[e.ID] = e
updatedEndpoints = append(updatedEndpoints, e)
}
}

Expand Down
5 changes: 2 additions & 3 deletions extension/observer/k8sobserver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// nolint:gocritic
package k8sobserver

import (
Expand Down Expand Up @@ -73,7 +72,7 @@ func TestInvalidAuth(t *testing.T) {

factory := NewFactory()
factories.Extensions[typeStr] = factory
cfg, err := servicetest.LoadConfigAndValidate(filepath.Join("testdata/invalid_auth.yaml"), factories)
cfg, err := servicetest.LoadConfigAndValidate(filepath.Join("testdata", "invalid_auth.yaml"), factories)
require.NotNil(t, cfg)
require.EqualError(t, err, `extension "k8s_observer" has invalid configuration: invalid authType for kubernetes: not a real auth type`)
}
Expand All @@ -84,7 +83,7 @@ func TestInvalidNoObserving(t *testing.T) {

factory := NewFactory()
factories.Extensions[typeStr] = factory
cfg, err := servicetest.LoadConfigAndValidate(filepath.Join("testdata/invalid_no_observing.yaml"), factories)
cfg, err := servicetest.LoadConfigAndValidate(filepath.Join("testdata", "invalid_no_observing.yaml"), factories)
require.NotNil(t, cfg)
require.EqualError(t, err, `extension "k8s_observer" has invalid configuration: one of observe_pods and observe_nodes must be true`)
}
4 changes: 2 additions & 2 deletions extension/observer/k8sobserver/mocks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// nolint:gocritic
package k8sobserver

import (
Expand Down Expand Up @@ -47,7 +46,8 @@ func (e *endpointSink) OnRemove(removed []observer.Endpoint) {
func (e *endpointSink) OnChange(changed []observer.Endpoint) {
e.Lock()
defer e.Unlock()
e.changed = append(e.removed, changed...)
e.changed = e.removed
e.changed = append(e.changed, changed...)
}

var _ observer.Notify = (*endpointSink)(nil)
Expand Down

0 comments on commit 0721367

Please sign in to comment.