Span names shouldn't be prefixed with Sent and Recv

Instead set them for some exporters that cannot differentiate client spans from server spans in their model. Fixes #607.
census-instrumentation · Mar 20, 2018 · 0788480 · 0788480
1 parent 7b6adb7
commit 0788480
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 56 deletions.
diff --git a/exporter/jaeger/jaeger.go b/exporter/jaeger/jaeger.go
@@ -158,7 +158,7 @@ func (e *Exporter) ExportSpan(data *trace.SpanData) {
 		TraceIdLow:    bytesToInt64(data.TraceID[8:16]),
 		SpanId:        bytesToInt64(data.SpanID[:]),
 		ParentSpanId:  bytesToInt64(data.ParentSpanID[:]),
-		OperationName: data.Name,
+		OperationName: name(data),
 		Flags:         int32(data.TraceOptions),
 		StartTime:     data.StartTime.UnixNano() / 1000,
 		Duration:      data.EndTime.Sub(data.StartTime).Nanoseconds() / 1000,
@@ -170,6 +170,17 @@ func (e *Exporter) ExportSpan(data *trace.SpanData) {
 	// TODO(jbd): Handle oversized bundlers.
 }
 
+func name(sd *trace.SpanData) string {
+	n := sd.Name
+	switch sd.SpanKind {
+	case trace.SpanKindClient:
+		n = "Sent." + n
+	case trace.SpanKindServer:
+		n = "Recv." + n
+	}
+	return n
+}
+
 func attributeToTag(key string, a interface{}) *gen.Tag {
 	var tag *gen.Tag
 	switch value := a.(type) {

diff --git a/exporter/stackdriver/trace_proto.go b/exporter/stackdriver/trace_proto.go
@@ -52,10 +52,18 @@ func protoFromSpanData(s *trace.SpanData, projectID string) *tracepb.Span {
 	traceIDString := s.SpanContext.TraceID.String()
 	spanIDString := s.SpanContext.SpanID.String()
 
+	name := s.Name
+	switch s.SpanKind {
+	case trace.SpanKindClient:
+		name = "Sent." + name
+	case trace.SpanKindServer:
+		name = "Recv." + name
+	}
+
 	sp := &tracepb.Span{
 		Name:                    "projects/" + projectID + "/traces/" + traceIDString + "/spans/" + spanIDString,
 		SpanId:                  spanIDString,
-		DisplayName:             trunc(s.Name, 128),
+		DisplayName:             trunc(name, 128),
 		StartTime:               timestampProto(s.StartTime),
 		EndTime:                 timestampProto(s.EndTime),
 		SameProcessAsParentSpan: &wrapperspb.BoolValue{Value: !s.HasRemoteParent},

diff --git a/exporter/zipkin/zipkin.go b/exporter/zipkin/zipkin.go
@@ -18,7 +18,6 @@ package zipkin // import "go.opencensus.io/exporter/zipkin"
 import (
 	"encoding/binary"
 	"strconv"
-	"strings"
 
 	"github.com/openzipkin/zipkin-go/model"
 	"github.com/openzipkin/zipkin-go/reporter"
@@ -102,23 +101,12 @@ func convertSpanID(s trace.SpanID) model.ID {
 }
 
 func spanKind(s *trace.SpanData) model.Kind {
-	if s.HasRemoteParent {
-		return model.Server
-	}
-	if strings.HasPrefix(s.Name, "Sent.") {
+	switch s.SpanKind {
+	case trace.SpanKindClient:
 		return model.Client
-	}
-	if strings.HasPrefix(s.Name, "Recv.") {
+	case trace.SpanKindServer:
 		return model.Server
 	}
-	if len(s.MessageEvents) > 0 {
-		switch s.MessageEvents[0].EventType {
-		case trace.MessageEventTypeSent:
-			return model.Client
-		case trace.MessageEventTypeRecv:
-			return model.Server
-		}
-	}
 	return model.Undetermined
 }
 

diff --git a/exporter/zipkin/zipkin_test.go b/exporter/zipkin/zipkin_test.go
@@ -50,6 +50,7 @@ func TestExport(t *testing.T) {
 					TraceOptions: 1,
 				},
 				Name:      "name",
+				SpanKind:  trace.SpanKindClient,
 				StartTime: now,
 				EndTime:   now.Add(24 * time.Hour),
 				Attributes: map[string]interface{}{

diff --git a/plugin/ochttp/server.go b/plugin/ochttp/server.go
@@ -79,7 +79,7 @@ func (h *Handler) startTrace(w http.ResponseWriter, r *http.Request) (*http.Requ
 		SpanKind: trace.SpanKindServer,
 	}
 
-	name := spanNameFromURL("Recv", r.URL)
+	name := spanNameFromURL(r.URL)
 	ctx := r.Context()
 	var span *trace.Span
 	sc, ok := h.extractSpanContext(r)

diff --git a/plugin/ochttp/trace.go b/plugin/ochttp/trace.go
@@ -50,7 +50,7 @@ type traceTransport struct {
 // The created span can follow a parent span, if a parent is presented in
 // the request's context.
 func (t *traceTransport) RoundTrip(req *http.Request) (*http.Response, error) {
-	name := spanNameFromURL("Sent", req.URL)
+	name := spanNameFromURL(req.URL)
 	// TODO(jbd): Discuss whether we want to prefix
 	// outgoing requests with Sent.
 	parent := trace.FromContext(req.Context())
@@ -126,13 +126,8 @@ func (t *traceTransport) CancelRequest(req *http.Request) {
 	}
 }
 
-func spanNameFromURL(prefix string, u *url.URL) string {
-	host := u.Hostname()
-	port := ":" + u.Port()
-	if port == ":" || port == ":80" || port == ":443" {
-		port = ""
-	}
-	return prefix + "." + host + port + u.Path
+func spanNameFromURL(u *url.URL) string {
+	return u.Path
 }
 
 func requestAttrs(r *http.Request) []trace.Attribute {

diff --git a/plugin/ochttp/trace_test.go b/plugin/ochttp/trace_test.go
@@ -266,23 +266,22 @@ func TestEndToEnd(t *testing.T) {
 
 			var client, server *trace.SpanData
 			for _, sp := range spans {
-				if strings.HasPrefix(sp.Name, "Sent.") {
+				switch sp.SpanKind {
+				case trace.SpanKindClient:
 					client = sp
-					serverHostport := req.URL.Hostname() + ":" + req.URL.Port()
-					if got, want := client.Name, "Sent."+serverHostport+"/example/url/path"; got != want {
+					if got, want := client.Name, "/example/url/path"; got != want {
 						t.Errorf("Span name: %q; want %q", got, want)
 					}
-				} else if strings.HasPrefix(sp.Name, "Recv.") {
+				case trace.SpanKindServer:
 					server = sp
-					if got, want := server.Name, "Recv./example/url/path"; got != want {
+					if got, want := server.Name, "/example/url/path"; got != want {
 						t.Errorf("Span name: %q; want %q", got, want)
 					}
+				default:
+					t.Fatalf("server or client span missing")
 				}
 			}
 
-			if server == nil || client == nil {
-				t.Fatalf("server or client span missing")
-			}
 			if tt.wantSameTraceID {
 				if server.TraceID != client.TraceID {
 					t.Errorf("TraceID does not match: server.TraceID=%q client.TraceID=%q", server.TraceID, client.TraceID)
@@ -346,38 +345,25 @@ func serveHTTP(handler *Handler, done chan struct{}, wait chan time.Time) string
 
 func TestSpanNameFromURL(t *testing.T) {
 	tests := []struct {
-		prefix string
-		u      string
-		want   string
+		u    string
+		want string
 	}{
 		{
-			prefix: "Sent",
-			u:      "http://localhost:80/hello?q=a",
-			want:   "Sent.localhost/hello",
-		},
-		{
-			prefix: "Recv",
-			u:      "https://localhost:443/a",
-			want:   "Recv.localhost/a",
-		},
-		{
-			prefix: "Recv",
-			u:      "https://example.com:7654/a",
-			want:   "Recv.example.com:7654/a",
+			u:    "http://localhost:80/hello?q=a",
+			want: "Sent.localhost/hello",
 		},
 		{
-			prefix: "Sent",
-			u:      "/a/b?q=c",
-			want:   "Sent./a/b",
+			u:    "/a/b?q=c",
+			want: "Sent./a/b",
 		},
 	}
 	for _, tt := range tests {
-		t.Run(tt.prefix+"-"+tt.u, func(t *testing.T) {
+		t.Run(tt.u, func(t *testing.T) {
 			u, err := url.Parse(tt.u)
 			if err != nil {
 				t.Errorf("url.Parse() = %v", err)
 			}
-			if got := spanNameFromURL(tt.prefix, u); got != tt.want {
+			if got := spanNameFromURL(u); got != tt.want {
 				t.Errorf("spanNameFromURL() = %v, want %v", got, tt.want)
 			}
 		})
@@ -393,7 +379,7 @@ func TestRequestAttributes(t *testing.T) {
 		{
 			name: "GET example.com/hello",
 			makeReq: func() *http.Request {
-				req, _ := http.NewRequest("GET", "http://example.com/hello", nil)
+				req, _ := http.NewRequest("GET", "http://example.com:779/hello", nil)
 				req.Header.Add("User-Agent", "ua")
 				return req
 			},