You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2020/01/13 11:56:50 UTC

[GitHub] [servicecomb-service-center] humingcheng opened a new pull request #622: [SCB-1719] Support access log

humingcheng opened a new pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622
 
 
   Follow this checklist to help us incorporate your contribution quickly and easily:
   
    - [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SCB) filed for the change (usually before you start working on it).  Trivial changes like typos do not require a JIRA issue.  Your pull request should address just this issue, without pulling in other changes.
    - [x] Each commit in the pull request should have a meaningful subject line and body.
    - [x] Format the pull request title like `[SCB-XXX] Fixes bug in ApproximateQuantiles`, where you replace `SCB-XXX` with the appropriate JIRA issue.
    - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [x] Run `go build` `go test` `go fmt` `go vet` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
    - [x] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   ---
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] tianxiaoliang commented on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
tianxiaoliang commented on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573951619
 
 
   UT？

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] aseTo2016 commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
aseTo2016 commented on a change in pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367189475
 
 

 ##########
 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 &Handler{
+		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{}{}
 
 Review comment:
   老版的心跳有v3的接口，看下要不要加入，建议将/v4/:project/registry/microservices/:serviceId/instances/:instanceId/heartbeat在instance里面定义一个const，这里引用

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] humingcheng commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367399807
 
 

 ##########
 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"
 
 Review comment:
   按照SC的机制，不ok的话，用当前时间是不准确的，注明unknown反而反应真实情况

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] humingcheng commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r366693939
 
 

 ##########
 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 := &Handler{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 log等效，因此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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] humingcheng commented on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
