Skip to content

Commit

Permalink
fix: stop creating new otel interceptor to avoid memory leak (#15174)
Browse files Browse the repository at this point in the history
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
  • Loading branch information
alexmt authored Aug 23, 2023
1 parent 32f10de commit 24c080b
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 12 deletions.
5 changes: 2 additions & 3 deletions cmpserver/apiclient/clientset.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
log "github.com/sirupsen/logrus"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"

Expand Down Expand Up @@ -47,8 +46,8 @@ func NewConnection(address string) (*grpc.ClientConn, error) {
grpc.WithStreamInterceptor(grpc_retry.StreamClientInterceptor(retryOpts...)),
grpc.WithUnaryInterceptor(grpc_middleware.ChainUnaryClient(unaryInterceptors...)),
grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(MaxGRPCMessageSize), grpc.MaxCallSendMsgSize(MaxGRPCMessageSize)),
grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()),
grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor()),
grpc.WithUnaryInterceptor(grpc_util.OTELUnaryClientInterceptor()),
grpc.WithStreamInterceptor(grpc_util.OTELStreamClientInterceptor()),
}

dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
Expand Down
5 changes: 2 additions & 3 deletions pkg/apiclient/apiclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
"github.com/hashicorp/go-retryablehttp"
log "github.com/sirupsen/logrus"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"golang.org/x/oauth2"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -525,8 +524,8 @@ func (c *client) newConn() (*grpc.ClientConn, io.Closer, error) {
dialOpts = append(dialOpts, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(MaxGRPCMessageSize), grpc.MaxCallSendMsgSize(MaxGRPCMessageSize)))
dialOpts = append(dialOpts, grpc.WithStreamInterceptor(grpc_retry.StreamClientInterceptor(retryOpts...)))
dialOpts = append(dialOpts, grpc.WithUnaryInterceptor(grpc_middleware.ChainUnaryClient(grpc_retry.UnaryClientInterceptor(retryOpts...))))
dialOpts = append(dialOpts, grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()))
dialOpts = append(dialOpts, grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor()))
dialOpts = append(dialOpts, grpc.WithUnaryInterceptor(grpc_util.OTELUnaryClientInterceptor()))
dialOpts = append(dialOpts, grpc.WithStreamInterceptor(grpc_util.OTELStreamClientInterceptor()))

ctx := context.Background()

Expand Down
5 changes: 2 additions & 3 deletions reposerver/apiclient/clientset.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
log "github.com/sirupsen/logrus"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"
Expand Down Expand Up @@ -67,8 +66,8 @@ func NewConnection(address string, timeoutSeconds int, tlsConfig *TLSConfigurati
grpc.WithStreamInterceptor(grpc_retry.StreamClientInterceptor(retryOpts...)),
grpc.WithUnaryInterceptor(grpc_middleware.ChainUnaryClient(unaryInterceptors...)),
grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(MaxGRPCMessageSize), grpc.MaxCallSendMsgSize(MaxGRPCMessageSize)),
grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()),
grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor()),
grpc.WithUnaryInterceptor(argogrpc.OTELUnaryClientInterceptor()),
grpc.WithStreamInterceptor(argogrpc.OTELStreamClientInterceptor()),
}

tlsC := &tls.Config{}
Expand Down
5 changes: 2 additions & 3 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ import (
"github.com/argoproj/argo-cd/v2/pkg/apiclient"
accountpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/account"
applicationpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/application"

applicationsetpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/applicationset"
certificatepkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/certificate"
clusterpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster"
Expand Down Expand Up @@ -435,8 +434,8 @@ func (a *ArgoCDServer) Listen() (*Listeners, error) {
var dOpts []grpc.DialOption
dOpts = append(dOpts, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(apiclient.MaxGRPCMessageSize)))
dOpts = append(dOpts, grpc.WithUserAgent(fmt.Sprintf("%s/%s", common.ArgoCDUserAgentName, common.GetVersion().Version)))
dOpts = append(dOpts, grpc.WithUnaryInterceptor(otelgrpc.UnaryClientInterceptor()))
dOpts = append(dOpts, grpc.WithStreamInterceptor(otelgrpc.StreamClientInterceptor()))
dOpts = append(dOpts, grpc.WithUnaryInterceptor(grpc_util.OTELUnaryClientInterceptor()))
dOpts = append(dOpts, grpc.WithStreamInterceptor(grpc_util.OTELStreamClientInterceptor()))
if a.useTLS() {
// The following sets up the dial Options for grpc-gateway to talk to gRPC server over TLS.
// grpc-gateway is just translating HTTP/HTTPS requests as gRPC requests over localhost,
Expand Down
33 changes: 33 additions & 0 deletions util/grpc/trace.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package grpc

import (
"sync"

"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"google.golang.org/grpc"
)

var (
otelUnaryInterceptor grpc.UnaryClientInterceptor
otelStreamInterceptor grpc.StreamClientInterceptor
interceptorsInitialized = sync.Once{}
)

// otel interceptors must be created once to avoid memory leak
// see https://github.com/open-telemetry/opentelemetry-go-contrib/issues/4226 for details
func ensureInitialized() {
interceptorsInitialized.Do(func() {
otelUnaryInterceptor = otelgrpc.UnaryClientInterceptor()
otelStreamInterceptor = otelgrpc.StreamClientInterceptor()
})
}

func OTELUnaryClientInterceptor() grpc.UnaryClientInterceptor {
ensureInitialized()
return otelUnaryInterceptor
}

func OTELStreamClientInterceptor() grpc.StreamClientInterceptor {
ensureInitialized()
return otelStreamInterceptor
}

0 comments on commit 24c080b

Please sign in to comment.