MM-48345: Fix Critical level logging (#21952)

Critical wasn't added in the StdLevels slice
and due to that mlog.Critical wasn't printing anything.
This was a complete blindspot that was missed.

We have removed all references to mlog.Critical in the codebase
and pointed Critical to be Fatal in the library.

In v8, we will remove Critical altogether.

```release-note
Servers with an encrypted key will throw an error
during startup now.
```

app/migrations.go

@@ -46,7 +46,7 @@ func (s *Server) doAdvancedPermissionsMigration() {
 		// If this failed for reasons other than the role already existing, don't mark the migration as done.
 		fetchedRole, err := s.Store().Role().GetByName(context.Background(), role.Name)
 		if err != nil {
-			mlog.Critical("Failed to migrate role to database.", mlog.Err(err))
+			mlog.Fatal("Failed to migrate role to database.", mlog.Err(err))
 			allSucceeded = false
 			continue
 		}
@@ -59,7 +59,7 @@ func (s *Server) doAdvancedPermissionsMigration() {
 			role.Id = fetchedRole.Id
 			if _, err = s.Store().Role().Save(role); err != nil {
 				// Role is not the same, but failed to update.
-				mlog.Critical("Failed to migrate role to database.", mlog.Err(err))
+				mlog.Fatal("Failed to migrate role to database.", mlog.Err(err))
 				allSucceeded = false
 			}
 		}
@@ -81,7 +81,7 @@ func (s *Server) doAdvancedPermissionsMigration() {
 	}
 
 	if err := s.Store().System().Save(&system); err != nil {
-		mlog.Critical("Failed to mark advanced permissions migration as completed.", mlog.Err(err))
+		mlog.Fatal("Failed to mark advanced permissions migration as completed.", mlog.Err(err))
 	}
 }
 
@@ -114,21 +114,21 @@ func (s *Server) doEmojisPermissionsMigration() {
 	// Emoji creation is set to all by default
 	role, err = s.GetRoleByName(context.Background(), model.SystemUserRoleId)
 	if err != nil {
-		mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err))
+		mlog.Fatal("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err))
 		return
 	}
 
 	if role != nil {
 		role.Permissions = append(role.Permissions, model.PermissionCreateEmojis.Id, model.PermissionDeleteEmojis.Id)
 		if _, nErr := s.Store().Role().Save(role); nErr != nil {
-			mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(nErr))
+			mlog.Fatal("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(nErr))
 			return
 		}
 	}
 
 	systemAdminRole, err = s.GetRoleByName(context.Background(), model.SystemAdminRoleId)
 	if err != nil {
-		mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err))
+		mlog.Fatal("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err))
 		return
 	}
 
@@ -138,7 +138,7 @@ func (s *Server) doEmojisPermissionsMigration() {
 		model.PermissionDeleteOthersEmojis.Id,
 	)
 	if _, err := s.Store().Role().Save(systemAdminRole); err != nil {
-		mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err))
+		mlog.Fatal("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err))
 		return
 	}
 
@@ -148,7 +148,7 @@ func (s *Server) doEmojisPermissionsMigration() {
 	}
 
 	if err := s.Store().System().Save(&system); err != nil {
-		mlog.Critical("Failed to mark emojis permissions migration as completed.", mlog.Err(err))
+		mlog.Fatal("Failed to mark emojis permissions migration as completed.", mlog.Err(err))
 	}
 }
 
