Skip to content

Commit

Permalink
Fix gRPC service/method URL path parsing discrepancies
Browse files Browse the repository at this point in the history
  • Loading branch information
ash2k committed Aug 4, 2023
1 parent 83a3c26 commit 6cd00f1
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 65 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`
- Do not modify the origin request in RoundTripper in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#4033)
- Handle empty value of `OTEL_PROPAGATORS` environment variable the same way as when the variable is unset in `go.opentelemetry.io/contrib/propagators/autoprop`. (#4101)
- Fix gRPC service/method URL path parsing discrepancies. (#4135)

### Deprecated

Expand Down
14 changes: 11 additions & 3 deletions instrumentation/google.golang.org/grpc/otelgrpc/internal/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,21 @@ import (
// ParseFullMethod returns a span name following the OpenTelemetry semantic
// conventions as well as all applicable span attribute.KeyValue attributes based
// on a gRPC's FullMethod.
//
// Parsing is consistent with grpc-go implementation:
// https://github.com/grpc/grpc-go/blob/v1.57.0/internal/grpcutil/method.go#L26-L39
func ParseFullMethod(fullMethod string) (string, []attribute.KeyValue) {
name := strings.TrimLeft(fullMethod, "/")
service, method, found := strings.Cut(name, "/")
if !found {
if !strings.HasPrefix(fullMethod, "/") {
// Invalid format, does not follow `/package.service/method`.
return fullMethod, nil
}
name := fullMethod[1:]
pos := strings.LastIndex(name, "/")
if pos < 0 {
// Invalid format, does not follow `/package.service/method`.
return name, nil
}
service, method := name[:pos], name[pos+1:]

var attrs []attribute.KeyValue
if service != "" {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package internal // import "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/internal"

import (
"testing"

"go.opentelemetry.io/otel/attribute"
semconv "go.opentelemetry.io/otel/semconv/v1.17.0"

"github.com/stretchr/testify/assert"
)

func TestParseFullMethod(t *testing.T) {
cases := []struct {
input string
expectedName string
expectedAttrs []attribute.KeyValue
}{
{
input: "no_slash/method",
expectedName: "no_slash/method",
},
{
input: "/slash_but_no_second_slash",
expectedName: "slash_but_no_second_slash",
},
{
input: "/service_only/",
expectedName: "service_only/",
expectedAttrs: []attribute.KeyValue{
semconv.RPCService("service_only"),
},
},
{
input: "//method_only",
expectedName: "/method_only",
expectedAttrs: []attribute.KeyValue{
semconv.RPCMethod("method_only"),
},
},
{
input: "/service/method",
expectedName: "service/method",
expectedAttrs: []attribute.KeyValue{
semconv.RPCService("service"),
semconv.RPCMethod("method"),
},
},
}
for _, tc := range cases {
t.Run(tc.input, func(t *testing.T) {
name, attrs := ParseFullMethod(tc.input)
assert.Equal(t, tc.expectedName, name)
assert.Equal(t, tc.expectedAttrs, attrs)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"time"

"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/internal"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/sdk/trace"
Expand Down Expand Up @@ -1056,64 +1055,3 @@ func TestStreamServerInterceptorEvents(t *testing.T) {
})
}
}

func TestParseFullMethod(t *testing.T) {
tests := []struct {
fullMethod string
name string
attr []attribute.KeyValue
}{
{
fullMethod: "/grpc.test.EchoService/Echo",
name: "grpc.test.EchoService/Echo",
attr: []attribute.KeyValue{
semconv.RPCService("grpc.test.EchoService"),
semconv.RPCMethod("Echo"),
},
}, {
fullMethod: "/com.example.ExampleRmiService/exampleMethod",
name: "com.example.ExampleRmiService/exampleMethod",
attr: []attribute.KeyValue{
semconv.RPCService("com.example.ExampleRmiService"),
semconv.RPCMethod("exampleMethod"),
},
}, {
fullMethod: "/MyCalcService.Calculator/Add",
name: "MyCalcService.Calculator/Add",
attr: []attribute.KeyValue{
semconv.RPCService("MyCalcService.Calculator"),
semconv.RPCMethod("Add"),
},
}, {
fullMethod: "/MyServiceReference.ICalculator/Add",
name: "MyServiceReference.ICalculator/Add",
attr: []attribute.KeyValue{
semconv.RPCService("MyServiceReference.ICalculator"),
semconv.RPCMethod("Add"),
},
}, {
fullMethod: "/MyServiceWithNoPackage/theMethod",
name: "MyServiceWithNoPackage/theMethod",
attr: []attribute.KeyValue{
semconv.RPCService("MyServiceWithNoPackage"),
semconv.RPCMethod("theMethod"),
},
}, {
fullMethod: "/pkg.srv",
name: "pkg.srv",
attr: []attribute.KeyValue(nil),
}, {
fullMethod: "/pkg.srv/",
name: "pkg.srv/",
attr: []attribute.KeyValue{
semconv.RPCService("pkg.srv"),
},
},
}

for _, test := range tests {
n, a := internal.ParseFullMethod(test.fullMethod)
assert.Equal(t, test.name, n)
assert.Equal(t, test.attr, a)
}
}

0 comments on commit 6cd00f1

Please sign in to comment.