Merge branch 'main' into serveroffered-values-bug-fix

client/clientimpl_test.go

@@ -5,7 +5,6 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"fmt"
-	"math/rand"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -14,6 +13,7 @@ import (
 	"time"
 
 	"github.com/oklog/ulid/v2"
+	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/proto"
@@ -94,10 +94,17 @@ func eventually(t *testing.T, f func() bool) {
 	assert.Eventually(t, f, 5*time.Second, 10*time.Millisecond)
 }
 
+func newInstanceUid(t *testing.T) types.InstanceUid {
+	uid, err := uuid.NewV7()
+	require.NoError(t, err)
+	b, err := uid.MarshalBinary()
+	require.NoError(t, err)
+	return types.InstanceUid(b)
+}
+
 func prepareSettings(t *testing.T, settings *types.StartSettings, c OpAMPClient) {
 	// Autogenerate instance id.
-	entropy := ulid.Monotonic(rand.New(rand.NewSource(99)), 0)
-	settings.InstanceUid = ulid.MustNew(ulid.Timestamp(time.Now()), entropy).String()
+	settings.InstanceUid = newInstanceUid(t)
 
 	// Make sure correct URL scheme is used, based on the type of the OpAMP client.
 	u, err := url.Parse(settings.OpAMPServerURL)
@@ -150,7 +157,7 @@ func TestInvalidInstanceId(t *testing.T) {
 	testClients(t, func(t *testing.T, client OpAMPClient) {
 		settings := createNoServerSettings()
 		prepareClient(t, &settings, client)
-		settings.InstanceUid = "invalidid"
+		settings.InstanceUid = types.InstanceUid{}
 
 		err := client.Start(context.Background(), settings)
 		assert.Error(t, err)
@@ -624,9 +631,7 @@ func TestAgentIdentification(t *testing.T) {
 	testClients(t, func(t *testing.T, client OpAMPClient) {
 		// Start a server.
 		srv := internal.StartMockServer(t)
-		newInstanceUid := ulid.MustNew(
-			ulid.Timestamp(time.Now()), ulid.Monotonic(rand.New(rand.NewSource(0)), 0),
-		)
+		newInstanceUid := newInstanceUid(t)
 		var rcvAgentInstanceUid atomic.Value
 		var sentInvalidId atomic.Bool
 		srv.OnMessage = func(msg *protobufs.AgentToServer) *protobufs.ServerToAgent {
@@ -636,7 +641,7 @@ func TestAgentIdentification(t *testing.T) {
 					InstanceUid: msg.InstanceUid,
 					AgentIdentification: &protobufs.AgentIdentification{
 						// If we sent the invalid one first, send a valid one now
-						NewInstanceUid: newInstanceUid.String(),
+						NewInstanceUid: newInstanceUid[:],
 					},
 				}
 			}
@@ -645,7 +650,7 @@ func TestAgentIdentification(t *testing.T) {
 				InstanceUid: msg.InstanceUid,
 				AgentIdentification: &protobufs.AgentIdentification{
 					// Start by sending an invalid id forcing an error.
-					NewInstanceUid: "",
+					NewInstanceUid: nil,
 				},
 			}
 		}
@@ -662,11 +667,11 @@ func TestAgentIdentification(t *testing.T) {
 		eventually(
 			t,
 			func() bool {
-				instanceUid, ok := rcvAgentInstanceUid.Load().(string)
+				instanceUid, ok := rcvAgentInstanceUid.Load().([]byte)
 				if !ok {
 					return false
 				}
-				return instanceUid == oldInstanceUid
+				return types.InstanceUid(instanceUid) == oldInstanceUid
 			},
 		)
 
@@ -677,11 +682,11 @@ func TestAgentIdentification(t *testing.T) {
 		eventually(
 			t,
 			func() bool {
-				instanceUid, ok := rcvAgentInstanceUid.Load().(string)
+				instanceUid, ok := rcvAgentInstanceUid.Load().([]byte)
 				if !ok {
 					return false
 				}
-				return instanceUid == oldInstanceUid
+				return types.InstanceUid(instanceUid) == oldInstanceUid
 			},
 		)
 
@@ -693,11 +698,11 @@ func TestAgentIdentification(t *testing.T) {
 		eventually(
 			t,
 			func() bool {
-				instanceUid, ok := rcvAgentInstanceUid.Load().(string)
+				instanceUid, ok := rcvAgentInstanceUid.Load().([]byte)
 				if !ok {
 					return false
 				}
-				return instanceUid == newInstanceUid.String()
+				return types.InstanceUid(instanceUid) == newInstanceUid
 			},
 		)
 

client/internal/receivedprocessor.go

@@ -2,7 +2,7 @@ package internal
 
 import (
 	"context"
-	"errors"
+	"fmt"
 
 	"github.com/open-telemetry/opamp-go/client/types"
 	"github.com/open-telemetry/opamp-go/protobufs"
@@ -225,13 +225,13 @@ func (r *receivedProcessor) processErrorResponse(ctx context.Context, body *prot
 }
 
 func (r *receivedProcessor) rcvAgentIdentification(ctx context.Context, agentId *protobufs.AgentIdentification) error {
-	if agentId.NewInstanceUid == "" {
-		err := errors.New("empty instance uid is not allowed")
+	if len(agentId.NewInstanceUid) != 16 {
+		err := fmt.Errorf("instance uid must be 16 bytes but is %d bytes long", len(agentId.NewInstanceUid))
 		r.logger.Debugf(ctx, err.Error())
 		return err
 	}
 
-	err := r.sender.SetInstanceUid(agentId.NewInstanceUid)
+	err := r.sender.SetInstanceUid(types.InstanceUid(agentId.NewInstanceUid))
 	if err != nil {
 		r.logger.Errorf(ctx, "Error while setting instance uid: %v", err)
 		return err

client/internal/sender.go

@@ -3,7 +3,7 @@ package internal
 import (
 	"errors"
 
-	"github.com/oklog/ulid/v2"
+	"github.com/open-telemetry/opamp-go/client/types"
 	"github.com/open-telemetry/opamp-go/protobufs"
 )
 
@@ -21,7 +21,7 @@ type Sender interface {
 	ScheduleSend()
 
 	// SetInstanceUid sets a new instanceUid to be used for all subsequent messages to be sent.
-	SetInstanceUid(instanceUid string) error
+	SetInstanceUid(instanceUid types.InstanceUid) error
 }
 
 // SenderCommon is partial Sender implementation that is common between WebSocket and plain
@@ -65,18 +65,15 @@ func (h *SenderCommon) NextMessage() *NextMessage {
 // SetInstanceUid sets a new instanceUid to be used for all subsequent messages to be sent.
 // Can be called concurrently, normally is called when a message is received from the
 // Server that instructs us to change our instance UID.
-func (h *SenderCommon) SetInstanceUid(instanceUid string) error {
-	if instanceUid == "" {
+func (h *SenderCommon) SetInstanceUid(instanceUid types.InstanceUid) error {
+	var emptyUid types.InstanceUid
+	if instanceUid == emptyUid {
 		return errors.New("cannot set instance uid to empty value")
 	}
 
-	if _, err := ulid.ParseStrict(instanceUid); err != nil {
-		return err
-	}
-
 	h.nextMessage.Update(
 		func(msg *protobufs.AgentToServer) {
-			msg.InstanceUid = instanceUid
+			msg.InstanceUid = instanceUid[:]
 		})
 
 	return nil

client/internal/wsreceiver_test.go

@@ -157,7 +157,7 @@ func TestDecodeMessage(t *testing.T) {
 	msgsToTest := []*protobufs.ServerToAgent{
 		{}, // Empty message
 		{
-			InstanceUid: "abcd",
+			InstanceUid: []byte("0123456789123456"),
 		},
 	}
 

client/types/instanceid.go

@@ -0,0 +1,3 @@
+package types
+
+type InstanceUid [16]byte

client/types/startsettings.go

@@ -21,7 +21,7 @@ type StartSettings struct {
 	TLSConfig *tls.Config
 
 	// Agent information.
-	InstanceUid string
+	InstanceUid InstanceUid
 
 	// Callbacks that the client will call after Start() returns nil.
 	Callbacks Callbacks

go.mod

@@ -4,8 +4,8 @@ go 1.20
 
 require (
 	github.com/cenkalti/backoff/v4 v4.2.1
+	github.com/google/uuid v1.6.0
 	github.com/gorilla/websocket v1.5.1
-	github.com/oklog/ulid/v2 v2.1.0
 	github.com/stretchr/testify v1.9.0
 	google.golang.org/protobuf v1.33.0
 )

go.sum

@@ -4,11 +4,10 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
+github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/gorilla/websocket v1.5.1 h1:gmztn0JnHVt9JZquRuzLw3g4wouNVzKL15iLr/zn/QY=
 github.com/gorilla/websocket v1.5.1/go.mod h1:x3kM2JMyaluk02fnUJpQuwD2dCS5NDG2ZHL0uE0tcaY=
-github.com/oklog/ulid/v2 v2.1.0 h1:+9lhoxAP56we25tyYETBBY1YLA2SaoLvUFgrP2miPJU=
-github.com/oklog/ulid/v2 v2.1.0/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNsTT1QQ=
-github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=

internal/examples/agent/agent/agent.go

@@ -10,16 +10,14 @@ import (
 	"crypto/x509/pkix"
 	"encoding/pem"
 	"fmt"
-	"math/rand"
 	"os"
 	"runtime"
 	"sort"
-	"time"
 
+	"github.com/google/uuid"
 	"github.com/knadh/koanf"
 	"github.com/knadh/koanf/parsers/yaml"
 	"github.com/knadh/koanf/providers/rawbytes"
-	"github.com/oklog/ulid/v2"
 
 	"github.com/open-telemetry/opamp-go/client"
 	"github.com/open-telemetry/opamp-go/client/types"
@@ -54,7 +52,7 @@ type Agent struct {
 
 	effectiveConfig string
 
-	instanceId ulid.ULID
+	instanceId uuid.UUID
 
 	agentDescription *protobufs.AgentDescription
 
@@ -82,7 +80,7 @@ func NewAgent(logger types.Logger, agentType string, agentVersion string) *Agent
 
 	agent.createAgentIdentity()
 	agent.logger.Debugf(context.Background(), "Agent starting, id=%v, type=%s, version=%s.",
-		agent.instanceId.String(), agentType, agentVersion)
+		agent.instanceId, agentType, agentVersion)
 
 	agent.loadLocalConfig()
 	if err := agent.connect(); err != nil {
@@ -107,7 +105,7 @@ func (agent *Agent) connect() error {
 	settings := types.StartSettings{
 		OpAMPServerURL: "wss://127.0.0.1:4320/v1/opamp",
 		TLSConfig:      tlsConfig,
-		InstanceUid:    agent.instanceId.String(),
+		InstanceUid:    types.InstanceUid(agent.instanceId),
 		Callbacks: types.CallbacksStruct{
 			OnConnectFunc: func(ctx context.Context) {
 				agent.logger.Debugf(ctx, "Connected to the server.")
@@ -167,8 +165,11 @@ func (agent *Agent) disconnect(ctx context.Context) {
 
 func (agent *Agent) createAgentIdentity() {
 	// Generate instance id.
-	entropy := ulid.Monotonic(rand.New(rand.NewSource(0)), 0)
-	agent.instanceId = ulid.MustNew(ulid.Timestamp(time.Now()), entropy)
+	uid, err := uuid.NewV7()
+	if err != nil {
+		panic(err)
+	}
+	agent.instanceId = uid
 
 	hostname, _ := os.Hostname()
 
@@ -209,10 +210,10 @@ func (agent *Agent) createAgentIdentity() {
 	}
 }
 
-func (agent *Agent) updateAgentIdentity(ctx context.Context, instanceId ulid.ULID) {
+func (agent *Agent) updateAgentIdentity(ctx context.Context, instanceId uuid.UUID) {
 	agent.logger.Debugf(ctx, "Agent identify is being changed from id=%v to id=%v",
-		agent.instanceId.String(),
-		instanceId.String())
+		agent.instanceId,
+		instanceId)
 	agent.instanceId = instanceId
 
 	if agent.metricReporter != nil {
@@ -459,11 +460,12 @@ func (agent *Agent) onMessage(ctx context.Context, msg *types.MessageData) {
 	}
 
 	if msg.AgentIdentification != nil {
-		newInstanceId, err := ulid.Parse(msg.AgentIdentification.NewInstanceUid)
+		uid, err := uuid.FromBytes(msg.AgentIdentification.NewInstanceUid)
 		if err != nil {
-			agent.logger.Errorf(ctx, err.Error())
+			agent.logger.Errorf(ctx, "invalid NewInstanceUid: %v", err)
+			return
 		}
-		agent.updateAgentIdentity(ctx, newInstanceId)
+		agent.updateAgentIdentity(ctx, uid)
 	}
 
 	if configChanged {

internal/examples/agent/agent/metricreporter.go

@@ -9,7 +9,7 @@ import (
 	"os"
 	"time"
 
-	"github.com/oklog/ulid/v2"
+	"github.com/google/uuid"
 	"github.com/shirou/gopsutil/process"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
@@ -46,7 +46,7 @@ func NewMetricReporter(
 	dest *protobufs.TelemetryConnectionSettings,
 	agentType string,
 	agentVersion string,
-	instanceId ulid.ULID,
+	instanceId uuid.UUID,
 ) (*MetricReporter, error) {
 
 	// Check the destination credentials to make sure they look like a valid OTLP/HTTP

internal/examples/go.mod

@@ -4,6 +4,7 @@ go 1.20
 
 require (
 	github.com/cenkalti/backoff/v4 v4.2.1
+	github.com/google/uuid v1.6.0
 	github.com/knadh/koanf v1.3.3
 	github.com/oklog/ulid/v2 v2.1.0
 	github.com/open-telemetry/opamp-go v0.1.0

internal/examples/go.sum

@@ -47,6 +47,8 @@ github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
+github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
+github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/gorilla/websocket v1.5.1 h1:gmztn0JnHVt9JZquRuzLw3g4wouNVzKL15iLr/zn/QY=
 github.com/gorilla/websocket v1.5.1/go.mod h1:x3kM2JMyaluk02fnUJpQuwD2dCS5NDG2ZHL0uE0tcaY=
 github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.1 h1:/c3QmbOGMGTOumP2iT/rCwB7b0QDGLKzqOmktBjT+Is=

internal/examples/server/data/agent.go

@@ -11,6 +11,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/google/uuid"
 	"google.golang.org/protobuf/proto"
 
 	"github.com/open-telemetry/opamp-go/internal/examples/server/certman"
@@ -23,7 +24,8 @@ type Agent struct {
 	// Some fields in this struct are exported so that we can render them in the UI.
 
 	// Agent's instance id. This is an immutable field.
-	InstanceId InstanceId
+	InstanceId    InstanceId
+	InstanceIdStr string
 
 	// Connection to the Agent.
 	conn types.Connection
@@ -60,7 +62,7 @@ func NewAgent(
 	instanceId InstanceId,
 	conn types.Connection,
 ) *Agent {
-	agent := &Agent{InstanceId: instanceId, conn: conn}
+	agent := &Agent{InstanceId: instanceId, InstanceIdStr: uuid.UUID(instanceId).String(), conn: conn}
 	tslConn, ok := conn.Connection().(*tls.Conn)
 	if ok {
 		// Client is using TLS connection.
@@ -84,6 +86,7 @@ func (agent *Agent) CloneReadonly() *Agent {
 	defer agent.mux.RUnlock()
 	return &Agent{
 		InstanceId:                  agent.InstanceId,
+		InstanceIdStr:               uuid.UUID(agent.InstanceId).String(),
 		Status:                      proto.Clone(agent.Status).(*protobufs.AgentToServer),
 		EffectiveConfig:             agent.EffectiveConfig,
 		CustomInstanceConfig:        agent.CustomInstanceConfig,

internal/examples/server/data/instanceid.go

@@ -1,3 +1,5 @@
 package data
 
-type InstanceId string
+import "github.com/google/uuid"
+
+type InstanceId uuid.UUID