@@ -167,26 +167,26 @@ func (s *Server) doGuestRolesCreationMigration() {
 	allSucceeded := true
 	if _, err := s.Store().Role().GetByName(context.Background(), model.ChannelGuestRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.ChannelGuestRoleId]); err != nil {
-			mlog.Critical("Failed to create new guest role to database.", mlog.Err(err))
+			mlog.Fatal("Failed to create new guest role to database.", mlog.Err(err))
 			allSucceeded = false
 		}
 	}
 	if _, err := s.Store().Role().GetByName(context.Background(), model.TeamGuestRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.TeamGuestRoleId]); err != nil {
-			mlog.Critical("Failed to create new guest role to database.", mlog.Err(err))
+			mlog.Fatal("Failed to create new guest role to database.", mlog.Err(err))
 			allSucceeded = false
 		}
 	}
 	if _, err := s.Store().Role().GetByName(context.Background(), model.SystemGuestRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.SystemGuestRoleId]); err != nil {
-			mlog.Critical("Failed to create new guest role to database.", mlog.Err(err))
+			mlog.Fatal("Failed to create new guest role to database.", mlog.Err(err))
 			allSucceeded = false
 		}
 	}
 
 	schemes, err := s.Store().Scheme().GetAllPage("", 0, 1000000)
 	if err != nil {
-		mlog.Critical("Failed to get all schemes.", mlog.Err(err))
+		mlog.Fatal("Failed to get all schemes.", mlog.Err(err))
 		allSucceeded = false
 	}
 	for _, scheme := range schemes {
@@ -201,7 +201,7 @@ func (s *Server) doGuestRolesCreationMigration() {
 				}
 
 				if savedRole, err := s.Store().Role().Save(teamGuestRole); err != nil {
-					mlog.Critical("Failed to create new guest role for custom scheme.", mlog.Err(err))
+					mlog.Fatal("Failed to create new guest role for custom scheme.", mlog.Err(err))
 					allSucceeded = false
 				} else {
 					scheme.DefaultTeamGuestRole = savedRole.Name
@@ -217,7 +217,7 @@ func (s *Server) doGuestRolesCreationMigration() {
 			}
 
 			if savedRole, err := s.Store().Role().Save(channelGuestRole); err != nil {
-				mlog.Critical("Failed to create new guest role for custom scheme.", mlog.Err(err))
+				mlog.Fatal("Failed to create new guest role for custom scheme.", mlog.Err(err))
 				allSucceeded = false
 			} else {
 				scheme.DefaultChannelGuestRole = savedRole.Name
@@ -225,7 +225,7 @@ func (s *Server) doGuestRolesCreationMigration() {
 
 			_, err := s.Store().Scheme().Save(scheme)
 			if err != nil {
-				mlog.Critical("Failed to update custom scheme.", mlog.Err(err))
+				mlog.Fatal("Failed to update custom scheme.", mlog.Err(err))
 				allSucceeded = false
 			}
 		}
@@ -241,7 +241,7 @@ func (s *Server) doGuestRolesCreationMigration() {
 	}
 
 	if err := s.Store().System().Save(&system); err != nil {
-		mlog.Critical("Failed to mark guest roles creation migration as completed.", mlog.Err(err))
+		mlog.Fatal("Failed to mark guest roles creation migration as completed.", mlog.Err(err))
 	}
 }
 
@@ -260,19 +260,19 @@ func (s *Server) doSystemConsoleRolesCreationMigration() {
 	allSucceeded := true
 	if _, err := s.Store().Role().GetByName(context.Background(), model.SystemManagerRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.SystemManagerRoleId]); err != nil {
-			mlog.Critical("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemManagerRoleId))
+			mlog.Fatal("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemManagerRoleId))
 			allSucceeded = false
 		}
 	}
 	if _, err := s.Store().Role().GetByName(context.Background(), model.SystemReadOnlyAdminRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.SystemReadOnlyAdminRoleId]); err != nil {
-			mlog.Critical("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemReadOnlyAdminRoleId))
+			mlog.Fatal("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemReadOnlyAdminRoleId))
 			allSucceeded = false
 		}
 	}
 	if _, err := s.Store().Role().GetByName(context.Background(), model.SystemUserManagerRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.SystemUserManagerRoleId]); err != nil {
-			mlog.Critical("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemUserManagerRoleId))
+			mlog.Fatal("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemUserManagerRoleId))
 			allSucceeded = false
 		}
 	}
@@ -287,7 +287,7 @@ func (s *Server) doSystemConsoleRolesCreationMigration() {
 	}
 
 	if err := s.Store().System().Save(&system); err != nil {
-		mlog.Critical("Failed to mark system console roles creation migration as completed.", mlog.Err(err))
+		mlog.Fatal("Failed to mark system console roles creation migration as completed.", mlog.Err(err))
 	}
 }
 
@@ -302,7 +302,7 @@ func (s *Server) doCustomGroupAdminRoleCreationMigration() {
 	allSucceeded := true
 	if _, err := s.Store().Role().GetByName(context.Background(), model.SystemCustomGroupAdminRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.SystemCustomGroupAdminRoleId]); err != nil {
-			mlog.Critical("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemCustomGroupAdminRoleId))
+			mlog.Fatal("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemCustomGroupAdminRoleId))
 			allSucceeded = false
 		}
 	}
@@ -317,7 +317,7 @@ func (s *Server) doCustomGroupAdminRoleCreationMigration() {
 	}
 
 	if err := s.Store().System().Save(&system); err != nil {
-		mlog.Critical("Failed to mark custom group admin role creation migration as completed.", mlog.Err(err))
+		mlog.Fatal("Failed to mark custom group admin role creation migration as completed.", mlog.Err(err))
 	}
 }
 
@@ -337,7 +337,7 @@ func (s *Server) doContentExtractionConfigDefaultTrueMigration() {
 	}
 
 	if err := s.Store().System().Save(&system); err != nil {
-		mlog.Critical("Failed to mark content extraction config migration as completed.", mlog.Err(err))
+		mlog.Fatal("Failed to mark content extraction config migration as completed.", mlog.Err(err))
 	}
 }
 
@@ -352,31 +352,31 @@ func (s *Server) doPlaybooksRolesCreationMigration() {
 	allSucceeded := true
 	if _, err := s.Store().Role().GetByName(context.Background(), model.PlaybookAdminRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.PlaybookAdminRoleId]); err != nil {
-			mlog.Critical("Failed to create new playbook admin role to database.", mlog.Err(err))
+			mlog.Fatal("Failed to create new playbook admin role to database.", mlog.Err(err))
 			allSucceeded = false
 		}
 	}
 	if _, err := s.Store().Role().GetByName(context.Background(), model.PlaybookMemberRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.PlaybookMemberRoleId]); err != nil {
-			mlog.Critical("Failed to create new playbook member role to database.", mlog.Err(err))
+			mlog.Fatal("Failed to create new playbook member role to database.", mlog.Err(err))
 			allSucceeded = false
 		}
 	}
 	if _, err := s.Store().Role().GetByName(context.Background(), model.RunAdminRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.RunAdminRoleId]); err != nil {
-			mlog.Critical("Failed to create new run admin role to database.", mlog.Err(err))
+			mlog.Fatal("Failed to create new run admin role to database.", mlog.Err(err))
 			allSucceeded = false
 		}
 	}
 	if _, err := s.Store().Role().GetByName(context.Background(), model.RunMemberRoleId); err != nil {
 		if _, err := s.Store().Role().Save(roles[model.RunMemberRoleId]); err != nil {
-			mlog.Critical("Failed to create new run member role to database.", mlog.Err(err))
+			mlog.Fatal("Failed to create new run member role to database.", mlog.Err(err))
 			allSucceeded = false
 		}
 	}
 	schemes, err := s.Store().Scheme().GetAllPage(model.SchemeScopeTeam, 0, 1000000)
 	if err != nil {
-		mlog.Critical("Failed to get all schemes.", mlog.Err(err))
+		mlog.Fatal("Failed to get all schemes.", mlog.Err(err))
 		allSucceeded = false
 	}
 
