bump opamp go to v0.15.0 (#3159)

* bump opamp go to v0.15.0 * Fix test * update makefile to build with image * remove image pull policy * pin version for bridge test server
open-telemetry · Jul 24, 2024 · 934c1d8 · 934c1d8
1 parent e133eee
commit 934c1d8
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 26 deletions.
diff --git a/Makefile b/Makefile
@@ -35,6 +35,9 @@ TARGETALLOCATOR_IMG ?= ${IMG_PREFIX}/${TARGETALLOCATOR_IMG_REPO}:$(addprefix v,$
 OPERATOROPAMPBRIDGE_IMG_REPO ?= operator-opamp-bridge
 OPERATOROPAMPBRIDGE_IMG ?= ${IMG_PREFIX}/${OPERATOROPAMPBRIDGE_IMG_REPO}:$(addprefix v,${VERSION})
 
+BRIDGETESTSERVER_IMG_REPO ?= e2e-test-app-bridge-server
+BRIDGETESTSERVER_IMG ?= ${IMG_PREFIX}/${BRIDGETESTSERVER_IMG_REPO}:ve2e
+
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
 ifeq (,$(shell go env GOBIN))
 GOBIN=$(shell go env GOPATH)/bin
@@ -316,7 +319,7 @@ e2e-upgrade: undeploy chainsaw
 	$(CHAINSAW) test --test-dir ./tests/e2e-upgrade
 
 .PHONY: prepare-e2e
-prepare-e2e: chainsaw set-image-controller add-image-targetallocator add-image-opampbridge container container-target-allocator container-operator-opamp-bridge start-kind cert-manager install-metrics-server install-targetallocator-prometheus-crds load-image-all deploy
+prepare-e2e: chainsaw set-image-controller add-image-targetallocator add-image-opampbridge container container-target-allocator container-operator-opamp-bridge container-bridge-test-server start-kind cert-manager install-metrics-server install-targetallocator-prometheus-crds load-image-all deploy
 
 .PHONY: scorecard-tests
 scorecard-tests: operator-sdk
@@ -354,6 +357,11 @@ container-operator-opamp-bridge: GOOS = linux
 container-operator-opamp-bridge: operator-opamp-bridge
 	docker build -t ${OPERATOROPAMPBRIDGE_IMG} cmd/operator-opamp-bridge
 
+.PHONY: container-bridge-test-server
+container-bridge-test-server: GOOS = linux
+container-bridge-test-server:
+	docker build -t ${BRIDGETESTSERVER_IMG} tests/test-e2e-apps/bridge-server
+
 .PHONY: start-kind
 start-kind: kind
 ifeq (true,$(START_KIND_CLUSTER))
@@ -370,7 +378,7 @@ install-targetallocator-prometheus-crds:
 	./hack/install-targetallocator-prometheus-crds.sh
 
 .PHONY: load-image-all
-load-image-all: load-image-operator load-image-target-allocator load-image-operator-opamp-bridge
+load-image-all: load-image-operator load-image-target-allocator load-image-operator-opamp-bridge load-image-bridge-test-server
 
 .PHONY: load-image-operator
 load-image-operator: container kind
@@ -389,6 +397,9 @@ else
 	$(MAKE) container-target-allocator-push
 endif
 
+.PHONY: load-image-bridge-test-server
+load-image-bridge-test-server: container-bridge-test-server kind
+	$(KIND) load --name $(KIND_CLUSTER_NAME) docker-image ${BRIDGETESTSERVER_IMG}
 
 .PHONY: load-image-operator-opamp-bridge
 load-image-operator-opamp-bridge: container-operator-opamp-bridge kind

diff --git a/cmd/operator-opamp-bridge/agent/agent.go b/cmd/operator-opamp-bridge/agent/agent.go
@@ -22,7 +22,7 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
-	"github.com/oklog/ulid/v2"
+	"github.com/google/uuid"
 	"github.com/open-telemetry/opamp-go/client"
 	"github.com/open-telemetry/opamp-go/client/types"
 	"github.com/open-telemetry/opamp-go/protobufs"
@@ -44,7 +44,7 @@ type Agent struct {
 	startTime   uint64
 	lastHash    []byte
 
-	instanceId         ulid.ULID
+	instanceId         uuid.UUID
 	agentDescription   *protobufs.AgentDescription
 	remoteConfigStatus *protobufs.RemoteConfigStatus
 
@@ -211,7 +211,7 @@ func (agent *Agent) Start() error {
 	settings := types.StartSettings{
 		OpAMPServerURL: agent.config.Endpoint,
 		Header:         agent.config.Headers.ToHTTPHeader(),
-		InstanceUid:    agent.instanceId.String(),
+		InstanceUid:    types.InstanceUid(agent.instanceId),
 		Callbacks: types.CallbacksStruct{
 			OnConnectFunc:              agent.onConnect,
 			OnConnectFailedFunc:        agent.onConnectFailed,
@@ -274,7 +274,7 @@ func (agent *Agent) runHeartbeat() {
 
 // updateAgentIdentity receives a new instanced Id from the remote server and updates the agent's instanceID field.
 // The meter will be reinitialized by the onMessage function.
-func (agent *Agent) updateAgentIdentity(instanceId ulid.ULID) {
+func (agent *Agent) updateAgentIdentity(instanceId uuid.UUID) {
 	agent.logger.V(3).Info("Agent identity is being changed",
 		"old instanceId", agent.instanceId.String(),
 		"new instanceid", instanceId.String())
@@ -416,12 +416,12 @@ func (agent *Agent) onMessage(ctx context.Context, msg *types.MessageData) {
 	// The instance id is updated prior to the meter initialization so that the new meter will report using the updated
 	// instanceId.
 	if msg.AgentIdentification != nil {
-		newInstanceId, err := ulid.Parse(msg.AgentIdentification.NewInstanceUid)
+		uid, err := uuid.FromBytes(msg.AgentIdentification.NewInstanceUid)
 		if err != nil {
 			agent.logger.Error(err, "couldn't parse instance UID")
 			return
 		}
-		agent.updateAgentIdentity(newInstanceId)
+		agent.updateAgentIdentity(uid)
 	}
 
 	if msg.OwnMetricsConnSettings != nil {

diff --git a/cmd/operator-opamp-bridge/agent/agent_test.go b/cmd/operator-opamp-bridge/agent/agent_test.go
@@ -16,15 +16,14 @@ package agent
 
 import (
 	"context"
-	"crypto/rand"
 	"fmt"
 	"os"
 	"sort"
 	"testing"
 	"time"
 
 	"github.com/go-logr/logr"
-	"github.com/oklog/ulid/v2"
+	"github.com/google/uuid"
 	"github.com/open-telemetry/opamp-go/client"
 	"github.com/open-telemetry/opamp-go/client/types"
 	"github.com/open-telemetry/opamp-go/protobufs"
@@ -883,15 +882,20 @@ func Test_CanUpdateIdentity(t *testing.T) {
 	defer agent.Shutdown()
 	require.NoError(t, err, "should be able to start agent")
 	previousInstanceId := agent.instanceId.String()
-	entropy := ulid.Monotonic(rand.Reader, 0)
-	newId := ulid.MustNew(ulid.MaxTime(), entropy)
+	newId, err := uuid.NewV7()
+	require.NoError(t, err)
+	marshalledId, err := newId.MarshalBinary()
+	require.NoError(t, err)
 	agent.onMessage(context.Background(), &types.MessageData{
 		AgentIdentification: &protobufs.AgentIdentification{
-			NewInstanceUid: newId.String(),
+			NewInstanceUid: marshalledId,
 		},
 	})
 	assert.NotEqual(t, previousInstanceId, newId.String())
 	assert.Equal(t, agent.instanceId, newId)
+	parsedUUID, err := uuid.FromBytes(marshalledId)
+	require.NoError(t, err)
+	assert.Equal(t, newId, parsedUUID)
 }
 
 func getMessageDataFromConfigFile(filemap map[string]string) (*types.MessageData, error) {

diff --git a/cmd/operator-opamp-bridge/config/config.go b/cmd/operator-opamp-bridge/config/config.go
@@ -15,7 +15,6 @@
 package config
 
 import (
-	"crypto/rand"
 	"errors"
 	"fmt"
 	"io/fs"
@@ -25,7 +24,7 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
-	"github.com/oklog/ulid/v2"
+	"github.com/google/uuid"
 	opampclient "github.com/open-telemetry/opamp-go/client"
 	"github.com/open-telemetry/opamp-go/protobufs"
 	"github.com/spf13/pflag"
@@ -186,9 +185,13 @@ func keyValuePair(key string, value string) *protobufs.KeyValue {
 	}
 }
 
-func (c *Config) GetNewInstanceId() ulid.ULID {
-	entropy := ulid.Monotonic(rand.Reader, 0)
-	return ulid.MustNew(ulid.Timestamp(time.Now()), entropy)
+func (c *Config) GetNewInstanceId() uuid.UUID {
+	u, err := uuid.NewV7()
+	if err != nil {
+		// This really should never happen and if it does we should fail.
+		panic(err)
+	}
+	return u
 }
 
 func (c *Config) RemoteConfigEnabled() bool {

diff --git a/cmd/operator-opamp-bridge/metrics/reporter.go b/cmd/operator-opamp-bridge/metrics/reporter.go
@@ -22,7 +22,7 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
-	"github.com/oklog/ulid/v2"
+	"github.com/google/uuid"
 	"github.com/open-telemetry/opamp-go/protobufs"
 	"github.com/shirou/gopsutil/process"
 	"go.opentelemetry.io/otel/attribute"
@@ -53,7 +53,7 @@ type MetricReporter struct {
 // NewMetricReporter creates an OTLP/HTTP client to the destination address supplied by the server.
 // TODO: do more validation on the endpoint, allow for gRPC.
 // TODO: set global provider and add more metrics to be reported.
-func NewMetricReporter(logger logr.Logger, dest *protobufs.TelemetryConnectionSettings, agentType string, agentVersion string, instanceId ulid.ULID) (*MetricReporter, error) {
+func NewMetricReporter(logger logr.Logger, dest *protobufs.TelemetryConnectionSettings, agentType string, agentVersion string, instanceId uuid.UUID) (*MetricReporter, error) {
 
 	if dest.DestinationEndpoint == "" {
 		return nil, fmt.Errorf("metric destination must specify DestinationEndpoint")

diff --git a/go.mod b/go.mod
@@ -17,7 +17,7 @@ require (
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/oklog/run v1.1.0
 	github.com/oklog/ulid/v2 v2.1.0
-	github.com/open-telemetry/opamp-go v0.14.0
+	github.com/open-telemetry/opamp-go v0.15.0
 	github.com/openshift/api v0.0.0-20240124164020-e2ce40831f2e
 	github.com/operator-framework/operator-lib v0.14.0
 	github.com/prometheus-operator/prometheus-operator v0.75.0

diff --git a/go.sum b/go.sum
@@ -499,8 +499,8 @@ github.com/onsi/ginkgo/v2 v2.17.2 h1:7eMhcy3GimbsA3hEnVKdw/PQM9XN9krpKVXsZdph0/g
 github.com/onsi/ginkgo/v2 v2.17.2/go.mod h1:nP2DPOQoNsQmsVyv5rDA8JkXQoCs6goXIvr/PRJ1eCc=
 github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk=
 github.com/onsi/gomega v1.33.1/go.mod h1:U4R44UsT+9eLIaYRB2a5qajjtQYn0hauxvRm16AVYg0=
-github.com/open-telemetry/opamp-go v0.14.0 h1:KoziIK+wsFojhUXNTkCSTnCPf0eCMqFAaccOs0HrWIY=
-github.com/open-telemetry/opamp-go v0.14.0/go.mod h1:XOGCigljsLSTZ8FfLwvat0M1QDj3conIIgRa77BWrKs=
+github.com/open-telemetry/opamp-go v0.15.0 h1:X2TWhEsGQ8GP7Uos3Ic9v/1aFUqoECZXKS7xAF5HqsA=
+github.com/open-telemetry/opamp-go v0.15.0/go.mod h1:QyPeN56JXlcZt5yG5RMdZ50Ju+zMFs1Ihy/hwHyF8Oo=
 github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
 github.com/opencontainers/image-spec v1.0.2 h1:9yCKha/T5XdGtO0q9Q9a6T5NUCsTn/DrBg0D7ufOcFM=

diff --git a/tests/e2e-opampbridge/opampbridge/00-install.yaml b/tests/e2e-opampbridge/opampbridge/00-install.yaml
@@ -14,7 +14,7 @@ spec:
     spec:
       containers:
         - name: e2e-test-app-bridge-server
-          image: ghcr.io/open-telemetry/opentelemetry-operator/e2e-test-app-bridge-server:main
+          image: ghcr.io/open-telemetry/opentelemetry-operator/e2e-test-app-bridge-server:ve2e
           ports:
             - containerPort: 4320
             - containerPort: 4321

diff --git a/tests/test-e2e-apps/bridge-server/opampsrv/opampsrv.go b/tests/test-e2e-apps/bridge-server/opampsrv/opampsrv.go
@@ -110,8 +110,8 @@ func (srv *Server) onMessage(ctx context.Context, conn types.Connection, msg *pr
 		instanceId = data.InstanceId(u.Bytes())
 	} else if len(msg.InstanceUid) == 16 {
 		// This is a 16 byte, new style UID.
-		if parsedId, err := uuid.Parse(string(msg.InstanceUid)); err != nil {
-			srv.logger.Errorf(ctx, "Cannot parse UUID %s: %v", string(msg.InstanceUid), err)
+		if parsedId, err := uuid.FromBytes(msg.InstanceUid); err != nil {
+			srv.logger.Errorf(ctx, "Cannot parse UUID %s: %v", msg.InstanceUid, err)
 			return response
 		} else {
 			instanceId = data.InstanceId(parsedId)