From 620f9481b379e5220152926d826bf4db895f5e05 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Fri, 1 Sep 2023 16:15:02 -0400 Subject: [PATCH 1/6] GODRIVER-2904 Add environment log. --- .../topology/polling_srv_records_test.go | 1 + x/mongo/driver/topology/topology.go | 75 ++++++- x/mongo/driver/topology/topology_test.go | 206 ++++++++++++++++++ 3 files changed, 277 insertions(+), 5 deletions(-) diff --git a/x/mongo/driver/topology/polling_srv_records_test.go b/x/mongo/driver/topology/polling_srv_records_test.go index 0ca5c7cbce..7484109d4e 100644 --- a/x/mongo/driver/topology/polling_srv_records_test.go +++ b/x/mongo/driver/topology/polling_srv_records_test.go @@ -105,6 +105,7 @@ func (ss serverSorter) Less(i, j int) bool { } func compareHosts(t *testing.T, received []description.Server, expected []string) { + t.Helper() if len(received) != len(expected) { t.Fatalf("Number of hosts in topology does not match expected value. Got %v; want %v.", len(received), len(expected)) } diff --git a/x/mongo/driver/topology/topology.go b/x/mongo/driver/topology/topology.go index b0683021ee..c91180a10d 100644 --- a/x/mongo/driver/topology/topology.go +++ b/x/mongo/driver/topology/topology.go @@ -69,6 +69,63 @@ const ( SingleMode ) +type hostEnv int + +const ( + genuine hostEnv = iota + cosmosDB + doumentDB +) + +var ( + CosmosDBLog = `You appear to be connected to a CosmosDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/cosmosdb` + DoumentDBLog = `You appear to be connected to a DoumentDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/documentdb` +) + +type envMap struct { + host string + env hostEnv +} + +type hostLogger struct { + logger *logger.Logger + envs []envMap + logs map[hostEnv]*string +} + +func newHostLogger(l *logger.Logger) *hostLogger { + return &hostLogger{ + logger: l, + envs: []envMap{ + {".cosmos.azure.com", cosmosDB}, + {".docdb.amazonaws.com", doumentDB}, + {".docdb-elastic.amazonaws.com", doumentDB}, + }, + logs: map[hostEnv]*string{ + cosmosDB: &CosmosDBLog, + doumentDB: &DoumentDBLog, + }, + } +} + +func (l *hostLogger) log(host string) { + if h, _, err := net.SplitHostPort(host); err == nil { + host = h + } + for _, em := range l.envs { + if strings.HasSuffix(host, em.host) { + if log, ok := l.logs[em.env]; ok && log != nil { + l.logger.Print(logger.LevelInfo, + logger.ComponentTopology, + *log, + ) + l.logs[em.env] = nil + } + break + } + } +} + // Topology represents a MongoDB deployment. type Topology struct { state int64 @@ -321,13 +378,14 @@ func (t *Topology) Connect() error { } t.serversLock.Unlock() + uri, err := url.Parse(t.cfg.URI) + if err != nil { + return err + } + parsedHosts := strings.Split(uri.Host, ",") if t.pollingRequired { - uri, err := url.Parse(t.cfg.URI) - if err != nil { - return err - } // sanity check before passing the hostname to resolver - if parsedHosts := strings.Split(uri.Host, ","); len(parsedHosts) != 1 { + if len(parsedHosts) != 1 { return fmt.Errorf("URI with SRV must include one and only one hostname") } _, _, err = net.SplitHostPort(uri.Host) @@ -338,6 +396,11 @@ func (t *Topology) Connect() error { } go t.pollSRVRecords(uri.Host) t.pollingwg.Add(1) + } else { + logger := newHostLogger(t.cfg.logger) + for _, host := range parsedHosts { + logger.log(host) + } } t.subscriptionsClosed = false // explicitly set in case topology was disconnected and then reconnected @@ -817,10 +880,12 @@ func (t *Topology) processSRVResults(parsedHosts []string) bool { }) } // Add all added hosts until the number of servers reaches srvMaxHosts. + logger := newHostLogger(t.cfg.logger) for _, a := range diff.Added { if t.cfg.SRVMaxHosts > 0 && len(t.servers) >= t.cfg.SRVMaxHosts { break } + logger.log(a) addr := address.Address(a).Canonicalize() _ = t.addServer(addr) t.fsm.addServer(addr) diff --git a/x/mongo/driver/topology/topology_test.go b/x/mongo/driver/topology/topology_test.go index 773a8b6475..05491f8b5e 100644 --- a/x/mongo/driver/topology/topology_test.go +++ b/x/mongo/driver/topology/topology_test.go @@ -693,6 +693,212 @@ func TestTopologyConstruction(t *testing.T) { }) } +type mockLogSink struct { + msgs []string +} + +func (s *mockLogSink) Info(level int, msg string, keysAndValues ...interface{}) { + s.msgs = append(s.msgs, msg) +} +func (*mockLogSink) Error(error, string, ...interface{}) { + // Do nothing. +} + +// Note: SRV connection strings are intentionally untested, since initial +// lookup responses cannot be easily mocked. +func TestTopologyConstructionLogging(t *testing.T) { + sink := &mockLogSink{} + loggerOptions := options. + Logger(). + SetSink(sink). + SetComponentLevel(options.LogComponentTopology, options.LogLevelInfo) + t.Run("CosmosDB URIs", func(t *testing.T) { + testCases := []struct { + name string + uri string + msgs []string + }{ + { + name: "normal", + uri: "mongodb://a.mongo.cosmos.azure.com:19555/", + msgs: []string{CosmosDBLog}, + }, + { + name: "multiple hosts", + uri: "mongodb://a.mongo.cosmos.azure.com:1955,b.mongo.cosmos.azure.com:19555/", + msgs: []string{CosmosDBLog}, + }, + { + name: "case-insensitive matching", + uri: "mongodb://a.MONGO.COSMOS.AZURE.COM:19555/", + msgs: []string{}, + }, + { + name: "Mixing genuine and nongenuine hosts (unlikely in practice)", + uri: "mongodb://a.example.com:27017,b.mongo.cosmos.azure.com:19555/", + msgs: []string{CosmosDBLog}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + defer func() { + sink.msgs = []string{} + }() + cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(loggerOptions), nil) + require.Nil(t, err, "error constructing topology config: %v", err) + + topo, err := New(cfg) + require.Nil(t, err, "topology.New error: %v", err) + + err = topo.Connect() + require.Nil(t, err, "Connect error: %v", err) + + require.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) + }) + } + }) + t.Run("DocumentDB URIs", func(t *testing.T) { + testCases := []struct { + name string + uri string + msgs []string + }{ + { + name: "normal", + uri: "mongodb://a.docdb.amazonaws.com:27017/", + msgs: []string{DoumentDBLog}, + }, + { + name: "normal", + uri: "mongodb://a.docdb-elastic.amazonaws.com:27017/", + msgs: []string{DoumentDBLog}, + }, + { + name: "multiple hosts", + uri: "mongodb://a.docdb.amazonaws.com:27017,a.docdb-elastic.amazonaws.com:27017/", + msgs: []string{DoumentDBLog}, + }, + { + name: "case-insensitive matching", + uri: "mongodb://a.DOCDB.AMAZONAWS.COM:27017/", + msgs: []string{}, + }, + { + name: "case-insensitive matching", + uri: "mongodb://a.DOCDB-ELASTIC.AMAZONAWS.COM:27017/", + msgs: []string{}, + }, + { + name: "Mixing genuine and nongenuine hosts (unlikely in practice)", + uri: "mongodb://a.example.com:27017,b.docdb.amazonaws.com:27017/", + msgs: []string{DoumentDBLog}, + }, + { + name: "Mixing genuine and nongenuine hosts (unlikely in practice)", + uri: "mongodb://a.example.com:27017,b.docdb-elastic.amazonaws.com:27017/", + msgs: []string{DoumentDBLog}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + defer func() { + sink.msgs = []string{} + }() + cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(loggerOptions), nil) + require.Nil(t, err, "error constructing topology config: %v", err) + + topo, err := New(cfg) + require.Nil(t, err, "topology.New error: %v", err) + + err = topo.Connect() + require.Nil(t, err, "Connect error: %v", err) + + require.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) + }) + } + }) + t.Run("Mixing CosmosDB and DocumentDB URIs", func(t *testing.T) { + testCases := []struct { + name string + uri string + msgs []string + }{ + { + name: "Mixing hosts", + uri: "mongodb://a.mongo.cosmos.azure.com:19555,a.docdb.amazonaws.com:27017/", + msgs: []string{CosmosDBLog, DoumentDBLog}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + defer func() { + sink.msgs = []string{} + }() + cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(loggerOptions), nil) + require.Nil(t, err, "error constructing topology config: %v", err) + + topo, err := New(cfg) + require.Nil(t, err, "topology.New error: %v", err) + + err = topo.Connect() + require.Nil(t, err, "Connect error: %v", err) + + require.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) + }) + } + }) + t.Run("genuine URIs", func(t *testing.T) { + testCases := []struct { + name string + uri string + msgs []string + }{ + { + name: "normal", + uri: "mongodb://a.example.com:27017/", + msgs: []string{}, + }, + { + name: "multiple hosts", + uri: "mongodb://a.example.com:27017,b.example.com:27017/", + msgs: []string{}, + }, + { + name: "unexpected suffix", + uri: "mongodb://a.mongo.cosmos.azure.com.tld:19555/", + msgs: []string{}, + }, + { + name: "unexpected suffix", + uri: "mongodb://a.docdb.amazonaws.com.tld:27017/", + msgs: []string{}, + }, + { + name: "unexpected suffix", + uri: "mongodb://a.docdb-elastic.amazonaws.com.tld:27017/", + msgs: []string{}, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + defer func() { + sink.msgs = []string{} + }() + cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(loggerOptions), nil) + require.Nil(t, err, "error constructing topology config: %v", err) + + topo, err := New(cfg) + require.Nil(t, err, "topology.New error: %v", err) + + err = topo.Connect() + require.Nil(t, err, "Connect error: %v", err) + + require.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) + }) + } + }) +} + type inWindowServer struct { Address string `json:"address"` Type string `json:"type"` From 613cdc96b75cc38de10224bee9f0fbae06ae55b7 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Tue, 5 Sep 2023 13:27:21 -0400 Subject: [PATCH 2/6] minor updates --- x/mongo/driver/topology/topology.go | 8 ++++---- x/mongo/driver/topology/topology_test.go | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/x/mongo/driver/topology/topology.go b/x/mongo/driver/topology/topology.go index c91180a10d..2e3918a0c4 100644 --- a/x/mongo/driver/topology/topology.go +++ b/x/mongo/driver/topology/topology.go @@ -78,8 +78,8 @@ const ( ) var ( - CosmosDBLog = `You appear to be connected to a CosmosDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/cosmosdb` - DoumentDBLog = `You appear to be connected to a DoumentDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/documentdb` + cosmosDBLog = `You appear to be connected to a CosmosDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/cosmosdb` + doumentDBLog = `You appear to be connected to a DoumentDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/documentdb` ) type envMap struct { @@ -102,8 +102,8 @@ func newHostLogger(l *logger.Logger) *hostLogger { {".docdb-elastic.amazonaws.com", doumentDB}, }, logs: map[hostEnv]*string{ - cosmosDB: &CosmosDBLog, - doumentDB: &DoumentDBLog, + cosmosDB: &cosmosDBLog, + doumentDB: &doumentDBLog, }, } } diff --git a/x/mongo/driver/topology/topology_test.go b/x/mongo/driver/topology/topology_test.go index 05491f8b5e..80bf5b5217 100644 --- a/x/mongo/driver/topology/topology_test.go +++ b/x/mongo/driver/topology/topology_test.go @@ -697,7 +697,7 @@ type mockLogSink struct { msgs []string } -func (s *mockLogSink) Info(level int, msg string, keysAndValues ...interface{}) { +func (s *mockLogSink) Info(_ int, msg string, _ ...interface{}) { s.msgs = append(s.msgs, msg) } func (*mockLogSink) Error(error, string, ...interface{}) { @@ -721,12 +721,12 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "normal", uri: "mongodb://a.mongo.cosmos.azure.com:19555/", - msgs: []string{CosmosDBLog}, + msgs: []string{cosmosDBLog}, }, { name: "multiple hosts", uri: "mongodb://a.mongo.cosmos.azure.com:1955,b.mongo.cosmos.azure.com:19555/", - msgs: []string{CosmosDBLog}, + msgs: []string{cosmosDBLog}, }, { name: "case-insensitive matching", @@ -736,7 +736,7 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "Mixing genuine and nongenuine hosts (unlikely in practice)", uri: "mongodb://a.example.com:27017,b.mongo.cosmos.azure.com:19555/", - msgs: []string{CosmosDBLog}, + msgs: []string{cosmosDBLog}, }, } for _, tc := range testCases { @@ -766,17 +766,17 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "normal", uri: "mongodb://a.docdb.amazonaws.com:27017/", - msgs: []string{DoumentDBLog}, + msgs: []string{doumentDBLog}, }, { name: "normal", uri: "mongodb://a.docdb-elastic.amazonaws.com:27017/", - msgs: []string{DoumentDBLog}, + msgs: []string{doumentDBLog}, }, { name: "multiple hosts", uri: "mongodb://a.docdb.amazonaws.com:27017,a.docdb-elastic.amazonaws.com:27017/", - msgs: []string{DoumentDBLog}, + msgs: []string{doumentDBLog}, }, { name: "case-insensitive matching", @@ -791,12 +791,12 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "Mixing genuine and nongenuine hosts (unlikely in practice)", uri: "mongodb://a.example.com:27017,b.docdb.amazonaws.com:27017/", - msgs: []string{DoumentDBLog}, + msgs: []string{doumentDBLog}, }, { name: "Mixing genuine and nongenuine hosts (unlikely in practice)", uri: "mongodb://a.example.com:27017,b.docdb-elastic.amazonaws.com:27017/", - msgs: []string{DoumentDBLog}, + msgs: []string{doumentDBLog}, }, } for _, tc := range testCases { @@ -826,7 +826,7 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "Mixing hosts", uri: "mongodb://a.mongo.cosmos.azure.com:19555,a.docdb.amazonaws.com:27017/", - msgs: []string{CosmosDBLog, DoumentDBLog}, + msgs: []string{cosmosDBLog, doumentDBLog}, }, } for _, tc := range testCases { From ed734f1369b306c6bdcc3fef7ac52d79e40ea87e Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Wed, 6 Sep 2023 08:47:41 -0400 Subject: [PATCH 3/6] fix typos --- x/mongo/driver/topology/topology.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/mongo/driver/topology/topology.go b/x/mongo/driver/topology/topology.go index 2e3918a0c4..8d10492db2 100644 --- a/x/mongo/driver/topology/topology.go +++ b/x/mongo/driver/topology/topology.go @@ -78,8 +78,8 @@ const ( ) var ( - cosmosDBLog = `You appear to be connected to a CosmosDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/cosmosdb` - doumentDBLog = `You appear to be connected to a DoumentDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/documentdb` + cosmosDBLog = `You appear to be connected to a CosmosDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/cosmosdb` + documentDBLog = `You appear to be connected to a DocumentDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/documentdb` ) type envMap struct { @@ -103,7 +103,7 @@ func newHostLogger(l *logger.Logger) *hostLogger { }, logs: map[hostEnv]*string{ cosmosDB: &cosmosDBLog, - doumentDB: &doumentDBLog, + doumentDB: &documentDBLog, }, } } From 60d159d70d97b3949c4a9433cd827970b32e0679 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Wed, 6 Sep 2023 11:13:00 -0400 Subject: [PATCH 4/6] fix typos --- x/mongo/driver/topology/topology_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/x/mongo/driver/topology/topology_test.go b/x/mongo/driver/topology/topology_test.go index 80bf5b5217..f346880849 100644 --- a/x/mongo/driver/topology/topology_test.go +++ b/x/mongo/driver/topology/topology_test.go @@ -766,17 +766,17 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "normal", uri: "mongodb://a.docdb.amazonaws.com:27017/", - msgs: []string{doumentDBLog}, + msgs: []string{documentDBLog}, }, { name: "normal", uri: "mongodb://a.docdb-elastic.amazonaws.com:27017/", - msgs: []string{doumentDBLog}, + msgs: []string{documentDBLog}, }, { name: "multiple hosts", uri: "mongodb://a.docdb.amazonaws.com:27017,a.docdb-elastic.amazonaws.com:27017/", - msgs: []string{doumentDBLog}, + msgs: []string{documentDBLog}, }, { name: "case-insensitive matching", @@ -791,12 +791,12 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "Mixing genuine and nongenuine hosts (unlikely in practice)", uri: "mongodb://a.example.com:27017,b.docdb.amazonaws.com:27017/", - msgs: []string{doumentDBLog}, + msgs: []string{documentDBLog}, }, { name: "Mixing genuine and nongenuine hosts (unlikely in practice)", uri: "mongodb://a.example.com:27017,b.docdb-elastic.amazonaws.com:27017/", - msgs: []string{doumentDBLog}, + msgs: []string{documentDBLog}, }, } for _, tc := range testCases { @@ -826,7 +826,7 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "Mixing hosts", uri: "mongodb://a.mongo.cosmos.azure.com:19555,a.docdb.amazonaws.com:27017/", - msgs: []string{cosmosDBLog, doumentDBLog}, + msgs: []string{cosmosDBLog, documentDBLog}, }, } for _, tc := range testCases { From ff92957acf77c928eb598925c1fac33f47a5f766 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Thu, 7 Sep 2023 18:34:11 -0400 Subject: [PATCH 5/6] log before SRV lookup --- x/mongo/driver/topology/topology.go | 117 ++++++++++------------- x/mongo/driver/topology/topology_test.go | 23 +++-- 2 files changed, 63 insertions(+), 77 deletions(-) diff --git a/x/mongo/driver/topology/topology.go b/x/mongo/driver/topology/topology.go index 8d10492db2..d8a31308cf 100644 --- a/x/mongo/driver/topology/topology.go +++ b/x/mongo/driver/topology/topology.go @@ -74,57 +74,26 @@ type hostEnv int const ( genuine hostEnv = iota cosmosDB - doumentDB + documentDB ) -var ( - cosmosDBLog = `You appear to be connected to a CosmosDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/cosmosdb` - documentDBLog = `You appear to be connected to a DocumentDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/documentdb` +const ( + cosmosDBMsg = `You appear to be connected to a CosmosDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/cosmosdb` + documentDBMsg = `You appear to be connected to a DocumentDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/documentdb` ) -type envMap struct { - host string - env hostEnv -} - -type hostLogger struct { - logger *logger.Logger - envs []envMap - logs map[hostEnv]*string -} - -func newHostLogger(l *logger.Logger) *hostLogger { - return &hostLogger{ - logger: l, - envs: []envMap{ - {".cosmos.azure.com", cosmosDB}, - {".docdb.amazonaws.com", doumentDB}, - {".docdb-elastic.amazonaws.com", doumentDB}, - }, - logs: map[hostEnv]*string{ - cosmosDB: &cosmosDBLog, - doumentDB: &documentDBLog, - }, +var ( + thirdPartySuffixes = map[string]hostEnv{ + ".cosmos.azure.com": cosmosDB, + ".docdb.amazonaws.com": documentDB, + ".docdb-elastic.amazonaws.com": documentDB, } -} -func (l *hostLogger) log(host string) { - if h, _, err := net.SplitHostPort(host); err == nil { - host = h - } - for _, em := range l.envs { - if strings.HasSuffix(host, em.host) { - if log, ok := l.logs[em.env]; ok && log != nil { - l.logger.Print(logger.LevelInfo, - logger.ComponentTopology, - *log, - ) - l.logs[em.env] = nil - } - break - } + thirdPartyMessages = map[hostEnv]string{ + cosmosDB: cosmosDBMsg, + documentDB: documentDBMsg, } -} +) // Topology represents a MongoDB deployment. type Topology struct { @@ -218,13 +187,13 @@ func New(cfg *Config) (*Topology, error) { return t, nil } -func mustLogTopologyMessage(topo *Topology) bool { +func mustLogTopologyMessage(topo *Topology, level logger.Level) bool { return topo.cfg.logger != nil && topo.cfg.logger.LevelComponentEnabled( - logger.LevelDebug, logger.ComponentTopology) + level, logger.ComponentTopology) } -func logTopologyMessage(topo *Topology, msg string, keysAndValues ...interface{}) { - topo.cfg.logger.Print(logger.LevelDebug, +func logTopologyMessage(topo *Topology, level logger.Level, msg string, keysAndValues ...interface{}) { + topo.cfg.logger.Print(level, logger.ComponentTopology, msg, logger.SerializeTopology(logger.Topology{ @@ -240,8 +209,8 @@ func mustLogServerSelection(topo *Topology, level logger.Level) bool { func logServerSelection( ctx context.Context, - level logger.Level, topo *Topology, + level logger.Level, msg string, srvSelector description.ServerSelector, keysAndValues ...interface{}, @@ -281,7 +250,7 @@ func logServerSelectionSucceeded( portInt64, _ := strconv.ParseInt(port, 10, 32) - logServerSelection(ctx, logger.LevelDebug, topo, logger.ServerSelectionSucceeded, srvSelector, + logServerSelection(ctx, topo, logger.LevelDebug, logger.ServerSelectionSucceeded, srvSelector, logger.KeyServerHost, host, logger.KeyServerPort, portInt64) } @@ -292,7 +261,7 @@ func logServerSelectionFailed( srvSelector description.ServerSelector, err error, ) { - logServerSelection(ctx, logger.LevelDebug, topo, logger.ServerSelectionFailed, srvSelector, + logServerSelection(ctx, topo, logger.LevelDebug, logger.ServerSelectionFailed, srvSelector, logger.KeyFailure, err.Error()) } @@ -383,6 +352,25 @@ func (t *Topology) Connect() error { return err } parsedHosts := strings.Split(uri.Host, ",") + if mustLogTopologyMessage(t, logger.LevelInfo) { + hostSet := make(map[hostEnv]bool) + for _, host := range parsedHosts { + if h, _, err := net.SplitHostPort(host); err == nil { + host = h + } + for suffix, env := range thirdPartySuffixes { + if !strings.HasSuffix(host, suffix) { + continue + } + if logged, ok := hostSet[env]; ok && logged { + break + } else { + hostSet[env] = true + logTopologyMessage(t, logger.LevelInfo, thirdPartyMessages[env]) + } + } + } + } if t.pollingRequired { // sanity check before passing the hostname to resolver if len(parsedHosts) != 1 { @@ -396,11 +384,6 @@ func (t *Topology) Connect() error { } go t.pollSRVRecords(uri.Host) t.pollingwg.Add(1) - } else { - logger := newHostLogger(t.cfg.logger) - for _, host := range parsedHosts { - logger.log(host) - } } t.subscriptionsClosed = false // explicitly set in case topology was disconnected and then reconnected @@ -555,7 +538,7 @@ func (t *Topology) SelectServer(ctx context.Context, ss description.ServerSelect if !doneOnce { if mustLogServerSelection(t, logger.LevelDebug) { - logServerSelection(ctx, logger.LevelDebug, t, logger.ServerSelectionStarted, ss) + logServerSelection(ctx, t, logger.LevelDebug, logger.ServerSelectionStarted, ss) } // for the first pass, select a server from the current description. @@ -594,7 +577,7 @@ func (t *Topology) SelectServer(ctx context.Context, ss description.ServerSelect elapsed := time.Since(startTime) remainingTimeMS := t.cfg.ServerSelectionTimeout - elapsed - logServerSelection(ctx, logger.LevelInfo, t, logger.ServerSelectionWaiting, ss, + logServerSelection(ctx, t, logger.LevelInfo, logger.ServerSelectionWaiting, ss, logger.KeyRemainingTimeMS, remainingTimeMS.Milliseconds()) } @@ -880,12 +863,10 @@ func (t *Topology) processSRVResults(parsedHosts []string) bool { }) } // Add all added hosts until the number of servers reaches srvMaxHosts. - logger := newHostLogger(t.cfg.logger) for _, a := range diff.Added { if t.cfg.SRVMaxHosts > 0 && len(t.servers) >= t.cfg.SRVMaxHosts { break } - logger.log(a) addr := address.Address(a).Canonicalize() _ = t.addServer(addr) t.fsm.addServer(addr) @@ -1035,7 +1016,7 @@ func (t *Topology) publishServerClosedEvent(addr address.Address) { t.cfg.ServerMonitor.ServerClosed(serverClosed) } - if mustLogTopologyMessage(t) { + if mustLogTopologyMessage(t, logger.LevelDebug) { serverHost, serverPort, err := net.SplitHostPort(addr.String()) if err != nil { serverHost = addr.String() @@ -1044,7 +1025,7 @@ func (t *Topology) publishServerClosedEvent(addr address.Address) { portInt64, _ := strconv.ParseInt(serverPort, 10, 32) - logTopologyMessage(t, logger.TopologyServerClosed, + logTopologyMessage(t, logger.LevelDebug, logger.TopologyServerClosed, logger.KeyServerHost, serverHost, logger.KeyServerPort, portInt64) } @@ -1062,8 +1043,8 @@ func (t *Topology) publishTopologyDescriptionChangedEvent(prev description.Topol t.cfg.ServerMonitor.TopologyDescriptionChanged(topologyDescriptionChanged) } - if mustLogTopologyMessage(t) { - logTopologyMessage(t, logger.TopologyDescriptionChanged, + if mustLogTopologyMessage(t, logger.LevelDebug) { + logTopologyMessage(t, logger.LevelDebug, logger.TopologyDescriptionChanged, logger.KeyPreviousDescription, prev.String(), logger.KeyNewDescription, current.String()) } @@ -1079,8 +1060,8 @@ func (t *Topology) publishTopologyOpeningEvent() { t.cfg.ServerMonitor.TopologyOpening(topologyOpening) } - if mustLogTopologyMessage(t) { - logTopologyMessage(t, logger.TopologyOpening) + if mustLogTopologyMessage(t, logger.LevelDebug) { + logTopologyMessage(t, logger.LevelDebug, logger.TopologyOpening) } } @@ -1094,7 +1075,7 @@ func (t *Topology) publishTopologyClosedEvent() { t.cfg.ServerMonitor.TopologyClosed(topologyClosed) } - if mustLogTopologyMessage(t) { - logTopologyMessage(t, logger.TopologyClosed) + if mustLogTopologyMessage(t, logger.LevelDebug) { + logTopologyMessage(t, logger.LevelDebug, logger.TopologyClosed) } } diff --git a/x/mongo/driver/topology/topology_test.go b/x/mongo/driver/topology/topology_test.go index f346880849..fe65a3b5b4 100644 --- a/x/mongo/driver/topology/topology_test.go +++ b/x/mongo/driver/topology/topology_test.go @@ -721,12 +721,12 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "normal", uri: "mongodb://a.mongo.cosmos.azure.com:19555/", - msgs: []string{cosmosDBLog}, + msgs: []string{cosmosDBMsg}, }, { name: "multiple hosts", uri: "mongodb://a.mongo.cosmos.azure.com:1955,b.mongo.cosmos.azure.com:19555/", - msgs: []string{cosmosDBLog}, + msgs: []string{cosmosDBMsg}, }, { name: "case-insensitive matching", @@ -736,7 +736,7 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "Mixing genuine and nongenuine hosts (unlikely in practice)", uri: "mongodb://a.example.com:27017,b.mongo.cosmos.azure.com:19555/", - msgs: []string{cosmosDBLog}, + msgs: []string{cosmosDBMsg}, }, } for _, tc := range testCases { @@ -766,17 +766,17 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "normal", uri: "mongodb://a.docdb.amazonaws.com:27017/", - msgs: []string{documentDBLog}, + msgs: []string{documentDBMsg}, }, { name: "normal", uri: "mongodb://a.docdb-elastic.amazonaws.com:27017/", - msgs: []string{documentDBLog}, + msgs: []string{documentDBMsg}, }, { name: "multiple hosts", uri: "mongodb://a.docdb.amazonaws.com:27017,a.docdb-elastic.amazonaws.com:27017/", - msgs: []string{documentDBLog}, + msgs: []string{documentDBMsg}, }, { name: "case-insensitive matching", @@ -791,12 +791,12 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "Mixing genuine and nongenuine hosts (unlikely in practice)", uri: "mongodb://a.example.com:27017,b.docdb.amazonaws.com:27017/", - msgs: []string{documentDBLog}, + msgs: []string{documentDBMsg}, }, { name: "Mixing genuine and nongenuine hosts (unlikely in practice)", uri: "mongodb://a.example.com:27017,b.docdb-elastic.amazonaws.com:27017/", - msgs: []string{documentDBLog}, + msgs: []string{documentDBMsg}, }, } for _, tc := range testCases { @@ -826,7 +826,7 @@ func TestTopologyConstructionLogging(t *testing.T) { { name: "Mixing hosts", uri: "mongodb://a.mongo.cosmos.azure.com:19555,a.docdb.amazonaws.com:27017/", - msgs: []string{cosmosDBLog, documentDBLog}, + msgs: []string{cosmosDBMsg, documentDBMsg}, }, } for _, tc := range testCases { @@ -858,6 +858,11 @@ func TestTopologyConstructionLogging(t *testing.T) { uri: "mongodb://a.example.com:27017/", msgs: []string{}, }, + { + name: "srv", + uri: "mongodb+srv://test22.test.build.10gen.cc/?srvServiceName=customname", + msgs: []string{}, + }, { name: "multiple hosts", uri: "mongodb://a.example.com:27017,b.example.com:27017/", From 13ec0df994194934b1d61fae0cb9b72dc2693ed0 Mon Sep 17 00:00:00 2001 From: Qingyang Hu Date: Mon, 11 Sep 2023 18:21:57 -0400 Subject: [PATCH 6/6] improve logic --- x/mongo/driver/topology/topology.go | 74 +++++++++------------- x/mongo/driver/topology/topology_test.go | 81 +++++++++++++++--------- 2 files changed, 83 insertions(+), 72 deletions(-) diff --git a/x/mongo/driver/topology/topology.go b/x/mongo/driver/topology/topology.go index d8a31308cf..b79efed4ed 100644 --- a/x/mongo/driver/topology/topology.go +++ b/x/mongo/driver/topology/topology.go @@ -69,32 +69,6 @@ const ( SingleMode ) -type hostEnv int - -const ( - genuine hostEnv = iota - cosmosDB - documentDB -) - -const ( - cosmosDBMsg = `You appear to be connected to a CosmosDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/cosmosdb` - documentDBMsg = `You appear to be connected to a DocumentDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/documentdb` -) - -var ( - thirdPartySuffixes = map[string]hostEnv{ - ".cosmos.azure.com": cosmosDB, - ".docdb.amazonaws.com": documentDB, - ".docdb-elastic.amazonaws.com": documentDB, - } - - thirdPartyMessages = map[hostEnv]string{ - cosmosDB: cosmosDBMsg, - documentDB: documentDBMsg, - } -) - // Topology represents a MongoDB deployment. type Topology struct { state int64 @@ -202,6 +176,36 @@ func logTopologyMessage(topo *Topology, level logger.Level, msg string, keysAndV }, keysAndValues...)...) } +func logTopologyThirdPartyUsage(topo *Topology, parsedHosts []string) { + thirdPartyMessages := [2]string{ + `You appear to be connected to a CosmosDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/cosmosdb`, + `You appear to be connected to a DocumentDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/documentdb`, + } + + thirdPartySuffixes := map[string]int{ + ".cosmos.azure.com": 0, + ".docdb.amazonaws.com": 1, + ".docdb-elastic.amazonaws.com": 1, + } + + hostSet := make([]bool, len(thirdPartyMessages)) + for _, host := range parsedHosts { + if h, _, err := net.SplitHostPort(host); err == nil { + host = h + } + for suffix, env := range thirdPartySuffixes { + if !strings.HasSuffix(host, suffix) { + continue + } + if hostSet[env] { + break + } + hostSet[env] = true + logTopologyMessage(topo, logger.LevelInfo, thirdPartyMessages[env]) + } + } +} + func mustLogServerSelection(topo *Topology, level logger.Level) bool { return topo.cfg.logger != nil && topo.cfg.logger.LevelComponentEnabled( level, logger.ComponentServerSelection) @@ -353,23 +357,7 @@ func (t *Topology) Connect() error { } parsedHosts := strings.Split(uri.Host, ",") if mustLogTopologyMessage(t, logger.LevelInfo) { - hostSet := make(map[hostEnv]bool) - for _, host := range parsedHosts { - if h, _, err := net.SplitHostPort(host); err == nil { - host = h - } - for suffix, env := range thirdPartySuffixes { - if !strings.HasSuffix(host, suffix) { - continue - } - if logged, ok := hostSet[env]; ok && logged { - break - } else { - hostSet[env] = true - logTopologyMessage(t, logger.LevelInfo, thirdPartyMessages[env]) - } - } - } + logTopologyThirdPartyUsage(t, parsedHosts) } if t.pollingRequired { // sanity check before passing the hostname to resolver diff --git a/x/mongo/driver/topology/topology_test.go b/x/mongo/driver/topology/topology_test.go index fe65a3b5b4..6cf540a95e 100644 --- a/x/mongo/driver/topology/topology_test.go +++ b/x/mongo/driver/topology/topology_test.go @@ -707,12 +707,21 @@ func (*mockLogSink) Error(error, string, ...interface{}) { // Note: SRV connection strings are intentionally untested, since initial // lookup responses cannot be easily mocked. func TestTopologyConstructionLogging(t *testing.T) { - sink := &mockLogSink{} - loggerOptions := options. - Logger(). - SetSink(sink). - SetComponentLevel(options.LogComponentTopology, options.LogLevelInfo) + const ( + cosmosDBMsg = `You appear to be connected to a CosmosDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/cosmosdb` + documentDBMsg = `You appear to be connected to a DocumentDB cluster. For more information regarding feature compatibility and support please visit https://www.mongodb.com/supportability/documentdb` + ) + + newLoggerOptions := func(sink options.LogSink) *options.LoggerOptions { + return options. + Logger(). + SetSink(sink). + SetComponentLevel(options.LogComponentTopology, options.LogLevelInfo) + } + t.Run("CosmosDB URIs", func(t *testing.T) { + t.Parallel() + testCases := []struct { name string uri string @@ -740,24 +749,28 @@ func TestTopologyConstructionLogging(t *testing.T) { }, } for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { - defer func() { - sink.msgs = []string{} - }() - cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(loggerOptions), nil) + t.Parallel() + + sink := &mockLogSink{} + cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(newLoggerOptions(sink)), nil) require.Nil(t, err, "error constructing topology config: %v", err) topo, err := New(cfg) require.Nil(t, err, "topology.New error: %v", err) err = topo.Connect() - require.Nil(t, err, "Connect error: %v", err) + assert.Nil(t, err, "Connect error: %v", err) - require.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) + assert.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) }) } }) t.Run("DocumentDB URIs", func(t *testing.T) { + t.Parallel() + testCases := []struct { name string uri string @@ -800,24 +813,28 @@ func TestTopologyConstructionLogging(t *testing.T) { }, } for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { - defer func() { - sink.msgs = []string{} - }() - cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(loggerOptions), nil) + t.Parallel() + + sink := &mockLogSink{} + cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(newLoggerOptions(sink)), nil) require.Nil(t, err, "error constructing topology config: %v", err) topo, err := New(cfg) require.Nil(t, err, "topology.New error: %v", err) err = topo.Connect() - require.Nil(t, err, "Connect error: %v", err) + assert.Nil(t, err, "Connect error: %v", err) - require.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) + assert.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) }) } }) t.Run("Mixing CosmosDB and DocumentDB URIs", func(t *testing.T) { + t.Parallel() + testCases := []struct { name string uri string @@ -830,24 +847,28 @@ func TestTopologyConstructionLogging(t *testing.T) { }, } for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { - defer func() { - sink.msgs = []string{} - }() - cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(loggerOptions), nil) + t.Parallel() + + sink := &mockLogSink{} + cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(newLoggerOptions(sink)), nil) require.Nil(t, err, "error constructing topology config: %v", err) topo, err := New(cfg) require.Nil(t, err, "topology.New error: %v", err) err = topo.Connect() - require.Nil(t, err, "Connect error: %v", err) + assert.Nil(t, err, "Connect error: %v", err) - require.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) + assert.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) }) } }) t.Run("genuine URIs", func(t *testing.T) { + t.Parallel() + testCases := []struct { name string uri string @@ -885,20 +906,22 @@ func TestTopologyConstructionLogging(t *testing.T) { }, } for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { - defer func() { - sink.msgs = []string{} - }() - cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(loggerOptions), nil) + t.Parallel() + + sink := &mockLogSink{} + cfg, err := NewConfig(options.Client().ApplyURI(tc.uri).SetLoggerOptions(newLoggerOptions(sink)), nil) require.Nil(t, err, "error constructing topology config: %v", err) topo, err := New(cfg) require.Nil(t, err, "topology.New error: %v", err) err = topo.Connect() - require.Nil(t, err, "Connect error: %v", err) + assert.Nil(t, err, "Connect error: %v", err) - require.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) + assert.ElementsMatch(t, tc.msgs, sink.msgs, "expected messages to be %v, got %v", tc.msgs, sink.msgs) }) } })