From 925a09a5f22180b3250c2bd0c2a006cfea65aee5 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Tue, 18 Aug 2026 18:36:03 -0300 Subject: [PATCH 1/2] Remove dead Email login button color settings (#38021) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [MM-57557] Remove dead Email login button color settings from the server EmailSettings.LoginButtonColor, LoginButtonBorderColor and LoginButtonTextColor were plumbed into the client config as EmailLoginButtonColor / EmailLoginButtonBorderColor / EmailLoginButtonTextColor, but no client — web or mobile — ever consumed them, so the email login button was never colored by these values. Remove the fields from the config struct and its defaults, drop the three client config props, and update the config fixtures that carried them. Also fixes MM-57556 and MM-57804, and follows the same removal already done for the AD/LDAP (MM-70140) and SAML (MM-70141) equivalents. * [MM-57557] Remove Email login button colors from the webapp and docs Drop the three Email Login Button Color settings from the Admin Console Experimental Features section along with their en.json strings, remove the matching ClientConfig and AdminConfig EmailSettings entries to stay in sync with the server model, and delete the corresponding documentation entries. The experimental settings doc's jq example referenced EmailSettings.LoginButtonColor, which no longer exists; point it at EmailSettings.EmailBatchingBufferSize instead. --- .../experimental-configuration-settings.mdx | 51 ++----------------- .../support/api/cloud_default_config.json | 5 +- .../support/api/on_prem_default_config.json | 5 +- .../lib/src/server/default_config.ts | 3 -- server/channels/app/email/email_test.go | 3 -- server/config/client.go | 4 -- server/public/model/config.go | 15 ------ server/tests/test-config.json | 5 +- .../admin_console/admin_definition.tsx | 24 --------- webapp/channels/src/i18n/en.json | 6 --- webapp/platform/types/src/config.ts | 6 --- 11 files changed, 6 insertions(+), 121 deletions(-) diff --git a/docs/main/administration-guide/configure/experimental-configuration-settings.mdx b/docs/main/administration-guide/configure/experimental-configuration-settings.mdx index 92cf91175377..fe539f051665 100644 --- a/docs/main/administration-guide/configure/experimental-configuration-settings.mdx +++ b/docs/main/administration-guide/configure/experimental-configuration-settings.mdx @@ -13,10 +13,10 @@ Review and manage the following [experimental](/administration-guide/manage/feat -System admins managing a self-hosted Mattermost deployment can edit the `config.json` file as described in the following tables. Each configuration value below includes a JSON path to access the value programmatically in the `config.json` file using a JSON-aware tool. For example, one `LoginButtonColor` value is under `EmailSettings`. +System admins managing a self-hosted Mattermost deployment can edit the `config.json` file as described in the following tables. Each configuration value below includes a JSON path to access the value programmatically in the `config.json` file using a JSON-aware tool. For example, the `EmailBatchingBufferSize` value is under `EmailSettings`. -- If using a tool such as [jq](https://stedolan.github.io/jq/), you'd enter: `cat config/config.json | jq '.EmailSettings.LoginButtonColor'` -- When working with the `config.json` file manually, look for an object such as `EmailSettings`, then within that object, find the key `LoginButtonColor`. +- If using a tool such as [jq](https://stedolan.github.io/jq/), you'd enter: `cat config/config.json | jq '.EmailSettings.EmailBatchingBufferSize'` +- When working with the `config.json` file manually, look for an object such as `EmailSettings`, then within that object, find the key `EmailBatchingBufferSize`. @@ -101,51 +101,6 @@ Specify the maximum frequency, in seconds, which the batching job checks for new -### Email login button color - -Specify the color of the email login button for white labeling purposes. Use a hex code with a \#-sign before the code. This setting only applies to the mobile app. - - --- - - - - - -
This feature's config.json setting is "LoginButtonColor": "" with string input.
- -### Email login button border color - -Specify the color of the email login button border for white labeling purposes. Use a hex code with a \#-sign before the code. This setting only applies to the mobile app. - - --- - - - - - -
This feature's config.json setting is "LoginButtonBorderColor": "" with string input.
- -### Email login button text color - -Specify the color of the email login button text for white labeling purposes. Use a hex code with a \#-sign before the code. This setting only applies to the mobile app. - - --- - - - - - -
This feature's config.json setting is "LoginButtonTextColor": "" with string input.
- ### Enable account deactivation **True**: Ability for users to deactivate their own account from **Settings \> Advanced \> Deactivate Account**. If a user deactivates their own account, they will get an email notification confirming they were deactivated. Available only when authentication is set to use email/password. Not available when authentication uses SAML or AD/LDAP. diff --git a/e2e-tests/cypress/tests/support/api/cloud_default_config.json b/e2e-tests/cypress/tests/support/api/cloud_default_config.json index 1489c96cbd19..99de903bee63 100644 --- a/e2e-tests/cypress/tests/support/api/cloud_default_config.json +++ b/e2e-tests/cypress/tests/support/api/cloud_default_config.json @@ -175,10 +175,7 @@ "EmailBatchingInterval": 30, "EnablePreviewModeBanner": true, "SkipServerCertificateVerification": false, - "EmailNotificationContentsType": "full", - "LoginButtonColor": "#0000", - "LoginButtonBorderColor": "#2389D7", - "LoginButtonTextColor": "#2389D7" + "EmailNotificationContentsType": "full" }, "PrivacySettings": { "ShowEmailAddress": true, diff --git a/e2e-tests/cypress/tests/support/api/on_prem_default_config.json b/e2e-tests/cypress/tests/support/api/on_prem_default_config.json index 6a46c1383b4a..27bd956d616a 100644 --- a/e2e-tests/cypress/tests/support/api/on_prem_default_config.json +++ b/e2e-tests/cypress/tests/support/api/on_prem_default_config.json @@ -257,10 +257,7 @@ "EmailBatchingInterval": 30, "EnablePreviewModeBanner": true, "SkipServerCertificateVerification": false, - "EmailNotificationContentsType": "full", - "LoginButtonColor": "#0000", - "LoginButtonBorderColor": "#2389D7", - "LoginButtonTextColor": "#2389D7" + "EmailNotificationContentsType": "full" }, "RateLimitSettings": { "Enable": false, diff --git a/e2e-tests/playwright/lib/src/server/default_config.ts b/e2e-tests/playwright/lib/src/server/default_config.ts index b11d4f77ad06..57920db1ff6e 100644 --- a/e2e-tests/playwright/lib/src/server/default_config.ts +++ b/e2e-tests/playwright/lib/src/server/default_config.ts @@ -405,9 +405,6 @@ const defaultServerConfig: AdminConfig = { EnablePreviewModeBanner: true, SkipServerCertificateVerification: false, EmailNotificationContentsType: 'full', - LoginButtonColor: '#0000', - LoginButtonBorderColor: '#2389D7', - LoginButtonTextColor: '#2389D7', }, RateLimitSettings: { Enable: false, diff --git a/server/channels/app/email/email_test.go b/server/channels/app/email/email_test.go index c3dc783e83ca..0483b4384411 100644 --- a/server/channels/app/email/email_test.go +++ b/server/channels/app/email/email_test.go @@ -469,9 +469,6 @@ func TestMailServiceConfig(t *testing.T) { EnablePreviewModeBanner: new(bool), SkipServerCertificateVerification: new(bool), EmailNotificationContentsType: new(string), - LoginButtonColor: new(string), - LoginButtonBorderColor: new(string), - LoginButtonTextColor: new(string), }, } }, diff --git a/server/config/client.go b/server/config/client.go index d6226644da84..8f7c9e265dc5 100644 --- a/server/config/client.go +++ b/server/config/client.go @@ -322,10 +322,6 @@ func GenerateLimitedClientConfig(c *model.Config, telemetryID string, license *m props["EnableSignInWithEmail"] = strconv.FormatBool(*c.EmailSettings.EnableSignInWithEmail) props["EnableSignInWithUsername"] = strconv.FormatBool(*c.EmailSettings.EnableSignInWithUsername) - props["EmailLoginButtonColor"] = *c.EmailSettings.LoginButtonColor - props["EmailLoginButtonBorderColor"] = *c.EmailSettings.LoginButtonBorderColor - props["EmailLoginButtonTextColor"] = *c.EmailSettings.LoginButtonTextColor - props["TermsOfServiceLink"] = *c.SupportSettings.TermsOfServiceLink props["PrivacyPolicyLink"] = *c.SupportSettings.PrivacyPolicyLink props["AboutLink"] = *c.SupportSettings.AboutLink diff --git a/server/public/model/config.go b/server/public/model/config.go index 8d8277da7112..fb3608c99f86 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -2164,9 +2164,6 @@ type EmailSettings struct { EnablePreviewModeBanner *bool `access:"site_notifications"` SkipServerCertificateVerification *bool `access:"environment_smtp,write_restrictable,cloud_restrictable"` EmailNotificationContentsType *string `access:"site_notifications"` - LoginButtonColor *string `access:"experimental_features"` - LoginButtonBorderColor *string `access:"experimental_features"` - LoginButtonTextColor *string `access:"experimental_features"` } func (s *EmailSettings) SetDefaults(isUpdate bool) { @@ -2297,18 +2294,6 @@ func (s *EmailSettings) SetDefaults(isUpdate bool) { if s.EmailNotificationContentsType == nil { s.EmailNotificationContentsType = new(EmailNotificationContentsFull) } - - if s.LoginButtonColor == nil { - s.LoginButtonColor = new("#0000") - } - - if s.LoginButtonBorderColor == nil { - s.LoginButtonBorderColor = new("#2389D7") - } - - if s.LoginButtonTextColor == nil { - s.LoginButtonTextColor = new("#2389D7") - } } type RateLimitSettings struct { diff --git a/server/tests/test-config.json b/server/tests/test-config.json index a566a781efb7..780e905b53d9 100644 --- a/server/tests/test-config.json +++ b/server/tests/test-config.json @@ -164,10 +164,7 @@ "EmailBatchingInterval": 30, "EnablePreviewModeBanner": true, "SkipServerCertificateVerification": false, - "EmailNotificationContentsType": "full", - "LoginButtonColor": "", - "LoginButtonBorderColor": "", - "LoginButtonTextColor": "" + "EmailNotificationContentsType": "full" }, "RateLimitSettings": { "Enable": false, diff --git a/webapp/channels/src/components/admin_console/admin_definition.tsx b/webapp/channels/src/components/admin_console/admin_definition.tsx index 5e132d6860a7..709142a1d3f0 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition.tsx @@ -6548,30 +6548,6 @@ const AdminDefinition: AdminDefinitionType = { placeholder: defineMessage({id: 'admin.experimental.emailBatchingInterval.example', defaultMessage: 'E.g.: "30"'}), isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.EXPERIMENTAL.FEATURES)), }, - { - type: 'color', - key: 'EmailSettings.LoginButtonColor', - label: defineMessage({id: 'admin.experimental.emailSettingsLoginButtonColor.title', defaultMessage: 'Email Login Button Color:'}), - help_text: defineMessage({id: 'admin.experimental.emailSettingsLoginButtonColor.desc', defaultMessage: 'Specify the color of the email login button for white labeling purposes. Use a hex code with a #-sign before the code. This setting only applies to the mobile apps.'}), - help_text_markdown: false, - isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.EXPERIMENTAL.FEATURES)), - }, - { - type: 'color', - key: 'EmailSettings.LoginButtonBorderColor', - label: defineMessage({id: 'admin.experimental.emailSettingsLoginButtonBorderColor.title', defaultMessage: 'Email Login Button Border Color:'}), - help_text: defineMessage({id: 'admin.experimental.emailSettingsLoginButtonBorderColor.desc', defaultMessage: 'Specify the color of the email login button border for white labeling purposes. Use a hex code with a #-sign before the code. This setting only applies to the mobile apps.'}), - help_text_markdown: false, - isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.EXPERIMENTAL.FEATURES)), - }, - { - type: 'color', - key: 'EmailSettings.LoginButtonTextColor', - label: defineMessage({id: 'admin.experimental.emailSettingsLoginButtonTextColor.title', defaultMessage: 'Email Login Button Text Color:'}), - help_text: defineMessage({id: 'admin.experimental.emailSettingsLoginButtonTextColor.desc', defaultMessage: 'Specify the color of the email login button text for white labeling purposes. Use a hex code with a #-sign before the code. This setting only applies to the mobile apps.'}), - help_text_markdown: false, - isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.EXPERIMENTAL.FEATURES)), - }, { type: 'bool', key: 'TeamSettings.EnableUserDeactivation', diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 671634e64550..ff7502b787ca 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -1322,12 +1322,6 @@ "admin.experimental.emailBatchingInterval.desc": "Specify the maximum frequency, in seconds, which the batching job checks for new notifications. Longer batching intervals will increase performance.", "admin.experimental.emailBatchingInterval.example": "E.g.: \"30\"", "admin.experimental.emailBatchingInterval.title": "Email Batching Interval:", - "admin.experimental.emailSettingsLoginButtonBorderColor.desc": "Specify the color of the email login button border for white labeling purposes. Use a hex code with a #-sign before the code. This setting only applies to the mobile apps.", - "admin.experimental.emailSettingsLoginButtonBorderColor.title": "Email Login Button Border Color:", - "admin.experimental.emailSettingsLoginButtonColor.desc": "Specify the color of the email login button for white labeling purposes. Use a hex code with a #-sign before the code. This setting only applies to the mobile apps.", - "admin.experimental.emailSettingsLoginButtonColor.title": "Email Login Button Color:", - "admin.experimental.emailSettingsLoginButtonTextColor.desc": "Specify the color of the email login button text for white labeling purposes. Use a hex code with a #-sign before the code. This setting only applies to the mobile apps.", - "admin.experimental.emailSettingsLoginButtonTextColor.title": "Email Login Button Text Color:", "admin.experimental.enableChannelViewedMessages.desc": "This setting determines whether `channel_viewed` WebSocket events are sent, which synchronize unread notifications across clients and devices. Disabling the setting in larger deployments may improve server performance.", "admin.experimental.enableChannelViewedMessages.title": "Enable Channel Viewed WebSocket Messages:", "admin.experimental.enableOnboardingFlow.desc": "When true, new users are shown steps to complete as part of an onboarding process", diff --git a/webapp/platform/types/src/config.ts b/webapp/platform/types/src/config.ts index beee225ba478..b584c891890a 100644 --- a/webapp/platform/types/src/config.ts +++ b/webapp/platform/types/src/config.ts @@ -45,9 +45,6 @@ export type ClientConfig = { DiagnosticsEnabled: string; DisableRefetchingOnBrowserFocus: string; DisableWakeUpReconnectHandler: string; - EmailLoginButtonBorderColor: string; - EmailLoginButtonColor: string; - EmailLoginButtonTextColor: string; EmailNotificationContentsType: string; EnableAskCommunityLink: string; EnableBanner: string; @@ -654,9 +651,6 @@ export type EmailSettings = { EnablePreviewModeBanner: boolean; SkipServerCertificateVerification: boolean; EmailNotificationContentsType: string; - LoginButtonColor: string; - LoginButtonBorderColor: string; - LoginButtonTextColor: string; }; export type RateLimitSettings = { From ede2edab4dabc7d5777f02ec3135197a659615ca Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Tue, 18 Aug 2026 19:09:41 -0300 Subject: [PATCH 2/2] Enforce snake_case for mlog field keys (#37998) * introduce mlogFieldNaming * apply vet-fix changes * Cover every keyed mlog constructor in the analyzer fixture * clarify end result in comment * Check mlog field keys on explicitly instantiated constructors --- server/Makefile | 13 +- server/channels/api4/channel.go | 14 +- server/channels/api4/job.go | 2 +- server/channels/app/audit.go | 2 +- server/channels/app/brand.go | 4 +- server/channels/app/channel.go | 8 +- server/channels/app/email/email.go | 8 +- server/channels/app/expirynotify.go | 4 +- server/channels/app/file.go | 8 +- server/channels/app/notification.go | 8 +- server/channels/app/notification_push.go | 16 +- server/channels/app/opengraph.go | 8 +- server/channels/app/platform/cluster.go | 4 +- .../app/platform/cluster_discovery.go | 14 +- server/channels/app/platform/license.go | 2 +- server/channels/app/platform/web_hub.go | 2 +- server/channels/app/plugin_db_driver.go | 2 +- server/channels/app/post.go | 2 +- server/channels/app/post_metadata.go | 4 +- server/channels/app/product_notices.go | 2 +- server/channels/app/role.go | 4 +- server/channels/app/server.go | 14 +- server/channels/app/session.go | 2 +- .../app/slashcommands/command_invite.go | 2 +- server/channels/app/status.go | 4 +- server/channels/app/upload.go | 4 +- server/channels/audit/audit.go | 4 +- .../channels/manualtesting/manual_testing.go | 2 +- server/channels/store/sqlstore/testpool.go | 8 +- server/channels/web/static.go | 2 +- server/cmd/mattermost/commands/db_ping.go | 2 +- .../elasticsearch/common/indexing_job.go | 20 +- .../platform/services/remotecluster/ping.go | 16 +- .../platform/services/remotecluster/recv.go | 2 +- .../services/remotecluster/sendfile.go | 6 +- .../services/remotecluster/sendmsg.go | 6 +- .../remotecluster/sendprofileImage.go | 6 +- .../services/sharedchannel/attachment.go | 8 +- .../services/sharedchannel/membership.go | 2 +- .../services/sharedchannel/permalink.go | 2 +- .../services/sharedchannel/service.go | 4 +- .../services/sharedchannel/sync_recv.go | 10 +- .../services/sharedchannel/sync_send.go | 40 ++-- tools/mattermost-govet/README.md | 1 + tools/mattermost-govet/main.go | 2 + .../mlogFieldNaming/mlogFieldNaming.go | 213 ++++++++++++++++++ .../mlogFieldNaming/mlogFieldNaming_test.go | 88 ++++++++ .../mlogFieldNaming/testdata/src/a/a.go | 117 ++++++++++ .../testdata/src/a/a.go.golden | 117 ++++++++++ .../server/public/shared/mlog/mlog.go | 39 ++++ 50 files changed, 731 insertions(+), 143 deletions(-) create mode 100644 tools/mattermost-govet/mlogFieldNaming/mlogFieldNaming.go create mode 100644 tools/mattermost-govet/mlogFieldNaming/mlogFieldNaming_test.go create mode 100644 tools/mattermost-govet/mlogFieldNaming/testdata/src/a/a.go create mode 100644 tools/mattermost-govet/mlogFieldNaming/testdata/src/a/a.go.golden create mode 100644 tools/mattermost-govet/mlogFieldNaming/testdata/src/github.com/mattermost/mattermost/server/public/shared/mlog/mlog.go diff --git a/server/Makefile b/server/Makefile index 8142204ebcc4..1b27a330e0dc 100644 --- a/server/Makefile +++ b/server/Makefile @@ -942,6 +942,7 @@ vet: setup-go-work ## Run mattermost go vet specific checks cd ../tools/mattermost-govet && $(GO) install . $(GO) vet -vettool=$(GOBIN)/mattermost-govet \ -structuredLogging \ + -mlogFieldNaming \ -inconsistentReceiverName \ -emptyStrCmp \ -tFatal \ @@ -957,7 +958,17 @@ vet: setup-go-work ## Run mattermost go vet specific checks ./... ifeq ($(BUILD_ENTERPRISE_READY),true) ifneq ($(MM_NO_ENTERPRISE_LINT),true) - $(GO) vet -vettool=$(GOBIN)/mattermost-govet -structuredLogging -inconsistentReceiverName -emptyStrCmp -tFatal -configtelemetry -errorAssertions -requestCtxNaming -enterpriseLicense $(BUILD_ENTERPRISE_DIR)/... + $(GO) vet -vettool=$(GOBIN)/mattermost-govet -structuredLogging -mlogFieldNaming -inconsistentReceiverName -emptyStrCmp -tFatal -configtelemetry -errorAssertions -requestCtxNaming -enterpriseLicense $(BUILD_ENTERPRISE_DIR)/... + endif +endif + +.PHONY: vet-fix +vet-fix: setup-go-work ## Apply the suggested fixes from mattermost go vet checks + cd ../tools/mattermost-govet && $(GO) install . + $(GO) fix -fixtool=$(GOBIN)/mattermost-govet -mlogFieldNaming ./... +ifeq ($(BUILD_ENTERPRISE_READY),true) + ifneq ($(MM_NO_ENTERPRISE_LINT),true) + $(GO) fix -fixtool=$(GOBIN)/mattermost-govet -mlogFieldNaming $(BUILD_ENTERPRISE_DIR)/... endif endif diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index 84d569e1444f..c5ec618cc289 100644 --- a/server/channels/api4/channel.go +++ b/server/channels/api4/channel.go @@ -2469,7 +2469,7 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { var newChannelMembers []model.ChannelMember for _, userId := range userIds { if !model.IsValidId(userId) { - c.Logger.Warn("Error adding channel member, invalid UserId", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id)) + c.Logger.Warn("Error adding channel member, invalid UserId", mlog.String("user_id", userId), mlog.String("channel_id", channel.Id)) c.SetInvalidParam("user_id") lastError = c.Err continue @@ -2489,7 +2489,7 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { existingMember, err := c.App.GetChannelMember(c.AppContext, member.ChannelId, member.UserId) if err != nil { if err.Id != app.MissingChannelMemberError { - c.Logger.Warn("Error adding channel member, error getting channel member", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id), mlog.Err(err)) + c.Logger.Warn("Error adding channel member, error getting channel member", mlog.String("user_id", userId), mlog.String("channel_id", channel.Id), mlog.Err(err)) lastError = err continue } @@ -2502,12 +2502,12 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { newChannelMembers = append(newChannelMembers, *existingMember) continue } else if isSelfAdd && !canAddSelf { - c.Logger.Warn("Error adding channel member, Invalid Permission to add self", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id)) + c.Logger.Warn("Error adding channel member, Invalid Permission to add self", mlog.String("user_id", userId), mlog.String("channel_id", channel.Id)) c.SetPermissionError(model.PermissionJoinPublicChannels) lastError = c.Err continue } else if !isSelfAdd && !canAddOthers { - c.Logger.Warn("Error adding channel member, Invalid Permission to add others", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id)) + c.Logger.Warn("Error adding channel member, Invalid Permission to add others", mlog.String("user_id", userId), mlog.String("channel_id", channel.Id)) c.SetPermissionError(model.PermissionManagePublicChannelMembers) lastError = c.Err continue @@ -2516,7 +2516,7 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { if existingMember != nil { // user is already a member, go to next - c.Logger.Warn("User is already a channel member, skipping", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id)) + c.Logger.Warn("User is already a channel member, skipping", mlog.String("user_id", userId), mlog.String("channel_id", channel.Id)) newChannelMembers = append(newChannelMembers, *existingMember) continue } @@ -2526,7 +2526,7 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { PostRootID: postRootId, }) if err != nil { - c.Logger.Warn("Error adding channel member", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id), mlog.Err(err)) + c.Logger.Warn("Error adding channel member", mlog.String("user_id", userId), mlog.String("channel_id", channel.Id), mlog.Err(err)) lastError = err continue } @@ -2535,7 +2535,7 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { if postRootId != "" { err := c.App.UpdateThreadFollowForUserFromChannelAdd(c.AppContext, cm.UserId, channel.TeamId, postRootId) if err != nil { - c.Logger.Warn("Error adding channel member, error updating thread", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id), mlog.Err(err)) + c.Logger.Warn("Error adding channel member, error updating thread", mlog.String("user_id", userId), mlog.String("channel_id", channel.Id), mlog.Err(err)) lastError = err continue } diff --git a/server/channels/api4/job.go b/server/channels/api4/job.go index 9856de7a9632..65ec3588959d 100644 --- a/server/channels/api4/job.go +++ b/server/channels/api4/job.go @@ -221,7 +221,7 @@ func getJobs(c *Context, w http.ResponseWriter, r *http.Request) { for _, jType := range model.AllJobTypes { hasPermission, permissionRequired := c.App.SessionHasPermissionToReadJob(*c.AppContext.Session(), jType) if permissionRequired == nil { - c.Logger.Warn("The job types of a job you are trying to retrieve does not contain permissions", mlog.String("jobType", jType)) + c.Logger.Warn("The job types of a job you are trying to retrieve does not contain permissions", mlog.String("job_type", jType)) continue } if hasPermission { diff --git a/server/channels/app/audit.go b/server/channels/app/audit.go index 2b8a6f9e7328..88c6ce640065 100644 --- a/server/channels/app/audit.go +++ b/server/channels/app/audit.go @@ -163,7 +163,7 @@ func (s *Server) configureAudit(adt *audit.Audit, bAllowAdvancedLogging bool) er } func (s *Server) onAuditTargetQueueFull(qname string, maxQSize int) bool { - s.Log().Error("Audit queue full, dropping record.", mlog.String("qname", qname), mlog.Int("queueSize", maxQSize)) + s.Log().Error("Audit queue full, dropping record.", mlog.String("qname", qname), mlog.Int("queue_size", maxQSize)) return true // drop it } diff --git a/server/channels/app/brand.go b/server/channels/app/brand.go index 0fae9a974c6e..be0fadf37748 100644 --- a/server/channels/app/brand.go +++ b/server/channels/app/brand.go @@ -62,8 +62,8 @@ func (a *App) SaveBrandImage(rctx request.CTX, imageData *multipart.FileHeader) rctx.Logger().Warn( "Failed to backup old brand image", mlog.Err(err), - mlog.String("oldPath", oldPath), - mlog.String("newPath", newPath), + mlog.String("old_path", oldPath), + mlog.String("new_path", newPath), ) } } diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index a48c353c6c30..74f7a29d0d0b 100644 --- a/server/channels/app/channel.go +++ b/server/channels/app/channel.go @@ -75,7 +75,7 @@ func (a *App) JoinDefaultChannels(rctx request.CTX, teamID string, user *model.U for _, channelName := range a.DefaultChannelNames(rctx) { channel, channelErr := a.Srv().Store().Channel().GetByName(teamID, channelName, true) if channelErr != nil { - rctx.Logger().Warn("No default channel with this name", mlog.String("channelName", channelName), mlog.String("teamID", teamID), mlog.Err(channelErr)) + rctx.Logger().Warn("No default channel with this name", mlog.String("channel_name", channelName), mlog.String("team_id", teamID), mlog.Err(channelErr)) continue } @@ -3848,7 +3848,7 @@ func (a *App) MoveChannel(rctx request.CTX, team *model.Team, channel *model.Cha } for _, channelMember := range channelMembers { if _, ok := teamMembersMap[channelMember.UserId]; !ok { - rctx.Logger().Warn("Not member of the target team", mlog.String("userId", channelMember.UserId)) + rctx.Logger().Warn("Not member of the target team", mlog.String("user_id", channelMember.UserId)) } } return model.NewAppError("MoveChannel", "app.channel.move_channel.members_do_not_match.error", nil, "", http.StatusInternalServerError) @@ -3895,7 +3895,7 @@ func (a *App) MoveChannel(rctx request.CTX, team *model.Team, channel *model.Cha if webhook.ChannelId == channel.Id { webhook.TeamId = team.Id if _, err := a.Srv().Store().Webhook().UpdateIncoming(webhook); err != nil { - rctx.Logger().Warn("Failed to move incoming webhook to new team", mlog.String("webhook id", webhook.Id)) + rctx.Logger().Warn("Failed to move incoming webhook to new team", mlog.String("webhook_id", webhook.Id)) } } } @@ -3908,7 +3908,7 @@ func (a *App) MoveChannel(rctx request.CTX, team *model.Team, channel *model.Cha if webhook.ChannelId == channel.Id { webhook.TeamId = team.Id if _, err := a.Srv().Store().Webhook().UpdateOutgoing(webhook); err != nil { - rctx.Logger().Warn("Failed to move outgoing webhook to new team.", mlog.String("webhook id", webhook.Id)) + rctx.Logger().Warn("Failed to move outgoing webhook to new team.", mlog.String("webhook_id", webhook.Id)) } } } diff --git a/server/channels/app/email/email.go b/server/channels/app/email/email.go index 817e9cef38ca..0b4fdeb45b66 100644 --- a/server/channels/app/email/email.go +++ b/server/channels/app/email/email.go @@ -454,7 +454,7 @@ func (es *Service) SendInviteEmails(rctx request.CTX, inviteData InviteEmailData } if rateLimited { - mlog.Error("rate limit exceeded", mlog.Duration("RetryAfter", result.RetryAfter), mlog.Duration("ResetAfter", result.ResetAfter), mlog.String("user_id", inviteData.SenderUserID), + mlog.Error("rate limit exceeded", mlog.Duration("retry_after", result.RetryAfter), mlog.Duration("reset_after", result.ResetAfter), mlog.String("user_id", inviteData.SenderUserID), mlog.String("team_id", inviteData.Team.Id), mlog.String("retry_after_secs", fmt.Sprintf("%f", result.RetryAfter.Seconds())), mlog.String("reset_after_secs", fmt.Sprintf("%f", result.ResetAfter.Seconds()))) return RateLimitExceededError } @@ -547,7 +547,7 @@ func (es *Service) SendGuestInviteEmails( } if rateLimited { - mlog.Error("rate limit exceeded", mlog.Duration("RetryAfter", result.RetryAfter), mlog.Duration("ResetAfter", result.ResetAfter), mlog.String("user_id", senderUserId), + mlog.Error("rate limit exceeded", mlog.Duration("retry_after", result.RetryAfter), mlog.Duration("reset_after", result.ResetAfter), mlog.String("user_id", senderUserId), mlog.String("team_id", team.Id), mlog.String("retry_after_secs", fmt.Sprintf("%f", result.RetryAfter.Seconds())), mlog.String("reset_after_secs", fmt.Sprintf("%f", result.ResetAfter.Seconds()))) return RateLimitExceededError } @@ -670,7 +670,7 @@ func (es *Service) SendMagicLinkEmailSelfService( } if rateLimited { - mlog.Error("rate limit exceeded", mlog.Duration("RetryAfter", result.RetryAfter), mlog.Duration("ResetAfter", result.ResetAfter), mlog.String("email", invite), + mlog.Error("rate limit exceeded", mlog.Duration("retry_after", result.RetryAfter), mlog.Duration("reset_after", result.ResetAfter), mlog.String("email", invite), mlog.String("retry_after_secs", fmt.Sprintf("%f", result.RetryAfter.Seconds())), mlog.String("reset_after_secs", fmt.Sprintf("%f", result.ResetAfter.Seconds()))) return RateLimitExceededError } @@ -736,7 +736,7 @@ func (es *Service) SendInviteEmailsToTeamAndChannels(rctx request.CTX, inviteDat } if rateLimited { - mlog.Error("rate limit exceeded", mlog.Duration("RetryAfter", result.RetryAfter), mlog.Duration("ResetAfter", result.ResetAfter), mlog.String("user_id", inviteData.SenderUserID), + mlog.Error("rate limit exceeded", mlog.Duration("retry_after", result.RetryAfter), mlog.Duration("reset_after", result.ResetAfter), mlog.String("user_id", inviteData.SenderUserID), mlog.String("team_id", inviteData.Team.Id), mlog.String("retry_after_secs", fmt.Sprintf("%f", result.RetryAfter.Seconds())), mlog.String("reset_after_secs", fmt.Sprintf("%f", result.ResetAfter.Seconds()))) return nil, RateLimitExceededError } diff --git a/server/channels/app/expirynotify.go b/server/channels/app/expirynotify.go index c8ab82da2a86..93fc7cc5dd7d 100644 --- a/server/channels/app/expirynotify.go +++ b/server/channels/app/expirynotify.go @@ -52,7 +52,7 @@ func (a *App) NotifySessionsExpired() error { mlog.String("push_type", tmpMessage.Type), mlog.String("user_id", session.UserId), mlog.String("session_id", session.Id), - mlog.String("deviceId", model.RedactDeviceId(tmpMessage.DeviceId)), + mlog.String("device_id", model.RedactDeviceId(tmpMessage.DeviceId)), mlog.String("post_id", msg.PostId), )) @@ -83,7 +83,7 @@ func (a *App) NotifySessionsExpired() error { if err != nil { mlog.Error("Failed to update ExpiredNotify flag", mlog.String("sessionid", session.Id), - mlog.String("deviceId", model.RedactDeviceId(tmpMessage.DeviceId)), + mlog.String("device_id", model.RedactDeviceId(tmpMessage.DeviceId)), mlog.Err(err), ) } diff --git a/server/channels/app/file.go b/server/channels/app/file.go index 0d19b456b0a3..d63f6c9e44b0 100644 --- a/server/channels/app/file.go +++ b/server/channels/app/file.go @@ -879,10 +879,10 @@ func (a *App) UploadFileX(rctx request.CTX, channelID, name string, input io.Rea if !a.Srv().GoExtraction(func() { err := a.ExtractContentFromFileInfo(rctx, &infoCopy) if err != nil { - rctx.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id)) + rctx.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("file_info_id", infoCopy.Id)) } }) { - rctx.Logger().Warn("Content extraction queue is full, skipping inline extraction; this file's content will not be searchable until the scheduled content extraction catch-up job runs or an admin runs a content extraction job (e.g. mmctl extract)", mlog.String("fileInfoId", infoCopy.Id)) + rctx.Logger().Warn("Content extraction queue is full, skipping inline extraction; this file's content will not be searchable until the scheduled content extraction catch-up job runs or an admin runs a content extraction job (e.g. mmctl extract)", mlog.String("file_info_id", infoCopy.Id)) } } @@ -1148,10 +1148,10 @@ func (a *App) DoUploadFileExpectModification(rctx request.CTX, now time.Time, ra if !a.Srv().GoExtraction(func() { err := a.ExtractContentFromFileInfo(rctx, &infoCopy) if err != nil { - rctx.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id)) + rctx.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("file_info_id", infoCopy.Id)) } }) { - rctx.Logger().Warn("Content extraction queue is full, skipping inline extraction; this file's content will not be searchable until the scheduled content extraction catch-up job runs or an admin runs a content extraction job (e.g. mmctl extract)", mlog.String("fileInfoId", infoCopy.Id)) + rctx.Logger().Warn("Content extraction queue is full, skipping inline extraction; this file's content will not be searchable until the scheduled content extraction catch-up job runs or an admin runs a content extraction job (e.g. mmctl extract)", mlog.String("file_info_id", infoCopy.Id)) } } diff --git a/server/channels/app/notification.go b/server/channels/app/notification.go index 0503e8c77e3a..4efe985718d4 100644 --- a/server/channels/app/notification.go +++ b/server/channels/app/notification.go @@ -1080,7 +1080,7 @@ func (a *App) getExplicitMentionsAndKeywords(rctx request.CTX, post *model.Post, if _, ok := profileMap[user1]; ok { mentions.addMention(user1, DMMention) } else { - a.Log().Debug("missing profile: DM user not in profiles", mlog.String("userId", user1), mlog.String("channelId", channel.Id)) + a.Log().Debug("missing profile: DM user not in profiles", mlog.String("user_id", user1), mlog.String("channel_id", channel.Id)) } } @@ -1089,7 +1089,7 @@ func (a *App) getExplicitMentionsAndKeywords(rctx request.CTX, post *model.Post, if _, ok := profileMap[user2]; ok { mentions.addMention(user2, DMMention) } else { - a.Log().Debug("missing profile: DM user not in profiles", mlog.String("userId", user2), mlog.String("channelId", channel.Id)) + a.Log().Debug("missing profile: DM user not in profiles", mlog.String("user_id", user2), mlog.String("channel_id", channel.Id)) } } } @@ -1105,7 +1105,7 @@ func (a *App) getExplicitMentionsAndKeywords(rctx request.CTX, post *model.Post, if _, ok := profileMap[id]; ok { mentions.addMention(id, GMMention) } else { - a.Log().Debug("missing profile: GM user not in profiles", mlog.String("userId", id), mlog.String("channelId", channel.Id)) + a.Log().Debug("missing profile: GM user not in profiles", mlog.String("user_id", id), mlog.String("channel_id", channel.Id)) } } } @@ -1117,7 +1117,7 @@ func (a *App) getExplicitMentionsAndKeywords(rctx request.CTX, post *model.Post, if _, ok := profileMap[addedUserId]; ok { mentions.addMention(addedUserId, KeywordMention) } else { - a.Log().Debug("missing profile: user added to channel not in profiles", mlog.String("userId", addedUserId), mlog.String("channelId", channel.Id)) + a.Log().Debug("missing profile: user added to channel not in profiles", mlog.String("user_id", addedUserId), mlog.String("channel_id", channel.Id)) } } } diff --git a/server/channels/app/notification_push.go b/server/channels/app/notification_push.go index 087f8ca3f8a7..634bd86e14eb 100644 --- a/server/channels/app/notification_push.go +++ b/server/channels/app/notification_push.go @@ -176,7 +176,7 @@ func (a *App) sendPushNotificationToAllSessions(rctx request.CTX, msg *model.Pus mlog.String("reason", model.NotificationReasonSessionExpired), mlog.String("user_id", session.UserId), mlog.String("session_id", session.Id), - mlog.String("deviceId", model.RedactDeviceId(session.DeviceId)), + mlog.String("device_id", model.RedactDeviceId(session.DeviceId)), ) continue } @@ -219,13 +219,13 @@ func (a *App) sendPushNotificationToAllSessions(rctx request.CTX, msg *model.Pus }).SignedString(a.AsymmetricSigningKey()) if err != nil { rctx.Logger().LogM(mlog.MlvlNotificationError, "Notification error", - mlog.String("ackId", tmpMessage.AckId), + mlog.String("ack_id", tmpMessage.AckId), mlog.String("type", tmpMessage.Type), - mlog.String("userId", session.UserId), - mlog.String("postId", tmpMessage.PostId), - mlog.String("channelId", tmpMessage.ChannelId), + mlog.String("user_id", session.UserId), + mlog.String("post_id", tmpMessage.PostId), + mlog.String("channel_id", tmpMessage.ChannelId), mlog.String("session_id", session.Id), - mlog.String("deviceId", model.RedactDeviceId(tmpMessage.DeviceId)), + mlog.String("device_id", model.RedactDeviceId(tmpMessage.DeviceId)), mlog.String("status", err.Error()), ) continue @@ -249,7 +249,7 @@ func (a *App) sendPushNotificationToAllSessions(rctx request.CTX, msg *model.Pus mlog.String("sub_type", string(tmpMessage.SubType)), mlog.String("user_id", session.UserId), mlog.String("session_id", session.Id), - mlog.String("deviceId", model.RedactDeviceId(tmpMessage.DeviceId)), + mlog.String("device_id", model.RedactDeviceId(tmpMessage.DeviceId)), mlog.Err(err), ) continue @@ -263,7 +263,7 @@ func (a *App) sendPushNotificationToAllSessions(rctx request.CTX, msg *model.Pus mlog.String("sub_type", string(tmpMessage.SubType)), mlog.String("user_id", session.UserId), mlog.String("session_id", session.Id), - mlog.String("deviceId", model.RedactDeviceId(tmpMessage.DeviceId)), + mlog.String("device_id", model.RedactDeviceId(tmpMessage.DeviceId)), mlog.String("status", model.PushSendSuccess), ) diff --git a/server/channels/app/opengraph.go b/server/channels/app/opengraph.go index 0200c24456d1..8856b0115c12 100644 --- a/server/channels/app/opengraph.go +++ b/server/channels/app/opengraph.go @@ -56,7 +56,7 @@ func (a *App) parseOpenGraphMetadata(requestURL string, body io.Reader, contentT body = forceHTMLEncodingToUTF8(io.LimitReader(body, MaxOpenGraphResponseSize), contentType) if err := og.ProcessHTML(body); err != nil { - mlog.Warn("parseOpenGraphMetadata processing failed", mlog.String("requestURL", requestURL), mlog.Err(err)) + mlog.Warn("parseOpenGraphMetadata processing failed", mlog.String("request_url", requestURL), mlog.Err(err)) } makeOpenGraphURLsAbsolute(og, requestURL) @@ -81,7 +81,7 @@ func (a *App) parseOpenGraphMetadata(requestURL string, body io.Reader, contentT func forceHTMLEncodingToUTF8(body io.Reader, contentType string) io.Reader { r, err := charset.NewReader(body, contentType) if err != nil { - mlog.Warn("forceHTMLEncodingToUTF8 failed to convert", mlog.String("contentType", contentType), mlog.Err(err)) + mlog.Warn("forceHTMLEncodingToUTF8 failed to convert", mlog.String("content_type", contentType), mlog.Err(err)) return body } return r @@ -90,7 +90,7 @@ func forceHTMLEncodingToUTF8(body io.Reader, contentType string) io.Reader { func makeOpenGraphURLsAbsolute(og *opengraph.OpenGraph, requestURL string) { parsedRequestURL, err := url.Parse(requestURL) if err != nil { - mlog.Warn("makeOpenGraphURLsAbsolute failed to parse url", mlog.String("requestURL", requestURL), mlog.Err(err)) + mlog.Warn("makeOpenGraphURLsAbsolute failed to parse url", mlog.String("request_url", requestURL), mlog.Err(err)) return } @@ -101,7 +101,7 @@ func makeOpenGraphURLsAbsolute(og *opengraph.OpenGraph, requestURL string) { parsedResultURL, err := url.Parse(resultURL) if err != nil { - mlog.Warn("makeOpenGraphURLsAbsolute failed to parse result", mlog.String("requestURL", requestURL), mlog.Err(err)) + mlog.Warn("makeOpenGraphURLsAbsolute failed to parse result", mlog.String("request_url", requestURL), mlog.Err(err)) return resultURL } diff --git a/server/channels/app/platform/cluster.go b/server/channels/app/platform/cluster.go index 6e670ca527e3..1ebcf5896f16 100644 --- a/server/channels/app/platform/cluster.go +++ b/server/channels/app/platform/cluster.go @@ -151,7 +151,7 @@ func (ps *PlatformService) KVDelete(productID, key string) *model.AppError { func (ps *PlatformService) KVList(productID string, page, perPage int) ([]string, *model.AppError) { data, err := ps.Store.Plugin().List(productID, page*perPage, perPage) if err != nil { - ps.logger.Error("Failed to list plugin key values", mlog.Int("page", page), mlog.Int("perPage", perPage), mlog.Err(err)) + ps.logger.Error("Failed to list plugin key values", mlog.Int("page", page), mlog.Int("per_page", perPage), mlog.Err(err)) return nil, model.NewAppError("ListPluginKeys", "app.plugin_store.list.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } @@ -236,7 +236,7 @@ func (ps *PlatformService) PublishSkipClusterSend(event *model.WebSocketEvent) { func (ps *PlatformService) ListPluginKeys(pluginID string, page, perPage int) ([]string, *model.AppError) { data, err := ps.Store.Plugin().List(pluginID, page*perPage, perPage) if err != nil { - mlog.Error("Failed to list plugin key values", mlog.Int("page", page), mlog.Int("perPage", perPage), mlog.Err(err)) + mlog.Error("Failed to list plugin key values", mlog.Int("page", page), mlog.Int("per_page", perPage), mlog.Err(err)) return nil, model.NewAppError("ListPluginKeys", "app.plugin_store.list.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } diff --git a/server/channels/app/platform/cluster_discovery.go b/server/channels/app/platform/cluster_discovery.go index bd942fb6ac75..02b43068d4b8 100644 --- a/server/channels/app/platform/cluster_discovery.go +++ b/server/channels/app/platform/cluster_discovery.go @@ -28,34 +28,34 @@ func (cds *ClusterDiscoveryService) Start() { exists, err := cds.platform.Store.ClusterDiscovery().Exists(&cds.ClusterDiscovery) if err != nil { - mlog.Warn("ClusterDiscoveryService failed to check if row exists", mlog.String("ClusterDiscoveryID", cds.ClusterDiscovery.Id), mlog.Err(err)) + mlog.Warn("ClusterDiscoveryService failed to check if row exists", mlog.String("cluster_discovery_id", cds.ClusterDiscovery.Id), mlog.Err(err)) } else if exists { if _, err := cds.platform.Store.ClusterDiscovery().Delete(&cds.ClusterDiscovery); err != nil { - mlog.Warn("ClusterDiscoveryService failed to start clean", mlog.String("ClusterDiscoveryID", cds.ClusterDiscovery.Id), mlog.Err(err)) + mlog.Warn("ClusterDiscoveryService failed to start clean", mlog.String("cluster_discovery_id", cds.ClusterDiscovery.Id), mlog.Err(err)) } } if err := cds.platform.Store.ClusterDiscovery().Save(&cds.ClusterDiscovery); err != nil { - mlog.Error("ClusterDiscoveryService failed to save", mlog.String("ClusterDiscoveryID", cds.ClusterDiscovery.Id), mlog.Err(err)) + mlog.Error("ClusterDiscoveryService failed to save", mlog.String("cluster_discovery_id", cds.ClusterDiscovery.Id), mlog.Err(err)) return } go func() { - mlog.Debug("ClusterDiscoveryService ping writer started", mlog.String("ClusterDiscoveryID", cds.ClusterDiscovery.Id)) + mlog.Debug("ClusterDiscoveryService ping writer started", mlog.String("cluster_discovery_id", cds.ClusterDiscovery.Id)) ticker := time.NewTicker(DiscoveryServiceWritePing) defer func() { ticker.Stop() if _, err := cds.platform.Store.ClusterDiscovery().Delete(&cds.ClusterDiscovery); err != nil { - mlog.Warn("ClusterDiscoveryService failed to cleanup", mlog.String("ClusterDiscoveryID", cds.ClusterDiscovery.Id), mlog.Err(err)) + mlog.Warn("ClusterDiscoveryService failed to cleanup", mlog.String("cluster_discovery_id", cds.ClusterDiscovery.Id), mlog.Err(err)) } - mlog.Debug("ClusterDiscoveryService ping writer stopped", mlog.String("ClusterDiscoveryID", cds.ClusterDiscovery.Id)) + mlog.Debug("ClusterDiscoveryService ping writer stopped", mlog.String("cluster_discovery_id", cds.ClusterDiscovery.Id)) }() for { select { case <-ticker.C: if err := cds.platform.Store.ClusterDiscovery().SetLastPingAt(&cds.ClusterDiscovery); err != nil { - mlog.Error("ClusterDiscoveryService failed to write ping", mlog.String("ClusterDiscoveryID", cds.ClusterDiscovery.Id), mlog.Err(err)) + mlog.Error("ClusterDiscoveryService failed to write ping", mlog.String("cluster_discovery_id", cds.ClusterDiscovery.Id), mlog.Err(err)) } case <-cds.stop: return diff --git a/server/channels/app/platform/license.go b/server/channels/app/platform/license.go index 5836b404870b..46a16b3bcca8 100644 --- a/server/channels/app/platform/license.go +++ b/server/channels/app/platform/license.go @@ -421,7 +421,7 @@ func (ps *PlatformService) logLicense(message string, license *model.License) { if license.Features != nil { logger = logger.With( - mlog.Int("features.users", *license.Features.Users), + mlog.Int("features_users", *license.Features.Users), mlog.Map("features", license.Features.ToMap()), ) } diff --git a/server/channels/app/platform/web_hub.go b/server/channels/app/platform/web_hub.go index 413a4e3cf12b..37fbcce0037b 100644 --- a/server/channels/app/platform/web_hub.go +++ b/server/channels/app/platform/web_hub.go @@ -167,7 +167,7 @@ func (ps *PlatformService) GetHubForUserId(userID string) *Hub { hash.SetSeed(ps.hashSeed) _, err := hash.WriteString(userID) if err != nil { - ps.logger.Error("Unable to write userID to hash", mlog.String("userID", userID), mlog.Err(err)) + ps.logger.Error("Unable to write userID to hash", mlog.String("user_id", userID), mlog.Err(err)) } index := hash.Sum64() % uint64(len(ps.hubs)) diff --git a/server/channels/app/plugin_db_driver.go b/server/channels/app/plugin_db_driver.go index 4ab4e0c00e27..f8c93f08d49e 100644 --- a/server/channels/app/plugin_db_driver.go +++ b/server/channels/app/plugin_db_driver.go @@ -166,7 +166,7 @@ func (d *DriverImpl) ShutdownConns(pluginID string) { if entry.pluginID == pluginID { err := entry.conn.Close() if err != nil { - d.s.Log().Error("Error while closing DB connection", mlog.Err(err), mlog.String("pluginID", pluginID)) + d.s.Log().Error("Error while closing DB connection", mlog.Err(err), mlog.String("plugin_id", pluginID)) } delete(d.connMap, connID) } diff --git a/server/channels/app/post.go b/server/channels/app/post.go index f6465dcc836d..a4e6f1528dd1 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -2689,7 +2689,7 @@ func isCommentMention(user *model.User, post *model.Post, otherPosts map[string] } if _, ok := otherPosts[post.RootId]; !ok { - mlog.Warn("Can't determine the comment mentions as the rootPost is past the cloud plan's limit", mlog.String("rootPostID", post.RootId), mlog.String("commentID", post.Id)) + mlog.Warn("Can't determine the comment mentions as the rootPost is past the cloud plan's limit", mlog.String("root_post_id", post.RootId), mlog.String("comment_id", post.Id)) return false } diff --git a/server/channels/app/post_metadata.go b/server/channels/app/post_metadata.go index d96fb21ba452..5b4f4e0cb70a 100644 --- a/server/channels/app/post_metadata.go +++ b/server/channels/app/post_metadata.go @@ -180,7 +180,7 @@ func (a *App) OverrideIconURLIfEmoji(rctx request.CTX, post *model.Post) { if emojiURL, err := a.GetEmojiStaticURL(rctx, emojiName); err == nil { post.AddProp(model.PostPropsOverrideIconURL, emojiURL) } else { - rctx.Logger().Warn("Failed to retrieve URL for overridden profile icon (emoji)", mlog.String("emojiName", emojiName), mlog.Err(err)) + rctx.Logger().Warn("Failed to retrieve URL for overridden profile icon (emoji)", mlog.String("emoji_name", emojiName), mlog.Err(err)) } } @@ -631,7 +631,7 @@ func (a *App) getImagesForPost(rctx request.CTX, post *model.Post, isNewPost boo if !ok { rctx.Logger().Warn("Could not read the image data: the data could not be casted to OpenGraph", mlog.String("post_id", post.Id), - mlog.String("data type", fmt.Sprintf("%t", embed.Data)), + mlog.String("data_type", fmt.Sprintf("%t", embed.Data)), ) continue } diff --git a/server/channels/app/product_notices.go b/server/channels/app/product_notices.go index cfe352977c9e..7cae415f51fa 100644 --- a/server/channels/app/product_notices.go +++ b/server/channels/app/product_notices.go @@ -314,7 +314,7 @@ func (a *App) UpdateViewedProductNoticesForNewUser(userID string) { noticeIds = append(noticeIds, notice.ID) } if err := a.Srv().Store().ProductNotices().View(userID, noticeIds); err != nil { - mlog.Error("Cannot update product notices viewed state for user", mlog.String("userId", userID)) + mlog.Error("Cannot update product notices viewed state for user", mlog.String("user_id", userID)) } } diff --git a/server/channels/app/role.go b/server/channels/app/role.go index 4c088885339a..1c49854e8528 100644 --- a/server/channels/app/role.go +++ b/server/channels/app/role.go @@ -330,7 +330,7 @@ func (a *App) sendUpdatedRoleEvent(role *model.Role) *model.AppError { if totalBroadcasts >= maxBroadcasts { a.Log().Error("sendUpdatedRoleEvent: hit broadcast limit for team scheme", mlog.String("scheme_id", scheme.Id), - mlog.Int("totalBroadcasts", totalBroadcasts)) + mlog.Int("total_broadcasts", totalBroadcasts)) break } offset += pageSize @@ -353,7 +353,7 @@ func (a *App) sendUpdatedRoleEvent(role *model.Role) *model.AppError { if totalBroadcasts >= maxBroadcasts { a.Log().Error("sendUpdatedRoleEvent: hit broadcast limit for channel scheme", mlog.String("scheme_id", scheme.Id), - mlog.Int("totalBroadcasts", totalBroadcasts)) + mlog.Int("total_broadcasts", totalBroadcasts)) break } offset += pageSize diff --git a/server/channels/app/server.go b/server/channels/app/server.go index 0f70afa9c093..defab167a4d6 100644 --- a/server/channels/app/server.go +++ b/server/channels/app/server.go @@ -484,7 +484,7 @@ func NewServer(options ...Option) (*Server, error) { } s.clusterLeaderListenerId = s.AddClusterLeaderChangedListener(func() { - mlog.Info("Cluster leader changed. Determining if job schedulers should be running:", mlog.Bool("isLeader", s.IsLeader())) + mlog.Info("Cluster leader changed. Determining if job schedulers should be running:", mlog.Bool("is_leader", s.IsLeader())) if s.Jobs != nil { s.Jobs.HandleClusterLeaderChange(s.IsLeader()) } @@ -1958,13 +1958,13 @@ func runDNDStatusExpireJob(a *App) { } a.ch.srv.AddClusterLeaderChangedListener(func() { - mlog.Info("Cluster leader changed. Determining if unset DNS status task should be running", mlog.Bool("isLeader", a.IsLeader())) + mlog.Info("Cluster leader changed. Determining if unset DNS status task should be running", mlog.Bool("is_leader", a.IsLeader())) if a.IsLeader() { withMut(&a.ch.dndTaskMut, func() { a.ch.dndTask = model.CreateRecurringTaskFromNextIntervalTime("Unset DND Statuses", a.UpdateDNDStatusOfUsers, model.DNDExpiryInterval) }) } else { - mlog.Debug("This is no longer leader node. Cancelling the unset DND status task", mlog.Bool("isLeader", a.IsLeader())) + mlog.Debug("This is no longer leader node. Cancelling the unset DND status task", mlog.Bool("is_leader", a.IsLeader())) cancelTask(&a.ch.dndTaskMut, &a.ch.dndTask) } }) @@ -1982,7 +1982,7 @@ func runPostReminderJob(a *App) { } a.ch.srv.AddClusterLeaderChangedListener(func() { - mlog.Info("Cluster leader changed. Determining if post reminder task should be running", mlog.Bool("isLeader", a.IsLeader())) + mlog.Info("Cluster leader changed. Determining if post reminder task should be running", mlog.Bool("is_leader", a.IsLeader())) if a.IsLeader() { rctx := request.EmptyContext(a.Log()) withMut(&a.ch.postReminderMut, func() { @@ -1990,7 +1990,7 @@ func runPostReminderJob(a *App) { a.ch.postReminderTask = model.CreateRecurringTaskFromNextIntervalTime("Check Post reminders", fn, 5*time.Minute) }) } else { - mlog.Debug("This is no longer leader node. Cancelling the post reminder task", mlog.Bool("isLeader", a.IsLeader())) + mlog.Debug("This is no longer leader node. Cancelling the post reminder task", mlog.Bool("is_leader", a.IsLeader())) cancelTask(&a.ch.postReminderMut, &a.ch.postReminderTask) } }) @@ -2004,11 +2004,11 @@ func runScheduledPostJob(a *App) { } a.ch.srv.AddClusterLeaderChangedListener(func() { - mlog.Info("Cluster leader changed. Determining if scheduled posts task should be running", mlog.Bool("isLeader", a.IsLeader())) + mlog.Info("Cluster leader changed. Determining if scheduled posts task should be running", mlog.Bool("is_leader", a.IsLeader())) if a.IsLeader() { doRunScheduledPostJob(a) } else { - mlog.Debug("This is no longer leader node. Cancelling the scheduled post task", mlog.Bool("isLeader", a.IsLeader())) + mlog.Debug("This is no longer leader node. Cancelling the scheduled post task", mlog.Bool("is_leader", a.IsLeader())) cancelTask(&a.ch.scheduledPostMut, &a.ch.scheduledPostTask) } }) diff --git a/server/channels/app/session.go b/server/channels/app/session.go index 15d7f2f26519..f433136d1f9e 100644 --- a/server/channels/app/session.go +++ b/server/channels/app/session.go @@ -459,7 +459,7 @@ func (a *App) ExtendSessionExpiryIfNeeded(rctx request.CTX, session *model.Sessi rctx.Logger().Debug("Session extended", mlog.String("user_id", session.UserId), mlog.String("session_id", session.Id), - mlog.Int("newExpiry", newExpiry), + mlog.Int("new_expiry", newExpiry), mlog.Int("session_length", sessionLength), ) diff --git a/server/channels/app/slashcommands/command_invite.go b/server/channels/app/slashcommands/command_invite.go index 645f8b96ac4c..30d884329c59 100644 --- a/server/channels/app/slashcommands/command_invite.go +++ b/server/channels/app/slashcommands/command_invite.go @@ -312,7 +312,7 @@ func (i *InviteProvider) addUserToChannel(a *app.App, rctx request.CTX, args *mo err.Id == "api.channel.add_user.to.channel.failed.deleted.app_error" { return UserNotInTeam } - rctx.Logger().Warn("addUserToChannel had unexpected error.", mlog.String("UserId", userProfile.Id), mlog.Err(err)) + rctx.Logger().Warn("addUserToChannel had unexpected error.", mlog.String("user_id", userProfile.Id), mlog.Err(err)) return Unknown } diff --git a/server/channels/app/status.go b/server/channels/app/status.go index fe5803027bf7..ebd904ac3643 100644 --- a/server/channels/app/status.go +++ b/server/channels/app/status.go @@ -99,7 +99,7 @@ func (a *App) SetCustomStatus(rctx request.CTX, userID string, cs *model.CustomS } if err := user.SetCustomStatus(cs); err != nil { - rctx.Logger().Error("Failed to set custom status", mlog.String("userID", userID), mlog.Err(err)) + rctx.Logger().Error("Failed to set custom status", mlog.String("user_id", userID), mlog.Err(err)) } _, updateErr := a.UpdateUser(rctx, user, true) if updateErr != nil { @@ -107,7 +107,7 @@ func (a *App) SetCustomStatus(rctx request.CTX, userID string, cs *model.CustomS } if err = a.addRecentCustomStatus(rctx, userID, cs); err != nil { - rctx.Logger().Error("Can't add recent custom status for", mlog.String("userID", userID), mlog.Err(err)) + rctx.Logger().Error("Can't add recent custom status for", mlog.String("user_id", userID), mlog.Err(err)) } return nil diff --git a/server/channels/app/upload.go b/server/channels/app/upload.go index 5722133665f4..aa88ef0494ae 100644 --- a/server/channels/app/upload.go +++ b/server/channels/app/upload.go @@ -352,10 +352,10 @@ func (a *App) UploadData(rctx request.CTX, us *model.UploadSession, rd io.Reader if !a.Srv().GoExtraction(func() { err := a.ExtractContentFromFileInfo(rctx, &infoCopy) if err != nil { - rctx.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("fileInfoId", infoCopy.Id)) + rctx.Logger().Error("Failed to extract file content", mlog.Err(err), mlog.String("file_info_id", infoCopy.Id)) } }) { - rctx.Logger().Warn("Content extraction queue is full, skipping inline extraction; this file's content will not be searchable until the scheduled content extraction catch-up job runs or an admin runs a content extraction job (e.g. mmctl extract)", mlog.String("fileInfoId", infoCopy.Id)) + rctx.Logger().Warn("Content extraction queue is full, skipping inline extraction; this file's content will not be searchable until the scheduled content extraction catch-up job runs or an admin runs a content extraction job (e.g. mmctl extract)", mlog.String("file_info_id", infoCopy.Id)) } } diff --git a/server/channels/audit/audit.go b/server/channels/audit/audit.go index 7f0feeda879c..0e8ceb3174ed 100644 --- a/server/channels/audit/audit.go +++ b/server/channels/audit/audit.go @@ -73,7 +73,7 @@ func (a *Audit) onQueueFull(rec *mlog.LogRec, maxQueueSize int) bool { if a.OnQueueFull != nil { return a.OnQueueFull("main", maxQueueSize) } - mlog.Error("Audit logging queue full, dropping record.", mlog.Int("queueSize", maxQueueSize)) + mlog.Error("Audit logging queue full, dropping record.", mlog.Int("queue_size", maxQueueSize)) return true } @@ -81,7 +81,7 @@ func (a *Audit) onTargetQueueFull(target mlog.Target, rec *mlog.LogRec, maxQueue if a.OnQueueFull != nil { return a.OnQueueFull(fmt.Sprintf("%v", target), maxQueueSize) } - mlog.Error("Audit logging queue full for target, dropping record.", mlog.Any("target", target), mlog.Int("queueSize", maxQueueSize)) + mlog.Error("Audit logging queue full for target, dropping record.", mlog.Any("target", target), mlog.Int("queue_size", maxQueueSize)) return true } diff --git a/server/channels/manualtesting/manual_testing.go b/server/channels/manualtesting/manual_testing.go index 7490917496eb..cd11e9979a82 100644 --- a/server/channels/manualtesting/manual_testing.go +++ b/server/channels/manualtesting/manual_testing.go @@ -197,6 +197,6 @@ func getChannelID(a *app.App, channelname string, teamid string, userid string) return channel.Id, true } } - mlog.Debug("Could not find channel", mlog.String("Channel name", channelname), mlog.Int("Possibilities searched", len(channels))) + mlog.Debug("Could not find channel", mlog.String("channel_name", channelname), mlog.Int("possibilities_searched", len(channels))) return "", false } diff --git a/server/channels/store/sqlstore/testpool.go b/server/channels/store/sqlstore/testpool.go index 76125c8a7285..d739c1f2447b 100644 --- a/server/channels/store/sqlstore/testpool.go +++ b/server/channels/store/sqlstore/testpool.go @@ -27,7 +27,7 @@ type TestPoolEntry struct { } func NewTestPool(logger mlog.LoggerIFace, driverName string, poolSize int) (*TestPool, error) { - logger.Info("Creating test store pool", mlog.Int("poolSize", poolSize)) + logger.Info("Creating test store pool", mlog.Int("pool_size", poolSize)) entries := make(map[string]*TestPoolEntry, poolSize) @@ -69,7 +69,7 @@ func (p *TestPool) Get(t testing.TB) *TestPoolEntry { p.mut.Lock() defer p.mut.Unlock() - p.logger.Info("Getting from test store pool", mlog.Int("poolSize", len(p.entries))) + p.logger.Info("Getting from test store pool", mlog.Int("pool_size", len(p.entries))) var poolEntry *TestPoolEntry for _, entry := range p.entries { @@ -83,7 +83,7 @@ func (p *TestPool) Get(t testing.TB) *TestPoolEntry { return nil } - p.logger.Info("Got store from pool", mlog.String("datasource", *poolEntry.Settings.DataSource), mlog.Int("poolSize", len(p.entries))) + p.logger.Info("Got store from pool", mlog.String("datasource", *poolEntry.Settings.DataSource), mlog.Int("pool_size", len(p.entries))) dataSource := *poolEntry.Settings.DataSource @@ -91,7 +91,7 @@ func (p *TestPool) Get(t testing.TB) *TestPoolEntry { t.Cleanup(func() { p.mut.Lock() defer p.mut.Unlock() - p.logger.Info("Returning to test store pool", mlog.String("datasource", dataSource), mlog.Int("poolSize", len(p.entries))) + p.logger.Info("Returning to test store pool", mlog.String("datasource", dataSource), mlog.Int("pool_size", len(p.entries))) p.entries[dataSource] = poolEntry }) diff --git a/server/channels/web/static.go b/server/channels/web/static.go index bcc4e808281b..4432188c5a43 100644 --- a/server/channels/web/static.go +++ b/server/channels/web/static.go @@ -32,7 +32,7 @@ func (w *Web) InitStatic() { } staticDir, _ := fileutils.FindDir(model.ClientDir) - mlog.Debug("Using client directory", mlog.String("clientDir", staticDir)) + mlog.Debug("Using client directory", mlog.String("client_dir", staticDir)) subpath, _ := utils.GetSubpathFromConfig(w.srv.Config()) diff --git a/server/cmd/mattermost/commands/db_ping.go b/server/cmd/mattermost/commands/db_ping.go index 80729a24e407..a79ae303e5a2 100644 --- a/server/cmd/mattermost/commands/db_ping.go +++ b/server/cmd/mattermost/commands/db_ping.go @@ -92,7 +92,7 @@ func dbPingCmdF(command *cobra.Command, _ []string) error { defer cancel() return pingWithRetry(ctx, db, retryInterval, logger.With( - mlog.String("dataSource", sanitized), + mlog.String("data_source", sanitized), )) } diff --git a/server/enterprise/elasticsearch/common/indexing_job.go b/server/enterprise/elasticsearch/common/indexing_job.go index 2acdb08f6c6a..550075d7239d 100644 --- a/server/enterprise/elasticsearch/common/indexing_job.go +++ b/server/enterprise/elasticsearch/common/indexing_job.go @@ -458,12 +458,12 @@ func (worker *IndexerWorker) BulkIndexPosts(posts []*model.PostForIndexing, prog err = worker.addItemToBulkProcessor(indexName, indexOp, searchPost.Id, bytes.NewReader(data)) if err != nil { - worker.logger.Warn("Failed to add item to bulk processor", mlog.String("indexName", indexName), mlog.Err(err)) + worker.logger.Warn("Failed to add item to bulk processor", mlog.String("index_name", indexName), mlog.Err(err)) } } else { err := worker.addItemToBulkProcessor(indexName, deleteOp, post.Id, nil) if err != nil { - worker.logger.Warn("Failed to add item to bulk processor", mlog.String("indexName", indexName), mlog.Err(err)) + worker.logger.Warn("Failed to add item to bulk processor", mlog.String("index_name", indexName), mlog.Err(err)) } } } @@ -534,12 +534,12 @@ func (worker *IndexerWorker) BulkIndexFiles(files []*model.FileForIndexing, prog err = worker.addItemToBulkProcessor(indexName, indexOp, searchFile.Id, bytes.NewReader(data)) if err != nil { - worker.logger.Warn("Failed to add item to bulk processor", mlog.String("indexName", indexName), mlog.Err(err)) + worker.logger.Warn("Failed to add item to bulk processor", mlog.String("index_name", indexName), mlog.Err(err)) } } else { err := worker.addItemToBulkProcessor(indexName, deleteOp, file.Id, nil) if err != nil { - worker.logger.Warn("Failed to add item to bulk processor", mlog.String("indexName", indexName), mlog.Err(err)) + worker.logger.Warn("Failed to add item to bulk processor", mlog.String("index_name", indexName), mlog.Err(err)) } } } @@ -629,7 +629,7 @@ func BulkIndexChannels(config *model.Config, err = addItemToBulkProcessorFn(indexName, indexOp, searchChannel.Id, bytes.NewReader(data)) if err != nil { - logger.Warn("Failed to add item to bulk processor", mlog.String("indexName", indexName), mlog.Err(err)) + logger.Warn("Failed to add item to bulk processor", mlog.String("index_name", indexName), mlog.Err(err)) } } @@ -697,7 +697,7 @@ func (worker *IndexerWorker) BulkIndexUsers(users []*model.UserForIndexing, prog err = worker.addItemToBulkProcessor(indexName, indexOp, searchUser.Id, bytes.NewReader(data)) if err != nil { - worker.logger.Warn("Failed to add item to bulk processor", mlog.String("indexName", indexName), mlog.Err(err)) + worker.logger.Warn("Failed to add item to bulk processor", mlog.String("index_name", indexName), mlog.Err(err)) } } @@ -855,7 +855,7 @@ func setEntityCount(logger mlog.LoggerIFace, jobServer *jobs.JobServer, progress // on with the indexing job anyway. The only issue is that the progress % reporting will be inaccurate. if count, err := jobServer.Store.Post().AnalyticsPostCount(&model.PostCountOptions{}); err != nil { fallback := entityCountFallback(job, "total_posts_count", estimatedPostCount, progress.DonePostsCount) - logger.Warn("Worker: Failed to fetch total post count for job. A fallback value will be used for progress reporting.", mlog.Int("fallbackPostCount", fallback), mlog.Err(err)) + logger.Warn("Worker: Failed to fetch total post count for job. A fallback value will be used for progress reporting.", mlog.Int("fallback_post_count", fallback), mlog.Err(err)) progress.TotalPostsCount = fallback } else { progress.TotalPostsCount = count @@ -867,7 +867,7 @@ func setEntityCount(logger mlog.LoggerIFace, jobServer *jobs.JobServer, progress // Same possible fail as above can happen when counting channels if count, err := jobServer.Store.Channel().AnalyticsTypeCount("", ""); err != nil { fallback := entityCountFallback(job, "total_channels_count", estimatedChannelCount, progress.DoneChannelsCount) - logger.Warn("Worker: Failed to fetch total channel count for job. A fallback value will be used for progress reporting.", mlog.Int("fallbackChannelCount", fallback), mlog.Err(err)) + logger.Warn("Worker: Failed to fetch total channel count for job. A fallback value will be used for progress reporting.", mlog.Int("fallback_channel_count", fallback), mlog.Err(err)) progress.TotalChannelsCount = fallback } else { progress.TotalChannelsCount = count @@ -882,7 +882,7 @@ func setEntityCount(logger mlog.LoggerIFace, jobServer *jobs.JobServer, progress // since ExcludeRegularUsers is set to false }); err != nil { fallback := entityCountFallback(job, "total_users_count", estimatedUserCount, progress.DoneUsersCount) - logger.Warn("Worker: Failed to fetch total user count for job. A fallback value will be used for progress reporting.", mlog.Int("fallbackUserCount", fallback), mlog.Err(err)) + logger.Warn("Worker: Failed to fetch total user count for job. A fallback value will be used for progress reporting.", mlog.Int("fallback_user_count", fallback), mlog.Err(err)) progress.TotalUsersCount = fallback } else { progress.TotalUsersCount = count @@ -894,7 +894,7 @@ func setEntityCount(logger mlog.LoggerIFace, jobServer *jobs.JobServer, progress // Same possible fail as above can happen when counting files if count, err := jobServer.Store.FileInfo().CountAll(); err != nil { fallback := entityCountFallback(job, "total_files_count", estimatedFilesCount, progress.DoneFilesCount) - logger.Warn("Worker: Failed to fetch total files count for job. A fallback value will be used for progress reporting.", mlog.Int("fallbackFilesCount", fallback), mlog.Err(err)) + logger.Warn("Worker: Failed to fetch total files count for job. A fallback value will be used for progress reporting.", mlog.Int("fallback_files_count", fallback), mlog.Err(err)) progress.TotalFilesCount = fallback } else { progress.TotalFilesCount = count diff --git a/server/platform/services/remotecluster/ping.go b/server/platform/services/remotecluster/ping.go index 68f485a205a6..744b1b083ac8 100644 --- a/server/platform/services/remotecluster/ping.go +++ b/server/platform/services/remotecluster/ping.go @@ -21,8 +21,8 @@ func (rcs *Service) PingNow(rc *model.RemoteCluster) { if pingErr != nil { rcs.server.Log().LogM(mlog.MlvlRemoteClusterServiceWarn, "Remote cluster ping failed", mlog.String("remote", rc.DisplayName), - mlog.String("remoteId", rc.RemoteId), - mlog.String("pluginId", rc.PluginID), + mlog.String("remote_id", rc.RemoteId), + mlog.String("plugin_id", rc.PluginID), mlog.Err(pingErr), ) } @@ -152,7 +152,7 @@ func (rcs *Service) pingRemote(rc *model.RemoteCluster) error { if err := rcs.server.GetStore().RemoteCluster().SetLastPingAt(rc.RemoteId); err != nil { rcs.server.Log().LogM(mlog.MlvlRemoteClusterServiceError, "Failed to update LastPingAt for remote cluster", mlog.String("remote", rc.DisplayName), - mlog.String("remoteId", rc.RemoteId), + mlog.String("remote_id", rc.RemoteId), mlog.Err(err), ) } @@ -169,11 +169,11 @@ func (rcs *Service) pingRemote(rc *model.RemoteCluster) error { rcs.server.Log().Log(mlog.LvlRemoteClusterServiceDebug, "Remote cluster ping", mlog.String("remote", rc.DisplayName), - mlog.String("remoteId", rc.RemoteId), - mlog.String("pluginId", rc.PluginID), - mlog.Int("SentAt", ping.SentAt), - mlog.Int("RecvAt", ping.RecvAt), - mlog.Int("Diff", ping.RecvAt-ping.SentAt), + mlog.String("remote_id", rc.RemoteId), + mlog.String("plugin_id", rc.PluginID), + mlog.Int("sent_at", ping.SentAt), + mlog.Int("recv_at", ping.RecvAt), + mlog.Int("diff", ping.RecvAt-ping.SentAt), ) return nil } diff --git a/server/platform/services/remotecluster/recv.go b/server/platform/services/remotecluster/recv.go index 4d96ec8fc3f2..7ee541ce9943 100644 --- a/server/platform/services/remotecluster/recv.go +++ b/server/platform/services/remotecluster/recv.go @@ -33,7 +33,7 @@ func (rcs *Service) ReceiveIncomingMsg(rc *model.RemoteCluster, msg model.Remote for _, l := range listeners { if err := callback(l, msg, &rcSanitized, &response); err != nil { rcs.server.Log().LogM(mlog.MlvlRemoteClusterServiceError, "Error from remote cluster message listener", - mlog.String("msgId", msg.Id), mlog.String("topic", msg.Topic), mlog.String("remote", rc.DisplayName), mlog.Err(err)) + mlog.String("msg_id", msg.Id), mlog.String("topic", msg.Topic), mlog.String("remote", rc.DisplayName), mlog.Err(err)) response.Status = ResponseStatusFail response.Err = err.Error() diff --git a/server/platform/services/remotecluster/sendfile.go b/server/platform/services/remotecluster/sendfile.go index f3c807b670fe..78e02ddf57bc 100644 --- a/server/platform/services/remotecluster/sendfile.go +++ b/server/platform/services/remotecluster/sendfile.go @@ -61,7 +61,7 @@ func (rcs *Service) sendFile(task sendFileTask) { if err != nil { rcs.server.Log().LogM(mlog.MlvlRemoteClusterServiceError, "Remote Cluster send file failed", mlog.String("remote", task.rc.DisplayName), - mlog.String("uploadId", task.us.Id), + mlog.String("upload_id", task.us.Id), mlog.Err(err), ) response.Status = ResponseStatusFail @@ -69,7 +69,7 @@ func (rcs *Service) sendFile(task sendFileTask) { } else { rcs.server.Log().Log(mlog.LvlRemoteClusterServiceDebug, "Remote Cluster file sent successfully", mlog.String("remote", task.rc.DisplayName), - mlog.String("uploadId", task.us.Id), + mlog.String("upload_id", task.us.Id), ) response.Status = ResponseStatusOK response.SetPayload(fi) @@ -91,7 +91,7 @@ func (rcs *Service) sendFileToRemote(timeout time.Duration, task sendFileTask) ( rcs.server.Log().Log(mlog.LvlRemoteClusterServiceDebug, "sending file to remote...", mlog.String("remote", task.rc.DisplayName), - mlog.String("uploadId", task.us.Id), + mlog.String("upload_id", task.us.Id), mlog.String("file_path", task.us.Path), ) diff --git a/server/platform/services/remotecluster/sendmsg.go b/server/platform/services/remotecluster/sendmsg.go index e4a6efcc83b4..c2ecae9c59da 100644 --- a/server/platform/services/remotecluster/sendmsg.go +++ b/server/platform/services/remotecluster/sendmsg.go @@ -101,7 +101,7 @@ func (rcs *Service) sendMsg(task sendMsgTask) { if err != nil { rcs.server.Log().LogM(mlog.MlvlRemoteClusterServiceError, "Invalid siteURL while sending message to remote", mlog.String("remote", task.rc.DisplayName), - mlog.String("msgId", task.msg.Id), + mlog.String("msg_id", task.msg.Id), mlog.Err(err), ) errResp = err @@ -114,14 +114,14 @@ func (rcs *Service) sendMsg(task sendMsgTask) { if err != nil { rcs.server.Log().LogM(mlog.MlvlRemoteClusterServiceError, "Remote Cluster send message failed", mlog.String("remote", task.rc.DisplayName), - mlog.String("msgId", task.msg.Id), + mlog.String("msg_id", task.msg.Id), mlog.Err(err), ) errResp = err } else { rcs.server.Log().Log(mlog.LvlRemoteClusterServiceDebug, "Remote Cluster message sent successfully", mlog.String("remote", task.rc.DisplayName), - mlog.String("msgId", task.msg.Id), + mlog.String("msg_id", task.msg.Id), ) if err = json.Unmarshal(respJSON, &response); err != nil { diff --git a/server/platform/services/remotecluster/sendprofileImage.go b/server/platform/services/remotecluster/sendprofileImage.go index 4bad6f3e42ab..fb41f8ae7c29 100644 --- a/server/platform/services/remotecluster/sendprofileImage.go +++ b/server/platform/services/remotecluster/sendprofileImage.go @@ -60,7 +60,7 @@ func (rcs *Service) sendProfileImage(task sendProfileImageTask) { if err != nil { rcs.server.Log().LogM(mlog.MlvlRemoteClusterServiceWarn, "Remote Cluster send profile image failed", mlog.String("remote", task.rc.DisplayName), - mlog.String("UserId", task.userID), + mlog.String("user_id", task.userID), mlog.Err(err), ) response.Status = ResponseStatusFail @@ -68,7 +68,7 @@ func (rcs *Service) sendProfileImage(task sendProfileImageTask) { } else { rcs.server.Log().Log(mlog.LvlRemoteClusterServiceDebug, "Remote Cluster profile image sent successfully", mlog.String("remote", task.rc.DisplayName), - mlog.String("UserId", task.userID), + mlog.String("user_id", task.userID), ) response.Status = ResponseStatusOK } @@ -89,7 +89,7 @@ func (rcs *Service) sendProfileImageToRemote(timeout time.Duration, task sendPro rcs.server.Log().Log(mlog.LvlRemoteClusterServiceDebug, "sending profile image to remote...", mlog.String("remote", task.rc.DisplayName), - mlog.String("UserId", task.userID), + mlog.String("user_id", task.userID), ) user, err := rcs.server.GetStore().User().Get(request.EmptyContext(rcs.server.Log()), task.userID) diff --git a/server/platform/services/sharedchannel/attachment.go b/server/platform/services/sharedchannel/attachment.go index af3bf6b0dd81..ce85c4f54cea 100644 --- a/server/platform/services/sharedchannel/attachment.go +++ b/server/platform/services/sharedchannel/attachment.go @@ -107,7 +107,7 @@ func (scs *Service) sendAttachmentForRemote(fi *model.FileInfo, post *model.Post if !resp.IsSuccess() { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceError, "send file failed", mlog.String("remote", rc.DisplayName), - mlog.String("uploadId", usResp.Id), + mlog.String("upload_id", usResp.Id), mlog.String("err", resp.Err), ) return @@ -118,7 +118,7 @@ func (scs *Service) sendAttachmentForRemote(fi *model.FileInfo, post *model.Post if err2 := json.Unmarshal(resp.Payload, &fi); err2 != nil { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceWarn, "invalid file info response after send file", mlog.String("remote", rc.DisplayName), - mlog.String("uploadId", usResp.Id), + mlog.String("upload_id", usResp.Id), mlog.Err(err2), ) return @@ -128,7 +128,7 @@ func (scs *Service) sendAttachmentForRemote(fi *model.FileInfo, post *model.Post if err2 := scs.saveSharedAttachment(&fi, rc); err2 != nil { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceError, "error saving SharedChannelAttachment", mlog.String("remote", rc.DisplayName), - mlog.String("uploadId", usResp.Id), + mlog.String("upload_id", usResp.Id), mlog.Err(err2), ) return @@ -136,7 +136,7 @@ func (scs *Service) sendAttachmentForRemote(fi *model.FileInfo, post *model.Post scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "send file successful", mlog.String("remote", rc.DisplayName), - mlog.String("uploadId", usResp.Id), + mlog.String("upload_id", usResp.Id), ) }) } diff --git a/server/platform/services/sharedchannel/membership.go b/server/platform/services/sharedchannel/membership.go index 2f741d51b4a8..f749dae509bf 100644 --- a/server/platform/services/sharedchannel/membership.go +++ b/server/platform/services/sharedchannel/membership.go @@ -45,7 +45,7 @@ func (scs *Service) ForceMembershipSyncForRemote(rc *model.RemoteCluster) { if err != nil { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceError, "Failed to fetch shared channel remotes for membership sync", mlog.String("remote", rc.DisplayName), - mlog.String("remoteId", rc.RemoteId), + mlog.String("remote_id", rc.RemoteId), mlog.Err(err), ) return diff --git a/server/platform/services/sharedchannel/permalink.go b/server/platform/services/sharedchannel/permalink.go index 5d7d42cda673..325997c5ee31 100644 --- a/server/platform/services/sharedchannel/permalink.go +++ b/server/platform/services/sharedchannel/permalink.go @@ -44,7 +44,7 @@ func (scs *Service) processPermalinkToRemote(p *model.Post) string { return msg } if len(postList.Order) == 0 { - scs.server.Log().LogM(mlog.MlvlSharedChannelServiceWarn, "No post found for permalink", mlog.String("postID", postID)) + scs.server.Log().LogM(mlog.MlvlSharedChannelServiceWarn, "No post found for permalink", mlog.String("post_id", postID)) return msg } diff --git a/server/platform/services/sharedchannel/service.go b/server/platform/services/sharedchannel/service.go index 201f4e569891..57e412f58f50 100644 --- a/server/platform/services/sharedchannel/service.go +++ b/server/platform/services/sharedchannel/service.go @@ -281,7 +281,7 @@ func (scs *Service) onConnectionStateChange(rc *model.RemoteCluster, online bool scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Remote cluster connection status changed", mlog.String("remote", rc.DisplayName), - mlog.String("remoteId", rc.RemoteId), + mlog.String("remote_id", rc.RemoteId), mlog.Bool("online", online), ) } @@ -455,7 +455,7 @@ func (scs *Service) scheduleGlobalUserSync(rc *model.RemoteCluster) { scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Scheduled global user sync task for remote", mlog.String("remote", rc.DisplayName), - mlog.String("remoteId", rc.RemoteId), + mlog.String("remote_id", rc.RemoteId), ) }() } diff --git a/server/platform/services/sharedchannel/sync_recv.go b/server/platform/services/sharedchannel/sync_recv.go index a49a22a01e59..5a9dc22972cb 100644 --- a/server/platform/services/sharedchannel/sync_recv.go +++ b/server/platform/services/sharedchannel/sync_recv.go @@ -167,9 +167,9 @@ func (scs *Service) ProcessSyncMessage(rctx request.CTX, syncMsg *model.SyncMsg, if syncMsg.ChannelId != post.ChannelId { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceWarn, "ChannelId mismatch", mlog.String("remote", rc.Name), - mlog.String("sm.ChannelId", syncMsg.ChannelId), - mlog.String("sm.Post.ChannelId", post.ChannelId), - mlog.String("PostId", post.Id), + mlog.String("sm_channel_id", syncMsg.ChannelId), + mlog.String("sm_post_channel_id", post.ChannelId), + mlog.String("post_id", post.Id), ) syncResp.PostErrors = append(syncResp.PostErrors, post.Id) continue @@ -180,8 +180,8 @@ func (scs *Service) ProcessSyncMessage(rctx request.CTX, syncMsg *model.SyncMsg, team, err2 = scs.server.GetStore().Channel().GetTeamForChannel(syncMsg.ChannelId) if err2 != nil { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceError, "Error getting Team for Channel", - mlog.String("ChannelId", post.ChannelId), - mlog.String("PostId", post.Id), + mlog.String("channel_id", post.ChannelId), + mlog.String("post_id", post.Id), mlog.String("remote", rc.Name), mlog.Err(err2), ) diff --git a/server/platform/services/sharedchannel/sync_send.go b/server/platform/services/sharedchannel/sync_send.go index 0c8e248f36b7..5107e7490da8 100644 --- a/server/platform/services/sharedchannel/sync_send.go +++ b/server/platform/services/sharedchannel/sync_send.go @@ -91,7 +91,7 @@ func (scs *Service) NotifyUserProfileChanged(userID string) { scusers, err := scs.server.GetStore().SharedChannel().GetUsersForUser(userID) if err != nil { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceError, "Failed to fetch shared channel users", - mlog.String("userID", userID), + mlog.String("user_id", userID), mlog.Err(err), ) return @@ -130,7 +130,7 @@ func (scs *Service) NotifyUserStatusChanged(status *model.Status) { if status.UserId == "" { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceWarn, "Received invalid status for sync", - mlog.String("userID", status.UserId), + mlog.String("user_id", status.UserId), ) return } @@ -138,7 +138,7 @@ func (scs *Service) NotifyUserStatusChanged(status *model.Status) { scusers, err := scs.server.GetStore().SharedChannel().GetUsersForUser(status.UserId) if err != nil { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceError, "Failed to fetch shared channel users", - mlog.String("userID", status.UserId), + mlog.String("user_id", status.UserId), mlog.Err(err), ) return @@ -172,7 +172,7 @@ func (scs *Service) SendPendingInvitesForRemote(rc *model.RemoteCluster) { scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Processing pending invites for remote after reconnection", mlog.String("remote", rc.DisplayName), - mlog.String("remoteId", rc.RemoteId), + mlog.String("remote_id", rc.RemoteId), ) opts := model.SharedChannelRemoteFilterOpts{ @@ -183,7 +183,7 @@ func (scs *Service) SendPendingInvitesForRemote(rc *model.RemoteCluster) { if err != nil { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceError, "Failed to fetch shared channel remotes for pending invites", mlog.String("remote", rc.DisplayName), - mlog.String("remoteId", rc.RemoteId), + mlog.String("remote_id", rc.RemoteId), mlog.Err(err), ) return @@ -213,7 +213,7 @@ func (scs *Service) SendPendingInvitesForRemote(rc *model.RemoteCluster) { scs.server.Log().Log(mlog.LvlSharedChannelServiceDebug, "Pending invite sent", mlog.String("remote", rc.DisplayName), - mlog.String("remoteId", rc.RemoteId), + mlog.String("remote_id", rc.RemoteId), mlog.String("channel_id", scr.ChannelId), mlog.String("sharedchannelremote_id", scr.Id), ) @@ -234,7 +234,7 @@ func (scs *Service) ForceSyncForRemote(rc *model.RemoteCluster) { if err != nil { scs.server.Log().LogM(mlog.MlvlSharedChannelServiceError, "Failed to fetch shared channel remotes", mlog.String("remote", rc.DisplayName), - mlog.String("remoteId", rc.RemoteId), + mlog.String("remote_id", rc.RemoteId), mlog.Err(err), ) return @@ -347,8 +347,8 @@ func (scs *Service) doSync() time.Duration { scs.addTask(task) } else { scs.server.Log().Error("Failed to synchronize shared channel", - mlog.String("channelId", task.channelID), - mlog.String("remoteId", task.remoteID), + mlog.String("channel_id", task.channelID), + mlog.String("remote_id", task.remoteID), mlog.Err(err), ) } @@ -467,7 +467,7 @@ func (scs *Service) processTask(task syncTask) error { scs.addTask(rtask) } else { scs.server.Log().Error("Failed to synchronize shared channel for remote cluster", - mlog.String("channelId", rtask.channelID), + mlog.String("channel_id", rtask.channelID), mlog.String("remote", rc.DisplayName), mlog.Err(err), ) @@ -487,8 +487,8 @@ func (scs *Service) selfHealOrphanedSharedChannelRemote(channelID, remoteID stri if err != nil { // The SCR row is already gone (or unreadable); nothing left to self-heal. scs.server.Log().Warn("Skipping sync for deleted remote cluster", - mlog.String("channelId", channelID), - mlog.String("remoteId", remoteID), + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), mlog.Err(err), ) return @@ -499,26 +499,26 @@ func (scs *Service) selfHealOrphanedSharedChannelRemote(channelID, remoteID stri // remote no longer syncing is the expected steady state, so this is logged at // debug rather than warn to avoid noise. scs.server.Log().Debug("Skipping sync for deleted remote cluster", - mlog.String("channelId", channelID), - mlog.String("remoteId", remoteID), + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), ) return } if _, err := scs.server.GetStore().SharedChannel().DeleteRemote(scr.Id); err != nil { scs.server.Log().Warn("Failed to self-heal orphaned shared channel remote for deleted remote cluster", - mlog.String("channelId", channelID), - mlog.String("remoteId", remoteID), - mlog.String("sharedChannelRemoteId", scr.Id), + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), + mlog.String("shared_channel_remote_id", scr.Id), mlog.Err(err), ) return } scs.server.Log().Warn("Self-healed orphaned shared channel remote for deleted remote cluster", - mlog.String("channelId", channelID), - mlog.String("remoteId", remoteID), - mlog.String("sharedChannelRemoteId", scr.Id), + mlog.String("channel_id", channelID), + mlog.String("remote_id", remoteID), + mlog.String("shared_channel_remote_id", scr.Id), ) } diff --git a/tools/mattermost-govet/README.md b/tools/mattermost-govet/README.md index 9decfcdd4c80..0e6ad2ca7e12 100644 --- a/tools/mattermost-govet/README.md +++ b/tools/mattermost-govet/README.md @@ -9,6 +9,7 @@ This package contains mattermost-specific go-vet rules that are used to maintain 1. **license** - check the license header 1. **openApiSync** - check for inconsistencies between OpenAPI spec and the source code 1. **structuredLogging** - check invalid usage of logging (must use structured logging) +1. **mlogFieldNaming** - check that mlog field keys are snake_case 1. **tFatal** - check invalid usage of t.Fatal assertions (instead of testify methods) 1. **apiAuditLogs** - check that audit records are properly created in the API layer 1. **rawSql** - check invalid usage of raw SQL queries instead of using the squirrel lib diff --git a/tools/mattermost-govet/main.go b/tools/mattermost-govet/main.go index 071473a3adf5..0f3b629f458d 100644 --- a/tools/mattermost-govet/main.go +++ b/tools/mattermost-govet/main.go @@ -16,6 +16,7 @@ import ( "github.com/mattermost/mattermost/tools/mattermost-govet/immut" "github.com/mattermost/mattermost/tools/mattermost-govet/inconsistentReceiverName" "github.com/mattermost/mattermost/tools/mattermost-govet/license" + "github.com/mattermost/mattermost/tools/mattermost-govet/mlogFieldNaming" "github.com/mattermost/mattermost/tools/mattermost-govet/mutexLock" "github.com/mattermost/mattermost/tools/mattermost-govet/noSelectStar" "github.com/mattermost/mattermost/tools/mattermost-govet/openApiSync" @@ -33,6 +34,7 @@ func main() { license.Analyzer, license.EEAnalyzer, structuredLogging.Analyzer, + mlogFieldNaming.Analyzer, // appErrorWhere.Analyzer, tFatal.Analyzer, equalLenAsserts.Analyzer, diff --git a/tools/mattermost-govet/mlogFieldNaming/mlogFieldNaming.go b/tools/mattermost-govet/mlogFieldNaming/mlogFieldNaming.go new file mode 100644 index 000000000000..c90904afff96 --- /dev/null +++ b/tools/mattermost-govet/mlogFieldNaming/mlogFieldNaming.go @@ -0,0 +1,213 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package mlogFieldNaming + +import ( + "fmt" + "go/ast" + "go/constant" + "go/token" + "go/types" + "regexp" + "strconv" + "strings" + "unicode" + + "golang.org/x/tools/go/analysis" +) + +var Analyzer = &analysis.Analyzer{ + Name: "mlogFieldNaming", + Doc: "check that mlog field keys are snake_case", + Run: run, +} + +const mlogPkgPath = "github.com/mattermost/mattermost/server/public/shared/mlog" + +// keyedFieldConstructors are the mlog field constructors that take a field key +// as their first argument. +var keyedFieldConstructors = map[string]bool{ + "Any": true, + "Array": true, + "Bool": true, + "Duration": true, + "Float": true, + "Int": true, + "Map": true, + "Millis": true, + "NamedErr": true, + "String": true, + "Stringer": true, + "Time": true, + "Uint": true, +} + +var snakeCase = regexp.MustCompile(`^[a-z][a-z0-9]*(_[a-z0-9]+)*$`) + +func run(pass *analysis.Pass) (interface{}, error) { + for _, file := range pass.Files { + ast.Inspect(file, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok { + return true + } + + fun, ok := calleeSelector(call.Fun) + if !ok || !keyedFieldConstructors[fun.Sel.Name] { + return true + } + + if !isMlogPackage(pass, fun.X) { + return true + } + + if len(call.Args) == 0 { + return true + } + + arg := call.Args[0] + + key, ok := constantString(pass, arg) + if !ok { + // The key isn't statically known, so there's nothing to check. + return true + } + + if snakeCase.MatchString(key) { + return true + } + + pass.Report(diagnostic(arg, key)) + + return true + }) + } + + return nil, nil +} + +func diagnostic(arg ast.Expr, key string) analysis.Diagnostic { + // A fix can only rewrite the key in place when it is spelled out as a + // string literal. Keys that come from a named constant have to be renamed + // at the declaration instead, which is beyond what this analyzer offers. + lit, isLiteral := arg.(*ast.BasicLit) + isLiteral = isLiteral && lit.Kind == token.STRING + + fixed, canFix := toSnakeCase(key) + if !isLiteral || !canFix { + return analysis.Diagnostic{ + Pos: arg.Pos(), + End: arg.End(), + Message: fmt.Sprintf("mlog field key %q is not snake_case", key), + } + } + + return analysis.Diagnostic{ + Pos: arg.Pos(), + End: arg.End(), + Message: fmt.Sprintf("mlog field key %q is not snake_case, use %q", key, fixed), + SuggestedFixes: []analysis.SuggestedFix{{ + Message: fmt.Sprintf("Rename mlog field key to %q", fixed), + TextEdits: []analysis.TextEdit{{ + Pos: arg.Pos(), + End: arg.End(), + NewText: []byte(strconv.Quote(fixed)), + }}, + }}, + } +} + +// toSnakeCase converts a field key to snake_case, reporting whether the result +// is a valid snake_case key. Keys that cannot be converted mechanically (an +// empty key, or one that would start with a digit) are left to the author. +func toSnakeCase(key string) (string, bool) { + runes := []rune(key) + + var b strings.Builder + for i, r := range runes { + if !unicode.IsLetter(r) && !unicode.IsDigit(r) { + b.WriteRune('_') + continue + } + + // Break before an uppercase rune that starts a new word, either + // following a lowercase rune or a digit ("userId" -> "user_id"), or + // ending a run of uppercase runes ("requestURLPath" -> + // "request_url_path"). + if i > 0 && unicode.IsUpper(r) { + prev := runes[i-1] + startsWord := unicode.IsLower(prev) || unicode.IsDigit(prev) + endsAcronym := unicode.IsUpper(prev) && startsNewWord(runes, i+1) + if startsWord || endsAcronym { + b.WriteRune('_') + } + } + + b.WriteRune(unicode.ToLower(r)) + } + + fixed := strings.Trim(b.String(), "_") + for strings.Contains(fixed, "__") { + fixed = strings.ReplaceAll(fixed, "__", "_") + } + + return fixed, snakeCase.MatchString(fixed) +} + +// startsNewWord reports whether the lowercase run beginning at i is a new word +// rather than a plural suffix on the acronym that precedes it, so that +// "requestURLPath" breaks before "Path" but "userIDs" keeps its trailing "s" +// ("user_ids", not "user_i_ds"). +func startsNewWord(runes []rune, i int) bool { + if i >= len(runes) || !unicode.IsLower(runes[i]) { + return false + } + + // A lone "s" pluralizes the acronym instead of starting a word. + if runes[i] == 's' && (i+1 == len(runes) || !unicode.IsLower(runes[i+1])) { + return false + } + + return true +} + +// calleeSelector returns the pkg.Name selector a call expression resolves to. +// Most of the keyed constructors are generic, so an explicitly instantiated +// call such as mlog.Int[int64](...) reaches here as an index expression +// wrapping the selector rather than as the selector itself. +func calleeSelector(fun ast.Expr) (*ast.SelectorExpr, bool) { + switch expr := ast.Unparen(fun).(type) { + case *ast.IndexExpr: + fun = expr.X + case *ast.IndexListExpr: + fun = expr.X + } + + sel, ok := ast.Unparen(fun).(*ast.SelectorExpr) + + return sel, ok +} + +func isMlogPackage(pass *analysis.Pass, expr ast.Expr) bool { + ident, ok := expr.(*ast.Ident) + if !ok { + return false + } + + pkgName, ok := pass.TypesInfo.Uses[ident].(*types.PkgName) + if !ok { + return false + } + + return pkgName.Imported().Path() == mlogPkgPath +} + +func constantString(pass *analysis.Pass, expr ast.Expr) (string, bool) { + tv, ok := pass.TypesInfo.Types[expr] + if !ok || tv.Value == nil || tv.Value.Kind() != constant.String { + return "", false + } + + return constant.StringVal(tv.Value), true +} diff --git a/tools/mattermost-govet/mlogFieldNaming/mlogFieldNaming_test.go b/tools/mattermost-govet/mlogFieldNaming/mlogFieldNaming_test.go new file mode 100644 index 000000000000..ffccf86bf53b --- /dev/null +++ b/tools/mattermost-govet/mlogFieldNaming/mlogFieldNaming_test.go @@ -0,0 +1,88 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package mlogFieldNaming + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAll(t *testing.T) { + analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), Analyzer, "a") +} + +// TestFixtureCoversEveryConstructor keeps the fixture in step with +// keyedFieldConstructors, so that dropping a constructor cannot silently go +// unnoticed just because nothing exercises it. +func TestFixtureCoversEveryConstructor(t *testing.T) { + src, err := os.ReadFile(filepath.Join("testdata", "src", "a", "a.go")) + if err != nil { + t.Fatalf("reading fixture: %v", err) + } + + lines := strings.Split(string(src), "\n") + for name := range keyedFieldConstructors { + covered := false + for _, line := range lines { + if strings.Contains(line, "mlog."+name+`("`) && strings.Contains(line, "// want") { + covered = true + break + } + } + + if !covered { + t.Errorf("no diagnostic case for mlog.%s in testdata/src/a/a.go; add one so removing %q from keyedFieldConstructors fails a test", name, name) + } + } +} + +func TestToSnakeCase(t *testing.T) { + for _, tc := range []struct { + key string + want string + wantOK bool + }{ + {"user_id", "user_id", true}, + {"userId", "user_id", true}, + {"UserId", "user_id", true}, + {"userID", "user_id", true}, + {"ClusterDiscoveryID", "cluster_discovery_id", true}, + {"requestURL", "request_url", true}, + {"requestURLPath", "request_url_path", true}, + {"userIDs", "user_ids", true}, + {"channelIDs", "channel_ids", true}, + {"IDs", "ids", true}, + {"userIDsFoo", "user_ids_foo", true}, + {"userIDs.foo", "user_ids_foo", true}, + {"Diff", "diff", true}, + {"RecvAt", "recv_at", true}, + {"sharedChannelRemoteId", "shared_channel_remote_id", true}, + {"channel name", "channel_name", true}, + {"Possibilities searched", "possibilities_searched", true}, + {"features.users", "features_users", true}, + {"sm.Post.ChannelId", "sm_post_channel_id", true}, + {"user-id", "user_id", true}, + {"_user_id", "user_id", true}, + {"user__id", "user_id", true}, + {"sha256", "sha256", true}, + {"channelId2", "channel_id2", true}, + {"2fa", "", false}, + {"", "", false}, + {"___", "", false}, + } { + t.Run(tc.key, func(t *testing.T) { + got, ok := toSnakeCase(tc.key) + if ok != tc.wantOK { + t.Fatalf("toSnakeCase(%q) ok = %v, want %v", tc.key, ok, tc.wantOK) + } + if ok && got != tc.want { + t.Fatalf("toSnakeCase(%q) = %q, want %q", tc.key, got, tc.want) + } + }) + } +} diff --git a/tools/mattermost-govet/mlogFieldNaming/testdata/src/a/a.go b/tools/mattermost-govet/mlogFieldNaming/testdata/src/a/a.go new file mode 100644 index 000000000000..7b65cc4b2be6 --- /dev/null +++ b/tools/mattermost-govet/mlogFieldNaming/testdata/src/a/a.go @@ -0,0 +1,117 @@ +package a + +import ( + "errors" + "time" + + "github.com/mattermost/mattermost/server/public/shared/mlog" +) + +const ( + snakeKey = "batch_start_time" + camelKey = "batchStartTime" +) + +type notMlog struct{} + +func (notMlog) String(key string, val string) string { return key } + +func valid() { + mlog.Debug("message", + mlog.String("user_id", "abc"), + mlog.Int("count", 1), + mlog.Uint("size", uint(1)), + mlog.Float("ratio", 1.0), + mlog.Bool("is_leader", true), + mlog.Any("data", nil), + mlog.Array("ids", []string(nil)), + mlog.Map("props", map[string]string(nil)), + mlog.Duration("elapsed", nil), + mlog.Millis("started_at", 0), + mlog.Time("created_at", time.Time{}), + mlog.Stringer("channel_type", nil), + mlog.NamedErr("upload_err", nil), + mlog.Err(errors.New("boom")), + ) + + // Digits are allowed within and as whole segments. + mlog.Info("message", mlog.String("sha256", ""), mlog.String("channel_id_2", "")) + + // Keys resolved from constants are checked too. + mlog.Info("message", mlog.String(snakeKey, "")) +} + +func fixable() { + mlog.Debug("message", + mlog.String("userId", ""), // want `mlog field key "userId" is not snake_case, use "user_id"` + mlog.String("UserID", ""), // want `mlog field key "UserID" is not snake_case, use "user_id"` + mlog.Int("perPage", 0), // want `mlog field key "perPage" is not snake_case, use "per_page"` + mlog.String("ClusterDiscoveryID", ""), // want `mlog field key "ClusterDiscoveryID" is not snake_case, use "cluster_discovery_id"` + mlog.String("requestURLPath", ""), // want `mlog field key "requestURLPath" is not snake_case, use "request_url_path"` + mlog.String("Diff", ""), // want `mlog field key "Diff" is not snake_case, use "diff"` + mlog.String("channel name", ""), // want `mlog field key "channel name" is not snake_case, use "channel_name"` + mlog.String("features.users", ""), // want `mlog field key "features.users" is not snake_case, use "features_users"` + mlog.String("user-id", ""), // want `mlog field key "user-id" is not snake_case, use "user_id"` + mlog.String("_user_id", ""), // want `mlog field key "_user_id" is not snake_case, use "user_id"` + mlog.String("user__id", ""), // want `mlog field key "user__id" is not snake_case, use "user_id"` + mlog.String("channelId2", ""), // want `mlog field key "channelId2" is not snake_case, use "channel_id2"` + mlog.Array("userIDs", []string(nil)), // want `mlog field key "userIDs" is not snake_case, use "user_ids"` + ) +} + +// instantiated covers the generic constructors called with explicit type +// arguments, where the call expression wraps the selector in an index rather +// than being the selector itself. +func instantiated() { + mlog.Debug("message", + mlog.Int[int64]("count", 1), + mlog.Array[[]string, string]("ids", nil), + mlog.Map[map[string]string, string, string]("props", nil), + + mlog.Int[int64]("perPage", 1), // want `mlog field key "perPage" is not snake_case, use "per_page"` + mlog.Array[[]string, string]("userIDs", nil), // want `mlog field key "userIDs" is not snake_case, use "user_ids"` + mlog.Map[map[string]string, string, string]("mapProps", nil), // want `mlog field key "mapProps" is not snake_case, use "map_props"` + mlog.String[string](camelKey, ""), // want `mlog field key "batchStartTime" is not snake_case$` + ) +} + +func unfixable() { + mlog.Debug("message", + // Would start with a digit, so there is no mechanical rename. + mlog.String("2fa", ""), // want `mlog field key "2fa" is not snake_case$` + mlog.String("", ""), // want `mlog field key "" is not snake_case$` + ) + + // Renaming a constant's value is out of scope, so this is reported without + // a suggested fix. + mlog.Info("message", mlog.String(camelKey, "")) // want `mlog field key "batchStartTime" is not snake_case$` +} + +func ignored(key string) { + // Not the mlog package. + var n notMlog + _ = n.String("userId", "") + + // Not statically known. + mlog.Info("message", mlog.String(key, "")) +} + +// everyConstructor gives each keyed constructor its own diagnostic, so that +// dropping one from keyedFieldConstructors fails a test. +func everyConstructor() { + mlog.Debug("message", + mlog.Any("anyValue", nil), // want `mlog field key "anyValue" is not snake_case, use "any_value"` + mlog.Array("arrayIds", []string(nil)), // want `mlog field key "arrayIds" is not snake_case, use "array_ids"` + mlog.Bool("boolFlag", true), // want `mlog field key "boolFlag" is not snake_case, use "bool_flag"` + mlog.Duration("durationElapsed", nil), // want `mlog field key "durationElapsed" is not snake_case, use "duration_elapsed"` + mlog.Float("floatRatio", 1.0), // want `mlog field key "floatRatio" is not snake_case, use "float_ratio"` + mlog.Int("intCount", 1), // want `mlog field key "intCount" is not snake_case, use "int_count"` + mlog.Map("mapProps", map[string]string(nil)), // want `mlog field key "mapProps" is not snake_case, use "map_props"` + mlog.Millis("millisStartedAt", 0), // want `mlog field key "millisStartedAt" is not snake_case, use "millis_started_at"` + mlog.NamedErr("namedErr", nil), // want `mlog field key "namedErr" is not snake_case, use "named_err"` + mlog.String("stringName", ""), // want `mlog field key "stringName" is not snake_case, use "string_name"` + mlog.Stringer("stringerType", nil), // want `mlog field key "stringerType" is not snake_case, use "stringer_type"` + mlog.Time("timeCreatedAt", time.Time{}), // want `mlog field key "timeCreatedAt" is not snake_case, use "time_created_at"` + mlog.Uint("uintSize", uint(1)), // want `mlog field key "uintSize" is not snake_case, use "uint_size"` + ) +} diff --git a/tools/mattermost-govet/mlogFieldNaming/testdata/src/a/a.go.golden b/tools/mattermost-govet/mlogFieldNaming/testdata/src/a/a.go.golden new file mode 100644 index 000000000000..ce7a1c9e5f56 --- /dev/null +++ b/tools/mattermost-govet/mlogFieldNaming/testdata/src/a/a.go.golden @@ -0,0 +1,117 @@ +package a + +import ( + "errors" + "time" + + "github.com/mattermost/mattermost/server/public/shared/mlog" +) + +const ( + snakeKey = "batch_start_time" + camelKey = "batchStartTime" +) + +type notMlog struct{} + +func (notMlog) String(key string, val string) string { return key } + +func valid() { + mlog.Debug("message", + mlog.String("user_id", "abc"), + mlog.Int("count", 1), + mlog.Uint("size", uint(1)), + mlog.Float("ratio", 1.0), + mlog.Bool("is_leader", true), + mlog.Any("data", nil), + mlog.Array("ids", []string(nil)), + mlog.Map("props", map[string]string(nil)), + mlog.Duration("elapsed", nil), + mlog.Millis("started_at", 0), + mlog.Time("created_at", time.Time{}), + mlog.Stringer("channel_type", nil), + mlog.NamedErr("upload_err", nil), + mlog.Err(errors.New("boom")), + ) + + // Digits are allowed within and as whole segments. + mlog.Info("message", mlog.String("sha256", ""), mlog.String("channel_id_2", "")) + + // Keys resolved from constants are checked too. + mlog.Info("message", mlog.String(snakeKey, "")) +} + +func fixable() { + mlog.Debug("message", + mlog.String("user_id", ""), // want `mlog field key "userId" is not snake_case, use "user_id"` + mlog.String("user_id", ""), // want `mlog field key "UserID" is not snake_case, use "user_id"` + mlog.Int("per_page", 0), // want `mlog field key "perPage" is not snake_case, use "per_page"` + mlog.String("cluster_discovery_id", ""), // want `mlog field key "ClusterDiscoveryID" is not snake_case, use "cluster_discovery_id"` + mlog.String("request_url_path", ""), // want `mlog field key "requestURLPath" is not snake_case, use "request_url_path"` + mlog.String("diff", ""), // want `mlog field key "Diff" is not snake_case, use "diff"` + mlog.String("channel_name", ""), // want `mlog field key "channel name" is not snake_case, use "channel_name"` + mlog.String("features_users", ""), // want `mlog field key "features.users" is not snake_case, use "features_users"` + mlog.String("user_id", ""), // want `mlog field key "user-id" is not snake_case, use "user_id"` + mlog.String("user_id", ""), // want `mlog field key "_user_id" is not snake_case, use "user_id"` + mlog.String("user_id", ""), // want `mlog field key "user__id" is not snake_case, use "user_id"` + mlog.String("channel_id2", ""), // want `mlog field key "channelId2" is not snake_case, use "channel_id2"` + mlog.Array("user_ids", []string(nil)), // want `mlog field key "userIDs" is not snake_case, use "user_ids"` + ) +} + +// instantiated covers the generic constructors called with explicit type +// arguments, where the call expression wraps the selector in an index rather +// than being the selector itself. +func instantiated() { + mlog.Debug("message", + mlog.Int[int64]("count", 1), + mlog.Array[[]string, string]("ids", nil), + mlog.Map[map[string]string, string, string]("props", nil), + + mlog.Int[int64]("per_page", 1), // want `mlog field key "perPage" is not snake_case, use "per_page"` + mlog.Array[[]string, string]("user_ids", nil), // want `mlog field key "userIDs" is not snake_case, use "user_ids"` + mlog.Map[map[string]string, string, string]("map_props", nil), // want `mlog field key "mapProps" is not snake_case, use "map_props"` + mlog.String[string](camelKey, ""), // want `mlog field key "batchStartTime" is not snake_case$` + ) +} + +func unfixable() { + mlog.Debug("message", + // Would start with a digit, so there is no mechanical rename. + mlog.String("2fa", ""), // want `mlog field key "2fa" is not snake_case$` + mlog.String("", ""), // want `mlog field key "" is not snake_case$` + ) + + // Renaming a constant's value is out of scope, so this is reported without + // a suggested fix. + mlog.Info("message", mlog.String(camelKey, "")) // want `mlog field key "batchStartTime" is not snake_case$` +} + +func ignored(key string) { + // Not the mlog package. + var n notMlog + _ = n.String("userId", "") + + // Not statically known. + mlog.Info("message", mlog.String(key, "")) +} + +// everyConstructor gives each keyed constructor its own diagnostic, so that +// dropping one from keyedFieldConstructors fails a test. +func everyConstructor() { + mlog.Debug("message", + mlog.Any("any_value", nil), // want `mlog field key "anyValue" is not snake_case, use "any_value"` + mlog.Array("array_ids", []string(nil)), // want `mlog field key "arrayIds" is not snake_case, use "array_ids"` + mlog.Bool("bool_flag", true), // want `mlog field key "boolFlag" is not snake_case, use "bool_flag"` + mlog.Duration("duration_elapsed", nil), // want `mlog field key "durationElapsed" is not snake_case, use "duration_elapsed"` + mlog.Float("float_ratio", 1.0), // want `mlog field key "floatRatio" is not snake_case, use "float_ratio"` + mlog.Int("int_count", 1), // want `mlog field key "intCount" is not snake_case, use "int_count"` + mlog.Map("map_props", map[string]string(nil)), // want `mlog field key "mapProps" is not snake_case, use "map_props"` + mlog.Millis("millis_started_at", 0), // want `mlog field key "millisStartedAt" is not snake_case, use "millis_started_at"` + mlog.NamedErr("named_err", nil), // want `mlog field key "namedErr" is not snake_case, use "named_err"` + mlog.String("string_name", ""), // want `mlog field key "stringName" is not snake_case, use "string_name"` + mlog.Stringer("stringer_type", nil), // want `mlog field key "stringerType" is not snake_case, use "stringer_type"` + mlog.Time("time_created_at", time.Time{}), // want `mlog field key "timeCreatedAt" is not snake_case, use "time_created_at"` + mlog.Uint("uint_size", uint(1)), // want `mlog field key "uintSize" is not snake_case, use "uint_size"` + ) +} diff --git a/tools/mattermost-govet/mlogFieldNaming/testdata/src/github.com/mattermost/mattermost/server/public/shared/mlog/mlog.go b/tools/mattermost-govet/mlogFieldNaming/testdata/src/github.com/mattermost/mattermost/server/public/shared/mlog/mlog.go new file mode 100644 index 000000000000..cc32b6cbc09a --- /dev/null +++ b/tools/mattermost-govet/mlogFieldNaming/testdata/src/github.com/mattermost/mattermost/server/public/shared/mlog/mlog.go @@ -0,0 +1,39 @@ +package mlog + +type Field struct { + Key string +} + +func Any(key string, val any) Field { return Field{Key: key} } +func Duration(key string, val any) Field { return Field{Key: key} } +func Millis(key string, val int64) Field { return Field{Key: key} } +func NamedErr(key string, err error) Field { return Field{Key: key} } +func Stringer(key string, val any) Field { return Field{Key: key} } +func Time(key string, val any) Field { return Field{Key: key} } + +// The remaining constructors are generic, mirroring the real mlog package, so +// that the fixture exercises explicitly instantiated calls. + +func Array[S ~[]E, E any](key string, val S) Field { return Field{Key: key} } + +func Bool[T ~bool](key string, val T) Field { return Field{Key: key} } + +func Float[T ~float32 | ~float64](key string, val T) Field { return Field{Key: key} } + +func Int[T ~int | ~int8 | ~int16 | ~int32 | ~int64](key string, val T) Field { + return Field{Key: key} +} + +func Map[M ~map[K]V, K comparable, V any](key string, val M) Field { return Field{Key: key} } + +func String[T ~string | ~[]byte](key string, val T) Field { return Field{Key: key} } + +func Uint[T ~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 | ~uintptr](key string, val T) Field { + return Field{Key: key} +} + +func Err(err error) Field { return Field{Key: "error"} } + +func Debug(msg string, fields ...Field) {} +func Info(msg string, fields ...Field) {} +func Error(msg string, fields ...Field) {}