Repository: go-kratos/kratos
Pull request #2620 (2023-01-16 10:10:27)

feat(log): update zap interface
<!--
🎉 Thanks for sending a pull request to Kratos! Here are some tips for you:

1. If this is your first time contributing to Kratos, please read our contribution guide: https://go-kratos.dev/en/docs/community/contribution/
2. Ensure you have added or ran the appropriate tests and lint for your PR, please use `make lint` and `make test` before filing your PR, use `make clean` to tidy your go mod.
3. If the PR is unfinished, you may need to mark it as a WIP(Work In Progress) PR or draft PR
4. Please use a semantic commits format title, such as `<type>[optional scope]: <description>`, see: https://go-kratos.dev/docs/community/contribution#type
5. at the same time, please note that similar work should be submitted in one PR as far as possible to reduce the workload of reviewers. Do not split a work into multiple PR unless it should.
-->

<!--
🎉 感谢您向 Kratos 发送 PR 请求！以下是一些提示：
如果这是你第一次为 Kratos 贡献，请阅读我们的贡献指南：https://go-kratos.dev/en/docs/community/contribution/
2、确保您已经为您的 PR 添加或运行了适当的测试和lint，请在提交PR之前使用“make lint”和“make test”，使用“make clean”整理您的 go.mod。
3、如果请购单未完成，您可能需要将其标记为 WIP（在制品）PR 或 draft PR
4、请使用语义提交格式标题，如“<类型>[可选范围]：<说明>`，请参阅：https://go-kratos.dev/docs/community/contribution#type
5. 同时请注意，同类的工作请尽量在一个PR中提交，以减轻 review 者的工作负担，不要把一项工作拆分成很多个PR，除非它应该这样做。
-->

<!--
-->

#### Description (what this PR does / why we need it):
<!--
* The description should include the motivation for this PR or contrast this with previous behavior
-->


#### Which issue(s) this PR fixes (resolves / be part of):
<!--
* Automatically closes linked issue when PR is merged.
* If your PR is not fully resolved the issue, please use `part of #<issue number>` instead.

Usage: `fixes/resolves #<issue number>`, or `fixes/resolves (paste link of issue)`.
-->


#### Other special notes for the reviewers:
<!--
* Somethings that need extra attention for the reviewers
* Some additional notes, TODO list, etc.
-->



Patch:
diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
index 90e3bcb4..974ce964 100644
--- a/.github/pull_request_template.md
+++ b/.github/pull_request_template.md
@@ -3,22 +3,20 @@
 
 1. If this is your first time contributing to Kratos, please read our contribution guide: https://go-kratos.dev/en/docs/community/contribution/
 2. Ensure you have added or ran the appropriate tests and lint for your PR, please use `make lint` and `make test` before filing your PR, use `make clean` to tidy your go mod.
-3. If the PR is unfinished, you may need to mark it as a WIP(Work In Progress) PR or draft PR
+3. If the PR is unfinished, you may need to mark it as a WIP(Work In Progress) PR or Draft PR
 4. Please use a semantic commits format title, such as `<type>[optional scope]: <description>`, see: https://go-kratos.dev/docs/community/contribution#type
 5. at the same time, please note that similar work should be submitted in one PR as far as possible to reduce the workload of reviewers. Do not split a work into multiple PR unless it should.
 -->
 
 <!--
-🎉 感谢您向 Kratos 发送 PR 请求！以下是一些提示：
+🎉 感谢您向 Kratos 发送 PR！以下是一些提示：
 如果这是你第一次为 Kratos 贡献，请阅读我们的贡献指南：https://go-kratos.dev/en/docs/community/contribution/
 2、确保您已经为您的 PR 添加或运行了适当的测试和lint，请在提交PR之前使用“make lint”和“make test”，使用“make clean”整理您的 go.mod。
