[GitHub] [apisix-dashboard] tokers commented on a change in pull request #994: feat: add access log for Manager API

[GitBox <gi...@apache.org>]

tokers commented on a change in pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#discussion_r540011482



##########
File path: api/conf/conf.go
##########
@@ -149,6 +156,17 @@ func setConf() {
 			}
 		}
 
+		// access log
+		if config.Conf.Log.AccessLog.FilePath != "" {
+			AccessLogPath = config.Conf.Log.AccessLog.FilePath
+		}
+		if !filepath.IsAbs(AccessLogPath) {
+			AccessLogPath, err = filepath.Abs(WorkDir + "/" + AccessLogPath)

Review comment:
       Use `filepath.Join` instead of concatenating them manually.

##########
File path: api/filter/logging.go
##########
@@ -16,4 +16,53 @@
  */
 package filter
 
-//for logging access log, will refactor it in a new pr.
+import (
+	"bytes"
+	"time"
+
+	"github.com/gin-gonic/gin"
+	"go.uber.org/zap"
+)
+
+func RequestLogHandler(logger *zap.SugaredLogger) gin.HandlerFunc {
+	return func(c *gin.Context) {
+		start, host, remoteIP, path, method := time.Now(), c.Request.Host, c.ClientIP(), c.Request.URL.Path, c.Request.Method
+		query := c.Request.URL.RawQuery
+		requestId := c.Writer.Header().Get("X-Request-Id")
+
+		blw := &bodyLogWriter{body: bytes.NewBufferString(""), ResponseWriter: c.Writer}

Review comment:
       Why use `bytes.NewBufferString` but the parameter is an empty string, just use `bytes.NewBuffer` is enough.

##########
File path: api/conf/conf.go
##########
@@ -69,8 +70,14 @@ type ErrorLog struct {
 	FilePath string `yaml:"file_path"`
 }
 
+type AccessLog struct {
+	Level    string

Review comment:
       log level in access log is no sense.

##########
File path: api/conf/conf.go
##########
@@ -69,8 +70,14 @@ type ErrorLog struct {
 	FilePath string `yaml:"file_path"`
 }
 
+type AccessLog struct {
+	Level    string

Review comment:
       You can change the zapcore encoder to remove the level encoding.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org