humingcheng commented on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-574493296
 
 
   > UT？
   
   Done.

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] codecov-io edited a comment on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573651885
 
 
   # [Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=h1) Report
   > Merging [#622](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=desc) into [master](https://codecov.io/gh/apache/servicecomb-service-center/commit/1192e430dbc2ec0a803e62f9a22e5dc15ed2f897?src=pr&el=desc) will **decrease** coverage by `0.08%`.
   > The diff coverage is `24.41%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/graphs/tree.svg?width=650&token=GAaF7zrg8R&height=150&src=pr)](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #622      +/-   ##
   ==========================================
   - Coverage    57.3%   57.21%   -0.09%     
   ==========================================
     Files         205      206       +1     
     Lines       15877    15975      +98     
   ==========================================
   + Hits         9098     9140      +42     
   - Misses       6127     6174      +47     
   - Partials      652      661       +9
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [server/core/proto/types.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvcHJvdG8vdHlwZXMuZ28=) | `0% <ø> (ø)` | :arrow_up: |
   | [server/core/config.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvY29uZmlnLmdv) | `0% <0%> (ø)` | :arrow_up: |
   | [pkg/log/zap.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-cGtnL2xvZy96YXAuZ28=) | `80.83% <23.8%> (-8.59%)` | :arrow_down: |
   | [server/handler/accesslog/handler.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2hhbmRsZXIvYWNjZXNzbG9nL2hhbmRsZXIuZ28=) | `29.09% <29.09%> (ø)` | |
   | [syncer/pkg/utils/addr.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c3luY2VyL3BrZy91dGlscy9hZGRyLmdv) | `63.63% <0%> (-9.1%)` | :arrow_down: |
   | [server/metric/metrics.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL21ldHJpYy9tZXRyaWNzLmdv) | `80.82% <0%> (-3.18%)` | :arrow_down: |
   | [syncer/servicecenter/sync.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c3luY2VyL3NlcnZpY2VjZW50ZXIvc3luYy5nbw==) | `71.42% <0%> (-2.39%)` | :arrow_down: |
   | [scctl/pkg/plugin/diagnose/compare\_holder.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2NjdGwvcGtnL3BsdWdpbi9kaWFnbm9zZS9jb21wYXJlX2hvbGRlci5nbw==) | `94.87% <0%> (-1.29%)` | :arrow_down: |
   | [syncer/plugins/servicecenter/transform.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c3luY2VyL3BsdWdpbnMvc2VydmljZWNlbnRlci90cmFuc2Zvcm0uZ28=) | `53.57% <0%> (-0.72%)` | :arrow_down: |
   | ... and [4 more](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=footer). Last update [1192e43...0c65d42](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] humingcheng commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367399975
 
 

 ##########
 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 &Handler{
+		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:
   已修改

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
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:
users@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

Posted by GitBox <gi...@apache.org>.
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 := &Handler{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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] humingcheng commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367399880
 
 

 ##########
 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:
   已修改

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] coveralls edited a comment on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573651788
 
 
   
   [![Coverage Status](https://coveralls.io/builds/28169162/badge)](https://coveralls.io/builds/28169162)
   
   Coverage decreased (-0.06%) to 59.738% when pulling **2523ff31d891c309f75b8d7c7668861ae71e1838 on humingcheng:master** into **c1b43ad0514ac4bdbbe08461ded62caf2a7aeee5 on apache:master**.
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] little-cui merged pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
little-cui merged pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622
 
 
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] codecov-io edited a comment on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573651885
 
 
   # [Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=h1) Report
   > Merging [#622](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=desc) into [master](https://codecov.io/gh/apache/servicecomb-service-center/commit/1192e430dbc2ec0a803e62f9a22e5dc15ed2f897?src=pr&el=desc) will **increase** coverage by `0.02%`.
   > The diff coverage is `38.35%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/graphs/tree.svg?width=650&token=GAaF7zrg8R&height=150&src=pr)](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #622      +/-   ##
   ==========================================
   + Coverage    57.3%   57.32%   +0.02%     
   ==========================================
     Files         205      206       +1     
     Lines       15877    15962      +85     
   ==========================================
   + Hits         9098     9150      +52     
   - Misses       6127     6157      +30     
   - Partials      652      655       +3
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [server/core/proto/types.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvcHJvdG8vdHlwZXMuZ28=) | `0% <ø> (ø)` | :arrow_up: |
   | [server/core/config.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvY29uZmlnLmdv) | `0% <0%> (ø)` | :arrow_up: |
   | [server/broker/util.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2Jyb2tlci91dGlsLmdv) | `53.58% <100%> (+0.37%)` | :arrow_up: |
   | [pkg/log/zap.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-cGtnL2xvZy96YXAuZ28=) | `93.85% <100%> (+4.43%)` | :arrow_up: |
   | [server/handler/accesslog/handler.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2hhbmRsZXIvYWNjZXNzbG9nL2hhbmRsZXIuZ28=) | `23.8% <23.8%> (ø)` | |
   | [syncer/pkg/utils/addr.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c3luY2VyL3BrZy91dGlscy9hZGRyLmdv) | `63.63% <0%> (-9.1%)` | :arrow_down: |
   | [server/metric/metrics.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL21ldHJpYy9tZXRyaWNzLmdv) | `80.82% <0%> (-3.18%)` | :arrow_down: |
   | [server/core/proto/parser.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvcHJvdG8vcGFyc2VyLmdv) | `92.85% <0%> (-2.39%)` | :arrow_down: |
   | [scctl/pkg/plugin/diagnose/compare\_holder.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2NjdGwvcGtnL3BsdWdpbi9kaWFnbm9zZS9jb21wYXJlX2hvbGRlci5nbw==) | `94.87% <0%> (-1.29%)` | :arrow_down: |
   | ... and [6 more](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=footer). Last update [1192e43...c152ccc](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] codecov-io edited a comment on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573651885
 
 
   # [Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=h1) Report
   > Merging [#622](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=desc) into [master](https://codecov.io/gh/apache/servicecomb-service-center/commit/c1b43ad0514ac4bdbbe08461ded62caf2a7aeee5?src=pr&el=desc) will **decrease** coverage by `0.17%`.
   > The diff coverage is `26.96%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/graphs/tree.svg?width=650&token=GAaF7zrg8R&height=150&src=pr)](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #622      +/-   ##
   ==========================================
   - Coverage   57.35%   57.17%   -0.18%     
   ==========================================
     Files         205      206       +1     
     Lines       15905    15983      +78     
   ==========================================
   + Hits         9122     9139      +17     
   - Misses       6127     6181      +54     
   - Partials      656      663       +7
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [server/core/proto/types.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvcHJvdG8vdHlwZXMuZ28=) | `0% <ø> (ø)` | :arrow_up: |
   | [server/core/config.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvY29uZmlnLmdv) | `0% <0%> (ø)` | :arrow_up: |
   | [pkg/log/logger.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-cGtnL2xvZy9sb2dnZXIuZ28=) | `95.91% <100%> (+0.17%)` | :arrow_up: |
   | [pkg/log/zap.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-cGtnL2xvZy96YXAuZ28=) | `81.74% <27.27%> (-8.17%)` | :arrow_down: |
   | [server/handler/accesslog/handler.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2hhbmRsZXIvYWNjZXNzbG9nL2hhbmRsZXIuZ28=) | `29.09% <29.09%> (ø)` | |
   | [server/notify/listwatcher.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL25vdGlmeS9saXN0d2F0Y2hlci5nbw==) | `69.44% <0%> (-9.73%)` | :arrow_down: |
   | [server/core/proto/parser.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvcHJvdG8vcGFyc2VyLmdv) | `92.85% <0%> (-2.39%)` | :arrow_down: |
   | [syncer/servicecenter/sync.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c3luY2VyL3NlcnZpY2VjZW50ZXIvc3luYy5nbw==) | `71.42% <0%> (-2.39%)` | :arrow_down: |
   | [server/broker/service.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2Jyb2tlci9zZXJ2aWNlLmdv) | `55.7% <0%> (-0.16%)` | :arrow_down: |
   | ... and [2 more](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=footer). Last update [c1b43ad...2523ff3](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] codecov-io commented on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573651885
 
 
   # [Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=h1) Report
   > Merging [#622](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=desc) into [master](https://codecov.io/gh/apache/servicecomb-service-center/commit/1192e430dbc2ec0a803e62f9a22e5dc15ed2f897?src=pr&el=desc) will **decrease** coverage by `<.01%`.
   > The diff coverage is `0%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/graphs/tree.svg?width=650&token=GAaF7zrg8R&height=150&src=pr)](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #622      +/-   ##
   ==========================================
   - Coverage    57.3%   57.29%   -0.01%     
   ==========================================
     Files         205      205              
     Lines       15877    15879       +2     
   ==========================================
     Hits         9098     9098              
   - Misses       6127     6128       +1     
   - Partials      652      653       +1
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [server/core/proto/types.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvcHJvdG8vdHlwZXMuZ28=) | `0% <ø> (ø)` | :arrow_up: |
   | [server/core/config.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvY29uZmlnLmdv) | `0% <0%> (ø)` | :arrow_up: |
   | [syncer/pkg/utils/addr.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c3luY2VyL3BrZy91dGlscy9hZGRyLmdv) | `63.63% <0%> (-9.1%)` | :arrow_down: |
   | [server/core/proto/parser.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvcHJvdG8vcGFyc2VyLmdv) | `92.85% <0%> (-2.39%)` | :arrow_down: |
   | [syncer/servicecenter/sync.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c3luY2VyL3NlcnZpY2VjZW50ZXIvc3luYy5nbw==) | `71.42% <0%> (-2.39%)` | :arrow_down: |
   | [scctl/pkg/plugin/diagnose/compare\_holder.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2NjdGwvcGtnL3BsdWdpbi9kaWFnbm9zZS9jb21wYXJlX2hvbGRlci5nbw==) | `94.87% <0%> (-1.29%)` | :arrow_down: |
   | [server/broker/util.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2Jyb2tlci91dGlsLmdv) | `53.47% <0%> (+0.26%)` | :arrow_up: |
   | [syncer/pkg/syssig/signal.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c3luY2VyL3BrZy9zeXNzaWcvc2lnbmFsLmdv) | `67.74% <0%> (+3.22%)` | :arrow_up: |
   | [syncer/pkg/utils/files.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c3luY2VyL3BrZy91dGlscy9maWxlcy5nbw==) | `93.33% <0%> (+6.66%)` | :arrow_up: |
   | [server/service/util/util.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL3NlcnZpY2UvdXRpbC91dGlsLmdv) | `90% <0%> (+10%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=footer). Last update [1192e43...88cbf6e](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] coveralls edited a comment on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573651788
 
 
   
   [![Coverage Status](https://coveralls.io/builds/28193724/badge)](https://coveralls.io/builds/28193724)
   
   Coverage decreased (-0.09%) to 59.707% when pulling **0d22df4176c73b655294e685caf7b31be6c86bdb on humingcheng:master** into **c1b43ad0514ac4bdbbe08461ded62caf2a7aeee5 on apache:master**.
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] coveralls edited a comment on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573651788
 
 
   
   [![Coverage Status](https://coveralls.io/builds/28150142/badge)](https://coveralls.io/builds/28150142)
   
   Coverage decreased (-0.1%) to 59.684% when pulling **0c65d4235e0f4a00df1257f35f5a0b91dfb8f279 on humingcheng:master** into **214b429503ec8927d84285be411c4811daecdbed on apache:master**.
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] humingcheng commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367398763
 
 

 ##########
 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 &Handler{
+		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{}{}
 
 Review comment:
   v3的heartbeat已经加入；硬编码的地方很多，一次改不完，记个问题，后面一起改掉；

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] coveralls edited a comment on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573651788
 
 
   
   [![Coverage Status](https://coveralls.io/builds/28114888/badge)](https://coveralls.io/builds/28114888)
   
   Coverage decreased (-0.1%) to 59.672% when pulling **c152ccc57e3201a717f032edb982de71b5588550 on humingcheng:master** into **214b429503ec8927d84285be411c4811daecdbed on apache:master**.
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] aseTo2016 commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
aseTo2016 commented on a change in pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367188934
 
 

 ##########
 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 &Handler{
+		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)
 
 Review comment:
   有大小就指定为1吧

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
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 &Handler{
+		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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] aseTo2016 commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
aseTo2016 commented on a change in pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367187387
 
 

 ##########
 File path: pkg/log/zap.go
 ##########
 @@ -214,11 +230,12 @@ func (l *Logger) Sync() {
 }
 
 func NewLogger(cfg Config) *Logger {
-	l := zap.New(toZapConfig(cfg),
-		zap.ErrorOutput(StderrSyncer),
-		zap.AddCaller(),
-		zap.AddCallerSkip(cfg.CallerSkip),
-	)
+	opts := make([]zap.Option, 0)
 
 Review comment:
   指定一个cap吧

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] aseTo2016 commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
aseTo2016 commented on a change in pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367190225
 
 

 ##########
 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"
 
 Review comment:
   ok如果为false，建议使用当前时间，不要用个unknown

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] humingcheng commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
humingcheng commented on a change in pull request #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#discussion_r367395172
 
 

 ##########
 File path: pkg/log/zap.go
 ##########
 @@ -214,11 +230,12 @@ func (l *Logger) Sync() {
 }
 
 func NewLogger(cfg Config) *Logger {
-	l := zap.New(toZapConfig(cfg),
-		zap.ErrorOutput(StderrSyncer),
-		zap.AddCaller(),
-		zap.AddCallerSkip(cfg.CallerSkip),
-	)
+	opts := make([]zap.Option, 0)
 
 Review comment:
   Done.

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] codecov-io edited a comment on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573651885
 
 
   # [Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=h1) Report
   > Merging [#622](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=desc) into [master](https://codecov.io/gh/apache/servicecomb-service-center/commit/c1b43ad0514ac4bdbbe08461ded62caf2a7aeee5?src=pr&el=desc) will **decrease** coverage by `0.12%`.
   > The diff coverage is `38.2%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/graphs/tree.svg?width=650&token=GAaF7zrg8R&height=150&src=pr)](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #622      +/-   ##
   ==========================================
   - Coverage   57.35%   57.22%   -0.13%     
   ==========================================
     Files         205      206       +1     
     Lines       15905    15983      +78     
   ==========================================
   + Hits         9122     9147      +25     
   - Misses       6127     6175      +48     
   - Partials      656      661       +5
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [server/core/proto/types.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvcHJvdG8vdHlwZXMuZ28=) | `0% <ø> (ø)` | :arrow_up: |
   | [server/core/config.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvY29uZmlnLmdv) | `0% <0%> (ø)` | :arrow_up: |
   | [pkg/log/zap.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-cGtnL2xvZy96YXAuZ28=) | `82.03% <33.33%> (-7.88%)` | :arrow_down: |
   | [server/handler/accesslog/handler.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2hhbmRsZXIvYWNjZXNzbG9nL2hhbmRsZXIuZ28=) | `47.27% <47.27%> (ø)` | |
   | [server/notify/listwatcher.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL25vdGlmeS9saXN0d2F0Y2hlci5nbw==) | `69.44% <0%> (-9.73%)` | :arrow_down: |
   | [syncer/serf/config.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c3luY2VyL3NlcmYvY29uZmlnLmdv) | `56.75% <0%> (-2.71%)` | :arrow_down: |
   | [server/core/proto/parser.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2NvcmUvcHJvdG8vcGFyc2VyLmdv) | `92.85% <0%> (-2.39%)` | :arrow_down: |
   | [server/broker/service.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2Jyb2tlci9zZXJ2aWNlLmdv) | `55.7% <0%> (-0.16%)` | :arrow_down: |
   | [server/broker/util.go](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree#diff-c2VydmVyL2Jyb2tlci91dGlsLmdv) | `53.47% <0%> (+0.26%)` | :arrow_up: |
   | ... and [2 more](https://codecov.io/gh/apache/servicecomb-service-center/pull/622/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=footer). Last update [c1b43ad...0d22df4](https://codecov.io/gh/apache/servicecomb-service-center/pull/622?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] coveralls commented on issue #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
coveralls commented on issue #622: [SCB-1719] Support access log
URL: https://github.com/apache/servicecomb-service-center/pull/622#issuecomment-573651788
 
 
   
   [![Coverage Status](https://coveralls.io/builds/28068388/badge)](https://coveralls.io/builds/28068388)
   
   Coverage decreased (-0.05%) to 59.693% when pulling **88cbf6eb43cefa571937b49224903501d0289cc6 on humingcheng:master** into **1192e430dbc2ec0a803e62f9a22e5dc15ed2f897 on apache:master**.
   

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


With regards,
Apache Git Services

[GitHub] [servicecomb-service-center] little-cui commented on a change in pull request #622: [SCB-1719] Support access log

Posted by GitBox <gi...@apache.org>.
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 := &Handler{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:
users@infra.apache.org


With regards,
Apache Git Services