Skip to content

Commit bda9fa2

Browse files
NikolaySBogdan Tsechoev
NikolayS
authored and
Bogdan Tsechoev
committed
Improve internal/log messages; message style guide in CONTRIBUTING.md
1 parent 8ffc5e8 commit bda9fa2

File tree

27 files changed

+121
-82
lines changed

27 files changed

+121
-82
lines changed

‎CONTRIBUTING.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,45 @@ We encourage you to follow the principles described in the following documents:
121121
- [Effective Go](https://go.dev/doc/effective_go)
122122
- [Go Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments)
123123

124+
### Message style guide
125+
Consistent messaging is important throughout the codebase. Follow these guidelines for errors, logs, and user-facing messages:
126+
127+
#### Error messages
128+
- Lowercase for internal errors and logs: `failed to start session` (no ending period)
129+
- Uppercase for user-facing errors: `Requested object does not exist. Specify your request.` (with ending period)
130+
- Omit articles ("a", "an", "the") for brevity: use `failed to update clone` not `failed to update the clone`
131+
- Be specific and actionable whenever possible
132+
- For variable interpolation, use consistent formatting: `failed to find clone: %s`
133+
134+
#### CLI output
135+
- Use concise, action-oriented language
136+
- Present tense with ellipsis for in-progress actions: `Creating clone...`
137+
- Ellipsis (`...`) indicates an ongoing process where the user should wait
138+
- Always follow up with a completion message when the operation finishes
139+
- Past tense with period for results: `Clone created successfully.`
140+
- Include relevant identifiers (IDs, names) in output
141+
142+
#### Progress indication
143+
- Use ellipsis (`...`) to indicate that an operation is in progress and the user should wait
144+
- For longer operations, consider providing percentage or step indicators: `Cloning database... (25%)`
145+
- When an operation with ellipsis completes, always provide a completion message without ellipsis
146+
- Example sequence:
147+
```
148+
Creating clone...
149+
Clone "test-clone" created successfully.
150+
```
151+
152+
#### UI messages
153+
- Be consistent with terminology across UI and documentation
154+
- For confirmations, use format: `{Resource} {action} successfully.`
155+
- For errors, provide clear next steps when possible
156+
- Use sentence case for all messages (capitalize first word only)
157+
158+
#### Commit messages
159+
- Start with lowercase type prefix: `fix:`, `feat:`, `docs:`, etc.
160+
- Use imperative mood: `add feature` not `added feature`
161+
- Provide context in the body if needed
162+
124163
### Documentation styleguide
125164
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.
126165

‎engine/cmd/database-lab/main.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func main() {
202202
if cfg.EmbeddedUI.Enabled {
203203
go func() {
204204
if err := embeddedUI.Run(ctx); err != nil {
205-
log.Err("Failed to start embedded UI container:", err.Error())
205+
log.Err("failed to start embedded UI container:", err.Error())
206206
return
207207
}
208208
}()
@@ -242,14 +242,14 @@ func main() {
242242
go billingSvc.CollectUsage(ctx, systemMetrics)
243243

244244
if err := retrievalSvc.Run(ctx); err != nil {
245-
log.Err("Failed to run the data retrieval service:", err)
245+
log.Err("failed to run data retrieval service:", err)
246246
log.Msg(contactSupport)
247247
}
248248

249249
defer retrievalSvc.Stop()
250250

251251
if err := logCleaner.ScheduleLogCleanupJob(cfg.Diagnostic); err != nil {
252-
log.Err("Failed to schedule a cleanup job of the diagnostic logs collector", err)
252+
log.Err("failed to schedule cleanup job of diagnostic logs collector", err)
253253
}
254254

255255
<-shutdownCh
@@ -387,7 +387,7 @@ func setReloadListener(ctx context.Context, engProp global.EngineProps, provisio
387387
platformSvc,
388388
embeddedUI, server,
389389
cleaner, logFilter, whs); err != nil {
390-
log.Err("Failed to reload configuration:", err)
390+
log.Err("failed to reload configuration:", err)
391391

392392
continue
393393
}
@@ -407,11 +407,11 @@ func shutdownDatabaseLabEngine(ctx context.Context, docker *client.Client, dbCfg
407407
log.Msg("Stopping auxiliary containers")
408408

409409
if err := cont.StopControlContainers(ctx, docker, dbCfg, instanceID, fsm); err != nil {
410-
log.Err("Failed to stop control containers", err)
410+
log.Err("failed to stop control containers", err)
411411
}
412412

413413
if err := cont.CleanUpSatelliteContainers(ctx, docker, instanceID); err != nil {
414-
log.Err("Failed to stop satellite containers", err)
414+
log.Err("failed to stop satellite containers", err)
415415
}
416416

417417
log.Msg("Auxiliary containers have been stopped")

‎engine/cmd/runci/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ func main() {
3232

3333
cfg, err := runci.LoadConfiguration()
3434
if err != nil {
35-
log.Errf("Failed to load config: %v", err)
35+
log.Errf("failed to load config: %v", err)
3636
return
3737
}
3838

3939
log.SetDebug(cfg.App.Debug)
4040
log.Dbg("Config loaded: ", cfg)
4141

4242
if cfg.App.VerificationToken == "" {
43-
log.Err("DB Migration Checker is insecure since the Verification Token is empty")
43+
log.Err("migration checker is insecure since verification token is empty")
4444
return
4545
}
4646

‎engine/internal/cloning/base.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,11 @@ func (c *Base) Run(ctx context.Context) error {
8686
}
8787

8888
if _, err := c.GetSnapshots(); err != nil {
89-
log.Err("No available snapshots: ", err)
89+
log.Err("no available snapshots:", err)
9090
}
9191

9292
if err := c.RestoreClonesState(); err != nil {
93-
log.Err("Failed to load stored sessions:", err)
93+
log.Err("failed to load stored sessions:", err)
9494
}
9595

9696
c.restartCloneContainers(ctx)
@@ -210,13 +210,13 @@ func (c *Base) CreateClone(cloneRequest *types.CloneCreateRequest) (*models.Clon
210210
session, err := c.provision.StartSession(clone, ephemeralUser, cloneRequest.ExtraConf)
211211
if err != nil {
212212
// TODO(anatoly): Empty room case.
213-
log.Errf("Failed to start session: %v.", err)
213+
log.Errf("failed to start session: %v", err)
214214

215215
if updateErr := c.UpdateCloneStatus(cloneID, models.Status{
216216
Code: models.StatusFatal,
217217
Message: errors.Cause(err).Error(),
218218
}); updateErr != nil {
219-
log.Errf("Failed to update clone status: %v", updateErr)
219+
log.Errf("failed to update clone status: %v", updateErr)
220220
}
221221

222222
return
@@ -247,7 +247,7 @@ func (c *Base) fillCloneSession(cloneID string, session *resources.Session) {
247247

248248
w, ok := c.clones[cloneID]
249249
if !ok {
250-
log.Errf("Clone %q not found", cloneID)
250+
log.Errf("clone %q not found", cloneID)
251251
return
252252
}
253253

@@ -370,13 +370,13 @@ func (c *Base) DestroyCloneSync(cloneID string) error {
370370

371371
func (c *Base) destroyClone(cloneID string, w *CloneWrapper) {
372372
if err := c.provision.StopSession(w.Session, w.Clone); err != nil {
373-
log.Errf("Failed to delete a clone: %v.", err)
373+
log.Errf("failed to delete clone: %v", err)
374374

375375
if updateErr := c.UpdateCloneStatus(cloneID, models.Status{
376376
Code: models.StatusFatal,
377377
Message: errors.Cause(err).Error(),
378378
}); updateErr != nil {
379-
log.Errf("Failed to update clone status: %v", updateErr)
379+
log.Errf("failed to update clone status: %v", updateErr)
380380
}
381381

382382
return
@@ -425,7 +425,7 @@ func (c *Base) refreshCloneMetadata(w *CloneWrapper) {
425425
sessionState, err := c.provision.GetSessionState(w.Session, w.Clone.Branch, w.Clone.ID)
426426
if err != nil {
427427
// Session not ready yet.
428-
log.Err(fmt.Errorf("failed to get a session state: %w", err))
428+
log.Err(fmt.Errorf("failed to get session state: %w", err))
429429

430430
return
431431
}
@@ -539,7 +539,7 @@ func (c *Base) ResetClone(cloneID string, resetOptions types.ResetCloneRequest)
539539

540540
snapshot, err := c.provision.ResetSession(w.Session, w.Clone, snapshotID)
541541
if err != nil {
542-
log.Errf("Failed to reset clone: %v", err)
542+
log.Errf("failed to reset clone: %v", err)
543543

544544
if updateErr := c.UpdateCloneStatus(cloneID, models.Status{
545545
Code: models.StatusFatal,
@@ -629,7 +629,7 @@ func (c *Base) GetClones() []*models.Clone {
629629
if cloneWrapper.Clone.Snapshot != nil {
630630
snapshot, err := c.getSnapshotByID(cloneWrapper.Clone.Snapshot.ID)
631631
if err != nil {
632-
log.Err("Snapshot not found: ", cloneWrapper.Clone.Snapshot.ID)
632+
log.Err("snapshot not found: ", cloneWrapper.Clone.Snapshot.ID)
633633
}
634634

635635
if snapshot != nil {
@@ -729,15 +729,15 @@ func (c *Base) destroyIdleClones(ctx context.Context) {
729729
default:
730730
isIdleClone, err := c.isIdleClone(cloneWrapper)
731731
if err != nil {
732-
log.Errf("Failed to check the idleness of clone %s: %v.", cloneWrapper.Clone.ID, err)
732+
log.Errf("failed to check idleness of clone %s: %v", cloneWrapper.Clone.ID, err)
733733
continue
734734
}
735735

736736
if isIdleClone {
737737
log.Msg(fmt.Sprintf("Idle clone %q is going to be removed.", cloneWrapper.Clone.ID))
738738

739739
if err = c.DestroyClone(cloneWrapper.Clone.ID); err != nil {
740-
log.Errf("Failed to destroy clone: %v.", err)
740+
log.Errf("failed to destroy clone: %v", err)
741741
continue
742742
}
743743
}
@@ -795,7 +795,7 @@ func hasNotQueryActivity(session *resources.Session) (bool, error) {
795795

796796
defer func() {
797797
if err := db.Close(); err != nil {
798-
log.Err("Cannot close database connection.")
798+
log.Err("cannot close database connection")
799799
}
800800
}()
801801

‎engine/internal/cloning/snapshots.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ func (c *Base) IncrementCloneNumber(snapshotID string) {
140140

141141
snapshot, ok := c.snapshotBox.items[snapshotID]
142142
if !ok {
143-
log.Err("Snapshot not found:", snapshotID)
143+
log.Err("snapshot not found:", snapshotID)
144144
return
145145
}
146146

@@ -153,12 +153,12 @@ func (c *Base) decrementCloneNumber(snapshotID string) {
153153

154154
snapshot, ok := c.snapshotBox.items[snapshotID]
155155
if !ok {
156-
log.Err("Snapshot not found:", snapshotID)
156+
log.Err("snapshot not found:", snapshotID)
157157
return
158158
}
159159

160160
if snapshot.NumClones == 0 {
161-
log.Err("The number of clones for the snapshot is negative. Snapshot ID:", snapshotID)
161+
log.Err("number of clones for snapshot is negative. Snapshot ID:", snapshotID)
162162
return
163163
}
164164

@@ -172,7 +172,7 @@ func (c *Base) GetCloneNumber(snapshotID string) int {
172172

173173
snapshot, ok := c.snapshotBox.items[snapshotID]
174174
if !ok {
175-
log.Err("Snapshot not found:", snapshotID)
175+
log.Err("snapshot not found:", snapshotID)
176176
return 0
177177
}
178178

‎engine/internal/cloning/storage.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,12 @@ func (c *Base) restartCloneContainers(ctx context.Context) {
6161
}
6262

6363
if err := c.provision.ReconnectClone(ctx, cloneName); err != nil {
64-
log.Err(fmt.Sprintf("Clone container %s cannot be reconnected to the internal network: %s", cloneName, err))
64+
log.Err(fmt.Sprintf("clone container %s cannot be reconnected to internal network: %s", cloneName, err))
6565
continue
6666
}
6767

6868
if err := c.provision.StartCloneContainer(ctx, cloneName); err != nil {
69-
log.Err(fmt.Sprintf("Clone container %s cannot start: %s", cloneName, err))
69+
log.Err(fmt.Sprintf("clone container %s cannot start: %s", cloneName, err))
7070
continue
7171
}
7272

@@ -114,11 +114,11 @@ func (c *Base) filterRunningClones(ctx context.Context) {
114114
func (c *Base) SaveClonesState() {
115115
sessionsPath, err := util.GetMetaPath(sessionsFilename)
116116
if err != nil {
117-
log.Err("failed to get path of a sessions file", err)
117+
log.Err("failed to get path of sessions file", err)
118118
}
119119

120120
if err := c.saveClonesState(sessionsPath); err != nil {
121-
log.Err("Failed to save the state of running clones", err)
121+
log.Err("failed to save state of running clones", err)
122122
}
123123
}
124124

‎engine/internal/diagnostic/logs.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,13 @@ func CollectContainerDiagnostics(ctx context.Context, client *client.Client, con
8787

8888
err = collectContainerLogs(ctx, client, diagnosticsDir, containerName)
8989
if err != nil {
90-
log.Warn("Failed to collect container logs ", containerName, err)
90+
log.Warn("failed to collect container logs ", containerName, err)
9191
}
9292

9393
err = collectPostgresLogs(ctx, client, diagnosticsDir, containerName, dbDataDir)
9494

9595
if err != nil {
96-
log.Warn("Failed to collect Postgres logs ", containerName, err)
96+
log.Warn("failed to collect Postgres logs ", containerName, err)
9797
}
9898
}
9999

@@ -107,7 +107,7 @@ func collectContainersOutput(ctx context.Context, client *client.Client, diagnos
107107
for _, containerName := range containerList {
108108
err = collectContainerLogs(ctx, client, diagnosticDir, containerName)
109109
if err != nil {
110-
log.Warn("Failed to collect container logs ", containerName, err)
110+
log.Warn("failed to collect container logs ", containerName, err)
111111
}
112112
}
113113

@@ -236,7 +236,7 @@ func extractTar(dir string, reader *tar.Reader, header *tar.Header) error {
236236

237237
defer func() {
238238
if err := f.Close(); err != nil {
239-
log.Err("Failed to close TAR stream", err)
239+
log.Err("failed to close TAR stream", err)
240240
}
241241
}()
242242

@@ -255,14 +255,14 @@ func cleanLogsFunc(logRetentionDays int) func() {
255255
log.Dbg("Cleaning old logs", logsDir)
256256

257257
if err != nil {
258-
log.Err("Failed to fetch logs dir", err)
258+
log.Err("failed to fetch logs dir", err)
259259
return
260260
}
261261

262262
err = cleanupLogsDir(logsDir, logRetentionDays)
263263

264264
if err != nil {
265-
log.Err("Failed to fetch logs dir", err)
265+
log.Err("failed to fetch logs dir", err)
266266
return
267267
}
268268
}
@@ -273,7 +273,7 @@ func cleanupLogsDir(logsDir string, logRetentionDays int) error {
273273
dirList, err := os.ReadDir(logsDir)
274274

275275
if err != nil {
276-
log.Err("Failed list logs directories", err)
276+
log.Err("failed to list logs directories", err)
277277
return err
278278
}
279279

@@ -285,7 +285,7 @@ func cleanupLogsDir(logsDir string, logRetentionDays int) error {
285285
dirTime, err := time.Parse(timeFormat, name)
286286

287287
if err != nil {
288-
log.Warn("Failed to parse time", name, err)
288+
log.Warn("failed to parse time", name, err)
289289
continue
290290
}
291291

@@ -296,7 +296,7 @@ func cleanupLogsDir(logsDir string, logRetentionDays int) error {
296296
log.Dbg("Removing old logs directory", name)
297297

298298
if err = os.RemoveAll(path.Join(logsDir, name)); err != nil {
299-
log.Err("Directory removal failed", err)
299+
log.Err("directory removal failed", err)
300300
}
301301
}
302302

‎engine/internal/observer/observer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func (o *Observer) processCSVLogFile(ctx context.Context, buf io.Writer, filenam
121121

122122
defer func() {
123123
if err := logFile.Close(); err != nil {
124-
log.Errf("Failed to close a CSV log file: %s", err.Error())
124+
log.Errf("failed to close CSV log file: %s", err.Error())
125125
}
126126
}()
127127

‎engine/internal/observer/observing_clone.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ func (c *ObservingClone) RunSession() error {
218218

219219
defer func() {
220220
if err := c.db.Close(ctx); err != nil {
221-
log.Err("Failed to close a database connection after observation for SessionID: ", c.session.SessionID)
221+
log.Err("failed to close database connection after observation for SessionID: ", c.session.SessionID)
222222
}
223223
}()
224224

@@ -255,7 +255,7 @@ func (c *ObservingClone) RunSession() error {
255255
log.Dbg("Stop observation for SessionID: ", c.session.SessionID)
256256

257257
if err := c.storeArtifacts(); err != nil {
258-
log.Err("Failed to store artifacts: ", err)
258+
log.Err("failed to store artifacts: ", err)
259259
}
260260

261261
c.done <- struct{}{}

0 commit comments

Comments
 (0)