Skip to content

Commit

Permalink
Set the message identifiers as UUIDv7 (#943)
Browse files Browse the repository at this point in the history
* edited the message format for message id, fixed the install test
  • Loading branch information
oliveromahony authored Dec 10, 2024
1 parent 0c83c0c commit 426612a
Show file tree
Hide file tree
Showing 23 changed files with 353 additions and 64 deletions.
13 changes: 1 addition & 12 deletions internal/collector/otel_collector_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"context"
"errors"
"fmt"
"strings"
"testing"

"github.com/nginx/agent/v3/test/protos"
Expand Down Expand Up @@ -119,23 +118,13 @@ func TestCollector_Init(t *testing.T) {
initError := collector.Init(context.Background(), nil)
require.NoError(t, initError)

validateLog(t, tt.expectedLog, logBuf)
helpers.ValidateLog(t, tt.expectedLog, logBuf)

require.NoError(t, collector.Close(context.TODO()))
})
}
}

func validateLog(t *testing.T, expectedLog string, logBuf *bytes.Buffer) {
t.Helper()

if expectedLog != "" {
if !strings.Contains(logBuf.String(), expectedLog) {
t.Errorf("Expected log to contain %q, but got %q", expectedLog, logBuf.String())
}
}
}

func TestCollector_InitAndClose(t *testing.T) {
conf := types.OTelConfig(t)
conf.Collector.Log.Path = ""
Expand Down
5 changes: 2 additions & 3 deletions internal/command/command_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ import (

"google.golang.org/protobuf/types/known/timestamppb"

"github.com/google/uuid"

mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
"github.com/nginx/agent/v3/internal/bus"
"github.com/nginx/agent/v3/internal/config"
"github.com/nginx/agent/v3/internal/datasource/proto"
"github.com/nginx/agent/v3/internal/grpc"
"github.com/nginx/agent/v3/internal/logger"
pkgConfig "github.com/nginx/agent/v3/pkg/config"
Expand Down Expand Up @@ -249,7 +248,7 @@ func (cp *CommandPlugin) createDataPlaneResponse(correlationID string, status mp
) *mpi.DataPlaneResponse {
return &mpi.DataPlaneResponse{
MessageMeta: &mpi.MessageMeta{
MessageId: uuid.NewString(),
MessageId: proto.GenerateMessageID(),
CorrelationId: correlationID,
Timestamp: timestamppb.Now(),
},
Expand Down
7 changes: 2 additions & 5 deletions internal/command/command_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package command
import (
"bytes"
"context"
"strings"
"testing"
"time"

Expand All @@ -18,6 +17,7 @@ import (
"github.com/nginx/agent/v3/internal/bus"
"github.com/nginx/agent/v3/internal/command/commandfakes"
"github.com/nginx/agent/v3/internal/grpc/grpcfakes"
"github.com/nginx/agent/v3/test/helpers"
"github.com/nginx/agent/v3/test/protos"
"github.com/nginx/agent/v3/test/stub"
"github.com/nginx/agent/v3/test/types"
Expand Down Expand Up @@ -211,10 +211,7 @@ func TestMonitorSubscribeChannel(t *testing.T) {

time.Sleep(100 * time.Millisecond)

// Verify the logger was called
if s := logBuf.String(); !strings.Contains(s, "Received management plane request") {
t.Errorf("Unexpected log %s", s)
}
helpers.ValidateLog(t, "Received management plane request", logBuf)

// Clear the log buffer
logBuf.Reset()
Expand Down
10 changes: 6 additions & 4 deletions internal/command/command_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import (
"time"

"github.com/cenkalti/backoff/v4"
"github.com/google/uuid"

mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
"github.com/nginx/agent/v3/internal/config"
"github.com/nginx/agent/v3/internal/datasource/proto"
"github.com/nginx/agent/v3/internal/grpc"
"github.com/nginx/agent/v3/internal/logger"

"google.golang.org/protobuf/types/known/timestamppb"

backoffHelpers "github.com/nginx/agent/v3/internal/backoff"
Expand Down Expand Up @@ -86,7 +88,7 @@ func (cs *CommandService) UpdateDataPlaneStatus(

request := &mpi.UpdateDataPlaneStatusRequest{
MessageMeta: &mpi.MessageMeta{
MessageId: uuid.NewString(),
MessageId: proto.GenerateMessageID(),
CorrelationId: correlationID,
Timestamp: timestamppb.Now(),
},
Expand Down Expand Up @@ -136,7 +138,7 @@ func (cs *CommandService) UpdateDataPlaneHealth(ctx context.Context, instanceHea

request := &mpi.UpdateDataPlaneHealthRequest{
MessageMeta: &mpi.MessageMeta{
MessageId: uuid.NewString(),
MessageId: proto.GenerateMessageID(),
CorrelationId: correlationID,
Timestamp: timestamppb.Now(),
},
Expand Down Expand Up @@ -214,7 +216,7 @@ func (cs *CommandService) CreateConnection(

request := &mpi.CreateConnectionRequest{
MessageMeta: &mpi.MessageMeta{
MessageId: uuid.NewString(),
MessageId: proto.GenerateMessageID(),
CorrelationId: correlationID,
Timestamp: timestamppb.Now(),
},
Expand Down
7 changes: 3 additions & 4 deletions internal/command/command_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"context"
"errors"
"log/slog"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -122,9 +121,9 @@ func TestCommandService_UpdateDataPlaneStatusSubscribeError(t *testing.T) {
err := commandService.UpdateDataPlaneStatus(ctx, protos.GetHostResource())
require.Error(t, err)

if s := logBuf.String(); !strings.Contains(s, "Failed to send update data plane status") {
t.Errorf("Unexpected log %s", s)
}
helpers.ValidateLog(t, "Failed to send update data plane status", logBuf)

logBuf.Reset()
}

func TestCommandService_CreateConnection(t *testing.T) {
Expand Down
31 changes: 31 additions & 0 deletions internal/datasource/proto/message.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (c) F5, Inc.
//
// This source code is licensed under the Apache License, Version 2.0 license found in the
// LICENSE file in the root directory of this source tree.

package proto

import (
"log/slog"
"time"

"github.com/google/uuid"
agentUuid "github.com/nginx/agent/v3/pkg/uuid"
)

// UUIDGenerator defines a function type for generating UUIDs.
type UUIDGenerator func() (uuid.UUID, error)

// DefaultUUIDGenerator is the production implementation for generating UUIDv7.
var defaultUUIDGenerator UUIDGenerator = uuid.NewUUID

// GenerateMessageID generates a unique message ID, falling back to sha256 and timestamp if UUID generation fails.
func GenerateMessageID() string {
uuidv7, err := defaultUUIDGenerator()
if err != nil {
slog.Debug("Issue generating uuidv7, using sha256 and timestamp instead", "error", err)
return agentUuid.Generate("%s", time.Now().String())
}

return uuidv7.String()
}
128 changes: 128 additions & 0 deletions internal/datasource/proto/message_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// Copyright (c) F5, Inc.
//
// This source code is licensed under the Apache License, Version 2.0 license found in the
// LICENSE file in the root directory of this source tree.

package proto

import (
"bytes"
"errors"
"regexp"
"testing"

"github.com/nginx/agent/v3/test/helpers"
"github.com/nginx/agent/v3/test/stub"
"github.com/stretchr/testify/assert"

"github.com/google/uuid"
)

func TestUUIDv7Regex(t *testing.T) {
// Define the UUIDv7 regex
uuidv7Regex := regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$`)

// Define test cases
tests := []struct {
name string
input string
expectMatch bool
}{
{
name: "Valid UUIDv7 with variant 8",
input: "01876395-3f9d-7c91-89a3-4f57e53a1a4b",
expectMatch: true,
},
{
name: "Valid UUIDv7 with variant a",
input: "01876395-3f9d-7c91-9f00-4f57e53a1a4b",
expectMatch: true,
},
{
name: "Invalid UUIDv7 - wrong version",
input: "01876395-3f9d-6c91-89a3-4f57e53a1a4b",
expectMatch: false,
},
{
name: "Invalid UUIDv7 - wrong variant",
input: "01876395-3f9d-7c91-7a00-4f57e53a1a4b",
expectMatch: false,
},
{
name: "Invalid UUIDv7 - extra characters",
input: "01876395-3f9d-7c91-89a3-4f57e53a1a4b123",
expectMatch: false,
},
{
name: "Invalid UUIDv7 - missing characters",
input: "01876395-3f9d-7c91-89a3-4f57e53a",
expectMatch: false,
},
}

// Iterate over test cases
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
match := uuidv7Regex.MatchString(tt.input)
assert.Equal(t, tt.expectMatch, match, "Regex match result did not match expectation")
})
}
}

func TestGenerateMessageID(t *testing.T) {
tests := []struct {
name string
mockFunc func() (uuid.UUID, error)
expected string
expectError bool
}{
{
name: "Valid UUID generation",
mockFunc: func() (uuid.UUID, error) {
return uuid.UUID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}, nil
},
expected: "01020304-0506-0708-090a-0b0c0d0e0f10",
expectError: false,
},
{
name: "Fallback UUID generation due to error",
mockFunc: func() (uuid.UUID, error) {
return uuid.Nil, errors.New("mock error")
},
expected: "", // Fallback UUIDs don't follow a fixed prefix but should not be empty
expectError: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
defaultUUIDGenerator = tt.mockFunc

if tt.expectError {
logBuf := &bytes.Buffer{}
stub.StubLoggerWith(logBuf)

got := GenerateMessageID()
assert.NotEmpty(t, got)

// Inspect logs
helpers.ValidateLog(t, "Issue generating uuidv7, using sha256 and timestamp instead", logBuf)

logBuf.Reset()
} else {
got := GenerateMessageID()

assert.Equal(t, tt.expected, got, "Expected UUID string to match")
}

// reset
defaultUUIDGenerator = uuid.NewUUID
})
}
defaultUUIDGenerator = func() (uuid.UUID, error) {
return uuid.UUID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}, nil
}

got := GenerateMessageID()
assert.Equal(t, "01020304-0506-0708-090a-0b0c0d0e0f10", got, "Expected correct UUID string")
}
9 changes: 5 additions & 4 deletions internal/file/file_manager_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ import (
"sync"
"sync/atomic"

"github.com/nginx/agent/v3/internal/datasource/proto"
"github.com/nginx/agent/v3/internal/model"

"github.com/cenkalti/backoff/v4"
"github.com/google/uuid"

mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
"github.com/nginx/agent/v3/internal/config"
"github.com/nginx/agent/v3/internal/grpc"
Expand Down Expand Up @@ -96,7 +97,7 @@ func (fms *FileManagerService) UpdateOverview(

request := &mpi.UpdateOverviewRequest{
MessageMeta: &mpi.MessageMeta{
MessageId: uuid.NewString(),
MessageId: proto.GenerateMessageID(),
CorrelationId: requestCorrelationID.Value.String(),
Timestamp: timestamppb.Now(),
},
Expand Down Expand Up @@ -168,7 +169,7 @@ func (fms *FileManagerService) UpdateFile(
Contents: contents,
},
MessageMeta: &mpi.MessageMeta{
MessageId: uuid.NewString(),
MessageId: proto.GenerateMessageID(),
CorrelationId: correlationID,
Timestamp: timestamppb.Now(),
},
Expand Down Expand Up @@ -325,7 +326,7 @@ func (fms *FileManagerService) fileUpdate(ctx context.Context, file *mpi.File) e
getFile := func() (*mpi.GetFileResponse, error) {
return fms.fileServiceClient.GetFile(ctx, &mpi.GetFileRequest{
MessageMeta: &mpi.MessageMeta{
MessageId: uuid.NewString(),
MessageId: proto.GenerateMessageID(),
CorrelationId: logger.GetCorrelationID(ctx),
Timestamp: timestamppb.Now(),
},
Expand Down
6 changes: 3 additions & 3 deletions internal/file/file_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ import (

"github.com/nginx/agent/v3/pkg/files"

"github.com/google/uuid"
mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
"github.com/nginx/agent/v3/internal/bus"
"github.com/nginx/agent/v3/internal/config"
"github.com/nginx/agent/v3/internal/datasource/proto"
"github.com/nginx/agent/v3/internal/grpc"
"github.com/nginx/agent/v3/internal/logger"
"github.com/nginx/agent/v3/internal/model"
Expand Down Expand Up @@ -319,7 +319,7 @@ func (fp *FilePlugin) handleConfigUploadRequest(ctx context.Context, msg *bus.Me

response := &mpi.DataPlaneResponse{
MessageMeta: &mpi.MessageMeta{
MessageId: uuid.NewString(),
MessageId: proto.GenerateMessageID(),
CorrelationId: correlationID,
Timestamp: timestamppb.Now(),
},
Expand All @@ -343,7 +343,7 @@ func (fp *FilePlugin) createDataPlaneResponse(correlationID string, status mpi.C
) *mpi.DataPlaneResponse {
return &mpi.DataPlaneResponse{
MessageMeta: &mpi.MessageMeta{
MessageId: uuid.NewString(),
MessageId: proto.GenerateMessageID(),
CorrelationId: correlationID,
Timestamp: timestamppb.Now(),
},
Expand Down
4 changes: 2 additions & 2 deletions internal/file/file_plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
"time"

"github.com/nginx/agent/v3/internal/bus/busfakes"
"github.com/nginx/agent/v3/internal/datasource/proto"

"github.com/google/uuid"
"google.golang.org/protobuf/types/known/timestamppb"

mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
Expand Down Expand Up @@ -441,7 +441,7 @@ func TestFilePlugin_Process_ConfigApplyRollbackCompleteTopic(t *testing.T) {

expectedResponse := &mpi.DataPlaneResponse{
MessageMeta: &mpi.MessageMeta{
MessageId: uuid.NewString(),
MessageId: proto.GenerateMessageID(),
CorrelationId: "dfsbhj6-bc92-30c1-a9c9-85591422068e",
Timestamp: timestamppb.Now(),
},
Expand Down
Loading

0 comments on commit 426612a

Please sign in to comment.