Update grpctrace instrumentation span names

CHANGELOG.md

@@ -56,6 +56,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Add test for api.standard `HTTPClientAttributesFromHTTPRequest`. (#905)
 - Bump github.com/golangci/golangci-lint from 1.27.0 to 1.28.1 in /tools. (#901, #913)
 - Update otel-colector example to use the v0.5.0 collector. (#915)
+- The `grpctrace` instrumentation uses a span name conforming to the OpenTelemetry semantic conventions (does not contain a leading slash (`/`)). (#922)
+- The `grpctrace` instrumentation includes an `rpc.method` attribute now set to the gRPC method name. (#900, #922)
+- The `grpctrace` instrumentation `rpc.service` attribute now contains the package name if one exists.
+   This is in accordance with OpenTelemetry semantic conventions. (#922)
 
 ## [0.7.0] - 2020-06-26
 

instrumentation/grpctrace/interceptor.go

@@ -81,13 +81,13 @@ func UnaryClientInterceptor(tracer trace.Tracer) grpc.UnaryClientInterceptor {
 		requestMetadata, _ := metadata.FromOutgoingContext(ctx)
 		metadataCopy := requestMetadata.Copy()
 
+		name, attr := spanInfo(method, cc.Target())
 		var span trace.Span
 		ctx, span = tracer.Start(
-			ctx, method,
+			ctx,
+			name,
 			trace.WithSpanKind(trace.SpanKindClient),
-			trace.WithAttributes(peerInfoFromTarget(cc.Target())...),
-			trace.WithAttributes(standard.RPCSystemGRPC),
-			trace.WithAttributes(serviceAndMethodFromFullName(method)...),
+			trace.WithAttributes(attr...),
 		)
 		defer span.End()
 
@@ -260,13 +260,13 @@ func StreamClientInterceptor(tracer trace.Tracer) grpc.StreamClientInterceptor {
 		requestMetadata, _ := metadata.FromOutgoingContext(ctx)
 		metadataCopy := requestMetadata.Copy()
 
+		name, attr := spanInfo(method, cc.Target())
 		var span trace.Span
 		ctx, span = tracer.Start(
-			ctx, method,
+			ctx,
+			name,
 			trace.WithSpanKind(trace.SpanKindClient),
-			trace.WithAttributes(peerInfoFromTarget(cc.Target())...),
-			trace.WithAttributes(standard.RPCSystemGRPC),
-			trace.WithAttributes(serviceAndMethodFromFullName(method)...),
+			trace.WithAttributes(attr...),
 		)
 
 		Inject(ctx, &metadataCopy)
@@ -315,13 +315,12 @@ func UnaryServerInterceptor(tracer trace.Tracer) grpc.UnaryServerInterceptor {
 			MultiKV: entries,
 		}))
 
+		name, attr := spanInfo(info.FullMethod, peerFromCtx(ctx))
 		ctx, span := tracer.Start(
 			trace.ContextWithRemoteSpanContext(ctx, spanCtx),
-			info.FullMethod,
+			name,
 			trace.WithSpanKind(trace.SpanKindServer),
-			trace.WithAttributes(peerInfoFromContext(ctx)...),
-			trace.WithAttributes(standard.RPCSystemGRPC),
-			trace.WithAttributes(serviceAndMethodFromFullName(info.FullMethod)...),
+			trace.WithAttributes(attr...),
 		)
 		defer span.End()
 
@@ -406,13 +405,12 @@ func StreamServerInterceptor(tracer trace.Tracer) grpc.StreamServerInterceptor {
 			MultiKV: entries,
 		}))
 
+		name, attr := spanInfo(info.FullMethod, peerFromCtx(ctx))
 		ctx, span := tracer.Start(
 			trace.ContextWithRemoteSpanContext(ctx, spanCtx),
-			info.FullMethod,
+			name,
 			trace.WithSpanKind(trace.SpanKindServer),
-			trace.WithAttributes(peerInfoFromContext(ctx)...),
-			trace.WithAttributes(standard.RPCSystemGRPC),
-			trace.WithAttributes(serviceAndMethodFromFullName(info.FullMethod)...),
+			trace.WithAttributes(attr...),
 		)
 		defer span.End()
 
@@ -427,11 +425,21 @@ func StreamServerInterceptor(tracer trace.Tracer) grpc.StreamServerInterceptor {
 	}
 }
 
-func peerInfoFromTarget(target string) []kv.KeyValue {
-	host, port, err := net.SplitHostPort(target)
+// spanInfo returns a span name and all appropriate attributes from the gRPC
+// method and peer address.
+func spanInfo(fullMethod, peerAddress string) (string, []kv.KeyValue) {
+	attrs := []kv.KeyValue{standard.RPCSystemGRPC}
+	name, mAttrs := parseFullMethod(fullMethod)
+	attrs = append(attrs, mAttrs...)
+	attrs = append(attrs, peerAttr(peerAddress)...)
+	return name, attrs
+}
 
+// peerAttr returns attributes about the peer address.
+func peerAttr(addr string) []kv.KeyValue {
+	host, port, err := net.SplitHostPort(addr)
 	if err != nil {
-		return []kv.KeyValue{}
+		return []kv.KeyValue(nil)
 	}
 
 	if host == "" {
@@ -444,24 +452,32 @@ func peerInfoFromTarget(target string) []kv.KeyValue {
 	}
 }
 
-func peerInfoFromContext(ctx context.Context) []kv.KeyValue {
+// peerFromCtx returns a peer address from a context, if one exists.
+func peerFromCtx(ctx context.Context) string {
 	p, ok := peer.FromContext(ctx)
-
 	if !ok {
-		return []kv.KeyValue{}
+		return ""
 	}
-
-	return peerInfoFromTarget(p.Addr.String())
+	return p.Addr.String()
 }
 
-func serviceAndMethodFromFullName(method string) []kv.KeyValue {
-	l := strings.LastIndexByte(method, '/')
-	if l == -1 {
-		return []kv.KeyValue{}
+// parseFullMethod returns a span name following the OpenTelemetry semantic
+// conventions as well as all applicable span kv.KeyValue attributes based
+// on a gRPC's FullMethod.
+func parseFullMethod(fullMethod string) (string, []kv.KeyValue) {
+	name := strings.TrimLeft(fullMethod, "/")
+	parts := strings.SplitN(name, "/", 2)
+	if len(parts) != 2 {
+		// Invalid format, does not follow `/package.service/method`.
+		return []kv.KeyValue(nil)
 	}
 
-	return []kv.KeyValue{
-		standard.RPCServiceKey.String(method[:l]),
-		standard.RPCMethodKey.String(method[l+1:]),
+	var attrs []kv.KeyValue
+	if service := parts[0]; service != "" {
+		attrs = append(attrs, standard.RPCServiceKey.String(service))
+	}
+	if method := parts[1]; method != "" {
+		attrs = append(attrs, standard.RPCMethodKey.String(method))
 	}
+	return name, attrs
 }

instrumentation/grpctrace/interceptor_test.go

@@ -90,15 +90,17 @@ func TestUnaryClientInterceptor(t *testing.T) {
 	uniInterceptorInvoker := &mockUICInvoker{}
 
 	checks := []struct {
+		method       string
 		name         string
 		expectedAttr map[kv.Key]value.Value
 		eventsAttr   []map[kv.Key]value.Value
 	}{
 		{
-			name: "/github.com.serviceName/bar",
+			method: "/github.com.serviceName/bar",
+			name:   "github.com.serviceName/bar",
 			expectedAttr: map[kv.Key]value.Value{
 				standard.RPCSystemKey:   value.String("grpc"),
-				standard.RPCServiceKey:  value.String("/github.com.serviceName"),
+				standard.RPCServiceKey:  value.String("github.com.serviceName"),
 				standard.RPCMethodKey:   value.String("bar"),
 				standard.NetPeerIPKey:   value.String("fake"),
 				standard.NetPeerPortKey: value.String("connection"),
@@ -117,10 +119,11 @@ func TestUnaryClientInterceptor(t *testing.T) {
 			},
 		},
 		{
-			name: "/serviceName/bar",
+			method: "/serviceName/bar",
+			name:   "serviceName/bar",
 			expectedAttr: map[kv.Key]value.Value{
 				standard.RPCSystemKey:   value.String("grpc"),
-				standard.RPCServiceKey:  value.String("/serviceName"),
+				standard.RPCServiceKey:  value.String("serviceName"),
 				standard.RPCMethodKey:   value.String("bar"),
 				standard.NetPeerIPKey:   value.String("fake"),
 				standard.NetPeerPortKey: value.String("connection"),
@@ -139,7 +142,8 @@ func TestUnaryClientInterceptor(t *testing.T) {
 			},
 		},
 		{
-			name: "serviceName/bar",
+			method: "serviceName/bar",
+			name:   "serviceName/bar",
 			expectedAttr: map[kv.Key]value.Value{
 				standard.RPCSystemKey:   value.String("grpc"),
 				standard.RPCServiceKey:  value.String("serviceName"),
@@ -161,7 +165,8 @@ func TestUnaryClientInterceptor(t *testing.T) {
 			},
 		},
 		{
-			name: "invalidName",
+			method: "invalidName",
+			name:   "invalidName",
 			expectedAttr: map[kv.Key]value.Value{
 				standard.RPCSystemKey:   value.String("grpc"),
 				standard.NetPeerIPKey:   value.String("fake"),
@@ -181,10 +186,11 @@ func TestUnaryClientInterceptor(t *testing.T) {
 			},
 		},
 		{
-			name: "/github.com.foo.serviceName_123/method",
+			method: "/github.com.foo.serviceName_123/method",
+			name:   "github.com.foo.serviceName_123/method",
 			expectedAttr: map[kv.Key]value.Value{
 				standard.RPCSystemKey:   value.String("grpc"),
-				standard.RPCServiceKey:  value.String("/github.com.foo.serviceName_123"),
+				standard.RPCServiceKey:  value.String("github.com.foo.serviceName_123"),
 				standard.RPCMethodKey:   value.String("method"),
 				standard.NetPeerIPKey:   value.String("fake"),
 				standard.NetPeerPortKey: value.String("connection"),
@@ -205,7 +211,7 @@ func TestUnaryClientInterceptor(t *testing.T) {
 	}
 
 	for _, check := range checks {
-		err = unaryInterceptor(context.Background(), check.name, req, reply, clientConn, uniInterceptorInvoker.invoker)
+		err = unaryInterceptor(context.Background(), check.method, req, reply, clientConn, uniInterceptorInvoker.invoker)
 		if err != nil {
 			t.Errorf("failed to run unary interceptor: %v", err)
 			continue
@@ -298,12 +304,13 @@ func TestStreamClientInterceptor(t *testing.T) {
 	streamCI := StreamClientInterceptor(tracer)
 
 	var mockClStr mockClientStream
-	methodName := "/github.com.serviceName/bar"
+	method := "/github.com.serviceName/bar"
+	name := "github.com.serviceName/bar"
 
 	streamClient, err := streamCI(context.Background(),
 		&grpc.StreamDesc{ServerStreams: true},
 		clientConn,
-		methodName,
+		method,
 		func(ctx context.Context,
 			desc *grpc.StreamDesc,
 			cc *grpc.ClientConn,
@@ -318,7 +325,7 @@ func TestStreamClientInterceptor(t *testing.T) {
 	}
 
 	// no span exported while stream is open
-	if _, ok := exp.spanMap[methodName]; ok {
+	if _, ok := exp.spanMap[name]; ok {
 		t.Fatalf("span shouldn't end while stream is open")
 	}
 
@@ -341,21 +348,21 @@ func TestStreamClientInterceptor(t *testing.T) {
 	for retry := 0; retry < 5; retry++ {
 		ok := false
 		exp.mu.Lock()
-		spanData, ok = exp.spanMap[methodName]
+		spanData, ok = exp.spanMap[name]
 		exp.mu.Unlock()
 		if ok {
 			break
 		}
 		time.Sleep(time.Second * 1)
 	}
 	if spanData == nil {
-		t.Fatalf("no span data found for name < %s >", methodName)
+		t.Fatalf("no span data found for name < %s >", name)
 	}
 
 	attrs := spanData.Attributes
 	expectedAttr := map[kv.Key]string{
 		standard.RPCSystemKey:   "grpc",
-		standard.RPCServiceKey:  "/github.com.serviceName",
+		standard.RPCServiceKey:  "github.com.serviceName",
 		standard.RPCMethodKey:   "bar",
 		standard.NetPeerIPKey:   "fake",
 		standard.NetPeerPortKey: "connection",
@@ -365,7 +372,7 @@ func TestStreamClientInterceptor(t *testing.T) {
 		expected, ok := expectedAttr[attr.Key]
 		if ok {
 			if expected != attr.Value.AsString() {
-				t.Errorf("name: %s invalid %s found. expected %s, actual %s", methodName, string(attr.Key),
+				t.Errorf("name: %s invalid %s found. expected %s, actual %s", name, string(attr.Key),
 					expected, attr.Value.AsString())
 			}
 		}
@@ -428,3 +435,64 @@ func TestServerInterceptorError(t *testing.T) {
 		kv.Int("message.uncompressed_size", 26),
 	}, span.MessageEvents[1].Attributes)
 }
+
+func TestParseFullMethod(t *testing.T) {
+	tests := []struct {
+		fullMethod string
+		name       string
+		attr       []kv.KeyValue
+	}{
+		{
+			fullMethod: "/grpc.test.EchoService/Echo",
+			name:       "grpc.test.EchoService/Echo",
+			attr: []kv.KeyValue{
+				standard.RPCServiceKey.String("grpc.test.EchoService"),
+				standard.RPCMethodKey.String("Echo"),
+			},
+		}, {
+			fullMethod: "/com.example.ExampleRmiService/exampleMethod",
+			name:       "com.example.ExampleRmiService/exampleMethod",
+			attr: []kv.KeyValue{
+				standard.RPCServiceKey.String("com.example.ExampleRmiService"),
+				standard.RPCMethodKey.String("exampleMethod"),
+			},
+		}, {
+			fullMethod: "/MyCalcService.Calculator/Add",
+			name:       "MyCalcService.Calculator/Add",
+			attr: []kv.KeyValue{
+				standard.RPCServiceKey.String("MyCalcService.Calculator"),
+				standard.RPCMethodKey.String("Add"),
+			},
+		}, {
+			fullMethod: "/MyServiceReference.ICalculator/Add",
+			name:       "MyServiceReference.ICalculator/Add",
+			attr: []kv.KeyValue{
+				standard.RPCServiceKey.String("MyServiceReference.ICalculator"),
+				standard.RPCMethodKey.String("Add"),
+			},
+		}, {
+			fullMethod: "/MyServiceWithNoPackage/theMethod",
+			name:       "MyServiceWithNoPackage/theMethod",
+			attr: []kv.KeyValue{
+				standard.RPCServiceKey.String("MyServiceWithNoPackage"),
+				standard.RPCMethodKey.String("theMethod"),
+			},
+		}, {
+			fullMethod: "/pkg.srv",
+			name:       "pkg.srv",
+			attr:       []kv.KeyValue(nil),
+		}, {
+			fullMethod: "/pkg.srv/",
+			name:       "pkg.srv/",
+			attr: []kv.KeyValue{
+				standard.RPCServiceKey.String("pkg.srv"),
+			},
+		},
+	}
+
+	for _, test := range tests {
+		n, a := parseFullMethod(test.fullMethod)
+		assert.Equal(t, test.name, n)
+		assert.Equal(t, test.attr, a)
+	}
+}