@@ -391,7 +391,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() {
 				}
 
 				if savedRole, err := s.Store().Role().Save(playbookAdminRole); err != nil {
-					mlog.Critical("Failed to create new playbook admin role for existing custom scheme.", mlog.Err(err))
+					mlog.Fatal("Failed to create new playbook admin role for existing custom scheme.", mlog.Err(err))
 					allSucceeded = false
 				} else {
 					scheme.DefaultPlaybookAdminRole = savedRole.Name
@@ -406,7 +406,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() {
 				}
 
 				if savedRole, err := s.Store().Role().Save(playbookMember); err != nil {
-					mlog.Critical("Failed to create new playbook member role for existing custom scheme.", mlog.Err(err))
+					mlog.Fatal("Failed to create new playbook member role for existing custom scheme.", mlog.Err(err))
 					allSucceeded = false
 				} else {
 					scheme.DefaultPlaybookMemberRole = savedRole.Name
@@ -422,7 +422,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() {
 				}
 
 				if savedRole, err := s.Store().Role().Save(runAdminRole); err != nil {
-					mlog.Critical("Failed to create new run admin role for existing custom scheme.", mlog.Err(err))
+					mlog.Fatal("Failed to create new run admin role for existing custom scheme.", mlog.Err(err))
 					allSucceeded = false
 				} else {
 					scheme.DefaultRunAdminRole = savedRole.Name
@@ -438,7 +438,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() {
 				}
 
 				if savedRole, err := s.Store().Role().Save(runMemberRole); err != nil {
-					mlog.Critical("Failed to create new run member role for existing custom scheme.", mlog.Err(err))
+					mlog.Fatal("Failed to create new run member role for existing custom scheme.", mlog.Err(err))
 					allSucceeded = false
 				} else {
 					scheme.DefaultRunMemberRole = savedRole.Name
@@ -446,7 +446,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() {
 			}
 			_, err := s.Store().Scheme().Save(scheme)
 			if err != nil {
-				mlog.Critical("Failed to update custom scheme.", mlog.Err(err))
+				mlog.Fatal("Failed to update custom scheme.", mlog.Err(err))
 				allSucceeded = false
 			}
 		}
@@ -462,7 +462,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() {
 	}
 
 	if err := s.Store().System().Save(&system); err != nil {
-		mlog.Critical("Failed to mark playbook roles creation migration as completed.", mlog.Err(err))
+		mlog.Fatal("Failed to mark playbook roles creation migration as completed.", mlog.Err(err))
 	}
 
 }
@@ -507,7 +507,7 @@ func (s *Server) doFirstAdminSetupCompleteMigration() {
 	}
 
 	if err := s.Store().System().Save(&system); err != nil {
-		mlog.Critical("Failed to mark first admin setup migration as completed.", mlog.Err(err))
+		mlog.Fatal("Failed to mark first admin setup migration as completed.", mlog.Err(err))
 	}
 }
 
@@ -534,7 +534,7 @@ func (s *Server) doRemainingSchemaMigrations() {
 	}
 
 	if err := s.Store().System().Save(&system); err != nil {
-		mlog.Critical("Failed to mark the remaining schema migrations as completed.", mlog.Err(err))
+		mlog.Fatal("Failed to mark the remaining schema migrations as completed.", mlog.Err(err))
 	}
 }
 
