[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log
little-cui commented on a change in pull request #622: [SCB-1719] Support
access log
URL:
https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367203221
##
File path: pkg/log/zap.go
##
@@ -59,6 +59,9 @@ type Config struct {
// days
LogBackupAge int
CallerSkip int
+ EnableTime bool // whether to record time
+ EnableLevel bool // whether to record level
+ EnableCaller bool // whether to record caller
Review comment:
应该WithXXX设置这个值
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log
little-cui commented on a change in pull request #622: [SCB-1719] Support
access log
URL:
https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367204153
##
File path: server/handler/accesslog/handler.go
##
@@ -0,0 +1,98 @@
+// 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.
+
+// +build go1.9
+
+package accesslog
+
+import (
+ "fmt"
+ "net/http"
+ "os"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/pkg/chain"
+ "github.com/apache/servicecomb-service-center/pkg/log"
+ "github.com/apache/servicecomb-service-center/pkg/rest"
+ "github.com/apache/servicecomb-service-center/pkg/util"
+ "github.com/apache/servicecomb-service-center/server/core"
+ svr "github.com/apache/servicecomb-service-center/server/rest"
+)
+
+// Handler implements chain.Handler
+// Handler records access log
+type Handler struct {
+ logger*log.Logger
+ whiteListAPIs map[string]struct{} // not record access log
+}
+
+// Handle handles the request
+func (l *Handler) Handle(i *chain.Invocation) {
+ matchPattern := i.Context().Value(rest.CTX_MATCH_PATTERN).(string)
+ if _, ok := l.whiteListAPIs[matchPattern]; ok {
+ i.Next()
+ return
+ }
+ startTimeStr := "unknown"
+ start, ok := i.Context().Value(svr.CTX_START_TIMESTAMP).(time.Time)
+ if ok {
+ startTimeStr = start.Format("2006-01-02T15:04:05.000Z07:00")
+ }
+ r := i.Context().Value(rest.CTX_REQUEST).(*http.Request)
+ w := i.Context().Value(rest.CTX_RESPONSE).(http.ResponseWriter)
+ i.Next(chain.WithAsyncFunc(func(_ chain.Result) {
+ delayByMillisecond := "unknown"
+ if ok {
+ delayByMillisecond = fmt.Sprintf("%d",
time.Since(start)/time.Millisecond)
+ }
+ statusCode := w.Header().Get(rest.HEADER_RESPONSE_STATUS)
+ // format: remoteIp requestReceiveTime "method requestUri
proto" statusCode requestBodySize delay(ms)
+ // example: 127.0.0.1 2006-01-02T15:04:05.000Z07:00 "GET
/v4/default/registry/microservices HTTP/1.1" 200 0 0
+ l.logger.Infof("%s %s \"%s %s %s\" %s %d %s",
+ util.GetIPFromContext(i.Context()),
+ startTimeStr,
+ r.Method,
+ r.RequestURI,
+ r.Proto,
+ statusCode,
+ r.ContentLength,
+ delayByMillisecond)
+ }))
+}
+
+// NewAccessLogHandler creates a Handler
+func NewAccessLogHandler(l *log.Logger, m map[string]struct{}) *Handler {
+ return {
+ logger:l,
+ whiteListAPIs: m}
+}
+
+// RegisterHandlers registers an access log handler to the handler chain
+func RegisterHandlers() {
+ if !core.ServerInfo.Config.EnableAccessLog {
+ return
+ }
+ logger := log.NewLogger(log.Config{
+ LoggerFile:
os.ExpandEnv(core.ServerInfo.Config.AccessLogFile),
+ LogFormatText: true,
+ LogRotateSize: int(core.ServerInfo.Config.LogRotateSize),
+ LogBackupCount: int(core.ServerInfo.Config.LogBackupCount),
+ })
+ whiteListAPIs := make(map[string]struct{}, 0)
+ // no access log for heartbeat
+
whiteListAPIs["/v4/:project/registry/microservices/:serviceId/instances/:instanceId/heartbeat"]
= struct{}{}
+ h := NewAccessLogHandler(logger, whiteListAPIs)
Review comment:
把whiteListAPIs开放出来接口，否则以后要加白名单，都要改此文件
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log
little-cui commented on a change in pull request #622: [SCB-1719] Support
access log
URL:
https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367203730
##
File path: server/core/config.go
##
@@ -143,5 +147,8 @@ func initLogger() {
LogFormatText: ServerInfo.Config.LogFormat == "text",
LogRotateSize: int(ServerInfo.Config.LogRotateSize),
LogBackupCount: int(ServerInfo.Config.LogBackupCount),
+ EnableLevel:true,
+ EnableCaller: true,
+ EnableTime: true,
Review comment:
既然都是true，默认为true即可，不用每个地方加设置值
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log
little-cui commented on a change in pull request #622: [SCB-1719] Support access log URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r366112743 ## File path: etc/conf/app.conf ## @@ -152,6 +152,12 @@ log_backup_count = 50 log_format = text # whether enable record syslog log_sys = false +# access log format: remoteIp requestReceiveTime "method requestUri proto" statusCode requestBodySize delay(ms) +# example: 127.0.0.1 2006-01-02T15:04:05.000Z07:00 "GET /v4/default/registry/microservices HTTP/1.1" 200 0 0 +# whether enable access log +enable_access_log = false +# access log file +access_log_file = "./access.log" Review comment: access的转存是否继承log的配置？如果是就说明清楚 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log
little-cui commented on a change in pull request #622: [SCB-1719] Support
access log
URL:
https://github.com/apache/servicecomb-service-center/pull/622#discussion_r366110702
##
File path: server/core/config.go
##
@@ -108,12 +108,14 @@ func newInfo() pb.ServerInformation {
CompactIndexDelta:
beego.AppConfig.DefaultInt64("compact_index_delta", 100),
CompactInterval:
beego.AppConfig.String("compact_interval"),
- LogRotateSize: maxLogFileSize,
- LogBackupCount: maxLogBackupCount,
- LogFilePath:beego.AppConfig.String("logfile"),
- LogLevel: beego.AppConfig.String("loglevel"),
- LogFormat:
beego.AppConfig.DefaultString("log_format", "text"),
- LogSys: beego.AppConfig.DefaultBool("log_sys",
false),
+ LogRotateSize: maxLogFileSize,
+ LogBackupCount: maxLogBackupCount,
+ LogFilePath: beego.AppConfig.String("logfile"),
+ LogLevel:beego.AppConfig.String("loglevel"),
+ LogFormat:
beego.AppConfig.DefaultString("log_format", "text"),
+ LogSys: beego.AppConfig.DefaultBool("log_sys",
false),
+ EnableAccessLog:
beego.AppConfig.String("enable_access_log") == "true",
+ AccessLogFile:
beego.AppConfig.DefaultString("access_log_file", "./access.log"),
Review comment:
同时也要支持环境变量注入
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log
little-cui commented on a change in pull request #622: [SCB-1719] Support
access log
URL:
https://github.com/apache/servicecomb-service-center/pull/622#discussion_r366111996
##
File path: server/handler/accesslog/handler.go
##
@@ -0,0 +1,99 @@
+// 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.
+
+// +build go1.9
+
+package accesslog
+
+import (
+ "fmt"
+ "go.uber.org/zap"
+ "net/http"
+ "os"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/pkg/chain"
+ "github.com/apache/servicecomb-service-center/pkg/log"
+ "github.com/apache/servicecomb-service-center/pkg/rest"
+ "github.com/apache/servicecomb-service-center/pkg/util"
+ "github.com/apache/servicecomb-service-center/server/core"
+ svr "github.com/apache/servicecomb-service-center/server/rest"
+)
+
+// Handler implements chain.Handler
+// Handler records access log
+type Handler struct {
+ logger*zap.SugaredLogger
+ whiteListAPIs map[string]struct{} // not record access log
+}
+
+// Handle handles the request
+func (l *Handler) Handle(i *chain.Invocation) {
+ matchPattern := i.Context().Value(rest.CTX_MATCH_PATTERN).(string)
+ if _, ok := l.whiteListAPIs[matchPattern]; ok {
+ i.Next()
+ return
+ }
+ startTimeStr := "unknown"
+ start, ok := i.Context().Value(svr.CTX_START_TIMESTAMP).(time.Time)
+ if ok {
+ startTimeStr = start.Format("2006-01-02T15:04:05.000Z07:00")
+ }
+ r := i.Context().Value(rest.CTX_REQUEST).(*http.Request)
+ w := i.Context().Value(rest.CTX_RESPONSE).(http.ResponseWriter)
+ i.Next(chain.WithAsyncFunc(func(_ chain.Result) {
+ delayByMillisecond := "unknown"
+ if ok {
+ delayByMillisecond = fmt.Sprintf("%d",
time.Since(start)/time.Millisecond)
+ }
+ statusCode := w.Header().Get(rest.HEADER_RESPONSE_STATUS)
+ // format: remoteIp requestReceiveTime "method requestUri
proto" statusCode requestBodySize delay(ms)
+ // example: 127.0.0.1 2006-01-02T15:04:05.000Z07:00 "GET
/v4/default/registry/microservices HTTP/1.1" 200 0 0
+ l.logger.Infof("%s %s \"%s %s %s\" %s %d %s",
+ util.GetIPFromContext(i.Context()),
+ startTimeStr,
+ r.Method,
+ r.RequestURI,
+ r.Proto,
+ statusCode,
+ r.ContentLength,
+ delayByMillisecond)
+ }))
+}
+
+// NewAccessLogHandler creates a Handler
+func NewAccessLogHandler(logFile string, rotateSize int, backupCount int)
*Handler {
+ h := {whiteListAPIs: make(map[string]struct{}, 0)}
+ if logFile == "" {
+ logFile = "./access.log"
+ }
+ logger := newZapLogger(logFile, rotateSize, backupCount)
Review comment:
不能直接依赖zap，应该通过接口方式实现
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log
little-cui commented on a change in pull request #622: [SCB-1719] Support
access log
URL:
https://github.com/apache/servicecomb-service-center/pull/622#discussion_r366112349
##
File path: server/handler/accesslog/handler.go
##
@@ -0,0 +1,99 @@
+// 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.
+
+// +build go1.9
+
+package accesslog
+
+import (
+ "fmt"
+ "go.uber.org/zap"
+ "net/http"
+ "os"
+ "time"
+
+ "github.com/apache/servicecomb-service-center/pkg/chain"
+ "github.com/apache/servicecomb-service-center/pkg/log"
+ "github.com/apache/servicecomb-service-center/pkg/rest"
+ "github.com/apache/servicecomb-service-center/pkg/util"
+ "github.com/apache/servicecomb-service-center/server/core"
+ svr "github.com/apache/servicecomb-service-center/server/rest"
+)
+
+// Handler implements chain.Handler
+// Handler records access log
+type Handler struct {
+ logger*zap.SugaredLogger
+ whiteListAPIs map[string]struct{} // not record access log
+}
+
+// Handle handles the request
+func (l *Handler) Handle(i *chain.Invocation) {
+ matchPattern := i.Context().Value(rest.CTX_MATCH_PATTERN).(string)
+ if _, ok := l.whiteListAPIs[matchPattern]; ok {
+ i.Next()
+ return
+ }
+ startTimeStr := "unknown"
+ start, ok := i.Context().Value(svr.CTX_START_TIMESTAMP).(time.Time)
+ if ok {
+ startTimeStr = start.Format("2006-01-02T15:04:05.000Z07:00")
+ }
+ r := i.Context().Value(rest.CTX_REQUEST).(*http.Request)
+ w := i.Context().Value(rest.CTX_RESPONSE).(http.ResponseWriter)
+ i.Next(chain.WithAsyncFunc(func(_ chain.Result) {
+ delayByMillisecond := "unknown"
+ if ok {
+ delayByMillisecond = fmt.Sprintf("%d",
time.Since(start)/time.Millisecond)
+ }
+ statusCode := w.Header().Get(rest.HEADER_RESPONSE_STATUS)
+ // format: remoteIp requestReceiveTime "method requestUri
proto" statusCode requestBodySize delay(ms)
+ // example: 127.0.0.1 2006-01-02T15:04:05.000Z07:00 "GET
/v4/default/registry/microservices HTTP/1.1" 200 0 0
+ l.logger.Infof("%s %s \"%s %s %s\" %s %d %s",
+ util.GetIPFromContext(i.Context()),
+ startTimeStr,
+ r.Method,
+ r.RequestURI,
+ r.Proto,
+ statusCode,
+ r.ContentLength,
+ delayByMillisecond)
+ }))
+}
+
+// NewAccessLogHandler creates a Handler
+func NewAccessLogHandler(logFile string, rotateSize int, backupCount int)
*Handler {
+ h := {whiteListAPIs: make(map[string]struct{}, 0)}
+ if logFile == "" {
+ logFile = "./access.log"
+ }
+ logger := newZapLogger(logFile, rotateSize, backupCount)
+ h.logger = logger.WithOptions(zap.ErrorOutput(log.StderrSyncer)).Sugar()
+ // no access log for heartbeat
+
h.whiteListAPIs["/v4/:project/registry/microservices/:serviceId/instances/:instanceId/heartbeat"]
= struct{}{}
Review comment:
心跳日志建议记录到access
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log
little-cui commented on a change in pull request #622: [SCB-1719] Support access log URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r366112543 ## File path: server/handler/accesslog/null.go ## @@ -0,0 +1,22 @@ +// 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. + +// +build !go1.9 Review comment: 没有版本要求就不需要申请 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log
little-cui commented on a change in pull request #622: [SCB-1719] Support
access log
URL:
https://github.com/apache/servicecomb-service-center/pull/622#discussion_r366111391
##
File path: server/core/config.go
##
@@ -108,12 +108,14 @@ func newInfo() pb.ServerInformation {
CompactIndexDelta:
beego.AppConfig.DefaultInt64("compact_index_delta", 100),
CompactInterval:
beego.AppConfig.String("compact_interval"),
- LogRotateSize: maxLogFileSize,
- LogBackupCount: maxLogBackupCount,
- LogFilePath:beego.AppConfig.String("logfile"),
- LogLevel: beego.AppConfig.String("loglevel"),
- LogFormat:
beego.AppConfig.DefaultString("log_format", "text"),
- LogSys: beego.AppConfig.DefaultBool("log_sys",
false),
+ LogRotateSize: maxLogFileSize,
+ LogBackupCount: maxLogBackupCount,
+ LogFilePath: beego.AppConfig.String("logfile"),
+ LogLevel:beego.AppConfig.String("loglevel"),
+ LogFormat:
beego.AppConfig.DefaultString("log_format", "text"),
+ LogSys: beego.AppConfig.DefaultBool("log_sys",
false),
+ EnableAccessLog:
beego.AppConfig.String("enable_access_log") == "true",
Review comment:
配置没有这个选项，就没必要定义了，可以在实现里面判断
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:
us...@infra.apache.org
With regards,
Apache Git Services
