Allow remote endpoint as functional option instead of only the servic…

…e name
openzipkin · Aug 12, 2020 · a4d4e71 · a4d4e71
1 parent b00ba74
commit a4d4e71
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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 {