Merge branch 'fix/error-message-style' into 'dle-4-0'

Improve internal/log messages; message style guide in CONTRIBUTING.md

See merge request postgres-ai/database-lab!1005

Author: bogdan-ts

CONTRIBUTING.md

@@ -121,6 +121,45 @@ We encourage you to follow the principles described in the following documents:
 - [Effective Go](https://go.dev/doc/effective_go)
 - [Go Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments)
 
+### Message style guide
+Consistent messaging is important throughout the codebase. Follow these guidelines for errors, logs, and user-facing messages:
+
+#### Error messages
+- Lowercase for internal errors and logs: `failed to start session` (no ending period)
+- Uppercase for user-facing errors: `Requested object does not exist. Specify your request.` (with ending period)
+- Omit articles ("a", "an", "the") for brevity: use `failed to update clone` not `failed to update the clone`
+- Be specific and actionable whenever possible
+- For variable interpolation, use consistent formatting: `failed to find clone: %s`
+
+#### CLI output
+- Use concise, action-oriented language
+- Present tense with ellipsis for in-progress actions: `Creating clone...` 
+  - Ellipsis (`...`) indicates an ongoing process where the user should wait
+  - Always follow up with a completion message when the operation finishes
+- Past tense with period for results: `Clone created successfully.`
+- Include relevant identifiers (IDs, names) in output
+
+#### Progress indication
+- Use ellipsis (`...`) to indicate that an operation is in progress and the user should wait
+- For longer operations, consider providing percentage or step indicators: `Cloning database... (25%)`
+- When an operation with ellipsis completes, always provide a completion message without ellipsis
+- Example sequence:
+  ```
+  Creating clone...
+  Clone "test-clone" created successfully.
+  ```
+
+#### UI messages
+- Be consistent with terminology across UI and documentation
+- For confirmations, use format: `{Resource} {action} successfully.`
+- For errors, provide clear next steps when possible
+- Use sentence case for all messages (capitalize first word only)
+
+#### Commit messages
+- Start with lowercase type prefix: `fix:`, `feat:`, `docs:`, etc.
+- Use imperative mood: `add feature` not `added feature`
+- Provide context in the body if needed
+
 ### Documentation styleguide
 Documentation for Database Lab Engine and additional components is hosted at https://postgres.ai/docs and is maintained in this GitLab repo: https://gitlab.com/postgres-ai/docs.
 

engine/cmd/database-lab/main.go

@@ -202,7 +202,7 @@ func main() {
 	if cfg.EmbeddedUI.Enabled {
 		go func() {
 			if err := embeddedUI.Run(ctx); err != nil {
-				log.Err("Failed to start embedded UI container:", err.Error())
+				log.Err("failed to start embedded UI container:", err.Error())
 				return
 			}
 		}()
@@ -242,14 +242,14 @@ func main() {
 	go billingSvc.CollectUsage(ctx, systemMetrics)
 
 	if err := retrievalSvc.Run(ctx); err != nil {
-		log.Err("Failed to run the data retrieval service:", err)
+		log.Err("failed to run data retrieval service:", err)
 		log.Msg(contactSupport)
 	}
 
 	defer retrievalSvc.Stop()
 
 	if err := logCleaner.ScheduleLogCleanupJob(cfg.Diagnostic); err != nil {
-		log.Err("Failed to schedule a cleanup job of the diagnostic logs collector", err)
+		log.Err("failed to schedule cleanup job of diagnostic logs collector", err)
 	}
 
 	<-shutdownCh
@@ -387,7 +387,7 @@ func setReloadListener(ctx context.Context, engProp global.EngineProps, provisio
 			platformSvc,
 			embeddedUI, server,
 			cleaner, logFilter, whs); err != nil {
-			log.Err("Failed to reload configuration:", err)
+			log.Err("failed to reload configuration:", err)
 
 			continue
 		}
@@ -407,11 +407,11 @@ func shutdownDatabaseLabEngine(ctx context.Context, docker *client.Client, dbCfg
 	log.Msg("Stopping auxiliary containers")
 
 	if err := cont.StopControlContainers(ctx, docker, dbCfg, instanceID, fsm); err != nil {
-		log.Err("Failed to stop control containers", err)
+		log.Err("failed to stop control containers", err)
 	}
 
 	if err := cont.CleanUpSatelliteContainers(ctx, docker, instanceID); err != nil {
-		log.Err("Failed to stop satellite containers", err)
+		log.Err("failed to stop satellite containers", err)
 	}
 
 	log.Msg("Auxiliary containers have been stopped")

engine/cmd/runci/main.go

@@ -32,15 +32,15 @@ func main() {
 
 	cfg, err := runci.LoadConfiguration()
 	if err != nil {
-		log.Errf("Failed to load config: %v", err)
+		log.Errf("failed to load config: %v", err)
 		return
 	}
 
 	log.SetDebug(cfg.App.Debug)
 	log.Dbg("Config loaded: ", cfg)
 
 	if cfg.App.VerificationToken == "" {
-		log.Err("DB Migration Checker is insecure since the Verification Token is empty")
+		log.Err("migration checker is insecure since verification token is empty")
 		return
 	}
 

engine/internal/cloning/base.go

@@ -86,11 +86,11 @@ func (c *Base) Run(ctx context.Context) error {
 	}
 
 	if _, err := c.GetSnapshots(); err != nil {
-		log.Err("No available snapshots: ", err)
+		log.Err("no available snapshots:", err)
 	}
 
 	if err := c.RestoreClonesState(); err != nil {
-		log.Err("Failed to load stored sessions:", err)
+		log.Err("failed to load stored sessions:", err)
 	}
 
 	c.restartCloneContainers(ctx)
@@ -210,13 +210,13 @@ func (c *Base) CreateClone(cloneRequest *types.CloneCreateRequest) (*models.Clon
 		session, err := c.provision.StartSession(clone, ephemeralUser, cloneRequest.ExtraConf)
 		if err != nil {
 			// TODO(anatoly): Empty room case.
-			log.Errf("Failed to start session: %v.", err)
+			log.Errf("failed to start session: %v", err)
 
 			if updateErr := c.UpdateCloneStatus(cloneID, models.Status{
 				Code:    models.StatusFatal,
 				Message: errors.Cause(err).Error(),
 			}); updateErr != nil {
-				log.Errf("Failed to update clone status: %v", updateErr)
+				log.Errf("failed to update clone status: %v", updateErr)
 			}
 
 			return
@@ -247,7 +247,7 @@ func (c *Base) fillCloneSession(cloneID string, session *resources.Session) {
 
 	w, ok := c.clones[cloneID]
 	if !ok {
-		log.Errf("Clone %q not found", cloneID)
+		log.Errf("clone %q not found", cloneID)
 		return
 	}
 
@@ -370,13 +370,13 @@ func (c *Base) DestroyCloneSync(cloneID string) error {
 
 func (c *Base) destroyClone(cloneID string, w *CloneWrapper) {
 	if err := c.provision.StopSession(w.Session, w.Clone); err != nil {
-		log.Errf("Failed to delete a clone: %v.", err)
+		log.Errf("failed to delete clone: %v", err)
 
 		if updateErr := c.UpdateCloneStatus(cloneID, models.Status{
 			Code:    models.StatusFatal,
 			Message: errors.Cause(err).Error(),
 		}); updateErr != nil {
-			log.Errf("Failed to update clone status: %v", updateErr)
+			log.Errf("failed to update clone status: %v", updateErr)
 		}
 
 		return
@@ -425,7 +425,7 @@ func (c *Base) refreshCloneMetadata(w *CloneWrapper) {
 	sessionState, err := c.provision.GetSessionState(w.Session, w.Clone.Branch, w.Clone.ID)
 	if err != nil {
 		// Session not ready yet.
-		log.Err(fmt.Errorf("failed to get a session state: %w", err))
+		log.Err(fmt.Errorf("failed to get session state: %w", err))
 
 		return
 	}
@@ -539,7 +539,7 @@ func (c *Base) ResetClone(cloneID string, resetOptions types.ResetCloneRequest)
 
 		snapshot, err := c.provision.ResetSession(w.Session, w.Clone, snapshotID)
 		if err != nil {
-			log.Errf("Failed to reset clone: %v", err)
+			log.Errf("failed to reset clone: %v", err)
 
 			if updateErr := c.UpdateCloneStatus(cloneID, models.Status{
 				Code:    models.StatusFatal,
@@ -629,7 +629,7 @@ func (c *Base) GetClones() []*models.Clone {
 		if cloneWrapper.Clone.Snapshot != nil {
 			snapshot, err := c.getSnapshotByID(cloneWrapper.Clone.Snapshot.ID)
 			if err != nil {
-				log.Err("Snapshot not found: ", cloneWrapper.Clone.Snapshot.ID)
+				log.Err("snapshot not found: ", cloneWrapper.Clone.Snapshot.ID)
 			}
 
 			if snapshot != nil {
@@ -729,15 +729,15 @@ func (c *Base) destroyIdleClones(ctx context.Context) {
 		default:
 			isIdleClone, err := c.isIdleClone(cloneWrapper)
 			if err != nil {
-				log.Errf("Failed to check the idleness of clone %s: %v.", cloneWrapper.Clone.ID, err)
+				log.Errf("failed to check idleness of clone %s: %v", cloneWrapper.Clone.ID, err)
 				continue
 			}
 
 			if isIdleClone {
 				log.Msg(fmt.Sprintf("Idle clone %q is going to be removed.", cloneWrapper.Clone.ID))
 
 				if err = c.DestroyClone(cloneWrapper.Clone.ID); err != nil {
-					log.Errf("Failed to destroy clone: %v.", err)
+					log.Errf("failed to destroy clone: %v", err)
 					continue
 				}
 			}
@@ -796,7 +796,7 @@ func hasNotQueryActivity(session *resources.Session) (bool, error) {
 
 	defer func() {
 		if err := db.Close(); err != nil {
-			log.Err("Cannot close database connection.")
+			log.Err("cannot close database connection")
 		}
 	}()
 

engine/internal/cloning/snapshots.go

@@ -141,7 +141,7 @@ func (c *Base) IncrementCloneNumber(snapshotID string) {
 
 	snapshot, ok := c.snapshotBox.items[snapshotID]
 	if !ok {
-		log.Err("Snapshot not found:", snapshotID)
+		log.Err("snapshot not found:", snapshotID)
 		return
 	}
 
@@ -154,12 +154,12 @@ func (c *Base) decrementCloneNumber(snapshotID string) {
 
 	snapshot, ok := c.snapshotBox.items[snapshotID]
 	if !ok {
-		log.Err("Snapshot not found:", snapshotID)
+		log.Err("snapshot not found:", snapshotID)
 		return
 	}
 
 	if snapshot.NumClones == 0 {
-		log.Err("The number of clones for the snapshot is negative. Snapshot ID:", snapshotID)
+		log.Err("number of clones for snapshot is negative. Snapshot ID:", snapshotID)
 		return
 	}
 
@@ -173,7 +173,7 @@ func (c *Base) GetCloneNumber(snapshotID string) int {
 
 	snapshot, ok := c.snapshotBox.items[snapshotID]
 	if !ok {
-		log.Err("Snapshot not found:", snapshotID)
+		log.Err("snapshot not found:", snapshotID)
 		return 0
 	}
 

engine/internal/cloning/storage.go

@@ -61,12 +61,12 @@ func (c *Base) restartCloneContainers(ctx context.Context) {
 		}
 
 		if err := c.provision.ReconnectClone(ctx, cloneName); err != nil {
-			log.Err(fmt.Sprintf("Clone container %s cannot be reconnected to the internal network: %s", cloneName, err))
+			log.Err(fmt.Sprintf("clone container %s cannot be reconnected to internal network: %s", cloneName, err))
 			continue
 		}
 
 		if err := c.provision.StartCloneContainer(ctx, cloneName); err != nil {
-			log.Err(fmt.Sprintf("Clone container %s cannot start: %s", cloneName, err))
+			log.Err(fmt.Sprintf("clone container %s cannot start: %s", cloneName, err))
 			continue
 		}
 
@@ -114,11 +114,11 @@ func (c *Base) filterRunningClones(ctx context.Context) {
 func (c *Base) SaveClonesState() {
 	sessionsPath, err := util.GetMetaPath(sessionsFilename)
 	if err != nil {
-		log.Err("failed to get path of a sessions file", err)
+		log.Err("failed to get path of sessions file", err)
 	}
 
 	if err := c.saveClonesState(sessionsPath); err != nil {
-		log.Err("Failed to save the state of running clones", err)
+		log.Err("failed to save state of running clones", err)
 	}
 }
 

engine/internal/diagnostic/logs.go

@@ -87,13 +87,13 @@ func CollectContainerDiagnostics(ctx context.Context, client *client.Client, con
 
 	err = collectContainerLogs(ctx, client, diagnosticsDir, containerName)
 	if err != nil {
-		log.Warn("Failed to collect container logs ", containerName, err)
+		log.Warn("failed to collect container logs ", containerName, err)
 	}
 
 	err = collectPostgresLogs(ctx, client, diagnosticsDir, containerName, dbDataDir)
 
 	if err != nil {
-		log.Warn("Failed to collect Postgres logs ", containerName, err)
+		log.Warn("failed to collect Postgres logs ", containerName, err)
 	}
 }
 
@@ -107,7 +107,7 @@ func collectContainersOutput(ctx context.Context, client *client.Client, diagnos
 	for _, containerName := range containerList {
 		err = collectContainerLogs(ctx, client, diagnosticDir, containerName)
 		if err != nil {
-			log.Warn("Failed to collect container logs ", containerName, err)
+			log.Warn("failed to collect container logs ", containerName, err)
 		}
 	}
 
@@ -236,7 +236,7 @@ func extractTar(dir string, reader *tar.Reader, header *tar.Header) error {
 
 		defer func() {
 			if err := f.Close(); err != nil {
-				log.Err("Failed to close TAR stream", err)
+				log.Err("failed to close TAR stream", err)
 			}
 		}()
 
@@ -255,14 +255,14 @@ func cleanLogsFunc(logRetentionDays int) func() {
 		log.Dbg("Cleaning old logs", logsDir)
 
 		if err != nil {
-			log.Err("Failed to fetch logs dir", err)
+			log.Err("failed to fetch logs dir", err)
 			return
 		}
 
 		err = cleanupLogsDir(logsDir, logRetentionDays)
 
 		if err != nil {
-			log.Err("Failed to fetch logs dir", err)
+			log.Err("failed to fetch logs dir", err)
 			return
 		}
 	}
@@ -273,7 +273,7 @@ func cleanupLogsDir(logsDir string, logRetentionDays int) error {
 	dirList, err := os.ReadDir(logsDir)
 
 	if err != nil {
-		log.Err("Failed list logs directories", err)
+		log.Err("failed to list logs directories", err)
 		return err
 	}
 
@@ -285,7 +285,7 @@ func cleanupLogsDir(logsDir string, logRetentionDays int) error {
 		dirTime, err := time.Parse(timeFormat, name)
 
 		if err != nil {
-			log.Warn("Failed to parse time", name, err)
+			log.Warn("failed to parse time", name, err)
 			continue
 		}
 
@@ -296,7 +296,7 @@ func cleanupLogsDir(logsDir string, logRetentionDays int) error {
 		log.Dbg("Removing old logs directory", name)
 
 		if err = os.RemoveAll(path.Join(logsDir, name)); err != nil {
-			log.Err("Directory removal failed", err)
+			log.Err("directory removal failed", err)
 		}
 	}
 

engine/internal/observer/observer.go

@@ -121,7 +121,7 @@ func (o *Observer) processCSVLogFile(ctx context.Context, buf io.Writer, filenam
 
 	defer func() {
 		if err := logFile.Close(); err != nil {
-			log.Errf("Failed to close a CSV log file: %s", err.Error())
+			log.Errf("failed to close CSV log file: %s", err.Error())
 		}
 	}()
 

engine/internal/observer/observing_clone.go

@@ -219,7 +219,7 @@ func (c *ObservingClone) RunSession() error {
 
 	defer func() {
 		if err := c.db.Close(ctx); err != nil {
-			log.Err("Failed to close a database connection after observation for SessionID: ", c.session.SessionID)
+			log.Err("failed to close database connection after observation for SessionID: ", c.session.SessionID)
 		}
 	}()
 
@@ -256,7 +256,7 @@ func (c *ObservingClone) RunSession() error {
 			log.Dbg("Stop observation for SessionID: ", c.session.SessionID)
 
 			if err := c.storeArtifacts(); err != nil {
-				log.Err("Failed to store artifacts: ", err)
+				log.Err("failed to store artifacts: ", err)
 			}
 
 			c.done <- struct{}{}