diff --git a/CHANGELOG.md b/CHANGELOG.md index 0df0168b922..eebdb179478 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,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) - Correlation Context extractor will no longer insert an empty map into the returned context when no valid values are extracted. (#923) ## [0.7.0] - 2020-06-26 diff --git a/instrumentation/grpctrace/interceptor.go b/instrumentation/grpctrace/interceptor.go index d6564825c43..3b4f0886dea 100644 --- a/instrumentation/grpctrace/interceptor.go +++ b/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 name, []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 } diff --git a/instrumentation/grpctrace/interceptor_test.go b/instrumentation/grpctrace/interceptor_test.go index 252f81061bb..fce3be59058 100644 --- a/instrumentation/grpctrace/interceptor_test.go +++ b/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,7 +348,7 @@ 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 @@ -349,13 +356,13 @@ func TestStreamClientInterceptor(t *testing.T) { 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) + } +}