Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GODRIVER-2943 Re-organize client entity expected log messages #1366

Merged
merged 12 commits into from
Dec 7, 2023
Merged
5 changes: 4 additions & 1 deletion mongo/integration/unified/client_entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ func newClientEntity(ctx context.Context, em *EntityMap, entityOptions *entityOp
}

if olm := entityOptions.ObserveLogMessages; olm != nil {
clientLogger := newLogger(olm, expectedLogMessageCount(ctx))
expectedLogMessagesCount := expectedLogMessagesCount(ctx, entityOptions.ID)
ignoreLogMessages := ignoreLogMessages(ctx, entityOptions.ID)

clientLogger := newLogger(olm, expectedLogMessagesCount, ignoreLogMessages)

wrap := func(str string) options.LogLevel {
return options.LogLevel(logger.ParseLevel(str))
Expand Down
34 changes: 28 additions & 6 deletions mongo/integration/unified/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,23 @@ const (
failPointsKey ctxKey = "test-failpoints"
// targetedFailPointsKey is used to store a map from a fail point name to the host on which the fail point is set.
targetedFailPointsKey ctxKey = "test-targeted-failpoints"
// expectedLogMessageCountKey is used to store the number of log messages expected to be received by the test runner.
expectedLogMessageCountKey ctxKey = "test-expected-log-message-count"
clientLogMessagesKey ctxKey = "test-expected-log-message-count"
ignoreLogMessagesKey ctxKey = "test-ignore-log-message-count"
)

// newTestContext creates a new Context derived from ctx with values initialized to store the state required for test
// execution.
func newTestContext(
ctx context.Context,
entityMap *EntityMap,
expectedLogMessageCount int,
clientLogMessages []*clientLogMessages,
hasOperationalFailPoint bool,
) context.Context {
ctx = context.WithValue(ctx, operationalFailPointKey, hasOperationalFailPoint)
ctx = context.WithValue(ctx, entitiesKey, entityMap)
ctx = context.WithValue(ctx, failPointsKey, make(map[string]*mongo.Client))
ctx = context.WithValue(ctx, targetedFailPointsKey, make(map[string]string))
ctx = context.WithValue(ctx, expectedLogMessageCountKey, expectedLogMessageCount)
ctx = context.WithValue(ctx, clientLogMessagesKey, clientLogMessages)
return ctx
}

Expand Down Expand Up @@ -84,6 +84,28 @@ func entities(ctx context.Context) *EntityMap {
return ctx.Value(entitiesKey).(*EntityMap)
}

func expectedLogMessageCount(ctx context.Context) int {
return ctx.Value(expectedLogMessageCountKey).(int)
func expectedLogMessagesCount(ctx context.Context, clientID string) int {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UTR expects this data to be client-specific. Before this change, this data was held at the test level which is incorrect. Suggest storing log message information on the context at the client-level.

messages := ctx.Value(clientLogMessagesKey).([]*clientLogMessages)

count := 0
for _, message := range messages {
if message.Client == clientID {
count += len(message.LogMessages)
}
}

return count
}

func ignoreLogMessages(ctx context.Context, clientID string) []*logMessage {
messages := ctx.Value(clientLogMessagesKey).([]*clientLogMessages)

ignoreMessages := []*logMessage{}
for _, message := range messages {
if message.Client == clientID {
ignoreMessages = append(ignoreMessages, message.IgnoreMessages...)
}
}

return ignoreMessages
}
28 changes: 18 additions & 10 deletions mongo/integration/unified/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package unified

import (
"context"
"sync"

"go.mongodb.org/mongo-driver/internal/logger"
Expand All @@ -33,19 +34,21 @@ type Logger struct {
// orderMu guards the order value, which increments each time the "Info"
// method is called. This is necessary since "Info" could be called from
// multiple go routines, e.g. SDAM logs.
orderMu sync.RWMutex
logQueue chan orderedLogMessage
orderMu sync.RWMutex
logQueue chan orderedLogMessage
ignoreMessages []*logMessage
}

func newLogger(olm *observeLogMessages, bufSize int) *Logger {
func newLogger(olm *observeLogMessages, bufSize int, ignoreMessages []*logMessage) *Logger {
if olm == nil {
return nil
}

return &Logger{
lastOrder: 1,
logQueue: make(chan orderedLogMessage, bufSize),
bufSize: bufSize,
lastOrder: 1,
logQueue: make(chan orderedLogMessage, bufSize),
bufSize: bufSize,
ignoreMessages: ignoreMessages,
}
}

Expand All @@ -65,8 +68,6 @@ func (log *Logger) Info(level int, msg string, args ...interface{}) {
return
}

defer func() { log.lastOrder++ }()

// Add the Diff back to the level, as there is no need to create a
// logging offset.
level = level + logger.DiffToInfo
Expand All @@ -76,12 +77,19 @@ func (log *Logger) Info(level int, msg string, args ...interface{}) {
panic(err)
}

for _, ignoreMessage := range log.ignoreMessages {
if err := verifyLogMatch(context.Background(), ignoreMessage, logMessage); err == nil {
return
}
}

defer func() { log.lastOrder++ }()

// Send the log message to the "orderedLogMessage" channel for
// validation.
log.logQueue <- orderedLogMessage{
order: log.lastOrder + 1,
logMessage: logMessage,
}
logMessage: logMessage}

// If the order has reached the buffer size, then close the channel.
if log.lastOrder == log.bufSize {
Expand Down
20 changes: 1 addition & 19 deletions mongo/integration/unified/logger_verification.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,6 @@ type clientLogMessages struct {
LogMessages []*logMessage `bson:"messages"`
}

// ignore checks to see if the message is in the "IgnoreMessages" slice.
func (clm clientLogMessages) ignore(ctx context.Context, msg *logMessage) bool {
for _, ignoreMessage := range clm.IgnoreMessages {
if err := verifyLogMatch(ctx, ignoreMessage, msg); err == nil {
return true
}
}

return false
}

// logMessageValidator defines the expectation for log messages across all
// clients.
type logMessageValidator struct {
Expand Down Expand Up @@ -191,8 +180,7 @@ type logQueues struct {
}

// partitionLogQueue will partition the expected logs into "unordered" and
// "ordered" log channels. This function will also remove any logs in the
// "ignoreMessages" list for a client.
// "ordered" log channels.
func partitionLogQueue(ctx context.Context, exp *clientLogMessages) logQueues {
orderedLogCh := make(chan *logMessage, len(exp.LogMessages))
unorderedLogCh := make(chan *logMessage, len(exp.LogMessages))
Expand Down Expand Up @@ -241,12 +229,6 @@ func matchOrderedLogs(ctx context.Context, logs logQueues) <-chan error {
defer close(errs)

for actual := range logs.ordered {
// Ignore logs that are in the "IngoreMessages" slice of
// the expected results.
if logs.expected.ignore(ctx, actual) {
continue
}

expected := expLogMessages[0]
if expected == nil {
continue
Expand Down
84 changes: 0 additions & 84 deletions mongo/integration/unified/logger_verification_test.go

This file was deleted.

10 changes: 1 addition & 9 deletions mongo/integration/unified/unified_spec_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ var (
// TODO(GODRIVER-2843): Fix and unskip these test cases.
"Find operation with snapshot": "Test fails frequently. See GODRIVER-2843",
"Write commands with snapshot session do not affect snapshot reads": "Test fails frequently. See GODRIVER-2843",
// TODO(GODRIVER-2943): Fix and unskip this test case.
"Topology lifecycle": "Test times out. See GODRIVER-2943",
}

logMessageValidatorTimeout = 10 * time.Millisecond
Expand Down Expand Up @@ -223,13 +221,7 @@ func (tc *TestCase) Run(ls LoggerSkipper) error {
return fmt.Errorf("schema version %q not supported: %v", tc.schemaVersion, err)
}

// Count the number of expected log messages over all clients.
var expectedLogCount int
for _, clientLog := range tc.ExpectLogMessages {
expectedLogCount += len(clientLog.LogMessages)
}

testCtx := newTestContext(context.Background(), tc.entities, expectedLogCount, tc.setsFailPoint())
testCtx := newTestContext(context.Background(), tc.entities, tc.ExpectLogMessages, tc.setsFailPoint())

defer func() {
// If anything fails while doing test cleanup, we only log the error because the actual test may have already
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,22 +220,6 @@
}
}
},
{
"level": "debug",
"component": "topology",
"data": {
"message": "Topology description changed",
"topologyId": {
"$$exists": true
},
"previousDescription": {
"$$exists": true
},
"newDescription": {
"$$exists": true
}
}
},
{
"level": "debug",
"component": "topology",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ tests:
topologyId: { $$exists: true }
serverHost: { $$type: string }
serverPort: { $$type: [int, long] }
# TODO(GODRIVER-2967): The following log message has been removed from
# the JSON analogue because it assumes that
# "TopologyDescriptionChangedEvent" should occur when a topolgoy is
# closed. This behavior is not clearly defined anywhere and some
# drivers support and some don't.
#
# Need to sync whenever GODRIVER-2967 is unblocked.
- level: debug
component: topology
data:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,22 +187,6 @@
}
}
},
{
"level": "debug",
"component": "topology",
"data": {
"message": "Topology description changed",
"topologyId": {
"$$exists": true
},
"previousDescription": {
"$$exists": true
},
"newDescription": {
"$$exists": true
}
}
},
{
"level": "debug",
"component": "topology",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,13 @@ tests:
topologyId: { $$exists: true }
serverHost: { $$type: string }
serverPort: { $$type: [int, long] }
# TODO(GODRIVER-2967): The following log message has been removed from
# the JSON analogue because it assumes that
# "TopologyDescriptionChangedEvent" should occur when a topolgoy is
# closed. This behavior is not clearly defined anywhere and some
# drivers support and some don't.
#
# Need to sync whenever GODRIVER-2967 is unblocked.
- level: debug
component: topology
data:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,22 +162,6 @@
}
}
},
{
"level": "debug",
"component": "topology",
"data": {
"message": "Topology description changed",
"topologyId": {
"$$exists": true
},
"previousDescription": {
"$$exists": true
},
"newDescription": {
"$$exists": true
}
}
},
{
"level": "debug",
"component": "topology",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ tests:
topologyId: { $$exists: true }
serverHost: { $$type: string }
serverPort: { $$type: [int, long] }
# TODO(GODRIVER-2967): The following log message has been removed from
# the JSON analogue because it assumes that
# "TopologyDescriptionChangedEvent" should occur when a topolgoy is
# closed. This behavior is not clearly defined anywhere and some
# drivers support and some don't.
#
# Need to sync whenever GODRIVER-2967 is unblocked.
- level: debug
component: topology
data:
Expand Down
Loading
Loading