@@ -552,7 +552,7 @@ func (s *Server) doAppMigrations() {
 	// migrations. For example, it needs the guest roles migration.
 	err := s.doPermissionsMigrations()
 	if err != nil {
-		mlog.Critical("(app.App).DoPermissionsMigrations failed", mlog.Err(err))
+		mlog.Fatal("(app.App).DoPermissionsMigrations failed", mlog.Err(err))
 	}
 	s.doContentExtractionConfigDefaultTrueMigration()
 	s.doPlaybooksRolesCreationMigration()

app/platform/metrics.go

@@ -111,7 +111,7 @@ func (pm *platformMetrics) startMetricsServer() error {
 	go func() {
 		close(notify)
 		if err := pm.server.Serve(l); err != nil && err != http.ErrServerClosed {
-			pm.logger.Critical(err.Error())
+			pm.logger.Fatal(err.Error())
 		}
 	}()
 

app/server.go

@@ -1059,7 +1059,7 @@ func (s *Server) Start() error {
 		}
 
 		if err != nil && err != http.ErrServerClosed {
-			mlog.Critical("Error starting server", mlog.Err(err))
+			mlog.Fatal("Error starting server", mlog.Err(err))
 			time.Sleep(time.Second)
 		}
 
@@ -1068,7 +1068,7 @@ func (s *Server) Start() error {
 
 	if *s.platform.Config().ServiceSettings.EnableLocalMode {
 		if err := s.startLocalModeServer(); err != nil {
-			mlog.Critical(err.Error())
+			mlog.Fatal(err.Error())
 		}
 	}
 
@@ -1100,7 +1100,7 @@ func (s *Server) startLocalModeServer() error {
 	go func() {
 		err = s.localModeServer.Serve(unixListener)
 		if err != nil && err != http.ErrServerClosed {
-			mlog.Critical("Error starting unix socket server", mlog.Err(err))
+			mlog.Fatal("Error starting unix socket server", mlog.Err(err))
 		}
 	}()
 	return nil

services/upgrader/upgrader_linux.go

@@ -333,7 +333,7 @@ func extractBinary(executablePath string, filename string) error {
 			if err != nil {
 				err2 := os.Rename(tmpFileName, executablePath)
 				if err2 != nil {
-					mlog.Critical("Unable to restore the backup of the executable file. Restore the executable file manually.")
+					mlog.Fatal("Unable to restore the backup of the executable file. Restore the executable file manually.")
 					return errors.Wrap(err2, "critical error: unable to upgrade the binary or restore the old binary version. Please restore it manually")
 				}
 				return err
@@ -342,13 +342,13 @@ func extractBinary(executablePath string, filename string) error {
 			if _, err = io.Copy(outFile, tarReader); err != nil {
 				err2 := os.Remove(executablePath)
 				if err2 != nil {
-					mlog.Critical("Unable to restore the backup of the executable file. Restore the executable file manually.")
+					mlog.Fatal("Unable to restore the backup of the executable file. Restore the executable file manually.")
 					return errors.Wrap(err2, "critical error: unable to upgrade the binary or restore the old binary version. Please restore it manually")
 				}
 
 				err2 = os.Rename(tmpFileName, executablePath)
 				if err2 != nil {
-					mlog.Critical("Unable to restore the backup of the executable file. Restore the executable file manually.")
+					mlog.Fatal("Unable to restore the backup of the executable file. Restore the executable file manually.")
 					return errors.Wrap(err2, "critical error: unable to upgrade the binary or restore the old binary version. Please restore it manually")
 				}
 				return err

shared/mlog/global.go

@@ -113,13 +113,10 @@ func Error(msg string, fields ...Field) {
 
 // Convenience method equivalent to calling `Log` with the `Critical` level.
 // DEPRECATED: Either use Error or Fatal.
+// Critical level isn't added in mlog/levels.go:StdAll so calling this doesn't
+// really work. For now we just call Fatal to atleast print something.
 func Critical(msg string, fields ...Field) {
-	logger := getGlobalLogger()
-	if logger == nil {
-		defaultLog(LvlCritical, msg, fields...)
-		return
-	}
-	logger.Critical(msg, fields...)
+	Fatal(msg, fields...)
 }
 
 func Fatal(msg string, fields ...Field) {

shared/mlog/global_test.go

@@ -24,7 +24,6 @@ func TestLoggingBeforeInitialized(t *testing.T) {
 		mlog.Debug("debug log")
 		mlog.Warn("warning log")
 		mlog.Error("error log")
-		mlog.Critical("critical log")
 	})
 }
 
@@ -40,14 +39,13 @@ func TestLoggingAfterInitialized(t *testing.T) {
 				Type:          "file",
 				Format:        "json",
 				FormatOptions: json.RawMessage(`{"enable_caller":true}`),
-				Levels:        []mlog.Level{mlog.LvlCritical, mlog.LvlError, mlog.LvlWarn, mlog.LvlInfo, mlog.LvlDebug},
+				Levels:        []mlog.Level{mlog.LvlError, mlog.LvlWarn, mlog.LvlInfo, mlog.LvlDebug},
 			},
 			[]string{
 				`{"timestamp":0,"level":"debug","msg":"real debug log","caller":"mlog/global_test.go:0"}`,
 				`{"timestamp":0,"level":"info","msg":"real info log","caller":"mlog/global_test.go:0"}`,
 				`{"timestamp":0,"level":"warn","msg":"real warning log","caller":"mlog/global_test.go:0"}`,
 				`{"timestamp":0,"level":"error","msg":"real error log","caller":"mlog/global_test.go:0"}`,
-				`{"timestamp":0,"level":"critical","msg":"real critical log","caller":"mlog/global_test.go:0"}`,
 			},
 		},
 		{
@@ -56,11 +54,10 @@ func TestLoggingAfterInitialized(t *testing.T) {
 				Type:          "file",
 				Format:        "json",
 				FormatOptions: json.RawMessage(`{"enable_caller":true}`),
-				Levels:        []mlog.Level{mlog.LvlCritical, mlog.LvlError},
+				Levels:        []mlog.Level{mlog.LvlError},
 			},
 			[]string{
 				`{"timestamp":0,"level":"error","msg":"real error log","caller":"mlog/global_test.go:0"}`,
-				`{"timestamp":0,"level":"critical","msg":"real critical log","caller":"mlog/global_test.go:0"}`,
 			},
 		},
 		{
@@ -69,14 +66,13 @@ func TestLoggingAfterInitialized(t *testing.T) {
 				Type:          "file",
 				Format:        "plain",
 				FormatOptions: json.RawMessage(`{"delim":" | ", "enable_caller":true}`),
-				Levels:        []mlog.Level{mlog.LvlCritical, mlog.LvlError, mlog.LvlWarn, mlog.LvlInfo, mlog.LvlDebug},
+				Levels:        []mlog.Level{mlog.LvlError, mlog.LvlWarn, mlog.LvlInfo, mlog.LvlDebug},
 			},
 			[]string{
 				`debug | TIME | real debug log | caller="mlog/global_test.go:0"`,
 				`info | TIME | real info log | caller="mlog/global_test.go:0"`,
 				`warn | TIME | real warning log | caller="mlog/global_test.go:0"`,
 				`error | TIME | real error log | caller="mlog/global_test.go:0"`,
-				`critical | TIME | real critical log | caller="mlog/global_test.go:0"`,
 			},
 		},
 		{
@@ -85,11 +81,10 @@ func TestLoggingAfterInitialized(t *testing.T) {
 				Type:          "file",
 				Format:        "plain",
 				FormatOptions: json.RawMessage(`{"delim":" | ", "enable_caller":true}`),
-				Levels:        []mlog.Level{mlog.LvlCritical, mlog.LvlError},
+				Levels:        []mlog.Level{mlog.LvlError},
 			},
 			[]string{
 				`error | TIME | real error log | caller="mlog/global_test.go:0"`,
-				`critical | TIME | real critical log | caller="mlog/global_test.go:0"`,
 			},
 		},
 	}
@@ -116,7 +111,6 @@ func TestLoggingAfterInitialized(t *testing.T) {
 			mlog.Info("real info log")
 			mlog.Warn("real warning log")
 			mlog.Error("real error log")
-			mlog.Critical("real critical log")
 
 			logger.Shutdown()