From 8e5ab886f4a66e3735b1ad1e2533ab4f715020e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 19 Dec 2023 14:21:45 +0100 Subject: [PATCH 1/3] Fix interceptor order to make sure tracing is set up first That fixes a problem where the appctx logger was using a wrong trace id, not the one from the trace parent. --- pkg/rgrpc/rgrpc.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/pkg/rgrpc/rgrpc.go b/pkg/rgrpc/rgrpc.go index 381c468c2d..966d349ac4 100644 --- a/pkg/rgrpc/rgrpc.go +++ b/pkg/rgrpc/rgrpc.go @@ -325,25 +325,24 @@ func (s *Server) getInterceptors(unprotected []string) ([]grpc.ServerOption, err return nil, errors.Wrap(err, "rgrpc: error creating unary auth interceptor") } - unaryInterceptors := []grpc.UnaryServerInterceptor{authUnary} - for _, t := range unaryTriples { - unaryInterceptors = append(unaryInterceptors, t.Interceptor) - s.log.Info().Msgf("rgrpc: chaining grpc unary interceptor %s with priority %d", t.Name, t.Priority) - } - - unaryInterceptors = append(unaryInterceptors, + unaryInterceptors := []grpc.UnaryServerInterceptor{ otelgrpc.UnaryServerInterceptor( otelgrpc.WithTracerProvider(s.tracerProvider), - otelgrpc.WithPropagators(rtrace.Propagator)), - ) - - unaryInterceptors = append([]grpc.UnaryServerInterceptor{ + otelgrpc.WithPropagators(rtrace.Propagator), + ), appctx.NewUnary(s.log, s.tracerProvider), token.NewUnary(), useragent.NewUnary(), log.NewUnary(), recovery.NewUnary(), - }, unaryInterceptors...) + authUnary, + } + + for _, t := range unaryTriples { + unaryInterceptors = append(unaryInterceptors, t.Interceptor) + s.log.Info().Msgf("rgrpc: chaining grpc unary interceptor %s with priority %d", t.Name, t.Priority) + } + unaryChain := grpc_middleware.ChainUnaryServer(unaryInterceptors...) streamTriples := []*streamInterceptorTriple{} From fce42c61144356de8914f4db6b936e9d61747562 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 19 Dec 2023 15:19:30 +0100 Subject: [PATCH 2/3] Add the opentelemetry interceptor to the stream chain as well --- pkg/rgrpc/rgrpc.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/pkg/rgrpc/rgrpc.go b/pkg/rgrpc/rgrpc.go index 966d349ac4..4168c4f064 100644 --- a/pkg/rgrpc/rgrpc.go +++ b/pkg/rgrpc/rgrpc.go @@ -371,20 +371,23 @@ func (s *Server) getInterceptors(unprotected []string) ([]grpc.ServerOption, err return nil, errors.Wrap(err, "rgrpc: error creating stream auth interceptor") } - streamInterceptors := []grpc.StreamServerInterceptor{authStream} - for _, t := range streamTriples { - streamInterceptors = append(streamInterceptors, t.Interceptor) - s.log.Info().Msgf("rgrpc: chaining grpc streaming interceptor %s with priority %d", t.Name, t.Priority) - } - - streamInterceptors = append([]grpc.StreamServerInterceptor{ - authStream, + streamInterceptors := []grpc.StreamServerInterceptor{ + otelgrpc.StreamServerInterceptor( + otelgrpc.WithTracerProvider(s.tracerProvider), + otelgrpc.WithPropagators(rtrace.Propagator), + ), appctx.NewStream(s.log, s.tracerProvider), token.NewStream(), useragent.NewStream(), log.NewStream(), recovery.NewStream(), - }, streamInterceptors...) + authStream, + } + + for _, t := range streamTriples { + streamInterceptors = append(streamInterceptors, t.Interceptor) + s.log.Info().Msgf("rgrpc: chaining grpc streaming interceptor %s with priority %d", t.Name, t.Priority) + } streamChain := grpc_middleware.ChainStreamServer(streamInterceptors...) opts := []grpc.ServerOption{ From 719d82e61850899a22b4f6f44429da8f5d3e46ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Tue, 19 Dec 2023 15:04:31 +0100 Subject: [PATCH 3/3] Add changelog --- changelog/unreleased/fix-disconnected-traces.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/fix-disconnected-traces.md diff --git a/changelog/unreleased/fix-disconnected-traces.md b/changelog/unreleased/fix-disconnected-traces.md new file mode 100644 index 0000000000..eb4242db32 --- /dev/null +++ b/changelog/unreleased/fix-disconnected-traces.md @@ -0,0 +1,5 @@ +Bugfix: Fix disconnected traces + +We fixed a problem where the appctx logger was using a new traceid instead of picking up the one from the trace parent. + +https://github.com/cs3org/reva/pull/4423