From b00ba7449e453d0f8db141597272a08b1f915d25 Mon Sep 17 00:00:00 2001 From: dengliming Date: Tue, 11 Aug 2020 23:48:48 +0800 Subject: [PATCH 1/2] Add support for `RemoteEndpoint` in transport/client. --- middleware/http/client.go | 21 ++++++++++++++++----- middleware/http/client_test.go | 7 +++++++ middleware/http/transport.go | 12 +++++++++++- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/middleware/http/client.go b/middleware/http/client.go index 10087de..26b773b 100644 --- a/middleware/http/client.go +++ b/middleware/http/client.go @@ -30,10 +30,11 @@ var ErrValidTracerRequired = errors.New("valid tracer required") // Client holds a Zipkin instrumented HTTP Client. type Client struct { *http.Client - tracer *zipkin.Tracer - httpTrace bool - defaultTags map[string]string - transportOptions []TransportOption + tracer *zipkin.Tracer + httpTrace bool + defaultTags map[string]string + transportOptions []TransportOption + remoteServiceName string } // ClientOption allows optional configuration of Client. @@ -71,6 +72,14 @@ func TransportOptions(options ...TransportOption) ClientOption { } } +// WithRemoteServiceName will set the value for the remote endpoint's service name on +// all spans. +func WithRemoteServiceName(name string) ClientOption { + return func(c *Client) { + c.remoteServiceName = name + } +} + // NewClient returns an HTTP Client adding Zipkin instrumentation around an // embedded standard Go http.Client. func NewClient(tracer *zipkin.Tracer, options ...ClientOption) (*Client, error) { @@ -88,6 +97,7 @@ func NewClient(tracer *zipkin.Tracer, options ...ClientOption) (*Client, error) // the following Client settings override provided transport settings. RoundTripper(c.Client.Transport), TransportTrace(c.httpTrace), + TransportRemoteServiceName(c.remoteServiceName), ) transport, err := NewTransport(tracer, c.transportOptions...) if err != nil { @@ -106,7 +116,8 @@ func (c *Client) DoWithAppSpan(req *http.Request, name string) (res *http.Respon parentContext = span.Context() } - appSpan := c.tracer.StartSpan(name, zipkin.Parent(parentContext)) + ep, _ := zipkin.NewEndpoint(c.remoteServiceName, req.Host) + appSpan := c.tracer.StartSpan(name, zipkin.Parent(parentContext), zipkin.RemoteEndpoint(ep)) zipkin.TagHTTPMethod.Set(appSpan, req.Method) zipkin.TagHTTPPath.Set(appSpan, req.URL.Path) diff --git a/middleware/http/client_test.go b/middleware/http/client_test.go index 4a0268d..d051e09 100644 --- a/middleware/http/client_test.go +++ b/middleware/http/client_test.go @@ -41,12 +41,14 @@ func TestHTTPClient(t *testing.T) { "conf.timeout": "default", } + remoteServiceName := "google-service" client, err := httpclient.NewClient( tracer, httpclient.WithClient(&http.Client{}), httpclient.ClientTrace(true), httpclient.ClientTags(clientTags), httpclient.TransportOptions(httpclient.TransportTags(transportTags)), + httpclient.WithRemoteServiceName(remoteServiceName), ) if err != nil { t.Fatalf("unable to create http client: %+v", err) @@ -65,6 +67,11 @@ func TestHTTPClient(t *testing.T) { t.Errorf("Span Count want 2+, have %d", len(spans)) } + remoteEndpoint := spans[0].RemoteEndpoint + if remoteEndpoint == nil || remoteEndpoint.ServiceName != remoteServiceName { + t.Errorf("Span remoteEndpoint ServiceName want %s, have %s", remoteServiceName, remoteEndpoint.ServiceName) + } + req, _ = http.NewRequest("GET", "https://www.google.com", nil) res, err = client.Do(req) diff --git a/middleware/http/transport.go b/middleware/http/transport.go index 981013c..6dcbe33 100644 --- a/middleware/http/transport.go +++ b/middleware/http/transport.go @@ -58,6 +58,7 @@ type transport struct { errResponseReader *ErrResponseReader logger *log.Logger requestSampler RequestSamplerFunc + remoteServiceName string } // TransportOption allows one to configure optional transport configuration. @@ -100,6 +101,14 @@ func TransportErrResponseReader(r ErrResponseReader) TransportOption { } } +// TransportRemoteServiceName will set the value for the remote endpoint's service name on +// all spans. +func TransportRemoteServiceName(name string) TransportOption { + return func(c *transport) { + c.remoteServiceName = name + } +} + // TransportLogger allows to plug a logger into the transport func TransportLogger(l *log.Logger) TransportOption { return func(t *transport) { @@ -142,8 +151,9 @@ func NewTransport(tracer *zipkin.Tracer, options ...TransportOption) (http.Round // RoundTrip satisfies the RoundTripper interface. func (t *transport) RoundTrip(req *http.Request) (res *http.Response, err error) { + ep, _ := zipkin.NewEndpoint(t.remoteServiceName, req.Host) sp, _ := t.tracer.StartSpanFromContext( - req.Context(), req.URL.Scheme+"/"+req.Method, zipkin.Kind(model.Client), + req.Context(), req.URL.Scheme+"/"+req.Method, zipkin.Kind(model.Client), zipkin.RemoteEndpoint(ep), ) for k, v := range t.defaultTags { From a4d4e7132a3d9057f6b1b38d99872d957b27e8f8 Mon Sep 17 00:00:00 2001 From: dengliming Date: Wed, 12 Aug 2020 22:42:20 +0800 Subject: [PATCH 2/2] Allow remote endpoint as functional option instead of only the service name --- middleware/http/client.go | 22 ++++++++++------------ middleware/http/client_test.go | 13 ++++++++----- middleware/http/transport.go | 12 +++++------- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/middleware/http/client.go b/middleware/http/client.go index 26b773b..2a8b83e 100644 --- a/middleware/http/client.go +++ b/middleware/http/client.go @@ -30,11 +30,11 @@ var ErrValidTracerRequired = errors.New("valid tracer required") // Client holds a Zipkin instrumented HTTP Client. type Client struct { *http.Client - tracer *zipkin.Tracer - httpTrace bool - defaultTags map[string]string - transportOptions []TransportOption - remoteServiceName string + tracer *zipkin.Tracer + httpTrace bool + defaultTags map[string]string + transportOptions []TransportOption + remoteEndpoint *model.Endpoint } // ClientOption allows optional configuration of Client. @@ -72,11 +72,10 @@ func TransportOptions(options ...TransportOption) ClientOption { } } -// WithRemoteServiceName will set the value for the remote endpoint's service name on -// all spans. -func WithRemoteServiceName(name string) ClientOption { +// WithRemoteEndpoint will set the remote endpoint for all spans. +func WithRemoteEndpoint(remoteEndpoint *model.Endpoint) ClientOption { return func(c *Client) { - c.remoteServiceName = name + c.remoteEndpoint = remoteEndpoint } } @@ -97,7 +96,7 @@ func NewClient(tracer *zipkin.Tracer, options ...ClientOption) (*Client, error) // the following Client settings override provided transport settings. RoundTripper(c.Client.Transport), TransportTrace(c.httpTrace), - TransportRemoteServiceName(c.remoteServiceName), + TransportRemoteEndpoint(c.remoteEndpoint), ) transport, err := NewTransport(tracer, c.transportOptions...) if err != nil { @@ -116,8 +115,7 @@ func (c *Client) DoWithAppSpan(req *http.Request, name string) (res *http.Respon parentContext = span.Context() } - ep, _ := zipkin.NewEndpoint(c.remoteServiceName, req.Host) - appSpan := c.tracer.StartSpan(name, zipkin.Parent(parentContext), zipkin.RemoteEndpoint(ep)) + appSpan := c.tracer.StartSpan(name, zipkin.Parent(parentContext), zipkin.RemoteEndpoint(c.remoteEndpoint)) zipkin.TagHTTPMethod.Set(appSpan, req.Method) zipkin.TagHTTPPath.Set(appSpan, req.URL.Path) diff --git a/middleware/http/client_test.go b/middleware/http/client_test.go index d051e09..4dcb0a1 100644 --- a/middleware/http/client_test.go +++ b/middleware/http/client_test.go @@ -41,14 +41,14 @@ func TestHTTPClient(t *testing.T) { "conf.timeout": "default", } - remoteServiceName := "google-service" + remoteEndpoint, _ := zipkin.NewEndpoint("google-service", "1.2.3.4:80") client, err := httpclient.NewClient( tracer, httpclient.WithClient(&http.Client{}), httpclient.ClientTrace(true), httpclient.ClientTags(clientTags), httpclient.TransportOptions(httpclient.TransportTags(transportTags)), - httpclient.WithRemoteServiceName(remoteServiceName), + httpclient.WithRemoteEndpoint(remoteEndpoint), ) if err != nil { t.Fatalf("unable to create http client: %+v", err) @@ -67,9 +67,12 @@ func TestHTTPClient(t *testing.T) { t.Errorf("Span Count want 2+, have %d", len(spans)) } - remoteEndpoint := spans[0].RemoteEndpoint - if remoteEndpoint == nil || remoteEndpoint.ServiceName != remoteServiceName { - t.Errorf("Span remoteEndpoint ServiceName want %s, have %s", remoteServiceName, remoteEndpoint.ServiceName) + rep := spans[0].RemoteEndpoint + if rep == nil { + t.Errorf("Span remoteEndpoint must not nil") + } + if rep.ServiceName != remoteEndpoint.ServiceName { + t.Errorf("Span remoteEndpoint ServiceName want %s, have %s", remoteEndpoint.ServiceName, rep.ServiceName) } req, _ = http.NewRequest("GET", "https://www.google.com", nil) diff --git a/middleware/http/transport.go b/middleware/http/transport.go index 6dcbe33..7facdb5 100644 --- a/middleware/http/transport.go +++ b/middleware/http/transport.go @@ -58,7 +58,7 @@ type transport struct { errResponseReader *ErrResponseReader logger *log.Logger requestSampler RequestSamplerFunc - remoteServiceName string + remoteEndpoint *model.Endpoint } // TransportOption allows one to configure optional transport configuration. @@ -101,11 +101,10 @@ func TransportErrResponseReader(r ErrResponseReader) TransportOption { } } -// TransportRemoteServiceName will set the value for the remote endpoint's service name on -// all spans. -func TransportRemoteServiceName(name string) TransportOption { +// TransportRemoteEndpoint will set the remote endpoint for all spans. +func TransportRemoteEndpoint(remoteEndpoint *model.Endpoint) TransportOption { return func(c *transport) { - c.remoteServiceName = name + c.remoteEndpoint = remoteEndpoint } } @@ -151,9 +150,8 @@ func NewTransport(tracer *zipkin.Tracer, options ...TransportOption) (http.Round // RoundTrip satisfies the RoundTripper interface. func (t *transport) RoundTrip(req *http.Request) (res *http.Response, err error) { - ep, _ := zipkin.NewEndpoint(t.remoteServiceName, req.Host) sp, _ := t.tracer.StartSpanFromContext( - req.Context(), req.URL.Scheme+"/"+req.Method, zipkin.Kind(model.Client), zipkin.RemoteEndpoint(ep), + req.Context(), req.URL.Scheme+"/"+req.Method, zipkin.Kind(model.Client), zipkin.RemoteEndpoint(t.remoteEndpoint), ) for k, v := range t.defaultTags {