You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/11/04 08:12:45 UTC
[GitHub] [apisix-dashboard] nic-chen opened a new pull request #689: chore: refactor error log for `manager api`
nic-chen opened a new pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689
close #678
1. Retain error logs.
2. Support custom error log level
3. Support saving to a local file, and can customize the file path
4. Support log rotate
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] juzhiyuan merged pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-722735995
> `api/logs/.gitkeep` to `api/logs/placeholder.txe`, more readable.
ping @nic-chen
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-722183759
@nic-chen after you fixed the CI, then we can turn this PR to `review ready` ^_^
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r518496279
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,12 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": "/dev/stdout"
+ }
Review comment:
I think we can rebase the `v2.0` branch, and then we can remove this file
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-721626088
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=h1) Report
> Merging [#689](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=desc) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/1215c2a8b6c2f1cf6b876feb5d0572a54259d084?el=desc) will **increase** coverage by `0.19%`.
> The diff coverage is `52.17%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## v2.0 #689 +/- ##
==========================================
+ Coverage 42.82% 43.01% +0.19%
==========================================
Files 18 18
Lines 1212 1232 +20
==========================================
+ Hits 519 530 +11
- Misses 605 614 +9
Partials 88 88
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (ø)` | |
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `55.07% <0.00%> (-2.08%)` | :arrow_down: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.28% <69.23%> (+0.27%)` | :arrow_up: |
| [api/internal/core/store/query.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvcXVlcnkuZ28=) | `88.09% <100.00%> (+0.59%)` | :arrow_up: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `46.06% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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/apisix-dashboard/pull/689?src=pr&el=footer). Last update [1215c2a...acfabc3](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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
[GitHub] [apisix-dashboard] nic-chen commented on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-722383537
Relative path for log file depends on this pr #693
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-722736181
CI is failed, please fix them first
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517187384
##########
File path: api/filter/logging.go
##########
@@ -18,65 +18,61 @@ package filter
import (
"bytes"
- "io/ioutil"
- "time"
"github.com/gin-gonic/gin"
- "github.com/shiningrush/droplet/log"
- "github.com/sirupsen/logrus"
)
-func RequestLogHandler() gin.HandlerFunc {
- return func(c *gin.Context) {
- start, host, remoteIP, path, method := time.Now(), c.Request.Host, c.ClientIP(), c.Request.URL.Path, c.Request.Method
- var val interface{}
- if method == "GET" {
- val = c.Request.URL.Query()
- } else {
- val, _ = c.GetRawData()
-
- // set RequestBody back
- c.Request.Body = ioutil.NopCloser(bytes.NewReader(val.([]byte)))
- }
- c.Set("requestBody", val)
- uuid, _ := c.Get("X-Request-Id")
-
- param, _ := c.Get("requestBody")
-
- switch paramType := param.(type) {
- case []byte:
- param = string(param.([]byte))
- log.Infof("type of param: %#v", paramType)
- default:
- }
-
- blw := &bodyLogWriter{body: bytes.NewBufferString(""), ResponseWriter: c.Writer}
- c.Writer = blw
- c.Next()
- latency := time.Since(start) / 1000000
- statusCode := c.Writer.Status()
- respBody := blw.body.String()
- if uuid == "" {
- uuid = c.Writer.Header().Get("X-Request-Id")
- }
- var errs []string
- for _, err := range c.Errors {
- errs = append(errs, err.Error())
- }
- logger.WithFields(logrus.Fields{
- "requestId": uuid,
- "latency": latency,
- "remoteIp": remoteIP,
- "method": method,
- "path": path,
- "statusCode": statusCode,
- "host": host,
- "params": param,
- "respBody": respBody,
- "errMsg": errs,
- }).Info("")
- }
-}
+//func RequestLogHandler() gin.HandlerFunc {
Review comment:
and we need an issue too
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517737031
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
+ },
+ "rotate": {
Review comment:
I can not get your mind
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-721626088
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`v2.0@b4b646b`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## v2.0 #689 +/- ##
=======================================
Coverage ? 43.10%
=======================================
Files ? 18
Lines ? 1232
Branches ? 0
=======================================
Hits ? 531
Misses ? 613
Partials ? 88
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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/apisix-dashboard/pull/689?src=pr&el=footer). Last update [b4b646b...d0d72fe](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517760526
##########
File path: api/log/zap.go
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package log
+
+import (
+ "os"
+
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+
+ "github.com/apisix/manager-api/conf"
+)
+
+var logger *zap.SugaredLogger
+
+func init() {
+ writeSyncer := fileWriter()
+ encoder := getEncoder()
+ logLevel := getLogLevel()
+ core := zapcore.NewCore(encoder, writeSyncer, logLevel)
+
+ zapLogger := zap.New(core, zap.AddCaller())
+
+ logger = zapLogger.Sugar()
+}
+
+func getLogLevel() zapcore.LevelEnabler {
+ level := zapcore.WarnLevel
+ switch conf.ErrorLogLevel {
+ case "debug":
+ level = zapcore.DebugLevel
+ case "info":
+ level = zapcore.InfoLevel
+ case "error":
Review comment:
as we talked right now, we need to support them in this PR.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-721626088
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`v2.0@b4b646b`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## v2.0 #689 +/- ##
=======================================
Coverage ? 43.16%
=======================================
Files ? 18
Lines ? 1237
Branches ? 0
=======================================
Hits ? 534
Misses ? 614
Partials ? 89
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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/apisix-dashboard/pull/689?src=pr&el=footer). Last update [b4b646b...30d8b73](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517439485
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
Review comment:
Our goal is to let code tell us what is it and what it does :)
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517175722
##########
File path: api/filter/logging.go
##########
@@ -18,65 +18,61 @@ package filter
import (
"bytes"
- "io/ioutil"
- "time"
"github.com/gin-gonic/gin"
- "github.com/shiningrush/droplet/log"
- "github.com/sirupsen/logrus"
)
-func RequestLogHandler() gin.HandlerFunc {
- return func(c *gin.Context) {
- start, host, remoteIP, path, method := time.Now(), c.Request.Host, c.ClientIP(), c.Request.URL.Path, c.Request.Method
- var val interface{}
- if method == "GET" {
- val = c.Request.URL.Query()
- } else {
- val, _ = c.GetRawData()
-
- // set RequestBody back
- c.Request.Body = ioutil.NopCloser(bytes.NewReader(val.([]byte)))
- }
- c.Set("requestBody", val)
- uuid, _ := c.Get("X-Request-Id")
-
- param, _ := c.Get("requestBody")
-
- switch paramType := param.(type) {
- case []byte:
- param = string(param.([]byte))
- log.Infof("type of param: %#v", paramType)
- default:
- }
-
- blw := &bodyLogWriter{body: bytes.NewBufferString(""), ResponseWriter: c.Writer}
- c.Writer = blw
- c.Next()
- latency := time.Since(start) / 1000000
- statusCode := c.Writer.Status()
- respBody := blw.body.String()
- if uuid == "" {
- uuid = c.Writer.Header().Get("X-Request-Id")
- }
- var errs []string
- for _, err := range c.Errors {
- errs = append(errs, err.Error())
- }
- logger.WithFields(logrus.Fields{
- "requestId": uuid,
- "latency": latency,
- "remoteIp": remoteIP,
- "method": method,
- "path": path,
- "statusCode": statusCode,
- "host": host,
- "params": param,
- "respBody": respBody,
- "errMsg": errs,
- }).Info("")
- }
-}
+//func RequestLogHandler() gin.HandlerFunc {
Review comment:
It's for access log. It doesn't work now. we could remove it first, and refactor it in a new pr.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] codecov-io commented on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-721626088
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=h1) Report
> Merging [#689](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=desc) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/1215c2a8b6c2f1cf6b876feb5d0572a54259d084?el=desc) will **increase** coverage by `0.27%`.
> The diff coverage is `52.17%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## v2.0 #689 +/- ##
==========================================
+ Coverage 42.82% 43.10% +0.27%
==========================================
Files 18 18
Lines 1212 1232 +20
==========================================
+ Hits 519 531 +12
- Misses 605 613 +8
Partials 88 88
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (ø)` | |
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `55.07% <0.00%> (-2.08%)` | :arrow_down: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.94% <69.23%> (+0.93%)` | :arrow_up: |
| [api/internal/core/store/query.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvcXVlcnkuZ28=) | `88.09% <100.00%> (+0.59%)` | :arrow_up: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `46.06% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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/apisix-dashboard/pull/689?src=pr&el=footer). Last update [1215c2a...9b97ea1](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-721626088
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=h1) Report
> Merging [#689](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=desc) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/1215c2a8b6c2f1cf6b876feb5d0572a54259d084?el=desc) will **increase** coverage by `0.27%`.
> The diff coverage is `52.17%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## v2.0 #689 +/- ##
==========================================
+ Coverage 42.82% 43.10% +0.27%
==========================================
Files 18 18
Lines 1212 1232 +20
==========================================
+ Hits 519 531 +12
- Misses 605 613 +8
Partials 88 88
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (ø)` | |
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `55.07% <0.00%> (-2.08%)` | :arrow_down: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.94% <69.23%> (+0.93%)` | :arrow_up: |
| [api/internal/core/store/query.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvcXVlcnkuZ28=) | `88.09% <100.00%> (+0.59%)` | :arrow_up: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `46.06% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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/apisix-dashboard/pull/689?src=pr&el=footer). Last update [1215c2a...f3c5dc7](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-721626088
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=h1) Report
> Merging [#689](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=desc) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/1215c2a8b6c2f1cf6b876feb5d0572a54259d084?el=desc) will **increase** coverage by `0.19%`.
> The diff coverage is `52.17%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## v2.0 #689 +/- ##
==========================================
+ Coverage 42.82% 43.01% +0.19%
==========================================
Files 18 18
Lines 1212 1232 +20
==========================================
+ Hits 519 530 +11
- Misses 605 614 +9
Partials 88 88
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (ø)` | |
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `55.07% <0.00%> (-2.08%)` | :arrow_down: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.28% <69.23%> (+0.27%)` | :arrow_up: |
| [api/internal/core/store/query.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvcXVlcnkuZ28=) | `88.09% <100.00%> (+0.59%)` | :arrow_up: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `46.06% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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/apisix-dashboard/pull/689?src=pr&el=footer). Last update [1215c2a...6c07af5](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517398702
##########
File path: api/test/shell/cli_test.sh
##########
@@ -0,0 +1,49 @@
+#!/usr/bin/env bash
+
+#
+# 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.
+#
+
+# 'make init' operates scripts and related configuration files in the current directory
+# The 'apisix' command is a command in the /usr/local/apisix,
+# and the configuration file for the operation is in the /usr/local/apisix/conf
+
+set -ex
+
+clean_up() {
+ git checkout conf/conf.json
+}
+
+trap clean_up EXIT
+
+sed -i 's/"file_path": ""/"file_path": ".\/error.log"/' conf/conf.json
+sed -i 's/warn/info/' conf/conf.json
+
+go run main.go &
+
+sleep 3
+
+if [ ! -f "./error.log" ]; then
+ echo "failed: failed to write log"
+ exit 1
+fi
+
+if [ ! cat "./error.log" | grep INFO ]; then
+ echo "failed: failed to write log on right level"
+ exit 1
+fi
+
Review comment:
ok
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517438759
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
Review comment:
We’d better update our doc to let users know what this means.
I would prefer using a better way to resolve this confusion, instead of using docs. @membphis , do you have any experiences?
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517752842
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
+ },
+ "rotate": {
Review comment:
It means OK.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-722183100
`api/logs/.gitkeep` to `api/logs/placeholder.txe`, more readable.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r518476078
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,12 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": "/dev/stdout"
+ }
Review comment:
sure.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-721626088
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=h1) Report
> Merging [#689](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=desc) into [v2.0](https://codecov.io/gh/apache/apisix-dashboard/commit/1215c2a8b6c2f1cf6b876feb5d0572a54259d084?el=desc) will **increase** coverage by `0.27%`.
> The diff coverage is `52.17%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## v2.0 #689 +/- ##
==========================================
+ Coverage 42.82% 43.10% +0.27%
==========================================
Files 18 18
Lines 1212 1232 +20
==========================================
+ Hits 519 531 +12
- Misses 605 613 +8
Partials 88 88
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (ø)` | |
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `55.07% <0.00%> (-2.08%)` | :arrow_down: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.94% <69.23%> (+0.93%)` | :arrow_up: |
| [api/internal/core/store/query.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvcXVlcnkuZ28=) | `88.09% <100.00%> (+0.59%)` | :arrow_up: |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/689/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `46.06% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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/apisix-dashboard/pull/689?src=pr&el=footer). Last update [1215c2a...61a186b](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517752547
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
Review comment:
`case 1` + 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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517398528
##########
File path: api/log/zap.go
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package log
+
+import (
+ "os"
+
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+
+ "github.com/apisix/manager-api/conf"
+)
+
+var logger *zap.SugaredLogger
+
+func init() {
+ writeSyncer := fileWriter()
+ encoder := getEncoder()
+ logLevel := getLogLevel()
+ core := zapcore.NewCore(encoder, writeSyncer, logLevel)
+
+ zapLogger := zap.New(core, zap.AddCaller())
+
+ logger = zapLogger.Sugar()
+}
+
+func getLogLevel() zapcore.LevelEnabler {
+ level := zapcore.WarnLevel
+ switch conf.ErrorLogLevel {
+ case "debug":
+ level = zapcore.DebugLevel
+ case "info":
+ level = zapcore.InfoLevel
+ case "error":
Review comment:
log levels commonly used in Go libs:
debug info warn error panic fatal
what do you think ?
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
Review comment:
@membphis empty string means don't save to a file.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517755177
##########
File path: .github/workflows/cli_test.yml
##########
@@ -0,0 +1,31 @@
+name: Test CLI
Review comment:
ok, got it.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r518459947
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,12 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": "/dev/stdout"
+ }
Review comment:
we'll remove this file finally, all right?
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] nic-chen commented on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-722738292
> > `api/logs/.gitkeep` to `api/logs/placeholder.txe`, more readable.
>
> ping @nic-chen
OK
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517165851
##########
File path: api/conf/conf.go
##########
@@ -78,12 +84,38 @@ func setConf() {
if dagLibPath != "" {
DagLibPath = dagLibPath
}
+
//etcd
eTCDEndpoints := configuration.Get("conf.etcd.endpoints").String()
if eTCDEndpoints != "" {
ETCDEndpoints = eTCDEndpoints
}
+ //log
+ logLevel := configuration.Get("conf.log.error_log.level").String()
Review comment:
can we use JSONSchema to set the default value?
##########
File path: api/filter/logging.go
##########
@@ -18,65 +18,61 @@ package filter
import (
"bytes"
- "io/ioutil"
- "time"
"github.com/gin-gonic/gin"
- "github.com/shiningrush/droplet/log"
- "github.com/sirupsen/logrus"
)
-func RequestLogHandler() gin.HandlerFunc {
- return func(c *gin.Context) {
- start, host, remoteIP, path, method := time.Now(), c.Request.Host, c.ClientIP(), c.Request.URL.Path, c.Request.Method
- var val interface{}
- if method == "GET" {
- val = c.Request.URL.Query()
- } else {
- val, _ = c.GetRawData()
-
- // set RequestBody back
- c.Request.Body = ioutil.NopCloser(bytes.NewReader(val.([]byte)))
- }
- c.Set("requestBody", val)
- uuid, _ := c.Get("X-Request-Id")
-
- param, _ := c.Get("requestBody")
-
- switch paramType := param.(type) {
- case []byte:
- param = string(param.([]byte))
- log.Infof("type of param: %#v", paramType)
- default:
- }
-
- blw := &bodyLogWriter{body: bytes.NewBufferString(""), ResponseWriter: c.Writer}
- c.Writer = blw
- c.Next()
- latency := time.Since(start) / 1000000
- statusCode := c.Writer.Status()
- respBody := blw.body.String()
- if uuid == "" {
- uuid = c.Writer.Header().Get("X-Request-Id")
- }
- var errs []string
- for _, err := range c.Errors {
- errs = append(errs, err.Error())
- }
- logger.WithFields(logrus.Fields{
- "requestId": uuid,
- "latency": latency,
- "remoteIp": remoteIP,
- "method": method,
- "path": path,
- "statusCode": statusCode,
- "host": host,
- "params": param,
- "respBody": respBody,
- "errMsg": errs,
- }).Info("")
- }
-}
+//func RequestLogHandler() gin.HandlerFunc {
Review comment:
if we do not need those codes, I think we can remove them directly.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517285313
##########
File path: api/conf/conf.go
##########
@@ -78,12 +84,38 @@ func setConf() {
if dagLibPath != "" {
DagLibPath = dagLibPath
}
+
//etcd
eTCDEndpoints := configuration.Get("conf.etcd.endpoints").String()
if eTCDEndpoints != "" {
ETCDEndpoints = eTCDEndpoints
}
+ //log
+ logLevel := configuration.Get("conf.log.error_log.level").String()
Review comment:
if set the default value by config, I think don't need JSONSchema, just set default value in conf file.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517186818
##########
File path: api/filter/logging.go
##########
@@ -18,65 +18,61 @@ package filter
import (
"bytes"
- "io/ioutil"
- "time"
"github.com/gin-gonic/gin"
- "github.com/shiningrush/droplet/log"
- "github.com/sirupsen/logrus"
)
-func RequestLogHandler() gin.HandlerFunc {
- return func(c *gin.Context) {
- start, host, remoteIP, path, method := time.Now(), c.Request.Host, c.ClientIP(), c.Request.URL.Path, c.Request.Method
- var val interface{}
- if method == "GET" {
- val = c.Request.URL.Query()
- } else {
- val, _ = c.GetRawData()
-
- // set RequestBody back
- c.Request.Body = ioutil.NopCloser(bytes.NewReader(val.([]byte)))
- }
- c.Set("requestBody", val)
- uuid, _ := c.Get("X-Request-Id")
-
- param, _ := c.Get("requestBody")
-
- switch paramType := param.(type) {
- case []byte:
- param = string(param.([]byte))
- log.Infof("type of param: %#v", paramType)
- default:
- }
-
- blw := &bodyLogWriter{body: bytes.NewBufferString(""), ResponseWriter: c.Writer}
- c.Writer = blw
- c.Next()
- latency := time.Since(start) / 1000000
- statusCode := c.Writer.Status()
- respBody := blw.body.String()
- if uuid == "" {
- uuid = c.Writer.Header().Get("X-Request-Id")
- }
- var errs []string
- for _, err := range c.Errors {
- errs = append(errs, err.Error())
- }
- logger.WithFields(logrus.Fields{
- "requestId": uuid,
- "latency": latency,
- "remoteIp": remoteIP,
- "method": method,
- "path": path,
- "statusCode": statusCode,
- "host": host,
- "params": param,
- "respBody": respBody,
- "errMsg": errs,
- }).Info("")
- }
-}
+//func RequestLogHandler() gin.HandlerFunc {
Review comment:
agree +1
you can leave a comment in the source code
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-721626088
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`v2.0@b4b646b`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## v2.0 #689 +/- ##
=======================================
Coverage ? 43.01%
=======================================
Files ? 18
Lines ? 1232
Branches ? 0
=======================================
Hits ? 530
Misses ? 614
Partials ? 88
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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/apisix-dashboard/pull/689?src=pr&el=footer). Last update [b4b646b...ce70229](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-721626088
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`v2.0@b4b646b`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## v2.0 #689 +/- ##
=======================================
Coverage ? 43.01%
=======================================
Files ? 18
Lines ? 1232
Branches ? 0
=======================================
Hits ? 530
Misses ? 614
Partials ? 88
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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/apisix-dashboard/pull/689?src=pr&el=footer). Last update [b4b646b...74a89c4](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517752245
##########
File path: api/log/zap.go
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package log
+
+import (
+ "os"
+
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+
+ "github.com/apisix/manager-api/conf"
+)
+
+var logger *zap.SugaredLogger
+
+func init() {
+ writeSyncer := fileWriter()
+ encoder := getEncoder()
+ logLevel := getLogLevel()
+ core := zapcore.NewCore(encoder, writeSyncer, logLevel)
+
+ zapLogger := zap.New(core, zap.AddCaller())
+
+ logger = zapLogger.Sugar()
+}
+
+func getLogLevel() zapcore.LevelEnabler {
+ level := zapcore.WarnLevel
+ switch conf.ErrorLogLevel {
+ case "debug":
+ level = zapcore.DebugLevel
+ case "info":
+ level = zapcore.InfoLevel
+ case "error":
Review comment:
ok, I will at them all.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517440390
##########
File path: .github/workflows/cli_test.yml
##########
@@ -0,0 +1,31 @@
+name: Test CLI
Review comment:
Emmm any ideas? What this action does?
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517334311
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
Review comment:
what means empty string? It makes me confused
##########
File path: api/log/zap.go
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package log
+
+import (
+ "os"
+
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+
+ "github.com/apisix/manager-api/conf"
+)
+
+var logger *zap.SugaredLogger
+
+func init() {
+ writeSyncer := fileWriter()
+ encoder := getEncoder()
+ logLevel := getLogLevel()
+ core := zapcore.NewCore(encoder, writeSyncer, logLevel)
+
+ zapLogger := zap.New(core, zap.AddCaller())
+
+ logger = zapLogger.Sugar()
+}
+
+func getLogLevel() zapcore.LevelEnabler {
+ level := zapcore.WarnLevel
+ switch conf.ErrorLogLevel {
+ case "debug":
+ level = zapcore.DebugLevel
+ case "info":
+ level = zapcore.InfoLevel
+ case "error":
Review comment:
`notice`, `warn`, `crit`
can we add more log level?
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
+ },
+ "rotate": {
Review comment:
can we implement this feature later?
so we can remove those configurations at first.
##########
File path: .github/workflows/cli_test.yml
##########
@@ -0,0 +1,31 @@
+name: Test CLI
Review comment:
we can not use CLI here, we need a better name
##########
File path: api/test/shell/cli_test.sh
##########
@@ -0,0 +1,49 @@
+#!/usr/bin/env bash
+
+#
+# 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.
+#
+
+# 'make init' operates scripts and related configuration files in the current directory
+# The 'apisix' command is a command in the /usr/local/apisix,
+# and the configuration file for the operation is in the /usr/local/apisix/conf
+
+set -ex
+
+clean_up() {
+ git checkout conf/conf.json
+}
+
+trap clean_up EXIT
+
+sed -i 's/"file_path": ""/"file_path": ".\/error.log"/' conf/conf.json
+sed -i 's/warn/info/' conf/conf.json
+
+go run main.go &
+
+sleep 3
+
+if [ ! -f "./error.log" ]; then
+ echo "failed: failed to write log"
+ exit 1
+fi
+
+if [ ! cat "./error.log" | grep INFO ]; then
+ echo "failed: failed to write log on right level"
+ exit 1
+fi
+
Review comment:
only one blank line at the end of file.
and we need more test cases, check the error log when using different log level.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517735273
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
Review comment:
> We’d better update our doc to let users know what this means.
we should choose a good design first.
the user knows how to use it without any explanation is the best way.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-721626088
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`v2.0@b4b646b`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/689?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## v2.0 #689 +/- ##
=======================================
Coverage ? 43.16%
=======================================
Files ? 18
Lines ? 1237
Branches ? 0
=======================================
Hits ? 534
Misses ? 614
Partials ? 89
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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/apisix-dashboard/pull/689?src=pr&el=footer). Last update [b4b646b...2e2ac01](https://codecov.io/gh/apache/apisix-dashboard/pull/689?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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517690222
##########
File path: .github/workflows/cli_test.yml
##########
@@ -0,0 +1,31 @@
+name: Test CLI
Review comment:
It's not just for these cases. I think we could have other test cases on this CI. So I think it's OK for this.
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517436855
##########
File path: api/filter/logging.go
##########
@@ -18,65 +18,61 @@ package filter
import (
"bytes"
- "io/ioutil"
- "time"
"github.com/gin-gonic/gin"
- "github.com/shiningrush/droplet/log"
- "github.com/sirupsen/logrus"
)
-func RequestLogHandler() gin.HandlerFunc {
- return func(c *gin.Context) {
- start, host, remoteIP, path, method := time.Now(), c.Request.Host, c.ClientIP(), c.Request.URL.Path, c.Request.Method
- var val interface{}
- if method == "GET" {
- val = c.Request.URL.Query()
- } else {
- val, _ = c.GetRawData()
-
- // set RequestBody back
- c.Request.Body = ioutil.NopCloser(bytes.NewReader(val.([]byte)))
- }
- c.Set("requestBody", val)
- uuid, _ := c.Get("X-Request-Id")
-
- param, _ := c.Get("requestBody")
-
- switch paramType := param.(type) {
- case []byte:
- param = string(param.([]byte))
- log.Infof("type of param: %#v", paramType)
- default:
- }
-
- blw := &bodyLogWriter{body: bytes.NewBufferString(""), ResponseWriter: c.Writer}
- c.Writer = blw
- c.Next()
- latency := time.Since(start) / 1000000
- statusCode := c.Writer.Status()
- respBody := blw.body.String()
- if uuid == "" {
- uuid = c.Writer.Header().Get("X-Request-Id")
- }
- var errs []string
- for _, err := range c.Errors {
- errs = append(errs, err.Error())
- }
- logger.WithFields(logrus.Fields{
- "requestId": uuid,
- "latency": latency,
- "remoteIp": remoteIP,
- "method": method,
- "path": path,
- "statusCode": statusCode,
- "host": host,
- "params": param,
- "respBody": respBody,
- "errMsg": errs,
- }).Info("")
- }
-}
+//func RequestLogHandler() gin.HandlerFunc {
Review comment:
For those temp codes, we could use TODO, FIXME, NOTE keywords to let reviewer know what we want to do :)
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517283974
##########
File path: api/filter/logging.go
##########
@@ -18,65 +18,61 @@ package filter
import (
"bytes"
- "io/ioutil"
- "time"
"github.com/gin-gonic/gin"
- "github.com/shiningrush/droplet/log"
- "github.com/sirupsen/logrus"
)
-func RequestLogHandler() gin.HandlerFunc {
- return func(c *gin.Context) {
- start, host, remoteIP, path, method := time.Now(), c.Request.Host, c.ClientIP(), c.Request.URL.Path, c.Request.Method
- var val interface{}
- if method == "GET" {
- val = c.Request.URL.Query()
- } else {
- val, _ = c.GetRawData()
-
- // set RequestBody back
- c.Request.Body = ioutil.NopCloser(bytes.NewReader(val.([]byte)))
- }
- c.Set("requestBody", val)
- uuid, _ := c.Get("X-Request-Id")
-
- param, _ := c.Get("requestBody")
-
- switch paramType := param.(type) {
- case []byte:
- param = string(param.([]byte))
- log.Infof("type of param: %#v", paramType)
- default:
- }
-
- blw := &bodyLogWriter{body: bytes.NewBufferString(""), ResponseWriter: c.Writer}
- c.Writer = blw
- c.Next()
- latency := time.Since(start) / 1000000
- statusCode := c.Writer.Status()
- respBody := blw.body.String()
- if uuid == "" {
- uuid = c.Writer.Header().Get("X-Request-Id")
- }
- var errs []string
- for _, err := range c.Errors {
- errs = append(errs, err.Error())
- }
- logger.WithFields(logrus.Fields{
- "requestId": uuid,
- "latency": latency,
- "remoteIp": remoteIP,
- "method": method,
- "path": path,
- "statusCode": statusCode,
- "host": host,
- "params": param,
- "respBody": respBody,
- "errMsg": errs,
- }).Info("")
- }
-}
+//func RequestLogHandler() gin.HandlerFunc {
Review comment:
we had a issue:
https://github.com/apache/apisix-dashboard/issues/676
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517735545
##########
File path: api/log/zap.go
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package log
+
+import (
+ "os"
+
+ "go.uber.org/zap"
+ "go.uber.org/zap/zapcore"
+
+ "github.com/apisix/manager-api/conf"
+)
+
+var logger *zap.SugaredLogger
+
+func init() {
+ writeSyncer := fileWriter()
+ encoder := getEncoder()
+ logLevel := getLogLevel()
+ core := zapcore.NewCore(encoder, writeSyncer, logLevel)
+
+ zapLogger := zap.New(core, zap.AddCaller())
+
+ logger = zapLogger.Sugar()
+}
+
+func getLogLevel() zapcore.LevelEnabler {
+ level := zapcore.WarnLevel
+ switch conf.ErrorLogLevel {
+ case "debug":
+ level = zapcore.DebugLevel
+ case "info":
+ level = zapcore.InfoLevel
+ case "error":
Review comment:
do we have supported all of them? `debug info warn error panic fatal`
if yes, I think it is fine. please confirm
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517399432
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
+ },
+ "rotate": {
Review comment:
fair enough
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] membphis commented on a change in pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#discussion_r517733960
##########
File path: api/conf/conf.json
##########
@@ -10,6 +10,17 @@
"dag-lib-path": "",
"etcd": {
"endpoints": "127.0.0.1:2379"
+ },
+ "log": {
+ "error_log": {
+ "level": "warn",
+ "file_path": ""
Review comment:
> empty string means don't save to a file.
I do not think that is a good design.
How about this way?
```sh
# case 1:
file_path = "logs/error.log" # write error log to file `./logs/error.log` in current work dir
# case 2:
file_path = "/tmp/logs/error.log" # write error log to file `/tmp/logs/error.log`
# case 3:
file_path = "/dev/stdout" # print the error log to stand output device
# case 4
file_path = "/dev/stderr" # print the error log to error output device
```
For an empty string, it should be an invalid value, if the user uses this value, we'll throw an error message.
@nic-chen
----------------------------------------------------------------
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
[GitHub] [apisix-dashboard] nic-chen commented on pull request #689: chore: refactor error log for `manager api`
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #689:
URL: https://github.com/apache/apisix-dashboard/pull/689#issuecomment-722738466
> CI is failed, please fix them first
working on it
----------------------------------------------------------------
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