You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "justxuewei (via GitHub)" <gi...@apache.org> on 2023/04/09 03:25:50 UTC
[GitHub] [dubbo-go] justxuewei commented on a diff in pull request #2282: Enhance logger
justxuewei commented on code in PR #2282:
URL: https://github.com/apache/dubbo-go/pull/2282#discussion_r1160681581
##########
config/logger_config.go:
##########
@@ -18,163 +18,159 @@
package config
import (
- "net/url"
-)
+ "fmt"
+ "strconv"
-import (
getty "github.com/apache/dubbo-getty"
-
"github.com/creasty/defaults"
-
- "github.com/dubbogo/gost/encoding/yaml"
"github.com/dubbogo/gost/log/logger"
- "github.com/natefinch/lumberjack"
-
- "go.uber.org/zap"
- "go.uber.org/zap/zapcore"
-)
-
-import (
+ "dubbo.apache.org/dubbo-go/v3/common"
"dubbo.apache.org/dubbo-go/v3/common/constant"
+ "dubbo.apache.org/dubbo-go/v3/common/extension"
)
-type ZapConfig struct {
- Level string `default:"info" json:"level,omitempty" yaml:"level" property:"level"`
- Development bool `default:"false" json:"development,omitempty" yaml:"development" property:"development"`
- DisableCaller bool `default:"false" json:"disable-caller,omitempty" yaml:"disable-caller" property:"disable-caller"`
- DisableStacktrace bool `default:"false" json:"disable-stacktrace,omitempty" yaml:"disable-stacktrace" property:"disable-stacktrace"`
- Encoding string `default:"console" json:"encoding,omitempty" yaml:"encoding" property:"encoding"`
- EncoderConfig EncoderConfig `default:"" json:"encoder-config,omitempty" yaml:"encoder-config" property:"encoder-config"`
- OutputPaths []string `default:"[\"stderr\"]" json:"output-paths,omitempty" yaml:"output-paths" property:"output-paths"`
- ErrorOutputPaths []string `default:"[\"stderr\"]" json:"error-output-paths,omitempty" yaml:"error-output-paths" property:"error-output-paths"`
- InitialFields map[string]interface{} `default:"" json:"initial-fields,omitempty" yaml:"initial-fields" property:"initial-fields"`
-}
-
type LoggerConfig struct {
- LumberjackConfig *lumberjack.Logger `yaml:"lumberjack-config" json:"lumberjack-config,omitempty" property:"lumberjack-config"`
- ZapConfig ZapConfig `yaml:"zap-config" json:"zap-config,omitempty" property:"zap-config"`
+ // logger driver default zap
+ Driver string `default:"zap" yaml:"driver"`
+
+ // logger level
+ Level string `default:"info" yaml:"level"`
+
+ // logger formatter default text
+ Format string `default:"text" yaml:"format"`
+
+ // supports simultaneous file and console eg: console,file default console
+ Appender string `default:"console" yaml:"appender"`
+
+ // logger file
+ File *File `yaml:"file"`
}
-type EncoderConfig struct {
- MessageKey string `default:"message" json:"message-key,omitempty" yaml:"message-key" property:"message-key"`
- LevelKey string `default:"level" json:"level-key,omitempty" yaml:"level-key" property:"level-key"`
- TimeKey string `default:"time" json:"time-key,omitempty" yaml:"time-key" property:"time-key"`
- NameKey string `default:"logger" json:"name-key,omitempty" yaml:"name-key" property:"name-key"`
- CallerKey string `default:"caller" json:"caller-key,omitempty" yaml:"caller-key" property:"caller-key"`
- StacktraceKey string `default:"stacktrace" json:"stacktrace-key,omitempty" yaml:"stacktrace-key" property:"stacktrace-key"`
- EncodeLevel string `default:"capitalColor" json:"level-encoder" yaml:"level-encoder" property:"level-encoder"`
- EncodeTime string `default:"iso8601" json:"time-encoder" yaml:"time-encoder" property:"time-encoder"`
- EncodeDuration string `default:"seconds" json:"duration-encoder" yaml:"duration-encoder" property:"duration-encoder"`
- EncodeCaller string `default:"short" json:"caller-encoder" yaml:"calle-encoder" property:"caller-encoder"`
- Params map[string]string `yaml:"params" json:"params,omitempty"`
+type File struct {
+ // log file name default dubbo.log
+ Name string `default:"dubbo.log" yaml:"name"`
+
+ // log max size default 100Mb
+ MaxSize int `default:"100" yaml:"max-size"`
+
+ // log max backups default 5
+ MaxBackups int `default:"5" yaml:"max-backups"`
+
+ // log file max age default 3 day
+ MaxAge int `default:"3" yaml:"max-age"`
+
+ Compress *bool `default:"true" yaml:"compress"`
Review Comment:
The `compress` must be not nil. So the type of it should be `bool`.
##########
logger/logrus/logrus.go:
##########
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+package logrus
+
+import (
+ "io"
+ "os"
+ "strings"
+
+ "github.com/dubbogo/gost/log/logger"
+ "github.com/mattn/go-colorable"
+ "github.com/sirupsen/logrus"
+
+ "dubbo.apache.org/dubbo-go/v3/common"
+ "dubbo.apache.org/dubbo-go/v3/common/constant"
+ "dubbo.apache.org/dubbo-go/v3/common/extension"
+ . "dubbo.apache.org/dubbo-go/v3/logger"
+)
Review Comment:
The format of imports is wrong.
##########
config/logger_config_test.go:
##########
@@ -43,9 +40,9 @@ func TestLoggerInit(t *testing.T) {
loggerConfig := rootConfig.Logger
assert.NotNil(t, loggerConfig)
// default
- assert.Equal(t, "debug", loggerConfig.ZapConfig.Level)
- assert.Equal(t, "message", loggerConfig.ZapConfig.EncoderConfig.MessageKey)
- assert.Equal(t, "stacktrace", loggerConfig.ZapConfig.EncoderConfig.StacktraceKey)
+ //assert.Equal(t, "debug", loggerConfig.ZapConfig.Level)
+ //assert.Equal(t, "message", loggerConfig.ZapConfig.EncoderConfig.MessageKey)
+ //assert.Equal(t, "stacktrace", loggerConfig.ZapConfig.EncoderConfig.StacktraceKey)
Review Comment:
If the tests are useless, please remove them.
##########
common/extension/logger.go:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+package extension
+
+import (
+ "github.com/dubbogo/gost/log/logger"
+
+ "github.com/pkg/errors"
+)
+
+import (
+ "dubbo.apache.org/dubbo-go/v3/common"
+)
+
+var logs = make(map[string]func(config *common.URL) (logger.Logger, error))
+
+func SetLogger(driver string, log func(config *common.URL) (logger.Logger, error)) {
+ logs[driver] = log
+}
+
+func GetLogger(driver string, config *common.URL) (logger.Logger, error) {
+
+ if logs[driver] != nil {
+ return logs[driver](config)
+ } else {
+ return nil, errors.Errorf("logger for %s does not exist. "+
+ "please make sure that you have imported the package "+
+ "github.com/dubbogo/gost/log/logger/%s", driver, driver)
Review Comment:
```
return nil, errors.Errorf("Logger for %s does not exist. "+
"Please make sure that you have imported the package "+
"github.com/dubbogo/gost/log/logger/%s", driver, driver)
```
##########
config/logger_config_test.go:
##########
@@ -34,7 +31,7 @@ func TestLoggerInit(t *testing.T) {
assert.NotNil(t, rootConfig)
loggerConfig := rootConfig.Logger
assert.NotNil(t, loggerConfig)
- assert.Equal(t, []string{"stderr"}, loggerConfig.ZapConfig.OutputPaths)
+ //assert.Equal(t, []string{"stderr"}, loggerConfig.ZapConfig.OutputPaths)
Review Comment:
If the tests are useless, please remove them.
##########
common/extension/logger.go:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+package extension
+
+import (
+ "github.com/dubbogo/gost/log/logger"
+
+ "github.com/pkg/errors"
+)
+
+import (
+ "dubbo.apache.org/dubbo-go/v3/common"
+)
+
+var logs = make(map[string]func(config *common.URL) (logger.Logger, error))
+
+func SetLogger(driver string, log func(config *common.URL) (logger.Logger, error)) {
+ logs[driver] = log
+}
+
+func GetLogger(driver string, config *common.URL) (logger.Logger, error) {
Review Comment:
Why does here receive an unused parameter whose type is `*common.URL`?
##########
config/logger_config.go:
##########
@@ -18,163 +18,159 @@
package config
import (
- "net/url"
-)
+ "fmt"
+ "strconv"
-import (
getty "github.com/apache/dubbo-getty"
-
"github.com/creasty/defaults"
-
- "github.com/dubbogo/gost/encoding/yaml"
"github.com/dubbogo/gost/log/logger"
- "github.com/natefinch/lumberjack"
-
- "go.uber.org/zap"
- "go.uber.org/zap/zapcore"
-)
-
-import (
+ "dubbo.apache.org/dubbo-go/v3/common"
"dubbo.apache.org/dubbo-go/v3/common/constant"
+ "dubbo.apache.org/dubbo-go/v3/common/extension"
)
-type ZapConfig struct {
- Level string `default:"info" json:"level,omitempty" yaml:"level" property:"level"`
- Development bool `default:"false" json:"development,omitempty" yaml:"development" property:"development"`
- DisableCaller bool `default:"false" json:"disable-caller,omitempty" yaml:"disable-caller" property:"disable-caller"`
- DisableStacktrace bool `default:"false" json:"disable-stacktrace,omitempty" yaml:"disable-stacktrace" property:"disable-stacktrace"`
- Encoding string `default:"console" json:"encoding,omitempty" yaml:"encoding" property:"encoding"`
- EncoderConfig EncoderConfig `default:"" json:"encoder-config,omitempty" yaml:"encoder-config" property:"encoder-config"`
- OutputPaths []string `default:"[\"stderr\"]" json:"output-paths,omitempty" yaml:"output-paths" property:"output-paths"`
- ErrorOutputPaths []string `default:"[\"stderr\"]" json:"error-output-paths,omitempty" yaml:"error-output-paths" property:"error-output-paths"`
- InitialFields map[string]interface{} `default:"" json:"initial-fields,omitempty" yaml:"initial-fields" property:"initial-fields"`
-}
-
type LoggerConfig struct {
- LumberjackConfig *lumberjack.Logger `yaml:"lumberjack-config" json:"lumberjack-config,omitempty" property:"lumberjack-config"`
- ZapConfig ZapConfig `yaml:"zap-config" json:"zap-config,omitempty" property:"zap-config"`
+ // logger driver default zap
+ Driver string `default:"zap" yaml:"driver"`
Review Comment:
I think here should list the drivers we supports now.
##########
config/logger_config.go:
##########
@@ -18,163 +18,159 @@
package config
import (
- "net/url"
-)
+ "fmt"
+ "strconv"
-import (
getty "github.com/apache/dubbo-getty"
-
"github.com/creasty/defaults"
-
- "github.com/dubbogo/gost/encoding/yaml"
"github.com/dubbogo/gost/log/logger"
- "github.com/natefinch/lumberjack"
-
- "go.uber.org/zap"
- "go.uber.org/zap/zapcore"
-)
-
-import (
+ "dubbo.apache.org/dubbo-go/v3/common"
"dubbo.apache.org/dubbo-go/v3/common/constant"
+ "dubbo.apache.org/dubbo-go/v3/common/extension"
)
-type ZapConfig struct {
- Level string `default:"info" json:"level,omitempty" yaml:"level" property:"level"`
- Development bool `default:"false" json:"development,omitempty" yaml:"development" property:"development"`
- DisableCaller bool `default:"false" json:"disable-caller,omitempty" yaml:"disable-caller" property:"disable-caller"`
- DisableStacktrace bool `default:"false" json:"disable-stacktrace,omitempty" yaml:"disable-stacktrace" property:"disable-stacktrace"`
- Encoding string `default:"console" json:"encoding,omitempty" yaml:"encoding" property:"encoding"`
- EncoderConfig EncoderConfig `default:"" json:"encoder-config,omitempty" yaml:"encoder-config" property:"encoder-config"`
- OutputPaths []string `default:"[\"stderr\"]" json:"output-paths,omitempty" yaml:"output-paths" property:"output-paths"`
- ErrorOutputPaths []string `default:"[\"stderr\"]" json:"error-output-paths,omitempty" yaml:"error-output-paths" property:"error-output-paths"`
- InitialFields map[string]interface{} `default:"" json:"initial-fields,omitempty" yaml:"initial-fields" property:"initial-fields"`
-}
-
type LoggerConfig struct {
- LumberjackConfig *lumberjack.Logger `yaml:"lumberjack-config" json:"lumberjack-config,omitempty" property:"lumberjack-config"`
- ZapConfig ZapConfig `yaml:"zap-config" json:"zap-config,omitempty" property:"zap-config"`
+ // logger driver default zap
+ Driver string `default:"zap" yaml:"driver"`
+
+ // logger level
+ Level string `default:"info" yaml:"level"`
+
+ // logger formatter default text
+ Format string `default:"text" yaml:"format"`
+
+ // supports simultaneous file and console eg: console,file default console
+ Appender string `default:"console" yaml:"appender"`
+
+ // logger file
+ File *File `yaml:"file"`
}
-type EncoderConfig struct {
- MessageKey string `default:"message" json:"message-key,omitempty" yaml:"message-key" property:"message-key"`
- LevelKey string `default:"level" json:"level-key,omitempty" yaml:"level-key" property:"level-key"`
- TimeKey string `default:"time" json:"time-key,omitempty" yaml:"time-key" property:"time-key"`
- NameKey string `default:"logger" json:"name-key,omitempty" yaml:"name-key" property:"name-key"`
- CallerKey string `default:"caller" json:"caller-key,omitempty" yaml:"caller-key" property:"caller-key"`
- StacktraceKey string `default:"stacktrace" json:"stacktrace-key,omitempty" yaml:"stacktrace-key" property:"stacktrace-key"`
- EncodeLevel string `default:"capitalColor" json:"level-encoder" yaml:"level-encoder" property:"level-encoder"`
- EncodeTime string `default:"iso8601" json:"time-encoder" yaml:"time-encoder" property:"time-encoder"`
- EncodeDuration string `default:"seconds" json:"duration-encoder" yaml:"duration-encoder" property:"duration-encoder"`
- EncodeCaller string `default:"short" json:"caller-encoder" yaml:"calle-encoder" property:"caller-encoder"`
- Params map[string]string `yaml:"params" json:"params,omitempty"`
+type File struct {
+ // log file name default dubbo.log
+ Name string `default:"dubbo.log" yaml:"name"`
+
+ // log max size default 100Mb
+ MaxSize int `default:"100" yaml:"max-size"`
+
+ // log max backups default 5
+ MaxBackups int `default:"5" yaml:"max-backups"`
+
+ // log file max age default 3 day
+ MaxAge int `default:"3" yaml:"max-age"`
+
+ Compress *bool `default:"true" yaml:"compress"`
Review Comment:
Btw, I think `compressed` is a better choice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org