CBG-5109 Fix & vet log *fCtx function arguments

base/dcp_dest.go

@@ -285,7 +285,7 @@ func (d *DCPLoggingDest) DataDeleteEx(partition string, key []byte, seq uint64,
 
 func (d *DCPLoggingDest) SnapshotStart(partition string,
 	snapStart, snapEnd uint64) error {
-	TracefCtx(d.dest.loggingCtx, KeyDCP, "SnapshotStart:%d, %d, %d", partition, snapStart, snapEnd)
+	TracefCtx(d.dest.loggingCtx, KeyDCP, "SnapshotStart:%s, %d, %d", partition, snapStart, snapEnd)
 	return d.dest.SnapshotStart(partition, snapStart, snapEnd)
 }
 

base/dcp_feed_type.go

@@ -58,7 +58,7 @@ func cbgtRootCAsProvider(bucketName, bucketUUID, sourceParams string) func() *x5
 
 	if feedParams.DbName == "" {
 		// consider switching to AssertfCtx one CBG-4730 is fixed
-		InfofCtx(ctx, KeyDCP, "Database name not specified in dcp params %s during cbgtRootCAsProvider. Continuing without TLS authentication.")
+		InfofCtx(ctx, KeyDCP, "Database name not specified in dcp params %#v during cbgtRootCAsProvider. Continuing without TLS authentication.", UD(feedParams))
 		return nil
 	}
 

base/devmode_on.go

@@ -11,7 +11,9 @@
 
 package base
 
-import "context"
+import (
+	"context"
+)
 
 const cbSGDevModeBuildTagSet = true
 

base/logging.go

@@ -143,6 +143,10 @@ func PanicfCtx(ctx context.Context, format string, args ...any) {
 
 // FatalfCtx logs the given formatted string and args to the error log level and given log key and then exits.
 func FatalfCtx(ctx context.Context, format string, args ...any) {
+	// no-op call to enable golang.org/x/tools/go/analysis/passes/printf checking in go vet
+	if false {
+		_ = fmt.Sprintf(format, args...)
+	}
 	// Fall back to stdlib's log.Panicf if SG loggers aren't set up.
 	if errorLogger.Load() == nil {
 		log.Fatalf(format, args...)
@@ -155,16 +159,28 @@ func FatalfCtx(ctx context.Context, format string, args ...any) {
 
 // ErrorfCtx logs the given formatted string and args to the error log level and given log key.
 func ErrorfCtx(ctx context.Context, format string, args ...any) {
+	// no-op call to enable golang.org/x/tools/go/analysis/passes/printf checking in go vet
+	if false {
+		_ = fmt.Sprintf(format, args...)
+	}
 	logTo(ctx, LevelError, KeyAll, format, args...)
 }
 
 // WarnfCtx logs the given formatted string and args to the warn log level and given log key.
 func WarnfCtx(ctx context.Context, format string, args ...any) {
+	// no-op call to enable golang.org/x/tools/go/analysis/passes/printf checking in go vet
+	if false {
+		_ = fmt.Sprintf(format, args...)
+	}
 	logTo(ctx, LevelWarn, KeyAll, format, args...)
 }
 
 // InfofCtx logs the given formatted string and args to the info log level and given log key.
 func InfofCtx(ctx context.Context, logKey LogKey, format string, args ...any) {
+	// no-op call to enable golang.org/x/tools/go/analysis/passes/printf checking in go vet
+	if false {
+		_ = fmt.Sprintf(format, args...)
+	}
 	logTo(ctx, LevelInfo, logKey, format, args...)
 }
 
@@ -193,6 +209,10 @@ func RecordStats(statsJson string) {
 // logTo is the "core" logging function. All other logging functions (like Debugf(), WarnfCtx(), etc.) end up here.
 // The function will fan out the log to all of the various outputs for them to decide if they should log it or not.
 func logTo(ctx context.Context, logLevel LogLevel, logKey LogKey, format string, args ...any) {
+	// no-op call to enable golang.org/x/tools/go/analysis/passes/printf checking in go vet
+	if false {
+		_ = fmt.Sprintf(format, args...)
+	}
 	// Defensive bounds-check for log level. All callers of this function should be within this range.
 	if logLevel < LevelNone || logLevel >= levelCount {
 		return
@@ -272,7 +292,7 @@ func LogSyncGatewayVersion(ctx context.Context) {
 	msg := fmt.Sprintf("==== %s ====", LongVersionString)
 
 	// Log the startup indicator to the stderr.
-	ConsolefCtx(ctx, LevelNone, KeyNone, msg)
+	ConsolefCtx(ctx, LevelNone, KeyNone, "%s", msg)
 
 	// Log the startup indicator to ALL log files too.
 	msg = addPrefixes(msg, ctx, LevelNone, KeyNone)

base/logging_config.go

@@ -79,7 +79,7 @@ func InitLogging(ctx context.Context, logFilePath string,
 	if logFilePath == "" {
 		ConsolefCtx(ctx, LevelInfo, KeyNone, "Logging: Files disabled")
 		// Explicitly log this error to console
-		ConsolefCtx(ctx, LevelError, KeyNone, ErrUnsetLogFilePath.Error())
+		ConsolefCtx(ctx, LevelError, KeyNone, "%s", ErrUnsetLogFilePath.Error())
 		nilAllNonConsoleLoggers()
 		return nil
 	}

base/main_test_bucket_pool.go

@@ -571,7 +571,7 @@ func (tbp *TestBucketPool) setXDCRBucketSetting(ctx context.Context, bucket Buck
 	err, _ := RetryLoop(ctx, "setXDCRBucketSetting", func() (bool, error, any) {
 		output, statusCode, err := store.MgmtRequest(ctx, http.MethodPost, url, "application/x-www-form-urlencoded", strings.NewReader(posts.Encode()))
 		if err != nil {
-			tbp.Fatalf(ctx, "request to mobile XDCR bucket setting failed, status code: %d error: %w output: %s", statusCode, err, string(output))
+			tbp.Fatalf(ctx, "request to mobile XDCR bucket setting failed, status code: %d error: %s output: %s", statusCode, err, string(output))
 		}
 		if statusCode != http.StatusOK {
 			err := fmt.Errorf("request to mobile XDCR bucket setting failed with status code, %d, output: %s", statusCode, string(output))

base/main_test_bucket_pool_util.go

@@ -18,12 +18,20 @@ import (
 
 // Fatalf logs and exits.
 func (tbp *TestBucketPool) Fatalf(ctx context.Context, format string, args ...any) {
+	// no-op call to enable golang.org/x/tools/go/analysis/passes/printf checking in go vet
+	if false {
+		_ = fmt.Sprintf(format, args...)
+	}
 	format = addPrefixes(format, ctx, LevelNone, KeySGTest)
 	FatalfCtx(ctx, format, args...)
 }
 
 // Logf formats the given test bucket logging and logs to stderr.
 func (tbp *TestBucketPool) Logf(ctx context.Context, format string, args ...any) {
+	// no-op call to enable golang.org/x/tools/go/analysis/passes/printf checking in go vet
+	if false {
+		_ = fmt.Sprintf(format, args...)
+	}
 	if tbp != nil && !tbp.verbose.IsTrue() {
 		return
 	}

base/redactable_error.go

@@ -25,9 +25,13 @@ var (
 )
 
 // RedactErrorf creates a new redactable error.  Same signature as fmt.Errorf() for easy drop-in replacement.
-func RedactErrorf(fmt string, args ...any) *RedactableError {
+func RedactErrorf(format string, args ...any) *RedactableError {
+	// no-op call to enable golang.org/x/tools/go/analysis/passes/printf checking in go vet
+	if false {
+		_ = fmt.Errorf(format, args...)
+	}
 	return &RedactableError{
-		fmt:  fmt,
+		fmt:  format,
 		args: args,
 	}
 }

db/active_replicator_checkpointer.go

@@ -667,7 +667,7 @@ func (c *Checkpointer) setRetry(checkpoint *replicationCheckpoint, setFn setChec
 					base.InfofCtx(c.ctx, base.KeyReplicate, "Revision mismatch in setCheckpoint - updated from %q to %q based on existing checkpoint, will retry", checkpoint.Rev, existingCheckpoint.Rev)
 					checkpoint.Rev = existingCheckpoint.Rev
 				} else {
-					base.InfofCtx(c.ctx, base.KeyReplicate, "Revision mismatch in setCheckpoint, and unable to retrieve existing, will retry", getErr)
+					base.InfofCtx(c.ctx, base.KeyReplicate, "Revision mismatch in setCheckpoint, and unable to retrieve existing, will retry")
 					// pause before falling through to retry, in case of temporary failure on getFn
 					time.Sleep(time.Millisecond * 100)
 				}

db/attachment_compaction.go

@@ -125,7 +125,7 @@ func attachmentCompactMarkPhase(ctx context.Context, dataStore base.DataStore, c
 			if err != nil {
 				return failProcess(err, "[%s] Failed to mark attachment %s from doc %s with attachment docID %s. Err: %v", compactionLoggingID, base.UD(attachmentName), base.UD(docID), base.UD(attachmentDocID), err)
 			}
-			base.DebugfCtx(ctx, base.KeyAll, "[%s] Marked attachment %s from doc %s with attachment docID %s ; Event CAS: %s", compactionLoggingID, base.UD(attachmentName), base.UD(docID), base.UD(attachmentDocID), event.Cas)
+			base.DebugfCtx(ctx, base.KeyAll, "[%s] Marked attachment %s from doc %s with attachment docID %s ; Event CAS: %d", compactionLoggingID, base.UD(attachmentName), base.UD(docID), base.UD(attachmentDocID), event.Cas)
 			markedAttachmentCount.Add(1)
 		}
 		return true

db/background_mgr_attachment_migration.go

@@ -223,7 +223,7 @@ func (a *AttachmentMigrationManager) Run(ctx context.Context, options map[string
 		if failedDocs > 0 {
 			msg += fmt.Sprintf(" with %d docs failed", failedDocs)
 		}
-		base.InfofCtx(ctx, base.KeyAll, msg)
+		base.InfofCtx(ctx, base.KeyAll, "%s", msg)
 	case <-terminator.Done():
 		err = dcpClient.Close()
 		if err != nil {
@@ -239,7 +239,7 @@ func (a *AttachmentMigrationManager) Run(ctx context.Context, options map[string
 		if failedDocs > 0 {
 			msg += fmt.Sprintf(" with %d docs failed", failedDocs)
 		}
-		base.InfofCtx(ctx, base.KeyAll, msg)
+		base.InfofCtx(ctx, base.KeyAll, "%s", msg)
 	}
 	return nil
 }

db/blip_handler_collections.go

@@ -83,7 +83,7 @@ func (bh *blipHandler) handleGetCollections(rq *blip.Message) error {
 				collectionContexts[i] = newBlipSyncCollectionContext(bh.loggingCtx, collection)
 			} else {
 				errMsg := fmt.Sprintf("Unable to fetch client checkpoint %q for collection %s: %s", key, base.MD(scopeAndCollection).Redact(), err)
-				base.WarnfCtx(bh.loggingCtx, errMsg)
+				base.WarnfCtx(bh.loggingCtx, "%s", errMsg)
 				return base.NewHTTPError(http.StatusServiceUnavailable, errMsg)
 			}
 			continue

db/change_cache.go

@@ -702,12 +702,12 @@ func (c *changeCache) processUnusedSequenceRange(ctx context.Context, docID stri
 
 	fromSequence, err := strconv.ParseUint(sequences[0], 10, 64)
 	if err != nil {
-		base.WarnfCtx(ctx, "Unable to identify from sequence number for unused sequences notification with key: %s, error:", base.UD(docID), err)
+		base.WarnfCtx(ctx, "Unable to identify from sequence number for unused sequences notification with key: %s, error: %s", base.UD(docID), err)
 		return
 	}
 	toSequence, err := strconv.ParseUint(sequences[1], 10, 64)
 	if err != nil {
-		base.WarnfCtx(ctx, "Unable to identify to sequence number for unused sequence notification with key: %s, error:", base.UD(docID), err)
+		base.WarnfCtx(ctx, "Unable to identify to sequence number for unused sequence notification with key: %s, error: %s", base.UD(docID), err)
 		return
 	}
 

db/database.go

@@ -865,7 +865,7 @@ func (dc *DatabaseContext) TakeDbOffline(ctx context.Context, reason string) err
 			msg = "Unable to take Database offline, another operation was already in progress. Please try again."
 		}
 
-		base.InfofCtx(ctx, base.KeyCRUD, msg)
+		base.InfofCtx(ctx, base.KeyCRUD, "%s", msg)
 		return base.NewHTTPError(http.StatusServiceUnavailable, msg)
 	}
 }
@@ -2268,7 +2268,7 @@ func (db *DatabaseContext) StartOnlineProcesses(ctx context.Context) (returnedEr
 		db.Options.CacheOptions,
 		db.MetadataKeys,
 	); err != nil {
-		base.InfofCtx(ctx, base.KeyCache, "Error initializing the change cache", err)
+		base.InfofCtx(ctx, base.KeyCache, "Error initializing the change cache: %s", err)
 		return err
 	}
 

db/sg_replicate_cfg.go

@@ -538,7 +538,7 @@ func (m *sgReplicateManager) StartReplications(ctx context.Context) error {
 		if replicationCfg.AssignedNode == m.localNodeUUID {
 			activeReplicator, err := m.InitializeReplication(replicationCfg)
 			if err != nil {
-				base.WarnfCtx(m.loggingCtx, "Error initializing replication %s: %v", err)
+				base.WarnfCtx(m.loggingCtx, "Error initializing replication %s: %v", replicationID, err)
 				continue
 			}
 			m.activeReplicatorsLock.Lock()
@@ -800,7 +800,7 @@ func (m *sgReplicateManager) RefreshReplicationCfg(ctx context.Context) error {
 			if isChanged {
 				replicator, initError := m.InitializeReplication(replicationCfg)
 				if initError != nil {
-					base.WarnfCtx(m.loggingCtx, "Error initializing upserted replication %s: %v", initError)
+					base.WarnfCtx(m.loggingCtx, "Error initializing upserted replication %s: %v", replicationID, initError)
 				} else {
 					m.activeReplicators[replicationID] = replicator
 					activeReplicator = replicator

db/sg_replicate_conflict_resolver.go

@@ -285,7 +285,7 @@ func (i *ConflictResolverJSServer) EvaluateFunction(ctx context.Context, conflic
 	case map[string]any:
 		return result, nil
 	case error:
-		base.WarnfCtx(ctx, "conflictResolverRunner: "+result.Error())
+		base.WarnfCtx(ctx, "conflictResolverRunner: %s", result.Error())
 		return nil, result
 	default:
 		base.WarnfCtx(ctx, "Custom conflict resolution function returned non-document result %v Type: %T", result, result)

rest/config_manager.go

@@ -359,9 +359,9 @@ func (b *bootstrapContext) GetDatabaseConfigs(ctx context.Context, bucketName, g
 	for attempt = 1; attempt <= configFetchMaxRetryAttempts; attempt++ {
 		msg := fmt.Sprintf("Checking for database config (attempt %d/%d)", attempt, configFetchMaxRetryAttempts)
 		if attempt == 1 {
-			base.DebugfCtx(ctx, base.KeyConfig, msg)
+			base.DebugfCtx(ctx, base.KeyConfig, "%s", msg)
 		} else {
-			base.InfofCtx(ctx, base.KeyConfig, msg)
+			base.InfofCtx(ctx, base.KeyConfig, "%s", msg)
 		}
 		registry, err := b.getGatewayRegistry(ctx, bucketName)
 		if err != nil {

rest/doc_api.go

@@ -87,7 +87,7 @@ func (h *handler) handleGetDoc() error {
 		})
 		if err != nil {
 			if err == base.ErrImportCancelledPurged {
-				base.DebugfCtx(h.ctx(), base.KeyImport, fmt.Sprintf("Import cancelled as document %v is purged", base.UD(docid)))
+				base.DebugfCtx(h.ctx(), base.KeyImport, "Import cancelled as document %v is purged", base.UD(docid))
 				return nil
 			}
 			if h.collection.ForceAPIForbiddenErrors() && base.IsDocNotFoundError(err) {

rest/oidc_api_test.go

@@ -267,7 +267,7 @@ func (s *mockAuthServer) authHandler(w http.ResponseWriter, r *http.Request) {
 	}
 	code, err := base.GenerateRandomSecret()
 	if err != nil {
-		base.ErrorfCtx(r.Context(), err.Error())
+		base.ErrorfCtx(r.Context(), "%s", err.Error())
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}

rest/server_context.go

@@ -1182,7 +1182,7 @@ func dbcOptionsFromConfig(ctx context.Context, sc *ServerContext, config *DbConf
 	// Check for deprecated cache options. If new are set they will take priority but will still log warnings
 	warnings := config.deprecatedConfigCacheFallback()
 	for _, warnLog := range warnings {
-		base.WarnfCtx(ctx, warnLog)
+		base.WarnfCtx(ctx, "%s", warnLog)
 	}
 	// Set cache properties, if present
 	cacheOptions := db.DefaultCacheOptions()
@@ -2201,7 +2201,7 @@ func (sc *ServerContext) initializeBootstrapConnection(ctx context.Context) erro
 						base.WarnfCtx(ctx, "Couldn't load configs from bucket for group %q when polled: %v", sc.Config.Bootstrap.ConfigGroupID, err)
 					}
 					if count > 0 {
-						base.InfofCtx(ctx, base.KeyConfig, "Successfully fetched %d database configs for group %d from buckets in cluster", count, sc.Config.Bootstrap.ConfigGroupID)
+						base.InfofCtx(ctx, base.KeyConfig, "Successfully fetched %d database configs for group %q from buckets in cluster", count, sc.Config.Bootstrap.ConfigGroupID)
 					}
 				}
 			}
@@ -2265,7 +2265,7 @@ func (sc *ServerContext) CheckSupportedCouchbaseVersion(ctx context.Context) err
 	)
 
 	if !base.IsMinimumVersion(uint64(major), uint64(minor), CBXDCRCompatibleMajorVersion, CBXDCRCompatibleMinorVersion) {
-		base.ErrorfCtx(ctx, errMsg)
+		base.ErrorfCtx(ctx, "%s", errMsg)
 		return errors.New(errMsg)
 	}
 

rest/server_context_test.go

@@ -681,11 +681,11 @@ func TestLogFlush(t *testing.T) {
 
 			// Log some stuff (which will go into the memory loggers)
 			ctx := base.TestCtx(t)
-			base.ErrorfCtx(ctx, "error: "+testDirName)
-			base.WarnfCtx(ctx, "warn: "+testDirName)
-			base.InfofCtx(ctx, base.KeyAll, "info: "+testDirName)
-			base.DebugfCtx(ctx, base.KeyAll, "debug: "+testDirName)
-			base.TracefCtx(ctx, base.KeyAll, "trace: "+testDirName)
+			base.ErrorfCtx(ctx, "%s", "error: "+testDirName)
+			base.WarnfCtx(ctx, "%s", "warn: "+testDirName)
+			base.InfofCtx(ctx, base.KeyAll, "%s", "info: "+testDirName)
+			base.DebugfCtx(ctx, base.KeyAll, "%s", "debug: "+testDirName)
+			base.TracefCtx(ctx, base.KeyAll, "%s", "trace: "+testDirName)
 			base.RecordStats("{}")
 
 			config := DefaultStartupConfig(tempPath)