Repository: go-kratos/kratos
Pull request #1702 (2021-12-21 08:30:42)

feat(transport/grpc): gRPC client discovery supports incoming logger and timeout
…and timeout

<!--
🎉 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 mark it as a WIP(Work In Progress) PR or draft PR
4. Please use a conventional commits format title: `<type>[optional scope]: <description>`
    
    Some suggestion on <type>:

    fix: A bug fix
    feat: A new feature
    test: Adding missing tests or correcting existing tests
    refactor: A code change that neither fixes a bug nor adds a feature
    break: Changes has break change

    docs: Documentation only changes
    deps: Changes external dependencies
    style: Changes that do not affect the meaning of the code (white-space, formatting, etc)
    chore Daily work, examples, etc.
    ci: Changes to our CI configuration files and scripts
-->

#### 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 you PR is not fully resolved issue, please use `part of #<issue number>` instead.

Usage: `fixes/resolves #<issue number>`, or `fixes/resolves (paste link of issue)`.
-->
fixes #

#### Other special notes for reviewer:
<!--
* Somethings that need extra attention for reviewer
* Some additional notes, TODO list, etc.
-->


codecov-commenter: # [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
> Merging [#1702](https://codecov.io/gh/go-kratos/kratos/pull/1702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (f2f4aee) into [main](https://codecov.io/gh/go-kratos/kratos/commit/94d674fd4c2f5ff4c7230e2420a0f54438e35db5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (94d674f) will **decrease** coverage by `0.14%`.
> The diff coverage is `27.27%`.

> :exclamation: Current head f2f4aee differs from pull request most recent head ab2b001. Consider uploading reports for the commit ab2b001 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/go-kratos/kratos/pull/1702/graphs/tree.svg?width=650&height=150&src=pr&token=XwnmBTz78x&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)](https://codecov.io/gh/go-kratos/kratos/pull/1702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)

```diff
@@            Coverage Diff             @@
##             main    #1702      +/-   ##
==========================================
- Coverage   81.17%   81.02%   -0.15%     
==========================================
  Files          78       78              
  Lines        3431     3441      +10     
==========================================
+ Hits         2785     2788       +3     
- Misses        443      450       +7     
  Partials      203      203              
```


| [Impacted Files](https://codecov.io/gh/go-kratos/kratos/pull/1702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) | Coverage Δ | |
|---|---|---|
| [transport/grpc/client.go](https://codecov.io/gh/go-kratos/kratos/pull/1702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-dHJhbnNwb3J0L2dycGMvY2xpZW50Lmdv) | `85.22% <27.27%> (-7.09%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Last update [94d674f...ab2b001](https://codecov.io/gh/go-kratos/kratos/pull/1702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).


Patch:
diff --git a/contrib/registry/nacos/registry.go b/contrib/registry/nacos/registry.go
index 5a5d9744..baf9b473 100644
--- a/contrib/registry/nacos/registry.go
+++ b/contrib/registry/nacos/registry.go
@@ -81,7 +81,7 @@ func New(cli naming_client.INamingClient, opts ...Option) (r *Registry) {
 // Register the registration.
 func (r *Registry) Register(_ context.Context, si *registry.ServiceInstance) error {
 	if si.Name == "" {
-		return fmt.Errorf("kratos/nacos: serviceInstance.name cannot is empty")
+		return fmt.Errorf("kratos/nacos: serviceInstance.name can not be empty")
 	}
 	for _, endpoint := range si.Endpoints {
 		u, err := url.Parse(endpoint)
diff --git a/transport/grpc/client.go b/transport/grpc/client.go
index 9d7ede93..61ca85d1 100644
--- a/transport/grpc/client.go
+++ b/transport/grpc/client.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/go-kratos/kratos/v2/log"
 	"github.com/go-kratos/kratos/v2/middleware"
 	"github.com/go-kratos/kratos/v2/registry"
 	"github.com/go-kratos/kratos/v2/selector"
@@ -87,6 +88,13 @@ func WithFilter(filters ...selector.Filter) ClientOption {
 	}
 }
 
+// WithLogger with logger
+func WithLogger(log log.Logger) ClientOption {
+	return func(o *clientOptions) {
+		o.logger = log
+	}
+}
+
 // clientOptions is gRPC Client
 type clientOptions struct {
 	endpoint     string
@@ -98,6 +106,7 @@ type clientOptions struct {
 	grpcOpts     []grpc.DialOption
 	balancerName string
 	filters      []selector.Filter
+	logger       log.Logger
 }
 
 // Dial returns a GRPC connection.
@@ -114,6 +123,7 @@ func dial(ctx context.Context, insecure bool, opts ...ClientOption) (*grpc.Clien
 	options := clientOptions{
 		timeout:      2000 * time.Millisecond,
 		balancerName: wrr.Name,
+		logger:       log.DefaultLogger,
 	}
 	for _, o := range opts {
 		o(&options)
@@ -129,7 +139,14 @@ func dial(ctx context.Context, insecure bool, opts ...ClientOption) (*grpc.Clien
 		grpc.WithChainUnaryInterceptor(ints...),
 	}
 	if options.discovery != nil {
-		grpcOpts = append(grpcOpts, grpc.WithResolvers(discovery.NewBuilder(options.discovery, discovery.WithInsecure(insecure))))
+		grpcOpts = append(grpcOpts,
+			grpc.WithResolvers(
+				discovery.NewBuilder(
+					options.discovery,
+					discovery.WithInsecure(insecure),
+					discovery.WithLogger(options.logger),
+					discovery.WithTimeout(options.timeout),
+				)))
 	}
 	if insecure {
 		grpcOpts = append(grpcOpts, grpc.WithInsecure())


Test patch:
diff --git a/transport/grpc/client_test.go b/transport/grpc/client_test.go
index f618d9bf..3c510aff 100644
--- a/transport/grpc/client_test.go
+++ b/transport/grpc/client_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/go-kratos/kratos/v2/log"
 	"github.com/go-kratos/kratos/v2/middleware"
 	"github.com/go-kratos/kratos/v2/registry"
 	"github.com/stretchr/testify/assert"
@@ -59,6 +60,13 @@ func TestWithTLSConfig(t *testing.T) {
 	assert.Equal(t, v, o.tlsConf)
 }
 
+func TestWithLogger(t *testing.T) {
+	o := &clientOptions{}
+	v := log.DefaultLogger
+	WithLogger(v)(o)
+	assert.Equal(t, v, o.logger)
+}
+
 func EmptyMiddleware() middleware.Middleware {
 	return func(handler middleware.Handler) middleware.Handler {
 		return func(ctx context.Context, req interface{}) (reply interface{}, err error) {
@@ -112,3 +120,18 @@ func TestDial(t *testing.T) {
 	WithOptions(v...)(o)
 	assert.Equal(t, v, o.grpcOpts)
 }
+
+func TestDialConn(t *testing.T) {
+	_, err := dial(
+		context.Background(),
+		true,
+		WithDiscovery(&mockRegistry{}),
+		WithTimeout(10*time.Second),
+		WithLogger(log.DefaultLogger),
+		WithEndpoint("abc"),
+		WithMiddleware(EmptyMiddleware()),
+	)
+	if err != nil {
+		t.Error(err)
+	}
+}