From 02a3cfb021f4b0adad41bbbe384a31d616bc9dce Mon Sep 17 00:00:00 2001 From: Xinwei Xiong <3293172751NSS@gmail.com> Date: Mon, 4 Mar 2024 17:30:17 +0800 Subject: [PATCH] feat: Introduce Language-Specific Comment Detection Tool and Standardize Log Filename Convention (#1992) * feat: optimize openim reset code * feat: optimize openim reset code * feat: optimize openim reset code * feat: optimize openim reset code * feat: optimize openim reset code * feat: optimize openim reset code * feat: optimize openim reset code * feat: optimize openim reset code --- .golangci.yml | 1 + config/templates/prometheus-dashboard.yaml | 8 +- docs/contrib/environment.md | 2 +- go.work | 1 + internal/api/user.go | 2 +- internal/msggateway/message_handler.go | 12 +-- internal/msgtransfer/init.go | 2 +- internal/rpc/group/group.go | 2 +- pkg/common/cmd/root.go | 4 +- scripts/build-all-service.sh | 3 - scripts/common.sh | 3 - scripts/install/environment.sh | 2 +- scripts/install/openim-rpc.sh | 4 - scripts/install/openim-tools.sh | 3 - scripts/lib/init.sh | 3 - scripts/lib/logging.sh | 6 +- scripts/lib/release.sh | 1 + scripts/verify-annotation-language.sh | 46 +++++++++++ scripts/verify-pkg-names.sh | 3 - test/codescan/main.go | 1 - tools/codescan/checker/checker.go | 90 ++++++++++++++++++++++ tools/codescan/codescan.go | 20 +++++ tools/codescan/config.yaml | 7 ++ tools/codescan/config/config.go | 35 +++++++++ tools/codescan/go.mod | 3 + 25 files changed, 224 insertions(+), 40 deletions(-) create mode 100755 scripts/verify-annotation-language.sh delete mode 100644 test/codescan/main.go create mode 100644 tools/codescan/checker/checker.go create mode 100644 tools/codescan/codescan.go create mode 100644 tools/codescan/config.yaml create mode 100644 tools/codescan/config/config.go create mode 100644 tools/codescan/go.mod diff --git a/.golangci.yml b/.golangci.yml index 0a0d40c21..67419d05e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -66,6 +66,7 @@ run: - "mocks/" - ".github/" - "logs/" + - "_output/" - "components/" # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules": diff --git a/config/templates/prometheus-dashboard.yaml b/config/templates/prometheus-dashboard.yaml index 417f3d343..2e1ae7760 100644 --- a/config/templates/prometheus-dashboard.yaml +++ b/config/templates/prometheus-dashboard.yaml @@ -53,7 +53,7 @@ }, "id": 16, "panels": [], - "title": "openim自定义指标", + "title": "openim Custom Metrics", "type": "row" }, { @@ -144,7 +144,7 @@ "refId": "A" } ], - "title": "在线人数", + "title": "Online population", "type": "timeseries" }, { @@ -235,7 +235,7 @@ "refId": "A" } ], - "title": "登入/注册人数", + "title": "Login/registration numbers", "type": "timeseries" }, { @@ -1345,7 +1345,7 @@ "type": "timeseries" } ], - "title": "应用服务器流量指标", + "title": "Traffic indicators of the application server", "type": "row" } ], diff --git a/docs/contrib/environment.md b/docs/contrib/environment.md index dc11a3c7b..d2db7cbf3 100644 --- a/docs/contrib/environment.md +++ b/docs/contrib/environment.md @@ -449,7 +449,7 @@ This section involves configuring the log settings, including storage location, | Parameter | Example Value | Description | | ------------------------- | ------------------------ | --------------------------------- | -| LOG_STORAGE_LOCATION | "${OPENIM_ROOT}/logs/" | Location for storing logs | +| LOG_STORAGE_LOCATION | "${OPENIM_ROOT}/_output/logs/" | Location for storing logs | | LOG_ROTATION_TIME | "24" | Log rotation time (in hours) | | LOG_REMAIN_ROTATION_COUNT | "2" | Number of log rotations to retain | | LOG_REMAIN_LOG_LEVEL | "6" | Log level to retain | diff --git a/go.work b/go.work index 97d2816d6..be67ce842 100644 --- a/go.work +++ b/go.work @@ -3,6 +3,7 @@ go 1.19 use ( . ./test/typecheck + ./tools/codescan ./tools/changelog ./tools/component ./tools/data-conversion diff --git a/internal/api/user.go b/internal/api/user.go index 901998319..16efcd704 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -145,7 +145,7 @@ func (u *UserApi) GetUsersOnlineTokenDetail(c *gin.Context) { msgClient := msggateway.NewMsgGatewayClient(v) reply, err := msgClient.GetUsersOnlineStatus(c, &req) if err != nil { - log.ZWarn(c, "GetUsersOnlineStatus rpc err", err) + log.ZWarn(c, "GetUsersOnlineStatus rpc err", err) continue } else { wsResult = append(wsResult, reply.SuccessResult...) diff --git a/internal/msggateway/message_handler.go b/internal/msggateway/message_handler.go index 3c5bd6121..74bde1ff5 100644 --- a/internal/msggateway/message_handler.go +++ b/internal/msggateway/message_handler.go @@ -183,7 +183,7 @@ func (g GrpcHandler) PullMessageBySeqList(context context.Context, data *Req) ([ return nil, errs.Wrap(err, "error unmarshaling request") } if err := g.validate.Struct(data); err != nil { - return nil, err + return nil, errs.Wrap(err, "validation failed") } resp, err := g.msgRpcClient.PullMessageBySeqList(context, &req) if err != nil { @@ -191,7 +191,7 @@ func (g GrpcHandler) PullMessageBySeqList(context context.Context, data *Req) ([ } c, err := proto.Marshal(resp) if err != nil { - return nil, err + return nil, errs.Wrap(err, "error marshaling response") } return c, nil } @@ -199,7 +199,7 @@ func (g GrpcHandler) PullMessageBySeqList(context context.Context, data *Req) ([ func (g GrpcHandler) UserLogout(context context.Context, data *Req) ([]byte, error) { req := push.DelUserPushTokenReq{} if err := proto.Unmarshal(data.Data, &req); err != nil { - return nil, err + return nil, errs.Wrap(err, "error unmarshaling request") } resp, err := g.pushClient.DelUserPushToken(context, &req) if err != nil { @@ -207,7 +207,7 @@ func (g GrpcHandler) UserLogout(context context.Context, data *Req) ([]byte, err } c, err := proto.Marshal(resp) if err != nil { - return nil, err + return nil, errs.Wrap(err, "error marshaling response") } return c, nil } @@ -215,10 +215,10 @@ func (g GrpcHandler) UserLogout(context context.Context, data *Req) ([]byte, err func (g GrpcHandler) SetUserDeviceBackground(_ context.Context, data *Req) ([]byte, bool, error) { req := sdkws.SetAppBackgroundStatusReq{} if err := proto.Unmarshal(data.Data, &req); err != nil { - return nil, false, err + return nil, false, errs.Wrap(err, "error unmarshaling request") } if err := g.validate.Struct(data); err != nil { - return nil, false, err + return nil, false, errs.Wrap(err, "validation failed") } return nil, req.IsBackground, nil } diff --git a/internal/msgtransfer/init.go b/internal/msgtransfer/init.go index b5f8516f8..ca9ab5ac4 100644 --- a/internal/msgtransfer/init.go +++ b/internal/msgtransfer/init.go @@ -121,7 +121,7 @@ func (m *MsgTransfer) Start(prometheusPort int) error { var ( netDone = make(chan struct{}, 1) - netErr error + netErr error ) go m.historyCH.historyConsumerGroup.RegisterHandleAndConsumer(m.ctx, m.historyCH) diff --git a/internal/rpc/group/group.go b/internal/rpc/group/group.go index 60f6c3eb5..db5108f31 100644 --- a/internal/rpc/group/group.go +++ b/internal/rpc/group/group.go @@ -974,7 +974,7 @@ func (s *groupServer) SetGroupInfo(ctx context.Context, req *pbgroup.SetGroupInf if len(update) == 0 { return resp, nil } - if updateErr := s.db.UpdateGroup(ctx, group.GroupID, update); updateErr != nil { + if err := s.db.UpdateGroup(ctx, group.GroupID, update); err != nil { return nil, err } group, err = s.db.TakeGroup(ctx, req.GroupInfoForSet.GroupID) diff --git a/pkg/common/cmd/root.go b/pkg/common/cmd/root.go index 7256bd0ed..4f4c5e69b 100644 --- a/pkg/common/cmd/root.go +++ b/pkg/common/cmd/root.go @@ -47,7 +47,7 @@ type CmdOpts struct { func WithCronTaskLogName() func(*CmdOpts) { return func(opts *CmdOpts) { - opts.loggerPrefixName = "openim.crontask.log.all" + opts.loggerPrefixName = "openim-crontask" } } @@ -117,7 +117,7 @@ func (rc *RootCmd) initializeLogger(cmdOpts *CmdOpts) error { func defaultCmdOpts() *CmdOpts { return &CmdOpts{ - loggerPrefixName: "OpenIM.log.all", + loggerPrefixName: "openim-all", } } diff --git a/scripts/build-all-service.sh b/scripts/build-all-service.sh index 6335b0e08..eea380b4f 100755 --- a/scripts/build-all-service.sh +++ b/scripts/build-all-service.sh @@ -23,9 +23,6 @@ # Example: `scripts/build-go.sh WHAT=cmd/kubelet`. - - - OPENIM_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. source "${OPENIM_ROOT}/scripts/lib/init.sh" diff --git a/scripts/common.sh b/scripts/common.sh index 702f55588..f7155fca2 100755 --- a/scripts/common.sh +++ b/scripts/common.sh @@ -19,9 +19,6 @@ # Common utilities, variables and checks for all build scripts. - - - # Unset CDPATH, having it set messes up with script import paths unset CDPATH diff --git a/scripts/install/environment.sh b/scripts/install/environment.sh index 896288775..64e76853d 100755 --- a/scripts/install/environment.sh +++ b/scripts/install/environment.sh @@ -332,7 +332,7 @@ def "OPENIM_CONVERSATION_NAME" "Conversation" # OpenIM对话服务名称 def "OPENIM_THIRD_NAME" "Third" # OpenIM第三方服务名称 ###################### Log Configuration Variables ###################### -def "LOG_STORAGE_LOCATION" "${OPENIM_ROOT}/logs/" # 日志存储位置 +def "LOG_STORAGE_LOCATION" "${OPENIM_ROOT}/_output/logs/" # 日志存储位置 def "LOG_ROTATION_TIME" "24" # 日志轮替时间 def "LOG_REMAIN_ROTATION_COUNT" "2" # 保留的日志轮替数量 def "LOG_REMAIN_LOG_LEVEL" "6" # 保留的日志级别 diff --git a/scripts/install/openim-rpc.sh b/scripts/install/openim-rpc.sh index 023cb6594..e7ee430d2 100755 --- a/scripts/install/openim-rpc.sh +++ b/scripts/install/openim-rpc.sh @@ -38,10 +38,6 @@ # Note: Before executing this script, ensure that the necessary permissions are granted and relevant environmental variables are set. # - - - - OPENIM_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")"/../.. && pwd -P) [[ -z ${COMMON_SOURCED} ]] && source "${OPENIM_ROOT}"/scripts/install/common.sh diff --git a/scripts/install/openim-tools.sh b/scripts/install/openim-tools.sh index d8c27e985..4eb722c6e 100755 --- a/scripts/install/openim-tools.sh +++ b/scripts/install/openim-tools.sh @@ -39,9 +39,6 @@ # - - - OPENIM_ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")"/../.. && pwd -P) [[ -z ${COMMON_SOURCED} ]] && source "${OPENIM_ROOT}"/scripts/install/common.sh diff --git a/scripts/lib/init.sh b/scripts/lib/init.sh index 4cd6d9fb8..6f12db36e 100755 --- a/scripts/lib/init.sh +++ b/scripts/lib/init.sh @@ -14,9 +14,6 @@ # limitations under the License. - - - # Short-circuit if init.sh has already been sourced [[ $(type -t openim::init::loaded) == function ]] && return 0 diff --git a/scripts/lib/logging.sh b/scripts/lib/logging.sh index 7afb6bfce..bef3b5961 100755 --- a/scripts/lib/logging.sh +++ b/scripts/lib/logging.sh @@ -25,9 +25,9 @@ if [ -z "${OPENIM_OUTPUT+x}" ]; then fi # Set the log file path -LOG_FILE="${OPENIM_OUTPUT}/logs/openim_$(date '+%Y%m%d').log" -STDERR_LOG_FILE="${OPENIM_OUTPUT}/logs/openim_error_$(date '+%Y%m%d').log" -TMP_LOG_FILE="${OPENIM_OUTPUT}/logs/openim_tmp_$(date '+%Y%m%d').log" +LOG_FILE="${OPENIM_OUTPUT}/logs/openim-$(date '+%Y%m%d').log" +STDERR_LOG_FILE="${OPENIM_OUTPUT}/logs/openim-error-$(date '+%Y%m%d').log" +TMP_LOG_FILE="${OPENIM_OUTPUT}/logs/openim-tmp-$(date '+%Y%m%d').log" if [[ ! -d "${OPENIM_OUTPUT}/logs" ]]; then mkdir -p "${OPENIM_OUTPUT}/logs" diff --git a/scripts/lib/release.sh b/scripts/lib/release.sh index 521e5cedc..c1fbd00a1 100755 --- a/scripts/lib/release.sh +++ b/scripts/lib/release.sh @@ -152,6 +152,7 @@ function openim::release::package_src_tarball() { -path "${OPENIM_ROOT}"/.github\* -o \ -path "${OPENIM_ROOT}"/components\* -o \ -path "${OPENIM_ROOT}"/logs\* -o \ + -path "${OPENIM_ROOT}"/_output\* -o \ -path "${OPENIM_ROOT}"/.gitignore\* -o \ -path "${OPENIM_ROOT}"/.gsemver.yml\* -o \ -path "${OPENIM_ROOT}"/.config\* -o \ diff --git a/scripts/verify-annotation-language.sh b/scripts/verify-annotation-language.sh new file mode 100755 index 000000000..6b863776c --- /dev/null +++ b/scripts/verify-annotation-language.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# Copyright © 2023 OpenIM. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +# This script verifies whether codes follow golang convention. +# Usage: `scripts/verify-pkg-names.sh`. + +set -o errexit + +OPENIM_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. +source "${OPENIM_ROOT}/scripts/lib/init.sh" + +openim::golang::verify_go_version + +openim::golang::verify_go_version + +OPENIM_OUTPUT_HOSTBIN_TOOLS="${OPENIM_ROOT}/_output/bin/tools/linux/amd64" +CODESCAN_BINARY="${OPENIM_OUTPUT_HOSTBIN_TOOLS}/codescan" + +if [[ ! -f "${CODESCAN_BINARY}" ]]; then + echo "codescan binary not found, building..." + pushd "${OPENIM_ROOT}" >/dev/null + make build BINS="codescan" + popd >/dev/null +fi + +if [[ ! -f "${CODESCAN_BINARY}" ]]; then + echo "Failed to build codescan binary." + exit 1 +fi + +CONFIG_PATH="${OPENIM_ROOT}/tools/codescan/config.yaml" + +"${CODESCAN_BINARY}" -config "${CONFIG_PATH}" \ No newline at end of file diff --git a/scripts/verify-pkg-names.sh b/scripts/verify-pkg-names.sh index 7fce3d7ad..be1acd015 100755 --- a/scripts/verify-pkg-names.sh +++ b/scripts/verify-pkg-names.sh @@ -18,9 +18,6 @@ # Usage: `scripts/verify-pkg-names.sh`. - - - OPENIM_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. source "${OPENIM_ROOT}/scripts/lib/init.sh" diff --git a/test/codescan/main.go b/test/codescan/main.go deleted file mode 100644 index 06ab7d0f9..000000000 --- a/test/codescan/main.go +++ /dev/null @@ -1 +0,0 @@ -package main diff --git a/tools/codescan/checker/checker.go b/tools/codescan/checker/checker.go new file mode 100644 index 000000000..953236d0f --- /dev/null +++ b/tools/codescan/checker/checker.go @@ -0,0 +1,90 @@ +package checker + +import ( + "bufio" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/openimsdk/open-im-server/tools/codescan/config" +) + +type CheckResult struct { + FilePath string + Lines []int +} + +func checkFileForChineseComments(filePath string) ([]CheckResult, error) { + file, err := os.Open(filePath) + if err != nil { + return nil, err + } + defer file.Close() + + var results []CheckResult + scanner := bufio.NewScanner(file) + reg := regexp.MustCompile(`[\p{Han}]+`) + lineNumber := 0 + + var linesWithChinese []int + for scanner.Scan() { + lineNumber++ + if reg.FindString(scanner.Text()) != "" { + linesWithChinese = append(linesWithChinese, lineNumber) + } + } + + if len(linesWithChinese) > 0 { + results = append(results, CheckResult{ + FilePath: filePath, + Lines: linesWithChinese, + }) + } + + if err := scanner.Err(); err != nil { + return nil, err + } + + return results, nil +} + +func WalkDirAndCheckComments(cfg config.Config) error { + var allResults []CheckResult + err := filepath.Walk(cfg.Directory, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + for _, fileType := range cfg.FileTypes { + if filepath.Ext(path) == fileType { + results, err := checkFileForChineseComments(path) + if err != nil { + return err + } + if len(results) > 0 { + allResults = append(allResults, results...) + } + } + } + return nil + }) + + if err != nil { + return err + } + + if len(allResults) > 0 { + var errMsg strings.Builder + errMsg.WriteString("Files containing Chinese comments:\n") + for _, result := range allResults { + errMsg.WriteString(fmt.Sprintf("%s: Lines %v\n", result.FilePath, result.Lines)) + } + return fmt.Errorf(errMsg.String()) + } + + return nil +} diff --git a/tools/codescan/codescan.go b/tools/codescan/codescan.go new file mode 100644 index 000000000..c46f7a704 --- /dev/null +++ b/tools/codescan/codescan.go @@ -0,0 +1,20 @@ +package main + +import ( + "log" + + "github.com/openimsdk/open-im-server/tools/codescan/checker" + "github.com/openimsdk/open-im-server/tools/codescan/config" +) + +func main() { + cfg, err := config.ParseConfig() + if err != nil { + log.Fatalf("Error parsing config: %v", err) + } + + err = checker.WalkDirAndCheckComments(cfg) + if err != nil { + panic(err) + } +} diff --git a/tools/codescan/config.yaml b/tools/codescan/config.yaml new file mode 100644 index 000000000..9a81236b8 --- /dev/null +++ b/tools/codescan/config.yaml @@ -0,0 +1,7 @@ +directory: ./ +file_types: + - .go + - .yaml + - .yml +languages: + - Chinese \ No newline at end of file diff --git a/tools/codescan/config/config.go b/tools/codescan/config/config.go new file mode 100644 index 000000000..8f051e3da --- /dev/null +++ b/tools/codescan/config/config.go @@ -0,0 +1,35 @@ +package config + +import ( + "flag" + "log" + "os" + + "gopkg.in/yaml.v2" +) + +type Config struct { + Directory string `yaml:"directory"` + FileTypes []string `yaml:"file_types"` + Languages []string `yaml:"languages"` +} + +func ParseConfig() (Config, error) { + var configPath string + flag.StringVar(&configPath, "config", "./", "Path to config file") + flag.Parse() + + var config Config + if configPath != "" { + configFile, err := os.ReadFile(configPath) + if err != nil { + return Config{}, err + } + if err := yaml.Unmarshal(configFile, &config); err != nil { + return Config{}, err + } + } else { + log.Fatal("Config file must be provided") + } + return config, nil +} diff --git a/tools/codescan/go.mod b/tools/codescan/go.mod new file mode 100644 index 000000000..2ad132101 --- /dev/null +++ b/tools/codescan/go.mod @@ -0,0 +1,3 @@ +module github.com/openimsdk/open-im-server/tools/codescan + +go 1.19