Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

otelgrpc: Deprecate interceptors in favor of stats handlers #4534

Merged
merged 4 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Add metrics support (No-op, OTLP and Prometheus) to `go.opentelemetry.io/contrib/exporters/autoexport`. (#4229, #4479)
- Add support for `console` span exporter and metrics exporter in `go.opentelemetry.io/contrib/exporters/autoexport`. (#4486)
- Set unit and description on all instruments in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#4500)
- Add metric support for `grpc.StatsHandler` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#4356)
- Add metric support for `grpc.StatsHandler` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#4356)

### Changed

Expand All @@ -27,6 +27,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
### Deprecated

- In `go.opentelemetry.io/contrib/exporters/autoexport`, `Option` was renamed to `SpanOption`. The old name is deprecated but continues to be supported as an alias. (#4229)
- The interceptors (`UnaryClientInterceptor`, `StreamClientInterceptor`, `UnaryServerInterceptor`, `StreamServerInterceptor`, `WithInterceptorFilter`) are deprecated. Use stats handlers (`NewClientHandler`, `NewServerHandler`) instead. (#4534)

### Fixed

Expand Down
2 changes: 2 additions & 0 deletions instrumentation/google.golang.org/grpc/otelgrpc/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ func (o tracerProviderOption) apply(c *config) {
}

// WithInterceptorFilter returns an Option to use the request filter.
//
// Deprecated: Use stats handlers instead.
func WithInterceptorFilter(f Filter) Option {
return interceptorFilterOption{f: f}
}
Expand Down
29 changes: 3 additions & 26 deletions instrumentation/google.golang.org/grpc/otelgrpc/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,33 +13,10 @@
// limitations under the License.

/*
Package otelgrpc is the instrumentation library for [google.golang.org/grpc]
Package otelgrpc is the instrumentation library for [google.golang.org/grpc].

For now you can instrument your program which use [google.golang.org/grpc] in two ways:
Use [NewClientHandler] with [grpc.WithStatsHandler] to instrument a gRPC client.

- by [grpc.UnaryClientInterceptor], [grpc.UnaryServerInterceptor], [grpc.StreamClientInterceptor], [grpc.StreamServerInterceptor]
- by [stats.Handler]

Notice: Do not use both interceptors and [stats.Handler] at the same time! If so, you will get duplicated spans and the parent/child relationships between spans will also be broken.

We strongly still recommand you to use [stats.Handler], mainly for two reasons:

Functional advantages: [stats.Handler] has more information for user to build more flexible and granular metric, for example

- multiple different types of represent "data length": In [stats.InPayload], there exists "Length", "CompressedLength", "WireLength" to denote the size of uncompressed, compressed payload data, with or without framing data. But in interceptors, we can only got uncompressed data, and this feature is also removed due to performance problem.

- more accurate timestamp: [stats.InPayload]'s "RecvTime" and [stats.OutPayload]'s "SentTime" records more accurate timestamp that server got and sent the message, the timestamp recorded by interceptors depends on the location of this interceptors in the total interceptor chain.

- some other use cases: for example, catch failure of decoding message.

Performance advantages: If too many interceptors are registered in a service, the interceptor chain can become too long, which increases the latency and processing time of the entire RPC call.

[stats.Handler]: https://pkg.go.dev/google.golang.org/grpc/stats#Handler
[grpc.UnaryClientInterceptor]: https://pkg.go.dev/google.golang.org/grpc#UnaryClientInterceptor
[grpc.UnaryServerInterceptor]: https://pkg.go.dev/google.golang.org/grpc#UnaryServerInterceptor
[grpc.StreamClientInterceptor]: https://pkg.go.dev/google.golang.org/grpc#StreamClientInterceptor
[grpc.StreamServerInterceptor]: https://pkg.go.dev/google.golang.org/grpc#StreamServerInterceptor
[stats.OutPayload]: https://pkg.go.dev/google.golang.org/grpc/stats#OutPayload
[stats.InPayload]: https://pkg.go.dev/google.golang.org/grpc/stats#InPayload
Use [NewServerHandler] with [grpc.StatsHandler] to instrument a gRPC server.
*/
package otelgrpc // import "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,10 @@ import (
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
)

func ExampleStreamClientInterceptor() {
_, _ = grpc.Dial("localhost", grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor()))
func ExampleNewClientHandler() {
_, _ = grpc.Dial("localhost", grpc.WithStatsHandler(otelgrpc.NewClientHandler()))
}

func ExampleUnaryClientInterceptor() {
_, _ = grpc.Dial("localhost", grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()))
}

func ExampleStreamServerInterceptor() {
_ = grpc.NewServer(grpc.StreamInterceptor(otelgrpc.StreamServerInterceptor()))
}

func ExampleUnaryServerInterceptor() {
_ = grpc.NewServer(grpc.UnaryInterceptor(otelgrpc.UnaryServerInterceptor()))
func ExampleNewServerHandler() {
_ = grpc.NewServer(grpc.StatsHandler(otelgrpc.NewServerHandler()))
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ var (

// UnaryClientInterceptor returns a grpc.UnaryClientInterceptor suitable
// for use in a grpc.Dial call.
//
// Deprecated: Use [NewClientHandler] instead.
func UnaryClientInterceptor(opts ...Option) grpc.UnaryClientInterceptor {
cfg := newConfig(opts, "client")
tracer := cfg.TracerProvider.Tracer(
Expand Down Expand Up @@ -254,6 +256,8 @@ func (w *clientStream) sendStreamEvent(eventType streamEventType, err error) {

// StreamClientInterceptor returns a grpc.StreamClientInterceptor suitable
// for use in a grpc.Dial call.
//
// Deprecated: Use [NewClientHandler] instead.
func StreamClientInterceptor(opts ...Option) grpc.StreamClientInterceptor {
cfg := newConfig(opts, "client")
tracer := cfg.TracerProvider.Tracer(
Expand Down Expand Up @@ -324,6 +328,8 @@ 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(
Expand Down Expand Up @@ -445,6 +451,8 @@ func wrapServerStream(ctx context.Context, ss grpc.ServerStream, cfg *config) *s

// StreamServerInterceptor returns a grpc.StreamServerInterceptor suitable
// for use in a grpc.NewServer call.
//
// Deprecated: Use [NewServerHandler] instead.
func StreamServerInterceptor(opts ...Option) grpc.StreamServerInterceptor {
cfg := newConfig(opts, "server")
tracer := cfg.TracerProvider.Tracer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,21 +108,25 @@ func TestInterceptors(t *testing.T) {
listener, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err, "failed to open port")
err = newGrpcTest(listener, []grpc.DialOption{
//nolint:staticcheck // Interceptors are deprecated and will be removed in the next release.
grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor(
otelgrpc.WithTracerProvider(clientUnaryTP),
otelgrpc.WithMessageEvents(otelgrpc.ReceivedEvents, otelgrpc.SentEvents),
)),
//nolint:staticcheck // Interceptors are deprecated and will be removed in the next release.
grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor(
otelgrpc.WithTracerProvider(clientStreamTP),
otelgrpc.WithMessageEvents(otelgrpc.ReceivedEvents, otelgrpc.SentEvents),
)),
},
[]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),
otelgrpc.WithMessageEvents(otelgrpc.ReceivedEvents, otelgrpc.SentEvents),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,23 @@ func TestUnaryClientInterceptor(t *testing.T) {

sr := tracetest.NewSpanRecorder()
tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr))
//nolint:staticcheck // Interceptors are deprecated and will be removed in the next release.
unaryInterceptor := otelgrpc.UnaryClientInterceptor(
otelgrpc.WithTracerProvider(tp),
otelgrpc.WithMessageEvents(otelgrpc.ReceivedEvents, otelgrpc.SentEvents),
otelgrpc.WithSpanOptions(oteltrace.WithAttributes(attribute.Bool("custom", true))),
)
//nolint:staticcheck // Interceptors are deprecated and will be removed in the next release.
unaryInterceptorOnlySentEvents := otelgrpc.UnaryClientInterceptor(
otelgrpc.WithTracerProvider(tp),
otelgrpc.WithMessageEvents(otelgrpc.SentEvents),
)
//nolint:staticcheck // Interceptors are deprecated and will be removed in the next release.
unaryInterceptorOnlyReceivedEvents := otelgrpc.UnaryClientInterceptor(
otelgrpc.WithTracerProvider(tp),
otelgrpc.WithMessageEvents(otelgrpc.ReceivedEvents),
)
//nolint:staticcheck // Interceptors are deprecated and will be removed in the next release.
unaryInterceptorNoEvents := otelgrpc.UnaryClientInterceptor(
otelgrpc.WithTracerProvider(tp),
)
Expand Down Expand Up @@ -405,6 +409,7 @@ func createInterceptedStreamClient(t *testing.T, method string, opts clientStrea
if len(opts.Events) > 0 {
interceptorOpts = append(interceptorOpts, otelgrpc.WithMessageEvents(opts.Events...))
}
//nolint:staticcheck // Interceptors are deprecated and will be removed in the next release.
streamCI := otelgrpc.StreamClientInterceptor(interceptorOpts...)

streamClient, err := streamCI(
Expand Down Expand Up @@ -651,6 +656,7 @@ func TestStreamClientInterceptorCancelContext(t *testing.T) {
// tracer
sr := tracetest.NewSpanRecorder()
tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr))
//nolint:staticcheck // Interceptors are deprecated and will be removed in the next release.
streamCI := otelgrpc.StreamClientInterceptor(
otelgrpc.WithTracerProvider(tp),
otelgrpc.WithMessageEvents(otelgrpc.ReceivedEvents, otelgrpc.SentEvents),
Expand Down Expand Up @@ -714,6 +720,7 @@ func TestStreamClientInterceptorWithError(t *testing.T) {
// tracer
sr := tracetest.NewSpanRecorder()
tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr))
//nolint:staticcheck // Interceptors are deprecated and will be removed in the next release.
streamCI := otelgrpc.StreamClientInterceptor(
otelgrpc.WithTracerProvider(tp),
otelgrpc.WithMessageEvents(otelgrpc.ReceivedEvents, otelgrpc.SentEvents),
Expand Down Expand Up @@ -877,6 +884,7 @@ func TestUnaryServerInterceptor(t *testing.T) {
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),
Expand Down Expand Up @@ -937,6 +945,7 @@ func TestUnaryServerInterceptorEvents(t *testing.T) {
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()
Expand Down Expand Up @@ -998,6 +1007,7 @@ func TestStreamServerInterceptor(t *testing.T) {
sr := tracetest.NewSpanRecorder()
tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr))

//nolint:staticcheck // Interceptors are deprecated and will be removed in the next release.
usi := otelgrpc.StreamServerInterceptor(
otelgrpc.WithTracerProvider(tp),
)
Expand Down Expand Up @@ -1039,6 +1049,7 @@ func TestStreamServerInterceptorEvents(t *testing.T) {
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.StreamServerInterceptor(opts...)
stream := &mockServerStream{}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ func main() {
// handler to enable tracing.
log.Println("Starting the GRPC server, and using the OpenCensus binary propagation format.")
s := grpc.NewServer(
grpc.UnaryInterceptor(otelgrpc.UnaryServerInterceptor(otelgrpc.WithPropagators(opencensus.Binary{}))),
grpc.StreamInterceptor(otelgrpc.StreamServerInterceptor(otelgrpc.WithPropagators(opencensus.Binary{}))))
grpc.StatsHandler(otelgrpc.NewServerHandler(otelgrpc.WithPropagators(opencensus.Binary{}))))
pb.RegisterGreeterServer(s, &server{})

if err := s.Serve(lis); err != nil {
Expand Down
Loading