Repository: go-kratos/kratos
Pull request #1484 (2021-09-15 13:44:48)

test(middleware/tracing): tracing unit test enhancement
<!--
🎉 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
-->
unit test enhanced

#### Other special notes for reviewer:
<!--
* Somethings that need extra attention for reviewer
* Some additional notes, TODO list, etc.
-->
* update go version to 1.17
* use t.Setenv instead of os.Setenv
* enhance tracing test

codecov-commenter: # [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1484?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
> Merging [#1484](https://codecov.io/gh/go-kratos/kratos/pull/1484?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (4da4bc0) into [main](https://codecov.io/gh/go-kratos/kratos/commit/24ec23f0e41676bfe9726ce612005ac5ab85d67c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (24ec23f) will **increase** coverage by `2.79%`.
> The diff coverage is `0.00%`.

> :exclamation: Current head 4da4bc0 differs from pull request most recent head 1000f26. Consider uploading reports for the commit 1000f26 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/go-kratos/kratos/pull/1484/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/1484?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    #1484      +/-   ##
==========================================
+ Coverage   66.65%   69.45%   +2.79%     
==========================================
  Files          66       66              
  Lines        3038     3038              
==========================================
+ Hits         2025     2110      +85     
+ Misses        841      749      -92     
- Partials      172      179       +7     
```


| [Impacted Files](https://codecov.io/gh/go-kratos/kratos/pull/1484?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) | Coverage Δ | |
|---|---|---|
| [internal/host/host.go](https://codecov.io/gh/go-kratos/kratos/pull/1484/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-aW50ZXJuYWwvaG9zdC9ob3N0Lmdv) | `65.21% <0.00%> (ø)` | |
| [internal/context/context.go](https://codecov.io/gh/go-kratos/kratos/pull/1484/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-aW50ZXJuYWwvY29udGV4dC9jb250ZXh0Lmdv) | `100.00% <0.00%> (+2.98%)` | :arrow_up: |
| [middleware/tracing/span.go](https://codecov.io/gh/go-kratos/kratos/pull/1484/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-bWlkZGxld2FyZS90cmFjaW5nL3NwYW4uZ28=) | `61.62% <0.00%> (+61.62%)` | :arrow_up: |
| [middleware/tracing/tracing.go](https://codecov.io/gh/go-kratos/kratos/pull/1484/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-bWlkZGxld2FyZS90cmFjaW5nL3RyYWNpbmcuZ28=) | `100.00% <0.00%> (+83.33%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1484?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/1484?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Last update [24ec23f...1000f26](https://codecov.io/gh/go-kratos/kratos/pull/1484?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/.github/workflows/go.yml b/.github/workflows/go.yml
index 0e5e2684..ae4f7da6 100644
--- a/.github/workflows/go.yml
+++ b/.github/workflows/go.yml
@@ -32,7 +32,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@v2
         with:
-          go-version: 1.16
+          go-version: 1.17
 
       - name: Build
         run: go build ./...
diff --git a/transport/grpc/server.go b/transport/grpc/server.go
index 748302c2..31efebbf 100644
--- a/transport/grpc/server.go
+++ b/transport/grpc/server.go
@@ -89,6 +89,13 @@ func Options(opts ...grpc.ServerOption) ServerOption {
 	}
 }
 
+// Endpoint with server endpoint.
+func Endpoint(endpoint *url.URL) ServerOption {
+	return func(o *Server) {
+		o.endpoint = endpoint
+	}
+}
+
 // Server is a gRPC server wrapper.
 type Server struct {
 	*grpc.Server
@@ -151,6 +158,9 @@ func NewServer(opts ...ServerOption) *Server {
 //   grpc://127.0.0.1:9000?isSecure=false
 func (s *Server) Endpoint() (*url.URL, error) {
 	s.once.Do(func() {
+		if s.endpoint != nil {
+			return
+		}
 		lis, err := net.Listen(s.network, s.address)
 		if err != nil {
 			s.err = err
diff --git a/transport/http/calloption.go b/transport/http/calloption.go
index f6851895..7b0ed82d 100644
--- a/transport/http/calloption.go
+++ b/transport/http/calloption.go
@@ -2,8 +2,6 @@ package http
 
 import (
 	"net/http"
-
-	"github.com/go-kratos/kratos/v2/selector"
 )
 
 // CallOption configures a Call before it starts or extracts information from
@@ -22,7 +20,6 @@ type callInfo struct {
 	contentType  string
 	operation    string
 	pathTemplate string
-	filters      []selector.Filter
 }
 
 // EmptyCallOption does not alter the Call configuration.
@@ -93,22 +90,6 @@ func (o PathTemplateCallOption) before(c *callInfo) error {
 	return nil
 }
 
-// SelectFilter is http select filter
-func SelectFilter(filters ...selector.Filter) CallOption {
-	return SelectFilterCallOption{filters: filters}
-}
-
-// SelectFilterCallOption is set call select filters
-type SelectFilterCallOption struct {
-	EmptyCallOption
-	filters []selector.Filter
-}
-
-func (o SelectFilterCallOption) before(c *callInfo) error {
-	c.filters = o.filters
-	return nil
-}
-
 // Header returns a CallOptions that retrieves the http response header
 // from server reply.
 func Header(header *http.Header) CallOption {
diff --git a/transport/http/client.go b/transport/http/client.go
index 14dc9a3b..02542498 100644
--- a/transport/http/client.go
+++ b/transport/http/client.go
@@ -238,7 +238,7 @@ func (client *Client) invoke(ctx context.Context, req *http.Request, args interf
 				err  error
 				node selector.Node
 			)
-			if node, done, err = client.opts.selector.Select(ctx, selector.WithFilter(c.filters...)); err != nil {
+			if node, done, err = client.opts.selector.Select(ctx); err != nil {
 				return nil, errors.ServiceUnavailable("NODE_NOT_FOUND", err.Error())
 			}
 			if client.insecure {


Test patch:
diff --git a/config/env/env_test.go b/config/env/env_test.go
index 0ddb7c93..6ccf76d7 100644
--- a/config/env/env_test.go
+++ b/config/env/env_test.go
@@ -55,9 +55,7 @@ func TestEnvWithPrefix(t *testing.T) {
 	}
 
 	for k, v := range envs {
-		if err := os.Setenv(k, v); err != nil {
-			t.Fatal(err)
-		}
+		t.Setenv(k, v)
 	}
 
 	c := config.New(config.WithSource(
@@ -163,9 +161,7 @@ func TestEnvWithoutPrefix(t *testing.T) {
 	}
 
 	for k, v := range envs {
-		if err := os.Setenv(k, v); err != nil {
-			t.Fatal(err)
-		}
+		t.Setenv(k, v)
 	}
 
 	c := config.New(config.WithSource(
diff --git a/middleware/tracing/span_test.go b/middleware/tracing/span_test.go
new file mode 100644
index 00000000..c930154f
--- /dev/null
+++ b/middleware/tracing/span_test.go
@@ -0,0 +1,158 @@
+package tracing
+
+import (
+	"reflect"
+	"testing"
+
+	"go.opentelemetry.io/otel/attribute"
+	semconv "go.opentelemetry.io/otel/semconv/v1.4.0"
+)
+
+func Test_parseFullMethod(t *testing.T) {
+	tests := []struct {
+		name       string
+		fullMethod string
+		want       string
+		wantAttr   []attribute.KeyValue
+	}{
+		{
+			name:       "/foo.bar/hello",
+			fullMethod: "/foo.bar/hello",
+			want:       "foo.bar/hello",
+			wantAttr: []attribute.KeyValue{
+				semconv.RPCServiceKey.String("foo.bar"),
+				semconv.RPCMethodKey.String("hello"),
+			},
+		},
+		{
+			name:       "/foo.bar/hello/world",
+			fullMethod: "/foo.bar/hello/world",
+			want:       "foo.bar/hello/world",
+			wantAttr: []attribute.KeyValue{
+				semconv.RPCServiceKey.String("foo.bar"),
+				semconv.RPCMethodKey.String("hello/world"),
+			},
+		},
+		{
+			name:       "/hello",
+			fullMethod: "/hello",
+			want:       "hello",
+			wantAttr:   []attribute.KeyValue{attribute.Key("rpc.operation").String("/hello")},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, got1 := parseFullMethod(tt.fullMethod)
+			if got != tt.want {
+				t.Errorf("parseFullMethod() got = %v, want %v", got, tt.want)
+			}
+			if !reflect.DeepEqual(got1, tt.wantAttr) {
+				t.Errorf("parseFullMethod() got1 = %v, want %v", got1, tt.wantAttr)
+			}
+		})
+	}
+}
+
+func Test_peerAttr(t *testing.T) {
+	tests := []struct {
+		name string
+		addr string
+		want []attribute.KeyValue
+	}{
+		{
+			name: "nil addr",
+			addr: ":8080",
+			want: []attribute.KeyValue{
+				semconv.NetPeerIPKey.String("127.0.0.1"),
+				semconv.NetPeerPortKey.String("8080"),
+			},
+		},
+		{
+			name: "normal addr without port",
+			addr: "192.168.0.1",
+			want: []attribute.KeyValue(nil),
+		},
+		{
+			name: "normal addr with port",
+			addr: "192.168.0.1:8080",
+			want: []attribute.KeyValue{
+				semconv.NetPeerIPKey.String("192.168.0.1"),
+				semconv.NetPeerPortKey.String("8080"),
+			},
+		},
+		{
+			name: "dns addr",
+			addr: "foo:8080",
+			want: []attribute.KeyValue{
+				semconv.NetPeerIPKey.String("foo"),
+				semconv.NetPeerPortKey.String("8080"),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := peerAttr(tt.addr); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("peerAttr() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_parseTarget(t *testing.T) {
+	tests := []struct {
+		name        string
+		endpoint    string
+		wantAddress string
+		wantErr     bool
+	}{
+		{
+			name:        "http",
+			endpoint:    "http://foo.bar:8080",
+			wantAddress: "http://foo.bar:8080",
+			wantErr:     false,
+		},
+		{
+			name:        "http",
+			endpoint:    "http://127.0.0.1:8080",
+			wantAddress: "http://127.0.0.1:8080",
+			wantErr:     false,
+		},
+		{
+			name:        "without protocol",
+			endpoint:    "foo.bar:8080",
+			wantAddress: "foo.bar:8080",
+			wantErr:     false,
+		},
+		{
+			name:        "grpc",
+			endpoint:    "grpc://foo.bar",
+			wantAddress: "grpc://foo.bar",
+			wantErr:     false,
+		},
+		{
+			name:        "with path",
+			endpoint:    "/foo",
+			wantAddress: "foo",
+			wantErr:     false,
+		},
+		{
+			name:        "with path",
+			endpoint:    "http://127.0.0.1/hello",
+			wantAddress: "hello",
+			wantErr:     false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotAddress, err := parseTarget(tt.endpoint)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("parseTarget() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if gotAddress != tt.wantAddress {
+				t.Errorf("parseTarget() = %v, want %v", gotAddress, tt.wantAddress)
+			}
+		})
+	}
+}
diff --git a/middleware/tracing/tracing_test.go b/middleware/tracing/tracing_test.go
index 40ada4da..7593d1d3 100644
--- a/middleware/tracing/tracing_test.go
+++ b/middleware/tracing/tracing_test.go
@@ -3,15 +3,18 @@ package tracing
 import (
 	"context"
 	"net/http"
+	"os"
 	"testing"
 
+	"github.com/go-kratos/kratos/v2/log"
 	"github.com/go-kratos/kratos/v2/transport"
+	"github.com/stretchr/testify/assert"
 	"go.opentelemetry.io/otel/propagation"
 	tracesdk "go.opentelemetry.io/otel/sdk/trace"
 	"go.opentelemetry.io/otel/trace"
 )
 
-var _ transport.Transporter = &Transport{}
+var _ transport.Transporter = &mockTransport{}
 
 type headerCarrier http.Header
 
@@ -34,25 +37,25 @@ func (hc headerCarrier) Keys() []string {
 	return keys
 }
 
-type Transport struct {
+type mockTransport struct {
 	kind      transport.Kind
 	endpoint  string
 	operation string
 	header    headerCarrier
 }
 
-func (tr *Transport) Kind() transport.Kind            { return tr.kind }
-func (tr *Transport) Endpoint() string                { return tr.endpoint }
-func (tr *Transport) Operation() string               { return tr.operation }
-func (tr *Transport) RequestHeader() transport.Header { return tr.header }
-func (tr *Transport) ReplyHeader() transport.Header   { return tr.header }
+func (tr *mockTransport) Kind() transport.Kind            { return tr.kind }
+func (tr *mockTransport) Endpoint() string                { return tr.endpoint }
+func (tr *mockTransport) Operation() string               { return tr.operation }
+func (tr *mockTransport) RequestHeader() transport.Header { return tr.header }
+func (tr *mockTransport) ReplyHeader() transport.Header   { return tr.header }
 
-func TestTracing(t *testing.T) {
+func TestTracer(t *testing.T) {
 	carrier := headerCarrier{}
 	tp := tracesdk.NewTracerProvider(tracesdk.WithSampler(tracesdk.TraceIDRatioBased(0)))
 
 	// caller use Inject
-	tracer := NewTracer(
+	cliTracer := NewTracer(
 		trace.SpanKindClient,
 		WithTracerProvider(tp),
 		WithPropagator(
@@ -60,17 +63,17 @@ func TestTracing(t *testing.T) {
 		),
 	)
 
-	ts := &Transport{kind: transport.KindHTTP, header: carrier}
+	ts := &mockTransport{kind: transport.KindHTTP, header: carrier}
 
-	ctx, aboveSpan := tracer.Start(transport.NewClientContext(context.Background(), ts), ts.Operation(), ts.RequestHeader())
-	defer tracer.End(ctx, aboveSpan, nil, nil)
+	ctx, aboveSpan := cliTracer.Start(transport.NewClientContext(context.Background(), ts), ts.Operation(), ts.RequestHeader())
+	defer cliTracer.End(ctx, aboveSpan, nil, nil)
 
 	// server use Extract fetch traceInfo from carrier
-	tracer = NewTracer(trace.SpanKindServer, WithPropagator(propagation.NewCompositeTextMapPropagator(propagation.Baggage{}, propagation.TraceContext{})))
-	ts = &Transport{kind: transport.KindHTTP, header: carrier}
+	svrTracer := NewTracer(trace.SpanKindServer, WithPropagator(propagation.NewCompositeTextMapPropagator(propagation.Baggage{}, propagation.TraceContext{})))
+	ts = &mockTransport{kind: transport.KindHTTP, header: carrier}
 
-	ctx, span := tracer.Start(transport.NewServerContext(ctx, ts), ts.Operation(), ts.RequestHeader())
-	defer tracer.End(ctx, span, nil, nil)
+	ctx, span := svrTracer.Start(transport.NewServerContext(ctx, ts), ts.Operation(), ts.RequestHeader())
+	defer svrTracer.End(ctx, span, nil, nil)
 
 	if aboveSpan.SpanContext().TraceID() != span.SpanContext().TraceID() {
 		t.Fatalf("TraceID failed to deliver")
@@ -80,3 +83,110 @@ func TestTracing(t *testing.T) {
 		t.Fatalf("traceHeader failed to deliver")
 	}
 }
+
+func TestServer(t *testing.T) {
+	tr := &mockTransport{
+		kind:      transport.KindHTTP,
+		endpoint:  "server:2233",
+		operation: "/test.server/hello",
+		header:    headerCarrier{},
+	}
+
+	tracer := NewTracer(
+		trace.SpanKindClient,
+		WithTracerProvider(tracesdk.NewTracerProvider()),
+	)
+
+	logger := log.NewStdLogger(os.Stdout)
+	logger = log.With(logger, "span_id", SpanID())
+	logger = log.With(logger, "trace_id", TraceID())
+
+	var (
+		childSpanID  string
+		childTraceID string
+	)
+	next := func(ctx context.Context, req interface{}) (interface{}, error) {
+		_ = log.WithContext(ctx, logger).Log(log.LevelInfo,
+			"kind", "server",
+		)
+		childSpanID = SpanID()(ctx).(string)
+		childTraceID = TraceID()(ctx).(string)
+		return req.(string) + "https://go-kratos.dev", nil
+	}
+
+	var ctx context.Context
+	ctx, span := tracer.Start(
+		transport.NewServerContext(context.Background(), tr),
+		tr.Operation(),
+		tr.RequestHeader(),
+	)
+
+	_, err := Server(
+		WithTracerProvider(tracesdk.NewTracerProvider()),
+		WithPropagator(propagation.NewCompositeTextMapPropagator(propagation.Baggage{}, propagation.TraceContext{})),
+	)(next)(ctx, "test server: ")
+
+	span.End()
+	assert.NoError(t, err)
+	assert.NotEmpty(t, childSpanID)
+	assert.NotEqual(t, span.SpanContext().SpanID().String(), childSpanID)
+	assert.Equal(t, span.SpanContext().TraceID().String(), childTraceID)
+
+	_, err = Server(
+		WithTracerProvider(tracesdk.NewTracerProvider()),
+		WithPropagator(propagation.NewCompositeTextMapPropagator(propagation.Baggage{}, propagation.TraceContext{})),
+	)(next)(context.Background(), "test server: ")
+
+	assert.NoError(t, err)
+	assert.Empty(t, childSpanID)
+	assert.Empty(t, childTraceID)
+}
+
+func TestClient(t *testing.T) {
+	tr := &mockTransport{
+		kind:      transport.KindHTTP,
+		endpoint:  "server:2233",
+		operation: "/test.server/hello",
+		header:    headerCarrier{},
+	}
+
+	tracer := NewTracer(
+		trace.SpanKindClient,
+		WithTracerProvider(tracesdk.NewTracerProvider()),
+	)
+
+	logger := log.NewStdLogger(os.Stdout)
+	logger = log.With(logger, "span_id", SpanID())
+	logger = log.With(logger, "trace_id", TraceID())
+
+	var (
+		childSpanID  string
+		childTraceID string
+	)
+	next := func(ctx context.Context, req interface{}) (interface{}, error) {
+		_ = log.WithContext(ctx, logger).Log(log.LevelInfo,
+			"kind", "client",
+		)
+		childSpanID = SpanID()(ctx).(string)
+		childTraceID = TraceID()(ctx).(string)
+		return req.(string) + "https://go-kratos.dev", nil
+	}
+
+	var ctx context.Context
+	ctx, span := tracer.Start(
+		transport.NewClientContext(context.Background(), tr),
+		tr.Operation(),
+		tr.RequestHeader(),
+	)
+
+	_, err := Client(
+		WithTracerProvider(tracesdk.NewTracerProvider()),
+		WithPropagator(propagation.NewCompositeTextMapPropagator(propagation.Baggage{}, propagation.TraceContext{})),
+	)(next)(ctx, "test client: ")
+
+	span.End()
+	assert.NoError(t, err)
+	assert.NotEmpty(t, childSpanID)
+	assert.NotEqual(t, span.SpanContext().SpanID().String(), childSpanID)
+	assert.Equal(t, span.SpanContext().TraceID().String(), childTraceID)
+}
diff --git a/transport/grpc/server_test.go b/transport/grpc/server_test.go
index 3b9f1ede..10a3df25 100644
--- a/transport/grpc/server_test.go
+++ b/transport/grpc/server_test.go
@@ -82,6 +82,15 @@ func TestMiddleware(t *testing.T) {
 	assert.Equal(t, v, o.middleware)
 }
 
+func TestEndpoint(t *testing.T) {
+	u, err := url.Parse("grpc://hello/world")
+	assert.NoError(t, err)
+	o := &Server{}
+	Endpoint(u)(o)
+	assert.Equal(t, "hello", o.endpoint.Host)
+	assert.Equal(t, "grpc", o.endpoint.Scheme)
+}
+
 type mockLogger struct {
 	level log.Level
 	key   string