diff --git a/CHANGELOG.md b/CHANGELOG.md index 58ccfe27675..cb78602debd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The deprecated `SemVersion` function is removed in `go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin/test`, use `Version` function instead. (#7087) - The deprecated `SemVersion` function is removed in `go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho`, use `Version` function instead. (#7089) - The deprecated `SemVersion` function is removed in `go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws`, use `Version` function instead. (#7154) +- The deprecated `UnaryServerInterceptor` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` is removed, use `NewServerHandler` instead. (#7115) diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/benchmark_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/benchmark_test.go index 544b28811c3..d41899bc2a7 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/benchmark_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/benchmark_test.go @@ -70,14 +70,6 @@ func BenchmarkNoInstrumentation(b *testing.B) { benchmark(b, nil, nil) } -func BenchmarkUnaryServerInterceptor(b *testing.B) { - benchmark(b, nil, []grpc.ServerOption{ - grpc.UnaryInterceptor(otelgrpc.UnaryServerInterceptor( - otelgrpc.WithTracerProvider(tracerProvider), - )), - }) -} - func BenchmarkStreamServerInterceptor(b *testing.B) { benchmark(b, nil, []grpc.ServerOption{ grpc.StreamInterceptor(otelgrpc.StreamServerInterceptor( diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go b/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go index 7d5ed058082..c4f5a440bc2 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/interceptor.go @@ -11,7 +11,6 @@ import ( "io" "net" "strconv" - "time" "google.golang.org/grpc" grpc_codes "google.golang.org/grpc/codes" @@ -23,7 +22,6 @@ import ( "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/metric" semconv "go.opentelemetry.io/otel/semconv/v1.17.0" "go.opentelemetry.io/otel/trace" ) @@ -75,7 +73,7 @@ func UnaryClientInterceptor(opts ...Option) grpc.UnaryClientInterceptor { return invoker(ctx, method, req, reply, cc, callOpts...) } - name, attr, _ := telemetryAttributes(method, cc.Target()) + name, attr := telemetryAttributes(method, cc.Target()) startOpts := append([]trace.SpanStartOption{ trace.WithSpanKind(trace.SpanKindClient), @@ -235,7 +233,7 @@ func StreamClientInterceptor(opts ...Option) grpc.StreamClientInterceptor { return streamer(ctx, desc, cc, method, callOpts...) } - name, attr, _ := telemetryAttributes(method, cc.Target()) + name, attr := telemetryAttributes(method, cc.Target()) startOpts := append([]trace.SpanStartOption{ trace.WithSpanKind(trace.SpanKindClient), @@ -265,81 +263,6 @@ func StreamClientInterceptor(opts ...Option) grpc.StreamClientInterceptor { } } -// UnaryServerInterceptor returns a grpc.UnaryServerInterceptor suitable -// for use in a grpc.NewServer call. -// -// Deprecated: Use [NewServerHandler] instead. -func UnaryServerInterceptor(opts ...Option) grpc.UnaryServerInterceptor { - cfg := newConfig(opts, "server") - tracer := cfg.TracerProvider.Tracer( - ScopeName, - trace.WithInstrumentationVersion(Version()), - ) - - return func( - ctx context.Context, - req interface{}, - info *grpc.UnaryServerInfo, - handler grpc.UnaryHandler, - ) (interface{}, error) { - i := &InterceptorInfo{ - UnaryServerInfo: info, - Type: UnaryServer, - } - if cfg.InterceptorFilter != nil && !cfg.InterceptorFilter(i) { - return handler(ctx, req) - } - - ctx = extract(ctx, cfg.Propagators) - name, attr, metricAttrs := telemetryAttributes(info.FullMethod, peerFromCtx(ctx)) - - startOpts := append([]trace.SpanStartOption{ - trace.WithSpanKind(trace.SpanKindServer), - trace.WithAttributes(attr...), - }, - cfg.SpanStartOptions..., - ) - - ctx, span := tracer.Start( - trace.ContextWithRemoteSpanContext(ctx, trace.SpanContextFromContext(ctx)), - name, - startOpts..., - ) - defer span.End() - - if cfg.ReceivedEvent { - messageReceived.Event(ctx, 1, req) - } - - before := time.Now() - - resp, err := handler(ctx, req) - - s, _ := status.FromError(err) - if err != nil { - statusCode, msg := serverStatus(s) - span.SetStatus(statusCode, msg) - if cfg.SentEvent { - messageSent.Event(ctx, 1, s.Proto()) - } - } else { - if cfg.SentEvent { - messageSent.Event(ctx, 1, resp) - } - } - grpcStatusCodeAttr := statusCodeAttr(s.Code()) - span.SetAttributes(grpcStatusCodeAttr) - - // Use floating point division here for higher precision (instead of Millisecond method). - elapsedTime := float64(time.Since(before)) / float64(time.Millisecond) - - metricAttrs = append(metricAttrs, grpcStatusCodeAttr) - cfg.rpcDuration.Record(ctx, elapsedTime, metric.WithAttributeSet(attribute.NewSet(metricAttrs...))) - - return resp, err - } -} - // serverStream wraps around the embedded grpc.ServerStream, and intercepts the RecvMsg and // SendMsg method call. type serverStream struct { @@ -417,7 +340,7 @@ func StreamServerInterceptor(opts ...Option) grpc.StreamServerInterceptor { } ctx = extract(ctx, cfg.Propagators) - name, attr, _ := telemetryAttributes(info.FullMethod, peerFromCtx(ctx)) + name, attr := telemetryAttributes(info.FullMethod, peerFromCtx(ctx)) startOpts := append([]trace.SpanStartOption{ trace.WithSpanKind(trace.SpanKindServer), @@ -449,16 +372,15 @@ func StreamServerInterceptor(opts ...Option) grpc.StreamServerInterceptor { // telemetryAttributes returns a span name and span and metric attributes from // the gRPC method and peer address. -func telemetryAttributes(fullMethod, peerAddress string) (string, []attribute.KeyValue, []attribute.KeyValue) { +func telemetryAttributes(fullMethod, peerAddress string) (string, []attribute.KeyValue) { name, methodAttrs := internal.ParseFullMethod(fullMethod) peerAttrs := peerAttr(peerAddress) attrs := make([]attribute.KeyValue, 0, 1+len(methodAttrs)+len(peerAttrs)) attrs = append(attrs, RPCSystemGRPC) attrs = append(attrs, methodAttrs...) - metricAttrs := attrs[:1+len(methodAttrs)] attrs = append(attrs, peerAttrs...) - return name, attrs, metricAttrs + return name, attrs } // peerAttr returns attributes about the peer address. diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_test.go index 7473edee1ab..34b0e8ca129 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/test/grpc_test.go @@ -19,9 +19,6 @@ import ( "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/internal/test" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/sdk/instrumentation" - "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/metricdata" - "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" semconv "go.opentelemetry.io/otel/semconv/v1.17.0" @@ -86,11 +83,6 @@ func TestInterceptors(t *testing.T) { clientStreamSR := tracetest.NewSpanRecorder() clientStreamTP := trace.NewTracerProvider(trace.WithSpanProcessor(clientStreamSR)) - serverUnarySR := tracetest.NewSpanRecorder() - serverUnaryTP := trace.NewTracerProvider(trace.WithSpanProcessor(serverUnarySR)) - serverUnaryMetricReader := metric.NewManualReader() - serverUnaryMP := metric.NewMeterProvider(metric.WithReader(serverUnaryMetricReader)) - serverStreamSR := tracetest.NewSpanRecorder() serverStreamTP := trace.NewTracerProvider(trace.WithSpanProcessor(serverStreamSR)) @@ -110,12 +102,6 @@ func TestInterceptors(t *testing.T) { )), }, []grpc.ServerOption{ - //nolint:staticcheck // Interceptors are deprecated and will be removed in the next release. - grpc.UnaryInterceptor(otelgrpc.UnaryServerInterceptor( - otelgrpc.WithTracerProvider(serverUnaryTP), - otelgrpc.WithMeterProvider(serverUnaryMP), - otelgrpc.WithMessageEvents(otelgrpc.ReceivedEvents, otelgrpc.SentEvents), - )), //nolint:staticcheck // Interceptors are deprecated and will be removed in the next release. grpc.StreamInterceptor(otelgrpc.StreamServerInterceptor( otelgrpc.WithTracerProvider(serverStreamTP), @@ -135,11 +121,6 @@ func TestInterceptors(t *testing.T) { checkStreamClientSpans(t, clientStreamSR.Ended(), listener.Addr().String()) }) - t.Run("UnaryServerSpans", func(t *testing.T) { - checkUnaryServerSpans(t, serverUnarySR.Ended()) - checkUnaryServerRecords(t, serverUnaryMetricReader) - }) - t.Run("StreamServerSpans", func(t *testing.T) { checkStreamServerSpans(t, serverStreamSR.Ended()) }) @@ -568,74 +549,6 @@ func checkStreamServerSpans(t *testing.T, spans []trace.ReadOnlySpan) { }, pingPong.Attributes()) } -func checkUnaryServerSpans(t *testing.T, spans []trace.ReadOnlySpan) { - require.Len(t, spans, 2) - - emptySpan := spans[0] - assert.False(t, emptySpan.EndTime().IsZero()) - assert.Equal(t, "grpc.testing.TestService/EmptyCall", emptySpan.Name()) - assertEvents(t, []trace.Event{ - { - Name: "message", - Attributes: []attribute.KeyValue{ - otelgrpc.RPCMessageIDKey.Int(1), - otelgrpc.RPCMessageTypeKey.String("RECEIVED"), - }, - }, - { - Name: "message", - Attributes: []attribute.KeyValue{ - otelgrpc.RPCMessageIDKey.Int(1), - otelgrpc.RPCMessageTypeKey.String("SENT"), - }, - }, - }, emptySpan.Events()) - - port, ok := findAttribute(emptySpan.Attributes(), semconv.NetSockPeerPortKey) - assert.True(t, ok) - assert.ElementsMatch(t, []attribute.KeyValue{ - semconv.RPCMethod("EmptyCall"), - semconv.RPCService("grpc.testing.TestService"), - otelgrpc.RPCSystemGRPC, - otelgrpc.GRPCStatusCodeKey.Int64(int64(codes.OK)), - semconv.NetSockPeerAddr("127.0.0.1"), - port, - }, emptySpan.Attributes()) - - largeSpan := spans[1] - assert.False(t, largeSpan.EndTime().IsZero()) - assert.Equal(t, "grpc.testing.TestService/UnaryCall", largeSpan.Name()) - assertEvents(t, []trace.Event{ - { - Name: "message", - Attributes: []attribute.KeyValue{ - otelgrpc.RPCMessageIDKey.Int(1), - otelgrpc.RPCMessageTypeKey.String("RECEIVED"), - // largeReqSize from "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/test" + 12 (overhead). - }, - }, - { - Name: "message", - Attributes: []attribute.KeyValue{ - otelgrpc.RPCMessageIDKey.Int(1), - otelgrpc.RPCMessageTypeKey.String("SENT"), - // largeRespSize from "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/test" + 8 (overhead). - }, - }, - }, largeSpan.Events()) - - port, ok = findAttribute(largeSpan.Attributes(), semconv.NetSockPeerPortKey) - assert.True(t, ok) - assert.ElementsMatch(t, []attribute.KeyValue{ - semconv.RPCMethod("UnaryCall"), - semconv.RPCService("grpc.testing.TestService"), - otelgrpc.RPCSystemGRPC, - otelgrpc.GRPCStatusCodeKey.Int64(int64(codes.OK)), - semconv.NetSockPeerAddr("127.0.0.1"), - port, - }, largeSpan.Attributes()) -} - func assertEvents(t *testing.T, expected, actual []trace.Event) bool { //nolint:unparam if !assert.Len(t, actual, len(expected)) { return false @@ -654,47 +567,6 @@ func assertEvents(t *testing.T, expected, actual []trace.Event) bool { //nolint: return !failed } -func checkUnaryServerRecords(t *testing.T, reader metric.Reader) { - rm := metricdata.ResourceMetrics{} - err := reader.Collect(context.Background(), &rm) - assert.NoError(t, err) - require.Len(t, rm.ScopeMetrics, 1) - - want := metricdata.ScopeMetrics{ - Scope: wantInstrumentationScope, - Metrics: []metricdata.Metrics{ - { - Name: "rpc.server.duration", - Description: "Measures the duration of inbound RPC.", - Unit: "ms", - Data: metricdata.Histogram[float64]{ - Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[float64]{ - { - Attributes: attribute.NewSet( - semconv.RPCMethod("EmptyCall"), - semconv.RPCService("grpc.testing.TestService"), - otelgrpc.RPCSystemGRPC, - otelgrpc.GRPCStatusCodeKey.Int64(int64(codes.OK)), - ), - }, - { - Attributes: attribute.NewSet( - semconv.RPCMethod("UnaryCall"), - semconv.RPCService("grpc.testing.TestService"), - otelgrpc.RPCSystemGRPC, - otelgrpc.GRPCStatusCodeKey.Int64(int64(codes.OK)), - ), - }, - }, - }, - }, - }, - } - - metricdatatest.AssertEqual(t, want, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} - func findAttribute(kvs []attribute.KeyValue, key attribute.Key) (attribute.KeyValue, bool) { //nolint:unparam for _, kv := range kvs { if kv.Key == key { diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go index 056e08b00c2..97ec66657f6 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/test/interceptor_test.go @@ -16,9 +16,6 @@ import ( "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/internal/test" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" - "go.opentelemetry.io/otel/sdk/metric" - "go.opentelemetry.io/otel/sdk/metric/metricdata" - "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" semconv "go.opentelemetry.io/otel/semconv/v1.17.0" @@ -852,119 +849,6 @@ func assertServerSpan(t *testing.T, wantSpanCode codes.Code, wantSpanStatusDescr assert.Equal(t, attribute.Int64Value(int64(wantGrpcCode)), codeAttr.Value) } -// TestUnaryServerInterceptor tests the server interceptor for unary RPCs. -func TestUnaryServerInterceptor(t *testing.T) { - for _, check := range serverChecks { - name := check.grpcCode.String() - t.Run(name, func(t *testing.T) { - t.Setenv("OTEL_METRICS_EXEMPLAR_FILTER", "always_off") - sr := tracetest.NewSpanRecorder() - tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) - - mr := metric.NewManualReader() - mp := metric.NewMeterProvider(metric.WithReader(mr)) - - //nolint:staticcheck // Interceptors are deprecated and will be removed in the next release. - usi := otelgrpc.UnaryServerInterceptor( - otelgrpc.WithTracerProvider(tp), - otelgrpc.WithMeterProvider(mp), - ) - - serviceName := "TestGrpcService" - methodName := serviceName + "/" + name - fullMethodName := "/" + methodName - // call the unary interceptor - grpcErr := status.Error(check.grpcCode, check.grpcCode.String()) - handler := func(_ context.Context, _ interface{}) (interface{}, error) { - return nil, grpcErr - } - _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{FullMethod: fullMethodName}, handler) - assert.Equal(t, grpcErr, err) - - // validate span - span, ok := getSpanFromRecorder(sr, methodName) - require.True(t, ok, "missing span %s", methodName) - assertServerSpan(t, check.wantSpanCode, check.wantSpanStatusDescription, check.grpcCode, span) - - // validate metric - assertServerMetrics(t, mr, serviceName, name, check.grpcCode) - }) - } -} - -func TestUnaryServerInterceptorEvents(t *testing.T) { - testCases := []struct { - Name string - Events []otelgrpc.Event - }{ - { - Name: "No events", - Events: []otelgrpc.Event{}, - }, - { - Name: "With only received events", - Events: []otelgrpc.Event{otelgrpc.ReceivedEvents}, - }, - { - Name: "With only sent events", - Events: []otelgrpc.Event{otelgrpc.SentEvents}, - }, - { - Name: "With both events", - Events: []otelgrpc.Event{otelgrpc.ReceivedEvents, otelgrpc.SentEvents}, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.Name, func(t *testing.T) { - sr := tracetest.NewSpanRecorder() - tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr)) - opts := []otelgrpc.Option{ - otelgrpc.WithTracerProvider(tp), - } - if len(testCase.Events) > 0 { - opts = append(opts, otelgrpc.WithMessageEvents(testCase.Events...)) - } - //nolint:staticcheck // Interceptors are deprecated and will be removed in the next release. - usi := otelgrpc.UnaryServerInterceptor(opts...) - grpcCode := grpc_codes.OK - name := grpcCode.String() - // call the unary interceptor - grpcErr := status.Error(grpcCode, name) - handler := func(_ context.Context, _ interface{}) (interface{}, error) { - return nil, grpcErr - } - _, err := usi(context.Background(), &grpc_testing.SimpleRequest{}, &grpc.UnaryServerInfo{FullMethod: name}, handler) - assert.Equal(t, grpcErr, err) - - // validate span - span, ok := getSpanFromRecorder(sr, name) - require.True(t, ok, "missing span %s", name) - - // validate events and their attributes - if len(testCase.Events) == 0 { - assert.Empty(t, span.Events()) - } else { - assert.Len(t, span.Events(), len(testCase.Events)) - for i, event := range testCase.Events { - switch event { - case otelgrpc.ReceivedEvents: - assert.ElementsMatch(t, []attribute.KeyValue{ - attribute.Key("message.type").String("RECEIVED"), - attribute.Key("message.id").Int(1), - }, span.Events()[i].Attributes) - case otelgrpc.SentEvents: - assert.ElementsMatch(t, []attribute.KeyValue{ - attribute.Key("message.type").String("SENT"), - attribute.Key("message.id").Int(1), - }, span.Events()[i].Attributes) - } - } - } - }) - } -} - type mockServerStream struct { grpc.ServerStream } @@ -1078,37 +962,6 @@ func TestStreamServerInterceptorEvents(t *testing.T) { } } -func assertServerMetrics(t *testing.T, reader metric.Reader, serviceName, name string, code grpc_codes.Code) { - want := metricdata.ScopeMetrics{ - Scope: wantInstrumentationScope, - Metrics: []metricdata.Metrics{ - { - Name: "rpc.server.duration", - Description: "Measures the duration of inbound RPC.", - Unit: "ms", - Data: metricdata.Histogram[float64]{ - Temporality: metricdata.CumulativeTemporality, - DataPoints: []metricdata.HistogramDataPoint[float64]{ - { - Attributes: attribute.NewSet( - semconv.RPCMethod(name), - semconv.RPCService(serviceName), - otelgrpc.RPCSystemGRPC, - otelgrpc.GRPCStatusCodeKey.Int64(int64(code)), - ), - }, - }, - }, - }, - }, - } - rm := metricdata.ResourceMetrics{} - err := reader.Collect(context.Background(), &rm) - assert.NoError(t, err) - require.Len(t, rm.ScopeMetrics, 1) - metricdatatest.AssertEqual(t, want, rm.ScopeMetrics[0], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue()) -} - func BenchmarkStreamClientInterceptor(b *testing.B) { listener, err := net.Listen("tcp", "127.0.0.1:0") require.NoError(b, err, "failed to open port")