From bdc35ff7ad6c17a33b2e59581e69d447ee29fe69 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 7 Jul 2020 19:37:50 -0700 Subject: [PATCH 01/10] Update grpctrace instrumentation span names Span names MUST not contain the leading slash (`/`) that the grpc package prepends to all `FullMethod` values. This replaces the `serviceFromFullMethod` function with a parsing function. This parsing function returns an span name adhering to the OpenTelemetry semantic conventions as well as formatted span attributes. Additionally, the service name needs to include the package if one exists. This updates that attribute accordingly. Once #900 is merged the method attributes can be added by uncommenting. Resolves #916 --- instrumentation/grpctrace/interceptor.go | 49 +++++++--- instrumentation/grpctrace/interceptor_test.go | 93 +++++++++++++++---- 2 files changed, 111 insertions(+), 31 deletions(-) diff --git a/instrumentation/grpctrace/interceptor.go b/instrumentation/grpctrace/interceptor.go index 3d254c5dccd..990d79f18ba 100644 --- a/instrumentation/grpctrace/interceptor.go +++ b/instrumentation/grpctrace/interceptor.go @@ -81,12 +81,14 @@ func UnaryClientInterceptor(tracer trace.Tracer) grpc.UnaryClientInterceptor { requestMetadata, _ := metadata.FromOutgoingContext(ctx) metadataCopy := requestMetadata.Copy() + name, attr := parseFullMethod(method) var span trace.Span ctx, span = tracer.Start( - ctx, method, + ctx, + name, trace.WithSpanKind(trace.SpanKindClient), trace.WithAttributes(peerInfoFromTarget(cc.Target())...), - trace.WithAttributes(standard.RPCServiceKey.String(serviceFromFullMethod(method))), + trace.WithAttributes(attr...), ) defer span.End() @@ -259,12 +261,14 @@ func StreamClientInterceptor(tracer trace.Tracer) grpc.StreamClientInterceptor { requestMetadata, _ := metadata.FromOutgoingContext(ctx) metadataCopy := requestMetadata.Copy() + name, attr := parseFullMethod(method) var span trace.Span ctx, span = tracer.Start( - ctx, method, + ctx, + name, trace.WithSpanKind(trace.SpanKindClient), trace.WithAttributes(peerInfoFromTarget(cc.Target())...), - trace.WithAttributes(standard.RPCServiceKey.String(serviceFromFullMethod(method))), + trace.WithAttributes(attr...), ) Inject(ctx, &metadataCopy) @@ -313,12 +317,13 @@ func UnaryServerInterceptor(tracer trace.Tracer) grpc.UnaryServerInterceptor { MultiKV: entries, })) + name, attr := parseFullMethod(info.FullMethod) ctx, span := tracer.Start( trace.ContextWithRemoteSpanContext(ctx, spanCtx), - info.FullMethod, + name, trace.WithSpanKind(trace.SpanKindServer), trace.WithAttributes(peerInfoFromContext(ctx)...), - trace.WithAttributes(standard.RPCServiceKey.String(serviceFromFullMethod(info.FullMethod))), + trace.WithAttributes(attr...), ) defer span.End() @@ -403,12 +408,13 @@ func StreamServerInterceptor(tracer trace.Tracer) grpc.StreamServerInterceptor { MultiKV: entries, })) + name, attr := parseFullMethod(info.FullMethod) ctx, span := tracer.Start( trace.ContextWithRemoteSpanContext(ctx, spanCtx), - info.FullMethod, + name, trace.WithSpanKind(trace.SpanKindServer), trace.WithAttributes(peerInfoFromContext(ctx)...), - trace.WithAttributes(standard.RPCServiceKey.String(serviceFromFullMethod(info.FullMethod))), + trace.WithAttributes(attr...), ) defer span.End() @@ -427,7 +433,7 @@ func peerInfoFromTarget(target string) []kv.KeyValue { host, port, err := net.SplitHostPort(target) if err != nil { - return []kv.KeyValue{} + return []kv.KeyValue(nil) } if host == "" { @@ -444,19 +450,32 @@ func peerInfoFromContext(ctx context.Context) []kv.KeyValue { p, ok := peer.FromContext(ctx) if !ok { - return []kv.KeyValue{} + return []kv.KeyValue(nil) } return peerInfoFromTarget(p.Addr.String()) } -var fullMethodRegexp = regexp.MustCompile(`^\/?(?:\S+\.)?(\S+)\/\S+$`) +var fullMethodRegexp = regexp.MustCompile(`^\/?(((?:\S+\.)?\S+)\/(\S+))$`) -func serviceFromFullMethod(method string) string { - match := fullMethodRegexp.FindStringSubmatch(method) +// parseFullMethod returns an span name following the OpenTelemetry semantic +// conventions as well as all applicable span kv.KeyValue attributes based +// on a gRPC's FullMethod. If no name is parsable, full is returned with an +// empty attribute slice. +func parseFullMethod(full string) (string, []kv.KeyValue) { + match := fullMethodRegexp.FindStringSubmatch(full) if len(match) == 0 { - return "" + // Worse than incorrectly named spans is empty named spans. + return full, nil } - return match[1] + var attrs []kv.KeyValue + if match[2] != "" { + attrs = append(attrs, standard.RPCServiceKey.String(match[2])) + } + // TODO [MrAlias]: uncomment when #900 merges + //if match[3] != "" { + // attrs = append(attrs, standard.RPCMethodKey.String(match[3])) + //} + return match[1], attrs } diff --git a/instrumentation/grpctrace/interceptor_test.go b/instrumentation/grpctrace/interceptor_test.go index 6ec80cba85f..bef38a68a13 100644 --- a/instrumentation/grpctrace/interceptor_test.go +++ b/instrumentation/grpctrace/interceptor_test.go @@ -90,14 +90,16 @@ 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.RPCServiceKey: value.String("serviceName"), + standard.RPCServiceKey: value.String("github.com.serviceName"), standard.NetPeerIPKey: value.String("fake"), standard.NetPeerPortKey: value.String("connection"), }, @@ -115,7 +117,8 @@ func TestUnaryClientInterceptor(t *testing.T) { }, }, { - name: "/serviceName/bar", + method: "/serviceName/bar", + name: "serviceName/bar", expectedAttr: map[kv.Key]value.Value{ standard.RPCServiceKey: value.String("serviceName"), standard.NetPeerIPKey: value.String("fake"), @@ -135,7 +138,8 @@ func TestUnaryClientInterceptor(t *testing.T) { }, }, { - name: "serviceName/bar", + method: "serviceName/bar", + name: "serviceName/bar", expectedAttr: map[kv.Key]value.Value{ standard.RPCServiceKey: value.String("serviceName"), standard.NetPeerIPKey: value.String("fake"), @@ -155,9 +159,9 @@ func TestUnaryClientInterceptor(t *testing.T) { }, }, { - name: "invalidName", + method: "invalidName", + name: "invalidName", expectedAttr: map[kv.Key]value.Value{ - standard.RPCServiceKey: value.String(""), standard.NetPeerIPKey: value.String("fake"), standard.NetPeerPortKey: value.String("connection"), }, @@ -175,9 +179,10 @@ 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.RPCServiceKey: value.String("serviceName_123"), + standard.RPCServiceKey: value.String("github.com.foo.serviceName_123"), standard.NetPeerIPKey: value.String("fake"), standard.NetPeerPortKey: value.String("connection"), }, @@ -197,7 +202,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 @@ -290,12 +295,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, @@ -310,7 +316,7 @@ func TestStreamClientInterceptor(t *testing.T) { } // no span exported while stream is open - if _, ok := exp.spanMap[methodName]; ok { + if _, ok := exp.spanMap[method]; ok { t.Fatalf("span shouldn't end while stream is open") } @@ -333,7 +339,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 @@ -341,12 +347,12 @@ 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.RPCServiceKey: "serviceName", + standard.RPCServiceKey: "github.com.serviceName", standard.NetPeerIPKey: "fake", standard.NetPeerPortKey: "connection", } @@ -355,7 +361,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()) } } @@ -418,3 +424,58 @@ 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), + }, + } + + for _, test := range tests { + n, a := parseFullMethod(test.fullMethod) + assert.Equal(t, test.name, n) + assert.Equal(t, test.attr, a) + } +} From 09d004881f7263790ac2049f22950bac2e723302 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Tue, 7 Jul 2020 19:46:29 -0700 Subject: [PATCH 02/10] Update Changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9dcd8db84a7..690581323c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Ensure span status is not set to `Unknown` when no HTTP status code is provided as it is assumed to be `200 OK`. (#908) - Ensure `httptrace.clientTracer` closes `http.headers` span. (#912) - Prometheus exporter will not apply stale updates or forget inactive metrics. (#903) +- The `grpctrace` instrumentation uses a span name conforming to the OpenTelemetry semantic conventions (does not contain a leading slash (`/`)). (#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 From 8d86a53ee7962f1c6af1194b31bc11d45e939c1b Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 8 Jul 2020 08:40:07 -0700 Subject: [PATCH 03/10] Update comment to plural --- instrumentation/grpctrace/interceptor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/grpctrace/interceptor.go b/instrumentation/grpctrace/interceptor.go index 25be2472a95..2a22400e6d2 100644 --- a/instrumentation/grpctrace/interceptor.go +++ b/instrumentation/grpctrace/interceptor.go @@ -469,7 +469,7 @@ var fullMethodRegexp = regexp.MustCompile(`^\/?(((?:\S+\.)?\S+)\/(\S+))$`) func parseFullMethod(full string) (string, []kv.KeyValue) { match := fullMethodRegexp.FindStringSubmatch(full) if len(match) == 0 { - // Worse than incorrectly named spans is empty named spans. + // Worse than incorrectly named spans are empty named spans. return full, nil } From af7a91b44755f47fe0e050d21abafe8f1fd33917 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 8 Jul 2020 09:51:54 -0700 Subject: [PATCH 04/10] Switch from regexp to string parsing --- instrumentation/grpctrace/interceptor.go | 26 +++++++++---------- instrumentation/grpctrace/interceptor_test.go | 10 +++++-- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/instrumentation/grpctrace/interceptor.go b/instrumentation/grpctrace/interceptor.go index 2a22400e6d2..932e4caae8d 100644 --- a/instrumentation/grpctrace/interceptor.go +++ b/instrumentation/grpctrace/interceptor.go @@ -20,7 +20,7 @@ import ( "context" "io" "net" - "regexp" + "strings" "go.opentelemetry.io/otel/api/standard" @@ -460,25 +460,23 @@ func peerInfoFromContext(ctx context.Context) []kv.KeyValue { return peerInfoFromTarget(p.Addr.String()) } -var fullMethodRegexp = regexp.MustCompile(`^\/?(((?:\S+\.)?\S+)\/(\S+))$`) - // parseFullMethod returns an span name following the OpenTelemetry semantic // conventions as well as all applicable span kv.KeyValue attributes based -// on a gRPC's FullMethod. If no name is parsable, full is returned with an -// empty attribute slice. +// on a gRPC's FullMethod. func parseFullMethod(full string) (string, []kv.KeyValue) { - match := fullMethodRegexp.FindStringSubmatch(full) - if len(match) == 0 { - // Worse than incorrectly named spans are empty named spans. - return full, nil + name := strings.TrimLeft(full, "/") + parts := strings.SplitN(name, "/", 2) + if len(parts) != 2 { + // Invalid format, does not follow `/package.service/method`. + return name, nil } var attrs []kv.KeyValue - if match[2] != "" { - attrs = append(attrs, standard.RPCServiceKey.String(match[2])) + if service := parts[0]; service != "" { + attrs = append(attrs, standard.RPCServiceKey.String(service)) } - if match[3] != "" { - attrs = append(attrs, standard.RPCMethodKey.String(match[3])) + if method := parts[1]; method != "" { + attrs = append(attrs, standard.RPCMethodKey.String(method)) } - return match[1], attrs + return name, attrs } diff --git a/instrumentation/grpctrace/interceptor_test.go b/instrumentation/grpctrace/interceptor_test.go index b2c6265c490..b8f60be8616 100644 --- a/instrumentation/grpctrace/interceptor_test.go +++ b/instrumentation/grpctrace/interceptor_test.go @@ -478,9 +478,15 @@ func TestParseFullMethod(t *testing.T) { standard.RPCMethodKey.String("theMethod"), }, }, { - fullMethod: "/pkg.srv/", - name: "/pkg.srv/", + fullMethod: "/pkg.srv", + name: "pkg.srv", attr: []kv.KeyValue(nil), + }, { + fullMethod: "/pkg.srv/", + name: "pkg.srv/", + attr: []kv.KeyValue{ + standard.RPCServiceKey.String("pkg.srv"), + }, }, } From 9c93e4a5de47f77692d9b7095db7bd66504e33ac Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 8 Jul 2020 10:14:50 -0700 Subject: [PATCH 05/10] Consolidate attributes before creating span --- instrumentation/grpctrace/interceptor.go | 43 ++++++++++++------------ 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/instrumentation/grpctrace/interceptor.go b/instrumentation/grpctrace/interceptor.go index 932e4caae8d..e8fea1ae39d 100644 --- a/instrumentation/grpctrace/interceptor.go +++ b/instrumentation/grpctrace/interceptor.go @@ -81,14 +81,12 @@ func UnaryClientInterceptor(tracer trace.Tracer) grpc.UnaryClientInterceptor { requestMetadata, _ := metadata.FromOutgoingContext(ctx) metadataCopy := requestMetadata.Copy() - name, attr := parseFullMethod(method) + name, attr := spanInfo(method, cc.Target()) var span trace.Span ctx, span = tracer.Start( ctx, name, trace.WithSpanKind(trace.SpanKindClient), - trace.WithAttributes(peerInfoFromTarget(cc.Target())...), - trace.WithAttributes(standard.RPCSystemGRPC), trace.WithAttributes(attr...), ) defer span.End() @@ -262,14 +260,12 @@ func StreamClientInterceptor(tracer trace.Tracer) grpc.StreamClientInterceptor { requestMetadata, _ := metadata.FromOutgoingContext(ctx) metadataCopy := requestMetadata.Copy() - name, attr := parseFullMethod(method) + name, attr := spanInfo(method, cc.Target()) var span trace.Span ctx, span = tracer.Start( ctx, name, trace.WithSpanKind(trace.SpanKindClient), - trace.WithAttributes(peerInfoFromTarget(cc.Target())...), - trace.WithAttributes(standard.RPCSystemGRPC), trace.WithAttributes(attr...), ) @@ -319,13 +315,11 @@ func UnaryServerInterceptor(tracer trace.Tracer) grpc.UnaryServerInterceptor { MultiKV: entries, })) - name, attr := parseFullMethod(info.FullMethod) + name, attr := spanInfo(info.FullMethod, peerFromCtx(ctx)) ctx, span := tracer.Start( trace.ContextWithRemoteSpanContext(ctx, spanCtx), name, trace.WithSpanKind(trace.SpanKindServer), - trace.WithAttributes(peerInfoFromContext(ctx)...), - trace.WithAttributes(standard.RPCSystemGRPC), trace.WithAttributes(attr...), ) defer span.End() @@ -411,13 +405,11 @@ func StreamServerInterceptor(tracer trace.Tracer) grpc.StreamServerInterceptor { MultiKV: entries, })) - name, attr := parseFullMethod(info.FullMethod) + name, attr := spanInfo(info.FullMethod, peerFromCtx(ctx)) ctx, span := tracer.Start( trace.ContextWithRemoteSpanContext(ctx, spanCtx), name, trace.WithSpanKind(trace.SpanKindServer), - trace.WithAttributes(peerInfoFromContext(ctx)...), - trace.WithAttributes(standard.RPCSystemGRPC), trace.WithAttributes(attr...), ) defer span.End() @@ -433,9 +425,19 @@ 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(nil) } @@ -450,21 +452,20 @@ 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(nil) + return "" } - - return peerInfoFromTarget(p.Addr.String()) + return p.Addr.String() } // parseFullMethod returns an span name following the OpenTelemetry semantic // conventions as well as all applicable span kv.KeyValue attributes based // on a gRPC's FullMethod. -func parseFullMethod(full string) (string, []kv.KeyValue) { - name := strings.TrimLeft(full, "/") +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`. From 799d5400675064ef2c2ddf6162a08c7f9a6b13de Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 8 Jul 2020 10:20:11 -0700 Subject: [PATCH 06/10] Update Changelog with addition of rpc.method in grpctrace --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c47d2006acd..d9f5a631b01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - 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) From e016e0d604c6b83ff3183e22c5cea5d9d2e3958d Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 8 Jul 2020 10:22:10 -0700 Subject: [PATCH 07/10] Fix test spanMap lookup key --- instrumentation/grpctrace/interceptor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/grpctrace/interceptor_test.go b/instrumentation/grpctrace/interceptor_test.go index b8f60be8616..fce3be59058 100644 --- a/instrumentation/grpctrace/interceptor_test.go +++ b/instrumentation/grpctrace/interceptor_test.go @@ -325,7 +325,7 @@ func TestStreamClientInterceptor(t *testing.T) { } // no span exported while stream is open - if _, ok := exp.spanMap[method]; ok { + if _, ok := exp.spanMap[name]; ok { t.Fatalf("span shouldn't end while stream is open") } From ddb1c31154d7398e15587f0c816106da96f187e9 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 9 Jul 2020 10:26:26 -0700 Subject: [PATCH 08/10] Update instrumentation/grpctrace/interceptor.go Co-authored-by: ET --- instrumentation/grpctrace/interceptor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/grpctrace/interceptor.go b/instrumentation/grpctrace/interceptor.go index e8fea1ae39d..4e0cbe60765 100644 --- a/instrumentation/grpctrace/interceptor.go +++ b/instrumentation/grpctrace/interceptor.go @@ -461,7 +461,7 @@ func peerFromCtx(ctx context.Context) string { return p.Addr.String() } -// parseFullMethod returns an span name following the OpenTelemetry semantic +// 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) { From fa42268fa56965654319872cbdbe7f6fde89016a Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 9 Jul 2020 10:40:45 -0700 Subject: [PATCH 09/10] Unify on explicit typed return value --- instrumentation/grpctrace/interceptor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/grpctrace/interceptor.go b/instrumentation/grpctrace/interceptor.go index 4e0cbe60765..9cb2d2b7df4 100644 --- a/instrumentation/grpctrace/interceptor.go +++ b/instrumentation/grpctrace/interceptor.go @@ -469,7 +469,7 @@ func parseFullMethod(fullMethod string) (string, []kv.KeyValue) { parts := strings.SplitN(name, "/", 2) if len(parts) != 2 { // Invalid format, does not follow `/package.service/method`. - return name, nil + return []kv.KeyValue(nil) } var attrs []kv.KeyValue From f6e20271f3e7d43148cc35206c8f3d28c7d73912 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 9 Jul 2020 10:49:20 -0700 Subject: [PATCH 10/10] Fix copy paste error --- instrumentation/grpctrace/interceptor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/grpctrace/interceptor.go b/instrumentation/grpctrace/interceptor.go index 9cb2d2b7df4..3b4f0886dea 100644 --- a/instrumentation/grpctrace/interceptor.go +++ b/instrumentation/grpctrace/interceptor.go @@ -469,7 +469,7 @@ func parseFullMethod(fullMethod string) (string, []kv.KeyValue) { parts := strings.SplitN(name, "/", 2) if len(parts) != 2 { // Invalid format, does not follow `/package.service/method`. - return []kv.KeyValue(nil) + return name, []kv.KeyValue(nil) } var attrs []kv.KeyValue