Skip to content

Commit

Permalink
Use UUID instead of ULID
Browse files Browse the repository at this point in the history
Updates OpAMP spec to 0.9.0 and implements spec change open-telemetry/opamp-spec#186

The example server still accepts old-style ULID agent instances to
demonstrate how this change can be handled in a backward compatible way.
Replace ULIDs by 16 byte ids and recommend UUID v7 (open-telemetry#186)
  • Loading branch information
tigrannajaryan committed May 14, 2024
1 parent 359e77a commit d44e1d5
Show file tree
Hide file tree
Showing 24 changed files with 177 additions and 123 deletions.
36 changes: 20 additions & 16 deletions client/clientimpl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@ import (
"crypto/tls"
"crypto/x509"
"fmt"
"math/rand"
"net/http"
"net/http/httptest"
"net/url"
"sync/atomic"
"testing"
"time"

ulid "github.com/oklog/ulid/v2"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"
Expand Down Expand Up @@ -94,10 +93,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)
Expand Down Expand Up @@ -150,7 +156,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)
Expand Down Expand Up @@ -624,9 +630,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 {
Expand All @@ -636,7 +640,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[:],
},
}
}
Expand All @@ -645,7 +649,7 @@ func TestAgentIdentification(t *testing.T) {
InstanceUid: msg.InstanceUid,
AgentIdentification: &protobufs.AgentIdentification{
// Start by sending an invalid id forcing an error.
NewInstanceUid: "",
NewInstanceUid: nil,
},
}
}
Expand All @@ -662,11 +666,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
},
)

Expand All @@ -677,11 +681,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
},
)

Expand All @@ -693,11 +697,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
},
)

Expand Down
8 changes: 4 additions & 4 deletions client/internal/receivedprocessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
15 changes: 6 additions & 9 deletions client/internal/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion client/internal/wsreceiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func TestDecodeMessage(t *testing.T) {
msgsToTest := []*protobufs.ServerToAgent{
{}, // Empty message
{
InstanceUid: "abcd",
InstanceUid: []byte("0123456789123456"),
},
}

Expand Down
3 changes: 3 additions & 0 deletions client/types/instanceid.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package types

type InstanceUid [16]byte
2 changes: 1 addition & 1 deletion client/types/startsettings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
5 changes: 2 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
30 changes: 16 additions & 14 deletions internal/examples/agent/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -54,7 +52,7 @@ type Agent struct {

effectiveConfig string

instanceId ulid.ULID
instanceId uuid.UUID

agentDescription *protobufs.AgentDescription

Expand Down Expand Up @@ -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 {
Expand All @@ -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.")
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions internal/examples/agent/agent/metricreporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions internal/examples/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions internal/examples/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
7 changes: 5 additions & 2 deletions internal/examples/server/data/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion internal/examples/server/data/instanceid.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
package data

type InstanceId string
import "github.com/google/uuid"

type InstanceId uuid.UUID
Loading

0 comments on commit d44e1d5

Please sign in to comment.