Skip to content

Commit

Permalink
Span names shouldn't be prefixed with Sent and Recv
Browse files Browse the repository at this point in the history
Instead set them for some exporters that cannot
differentiate client spans from server spans in their model.

Also recored the request port.

Fixes census-instrumentation#607.
  • Loading branch information
rakyll committed Mar 20, 2018
1 parent 7b6adb7 commit d080bdf
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 46 deletions.
10 changes: 9 additions & 1 deletion exporter/stackdriver/trace_proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
18 changes: 3 additions & 15 deletions exporter/zipkin/zipkin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion plugin/ochttp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 5 additions & 8 deletions plugin/ochttp/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ var defaultFormat propagation.HTTPFormat = &b3.HTTPFormat{}
// Only trace exporters will need them.
const (
HostAttribute = "http.host"
PortAttribute = "http.port"
MethodAttribute = "http.method"
PathAttribute = "http.path"
UserAgentAttribute = "http.user_agent"
Expand All @@ -50,7 +51,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())
Expand Down Expand Up @@ -126,19 +127,15 @@ 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 {
return []trace.Attribute{
trace.StringAttribute(PathAttribute, r.URL.Path),
trace.StringAttribute(HostAttribute, r.URL.Host),
trace.StringAttribute(PortAttribute, r.URL.Port()),
trace.StringAttribute(MethodAttribute, r.Method),
trace.StringAttribute(UserAgentAttribute, r.UserAgent()),
}
Expand Down
29 changes: 8 additions & 21 deletions plugin/ochttp/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,38 +346,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",
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",
},
{
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)
}
})
Expand Down

0 comments on commit d080bdf

Please sign in to comment.