-3、如果请购单未完成，您可能需要将其标记为 WIP（在制品）PR 或 draft PR
+3、如果 PR 未完成，您可能需要将其标记为 WIP（Work In Progress）PR 或 Draft PR
 4、请使用语义提交格式标题，如“<类型>[可选范围]：<说明>`，请参阅：https://go-kratos.dev/docs/community/contribution#type
 5. 同时请注意，同类的工作请尽量在一个PR中提交，以减轻 review 者的工作负担，不要把一项工作拆分成很多个PR，除非它应该这样做。
 -->
 
-<!--
--->
 
 #### Description (what this PR does / why we need it):
 <!--
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
index 3a6bcb31..9d94238a 100644
--- a/.github/workflows/go.yml
+++ b/.github/workflows/go.yml
@@ -13,7 +13,7 @@ jobs:
   build:
     strategy:
       matrix:
-        go: [1.18,1.19]
+        go: [1.18,1.19,1.20.x]
     name: build & test
     runs-on: ubuntu-latest
     services:
@@ -82,4 +82,4 @@ jobs:
         run: |
           cd cmd/protoc-gen-go-http
           go build ./...
-          go test ./...
\ No newline at end of file
+          go test ./...
diff --git a/README.md b/README.md
index 815be2fc..6f768c4d 100644
--- a/README.md
+++ b/README.md
@@ -110,5 +110,5 @@ The following project had particular influence on kratos's design.
 - [go-kit/kit](https://github.com/go-kit/kit) is a programming toolkit for building microservices in go.
 - [asim/go-micro](https://github.com/asim/go-micro) a distributed systems development framework.
 - [google/go-cloud](https://github.com/google/go-cloud) is go cloud development kit.
-- [tal-tech/go-zero](https://github.com/tal-tech/go-zero) is a web and rpc framework with lots of builtin engineering practices.
+- [zeromicro/go-zero](https://github.com/zeromicro/go-zero) is a web and rpc framework with lots of builtin engineering practices.
 - [beego/beego](https://github.com/beego/beego) is a web framework including RESTful APIs, web apps and backend services.
diff --git a/cmd/kratos/go.mod b/cmd/kratos/go.mod
index d8ed9677..446b540c 100644
--- a/cmd/kratos/go.mod
+++ b/cmd/kratos/go.mod
@@ -13,5 +13,5 @@ require (
 	golang.org/x/sys v0.0.0-20211205182925-97ca703d548d // indirect
 	golang.org/x/text v0.3.7
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
-	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
+	gopkg.in/yaml.v3 v3.0.0 // indirect
 )
diff --git a/cmd/kratos/go.sum b/cmd/kratos/go.sum
index 5ed58fe4..9a599980 100644
--- a/cmd/kratos/go.sum
+++ b/cmd/kratos/go.sum
@@ -73,3 +73,5 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo=
 gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+gopkg.in/yaml.v3 v3.0.0 h1:hjy8E9ON/egN1tAYqKb61G10WtihqetD4sz2H+8nIeA=
+gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
diff --git a/contrib/log/zap/zap.go b/contrib/log/zap/zap.go
index 511381f9..094e1e65 100644
--- a/contrib/log/zap/zap.go
+++ b/contrib/log/zap/zap.go
@@ -19,13 +19,14 @@ func NewLogger(zlog *zap.Logger) *Logger {
 }
 
 func (l *Logger) Log(level log.Level, keyvals ...interface{}) error {
-	if len(keyvals) == 0 || len(keyvals)%2 != 0 {
+	keylen := len(keyvals)
+	if keylen == 0 || keylen%2 != 0 {
 		l.log.Warn(fmt.Sprint("Keyvalues must appear in pairs: ", keyvals))
 		return nil
 	}
 
-	var data []zap.Field
-	for i := 0; i < len(keyvals); i += 2 {
+	var data []zap.Field = make([]zap.Field, 0, (keylen/2)+1)
+	for i := 0; i < keylen; i += 2 {
 		data = append(data, zap.Any(fmt.Sprint(keyvals[i]), keyvals[i+1]))
 	}
 
diff --git a/contrib/registry/consul/client.go b/contrib/registry/consul/client.go
index 31d0da68..ea5fe846 100644
--- a/contrib/registry/consul/client.go
+++ b/contrib/registry/consul/client.go
@@ -2,6 +2,7 @@ package consul
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math/rand"
 	"net"
@@ -204,21 +205,32 @@ func (c *Client) Register(_ context.Context, svc *registry.ServiceInstance, enab
 			defer ticker.Stop()
 			for {
 				select {
+				case <-c.ctx.Done():
+					_ = c.cli.Agent().ServiceDeregister(svc.ID)
+					return
+				default:
+				}
+				select {
+				case <-c.ctx.Done():
+					_ = c.cli.Agent().ServiceDeregister(svc.ID)
+					return
 				case <-ticker.C:
+					// ensure that unregistered services will not be re-registered by mistake
+					if errors.Is(c.ctx.Err(), context.Canceled) || errors.Is(c.ctx.Err(), context.DeadlineExceeded) {
+						_ = c.cli.Agent().ServiceDeregister(svc.ID)
+						return
+					}
 					err = c.cli.Agent().UpdateTTL("service:"+svc.ID, "pass", "pass")
 					if err != nil {
 						log.Errorf("[Consul] update ttl heartbeat to consul failed! err=%v", err)
 						// when the previous report fails, try to re register the service
-						time.AfterFunc(time.Duration(rand.Intn(5))*time.Second, func() {
-							if err := c.cli.Agent().ServiceRegister(asr); err != nil {
-								log.Errorf("[Consul] re registry service failed!, err=%v", err)
-							} else {
-								log.Warn("[Consul] re registry of service occurred success")
-							}
-						})
+						time.Sleep(time.Duration(rand.Intn(5)) * time.Second)
+						if err := c.cli.Agent().ServiceRegister(asr); err != nil {
+							log.Errorf("[Consul] re registry service failed!, err=%v", err)
+						} else {
+							log.Warn("[Consul] re registry of service occurred success")
+						}
 					}
-				case <-c.ctx.Done():
-					return
 				}
 			}
 		}()
@@ -228,6 +240,6 @@ func (c *Client) Register(_ context.Context, svc *registry.ServiceInstance, enab
 
 // Deregister service by service ID
 func (c *Client) Deregister(_ context.Context, serviceID string) error {
-	c.cancel()
+	defer c.cancel()
 	return c.cli.Agent().ServiceDeregister(serviceID)
 }
diff --git a/middleware/recovery/recovery.go b/middleware/recovery/recovery.go
index dfa7cb7f..66dcec18 100644
--- a/middleware/recovery/recovery.go
+++ b/middleware/recovery/recovery.go
@@ -29,12 +29,6 @@ func WithHandler(h HandlerFunc) Option {
 	}
 }
 
-// WithLogger with recovery logger.
-// Deprecated: use global logger instead.
-func WithLogger(logger log.Logger) Option {
-	return func(o *options) {}
-}
-
 // Recovery is a server middleware that recovers from any panics.
 func Recovery(opts ...Option) middleware.Middleware {
 	op := options{
diff --git a/transport/grpc/resolver/discovery/builder.go b/transport/grpc/resolver/discovery/builder.go
index 015c20f1..6186cdf1 100644
--- a/transport/grpc/resolver/discovery/builder.go
+++ b/transport/grpc/resolver/discovery/builder.go
@@ -31,7 +31,7 @@ func WithInsecure(insecure bool) Option {
 	}
 }
 
-// WithInsecure with isSecure option.
+// WithSubset with subset size.
 func WithSubset(size int) Option {
 	return func(b *builder) {
 		b.subsetSize = size


Test patch:
diff --git a/contrib/polaris/registry_test.go b/contrib/polaris/registry_test.go
index 7295de53..a03f807e 100644
--- a/contrib/polaris/registry_test.go
+++ b/contrib/polaris/registry_test.go
@@ -29,8 +29,8 @@ func TestRegistry(t *testing.T) {
 	)
 
 	ins := &registry.ServiceInstance{
-		ID:      "test-ut-1",
-		Name:    "test-ut-1",
+		ID:      "test-ut",
+		Name:    "test-ut",
 		Version: "v1.0.0",
 		Endpoints: []string{
 			"grpc://127.0.0.1:8080",
@@ -50,115 +50,9 @@ func TestRegistry(t *testing.T) {
 		t.Fatal(err)
 	}
 	time.Sleep(time.Second * 3)
-	service, err := r.GetService(context.Background(), "test-ut-1")
+	service, err := r.GetService(context.Background(), "test-ut")
 	if err != nil {
 		t.Fatal(err)
 	}
 	t.Log(service)
 }
-
-func TestWatch(t *testing.T) {
-	sdk, err := polaris.NewSDKContextByAddress("127.0.0.1:8091")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	p := New(sdk)
-
-	r := p.Registry(
-		WithRegistryTimeout(time.Second),
-		WithRegistryHealthy(true),
-		WithRegistryIsolate(false),
-		WithRegistryRetryCount(3),
-		WithRegistryWeight(100),
-		WithRegistryTTL(1000),
-	)
-
-	err = r.Register(context.Background(), &registry.ServiceInstance{
-		ID:      "test-ut",
-		Name:    "test-ut",
-		Version: "v1.0.0",
-		Endpoints: []string{
-			"grpc://127.0.0.1:8080",
-			"http://127.0.0.1:9090",
-		},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	time.Sleep(time.Second * 3)
-	w, err := r.Watch(context.Background(), "test-ut")
-	if err != nil {
-		t.Fatal(err)
-	}
-	service, err := w.Next()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if len(service) != 1 {
-		t.Fatalf("want 1, got %d, service %+v", len(service), service)
-	}
-
-	err = r.Register(context.Background(), &registry.ServiceInstance{
-		ID:      "test-ut2",
-		Name:    "test-ut",
-		Version: "v1.0.0",
-		Endpoints: []string{
-			"grpc://127.0.0.2:8080",
-			"http://127.0.0.2:9090",
-		},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	time.Sleep(time.Second * 3)
-	service, err = w.Next()
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(service) != 2 {
-		t.Fatalf("want 1, got %d, service %+v", len(service), service)
-	}
-
-	err = r.Deregister(context.Background(), &registry.ServiceInstance{
-		ID:      "test-ut",
-		Name:    "test-ut",
-		Version: "v1.0.0",
-		Endpoints: []string{
-			"grpc://127.0.0.1:8080",
-			"http://127.0.0.1:9090",
-		},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	time.Sleep(time.Second * 3)
-	service, err = w.Next()
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(service) != 1 {
-		t.Fatalf("want 1, got %d, service %+v", len(service), service)
-	}
-	err = r.Deregister(context.Background(), &registry.ServiceInstance{
-		ID:      "test-ut",
-		Name:    "test-ut",
-		Version: "v1.0.0",
-		Endpoints: []string{
-			"grpc://127.0.0.2:8080",
-			"http://127.0.0.2:9090",
-		},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	time.Sleep(time.Second * 3)
-	service, err = w.Next()
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(service) != 0 {
-		t.Fatalf("want 0, got %d", len(service))
-	}
-}
diff --git a/contrib/registry/consul/registry_test.go b/contrib/registry/consul/registry_test.go
index a6025123..791c2efc 100644
--- a/contrib/registry/consul/registry_test.go
+++ b/contrib/registry/consul/registry_test.go
@@ -74,14 +74,14 @@ func TestRegistry_Register(t *testing.T) {
 				serverName: "server-1",
 				server: []*registry.ServiceInstance{
 					{
-						ID:        "1",
+						ID:        "2",
 						Name:      "server-1",
 						Version:   "v0.0.1",
 						Metadata:  nil,
 						Endpoints: []string{"http://127.0.0.1:8000"},
 					},
 					{
-						ID:        "1",
+						ID:        "2",
 						Name:      "server-1",
 						Version:   "v0.0.2",
 						Metadata:  nil,
@@ -91,7 +91,7 @@ func TestRegistry_Register(t *testing.T) {
 			},
 			want: []*registry.ServiceInstance{
 				{
-					ID:        "1",
+					ID:        "2",
 					Name:      "server-1",
 					Version:   "v0.0.2",
 					Metadata:  nil,
@@ -168,6 +168,13 @@ func TestRegistry_GetService(t *testing.T) {
 		Endpoints: []string{fmt.Sprintf("tcp://%s?isSecure=false", addr)},
 	}
 
+	instance2 := &registry.ServiceInstance{
+		ID:        "2",
+		Name:      "server-1",
+		Version:   "v0.0.1",
+		Endpoints: []string{fmt.Sprintf("tcp://%s?isSecure=false", addr)},
+	}
+
 	type fields struct {
 		registry *Registry
 	}
@@ -223,10 +230,10 @@ func TestRegistry_GetService(t *testing.T) {
 			want:    nil,
 			wantErr: true,
 			preFunc: func(t *testing.T) {
-				if err := r.Register(context.Background(), instance1); err != nil {
+				if err := r.Register(context.Background(), instance2); err != nil {
 					t.Error(err)
 				}
-				watch, err := r.Watch(context.Background(), instance1.Name)
+				watch, err := r.Watch(context.Background(), instance2.Name)
 				if err != nil {
 					t.Error(err)
 				}
@@ -236,7 +243,7 @@ func TestRegistry_GetService(t *testing.T) {
 				}
 			},
 			deferFunc: func(t *testing.T) {
-				err := r.Deregister(context.Background(), instance1)
+				err := r.Deregister(context.Background(), instance2)
 				if err != nil {
 					t.Error(err)
 				}
@@ -282,6 +289,20 @@ func TestRegistry_Watch(t *testing.T) {
 		Endpoints: []string{fmt.Sprintf("tcp://%s?isSecure=false", addr)},
 	}
 
+	instance2 := &registry.ServiceInstance{
+		ID:        "2",
+		Name:      "server-1",
+		Version:   "v0.0.1",
+		Endpoints: []string{fmt.Sprintf("tcp://%s?isSecure=false", addr)},
+	}
+
+	instance3 := &registry.ServiceInstance{
+		ID:        "3",
+		Name:      "server-1",
+		Version:   "v0.0.1",
+		Endpoints: []string{fmt.Sprintf("tcp://%s?isSecure=false", addr)},
+	}
+
 	type args struct {
 		ctx      context.Context
 		cancel   func()
@@ -316,7 +337,7 @@ func TestRegistry_Watch(t *testing.T) {
 			args: args{
 				ctx:      canceledCtx,
 				cancel:   cancel,
-				instance: instance1,
+				instance: instance2,
 				opts: []Option{
 					WithHealthCheck(false),
 				},
@@ -330,14 +351,14 @@ func TestRegistry_Watch(t *testing.T) {
 			name: "register with healthCheck",
 			args: args{
 				ctx:      context.Background(),
-				instance: instance1,
+				instance: instance3,
 				opts: []Option{
 					WithHeartbeat(true),
 					WithHealthCheck(true),
 					WithHealthCheckInterval(5),
 				},
 			},
-			want:    []*registry.ServiceInstance{instance1},
+			want:    []*registry.ServiceInstance{instance3},
 			wantErr: false,
 			preFunc: func(t *testing.T) {
 				lis, err := net.Listen("tcp", addr)
diff --git a/middleware/recovery/recovery_test.go b/middleware/recovery/recovery_test.go
index 985cb19b..cf4a7fbb 100644
--- a/middleware/recovery/recovery_test.go
+++ b/middleware/recovery/recovery_test.go
@@ -6,7 +6,6 @@ import (
 	"testing"
 
 	"github.com/go-kratos/kratos/v2/errors"
-	"github.com/go-kratos/kratos/v2/log"
 )
 
 func TestOnce(t *testing.T) {
@@ -35,8 +34,3 @@ func TestNotPanic(t *testing.T) {
 		t.Errorf("e isn't nil")
 	}
 }
-
-// Deprecated: Remove this test with WithLogger method.
-func TestWithLogger(t *testing.T) {
-	_ = WithLogger(log.DefaultLogger)
-}