From ab6732ba3290441c39d703b83289df30991bfb90 Mon Sep 17 00:00:00 2001 From: Afzal Ansari Date: Fri, 4 Aug 2023 20:45:18 +0530 Subject: [PATCH] Replace OpenTracing instrumentation with OpenTelemetry in grpc storage plugin (#4611) ## Which problem is this PR solving? * Part of #3381 * This PR adds `otelgrpc` plugin to storage ## Short description of the changes - Replaces `otgrpc` plugin storage with otel grpc interceptors --------- Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com> Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com> --- plugin/storage/grpc/README.md | 4 ++-- plugin/storage/grpc/config/config.go | 28 ++++++++++++++++------------ plugin/storage/grpc/factory.go | 6 +++++- plugin/storage/grpc/factory_test.go | 3 ++- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/plugin/storage/grpc/README.md b/plugin/storage/grpc/README.md index ec0dcbf113e..1fee18db23a 100644 --- a/plugin/storage/grpc/README.md +++ b/plugin/storage/grpc/README.md @@ -200,8 +200,8 @@ grpc.ServeWithGRPCServer(&shared.PluginServices{ ArchiveStore: memStorePlugin, }, func(options []googleGRPC.ServerOption) *googleGRPC.Server { return plugin.DefaultGRPCServer([]googleGRPC.ServerOption{ - googleGRPC.UnaryInterceptor(otgrpc.OpenTracingServerInterceptor(tracer)), - googleGRPC.StreamInterceptor(otgrpc.OpenTracingStreamServerInterceptor(tracer)), + grpc.UnaryInterceptor(otelgrpc.UnaryServerInterceptor(otelgrpc.WithTracerProvider(tracerProvider))), + grpc.StreamInterceptor(otelgrpc.StreamServerInterceptor(otelgrpc.WithTracerProvider(tracerProvider))), }) }) ``` diff --git a/plugin/storage/grpc/config/config.go b/plugin/storage/grpc/config/config.go index c5cc8154a4f..b81647916fa 100644 --- a/plugin/storage/grpc/config/config.go +++ b/plugin/storage/grpc/config/config.go @@ -21,10 +21,10 @@ import ( "runtime" "time" - "github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc" "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-plugin" - "github.com/opentracing/opentracing-go" + "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" + "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/credentials" @@ -60,16 +60,16 @@ type ClientPluginServices struct { // PluginBuilder is used to create storage plugins. Implemented by Configuration. type PluginBuilder interface { - Build(logger *zap.Logger) (*ClientPluginServices, error) + Build(logger *zap.Logger, tracerProvider trace.TracerProvider) (*ClientPluginServices, error) Close() error } // Build instantiates a PluginServices -func (c *Configuration) Build(logger *zap.Logger) (*ClientPluginServices, error) { +func (c *Configuration) Build(logger *zap.Logger, tracerProvider trace.TracerProvider) (*ClientPluginServices, error) { if c.PluginBinary != "" { - return c.buildPlugin(logger) + return c.buildPlugin(logger, tracerProvider) } else { - return c.buildRemote(logger) + return c.buildRemote(logger, tracerProvider) } } @@ -82,10 +82,12 @@ func (c *Configuration) Close() error { return c.RemoteTLS.Close() } -func (c *Configuration) buildRemote(logger *zap.Logger) (*ClientPluginServices, error) { +func (c *Configuration) buildRemote(logger *zap.Logger, tracerProvider trace.TracerProvider) (*ClientPluginServices, error) { opts := []grpc.DialOption{ - grpc.WithUnaryInterceptor(otgrpc.OpenTracingClientInterceptor(opentracing.GlobalTracer())), - grpc.WithStreamInterceptor(otgrpc.OpenTracingStreamClientInterceptor(opentracing.GlobalTracer())), + grpc.WithUnaryInterceptor( + otelgrpc.UnaryClientInterceptor(otelgrpc.WithTracerProvider(tracerProvider))), + grpc.WithStreamInterceptor( + otelgrpc.StreamClientInterceptor(otelgrpc.WithTracerProvider(tracerProvider))), grpc.WithBlock(), } if c.RemoteTLS.Enabled { @@ -123,10 +125,12 @@ func (c *Configuration) buildRemote(logger *zap.Logger) (*ClientPluginServices, }, nil } -func (c *Configuration) buildPlugin(logger *zap.Logger) (*ClientPluginServices, error) { +func (c *Configuration) buildPlugin(logger *zap.Logger, tracerProvider trace.TracerProvider) (*ClientPluginServices, error) { opts := []grpc.DialOption{ - grpc.WithUnaryInterceptor(otgrpc.OpenTracingClientInterceptor(opentracing.GlobalTracer())), - grpc.WithStreamInterceptor(otgrpc.OpenTracingStreamClientInterceptor(opentracing.GlobalTracer())), + grpc.WithUnaryInterceptor( + otelgrpc.UnaryClientInterceptor(otelgrpc.WithTracerProvider(tracerProvider))), + grpc.WithStreamInterceptor( + otelgrpc.StreamClientInterceptor(otelgrpc.WithTracerProvider(tracerProvider))), } tenancyMgr := tenancy.NewManager(&c.TenancyOpts) diff --git a/plugin/storage/grpc/factory.go b/plugin/storage/grpc/factory.go index 32c7f24dbbc..24a4712ff8d 100644 --- a/plugin/storage/grpc/factory.go +++ b/plugin/storage/grpc/factory.go @@ -20,6 +20,8 @@ import ( "io" "github.com/spf13/viper" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/metrics" @@ -41,6 +43,7 @@ type Factory struct { options Options metricsFactory metrics.Factory logger *zap.Logger + tracerProvider trace.TracerProvider builder config.PluginBuilder @@ -77,8 +80,9 @@ func (f *Factory) InitFromOptions(opts Options) { // Initialize implements storage.Factory func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger) error { f.metricsFactory, f.logger = metricsFactory, logger + f.tracerProvider = otel.GetTracerProvider() - services, err := f.builder.Build(logger) + services, err := f.builder.Build(logger, f.tracerProvider) if err != nil { return fmt.Errorf("grpc-plugin builder failed to create a store: %w", err) } diff --git a/plugin/storage/grpc/factory_test.go b/plugin/storage/grpc/factory_test.go index d157fac1f31..9525a4735aa 100644 --- a/plugin/storage/grpc/factory_test.go +++ b/plugin/storage/grpc/factory_test.go @@ -21,6 +21,7 @@ import ( "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace" "go.uber.org/zap" "github.com/jaegertracing/jaeger/pkg/config" @@ -43,7 +44,7 @@ type mockPluginBuilder struct { err error } -func (b *mockPluginBuilder) Build(logger *zap.Logger) (*grpcConfig.ClientPluginServices, error) { +func (b *mockPluginBuilder) Build(logger *zap.Logger, tracer trace.TracerProvider) (*grpcConfig.ClientPluginServices, error) { if b.err != nil { return nil, b.err }