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

chore(migrate): migrate otelgrpc pkg interceptor to stats handler(#18258) #18366

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions cmpserver/apiclient/clientset.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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 @@ -48,8 +49,7 @@ 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(grpc_util.OTELUnaryClientInterceptor()),
grpc.WithStreamInterceptor(grpc_util.OTELStreamClientInterceptor()),
grpc.WithStatsHandler(otelgrpc.NewClientHandler()),
}

dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
Expand Down
3 changes: 1 addition & 2 deletions cmpserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,11 @@ func NewServer(initConstants plugin.CMPServerInitConstants) (*ArgoCDCMPServer, e

serverLog := log.NewEntry(log.StandardLogger())
streamInterceptors := []grpc.StreamServerInterceptor{
otelgrpc.StreamServerInterceptor(), //nolint:staticcheck // TODO: ignore SA1019 for depreciation: see https://github.com/argoproj/argo-cd/issues/18258
grpc_logrus.StreamServerInterceptor(serverLog),
grpc_prometheus.StreamServerInterceptor,
grpc_util.PanicLoggerStreamServerInterceptor(serverLog),
}
unaryInterceptors := []grpc.UnaryServerInterceptor{
otelgrpc.UnaryServerInterceptor(), //nolint:staticcheck // TODO: ignore SA1019 for depreciation: see https://github.com/argoproj/argo-cd/issues/18258
grpc_logrus.UnaryServerInterceptor(serverLog),
grpc_prometheus.UnaryServerInterceptor,
grpc_util.PanicLoggerUnaryServerInterceptor(serverLog),
Expand All @@ -69,6 +67,7 @@ func NewServer(initConstants plugin.CMPServerInitConstants) (*ArgoCDCMPServer, e
MinTime: common.GetGRPCKeepAliveEnforcementMinimum(),
},
),
grpc.StatsHandler(otelgrpc.NewServerHandler()),
}

return &ArgoCDCMPServer{
Expand Down
4 changes: 2 additions & 2 deletions pkg/apiclient/apiclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ 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 @@ -520,8 +521,7 @@ 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(grpc_util.OTELUnaryClientInterceptor()))
dialOpts = append(dialOpts, grpc.WithStreamInterceptor(grpc_util.OTELStreamClientInterceptor()))
dialOpts = append(dialOpts, grpc.WithStatsHandler(otelgrpc.NewClientHandler()))

ctx := context.Background()

Expand Down
4 changes: 2 additions & 2 deletions reposerver/apiclient/clientset.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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 @@ -66,8 +67,7 @@ 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(argogrpc.OTELUnaryClientInterceptor()),
grpc.WithStreamInterceptor(argogrpc.OTELStreamClientInterceptor()),
grpc.WithStatsHandler(otelgrpc.NewClientHandler()),
}

tlsC := &tls.Config{}
Expand Down
6 changes: 2 additions & 4 deletions reposerver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@ import (
"os"
"path/filepath"

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

grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
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/health"
Expand Down Expand Up @@ -70,13 +69,11 @@ func NewServer(metricsServer *metrics.MetricsServer, cache *reposervercache.Cach

serverLog := log.NewEntry(log.StandardLogger())
streamInterceptors := []grpc.StreamServerInterceptor{
otelgrpc.StreamServerInterceptor(), //nolint:staticcheck // TODO: ignore SA1019 for depreciation: see https://github.com/argoproj/argo-cd/issues/18258
grpc_logrus.StreamServerInterceptor(serverLog),
grpc_prometheus.StreamServerInterceptor,
grpc_util.PanicLoggerStreamServerInterceptor(serverLog),
}
unaryInterceptors := []grpc.UnaryServerInterceptor{
otelgrpc.UnaryServerInterceptor(), //nolint:staticcheck // TODO: ignore SA1019 for depreciation: see https://github.com/argoproj/argo-cd/issues/18258
grpc_logrus.UnaryServerInterceptor(serverLog),
grpc_prometheus.UnaryServerInterceptor,
grpc_util.PanicLoggerUnaryServerInterceptor(serverLog),
Expand All @@ -93,6 +90,7 @@ func NewServer(metricsServer *metrics.MetricsServer, cache *reposervercache.Cach
MinTime: common.GetGRPCKeepAliveEnforcementMinimum(),
},
),
grpc.StatsHandler(otelgrpc.NewServerHandler()),
}

// We do allow for non-TLS servers to be created, in case of mTLS will be
Expand Down
3 changes: 2 additions & 1 deletion server/application/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/argoproj/gitops-engine/pkg/utils/text"
"github.com/argoproj/pkg/sync"
jsonpatch "github.com/evanphx/json-patch"
grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand Down Expand Up @@ -647,7 +648,7 @@ func (s *Server) GetManifestsWithFiles(stream application.ApplicationService_Get
ProjectSourceRepos: proj.Spec.SourceRepos,
}

repoStreamClient, err := client.GenerateManifestWithFiles(stream.Context())
repoStreamClient, err := client.GenerateManifestWithFiles(stream.Context(), grpc_retry.Disable())
if err != nil {
return fmt.Errorf("error opening stream: %w", err)
}
Expand Down
7 changes: 3 additions & 4 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,8 +472,7 @@ 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(grpc_util.OTELUnaryClientInterceptor()))
dOpts = append(dOpts, grpc.WithStreamInterceptor(grpc_util.OTELStreamClientInterceptor()))
dOpts = append(dOpts, grpc.WithStatsHandler(otelgrpc.NewClientHandler()))
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 Expand Up @@ -787,7 +786,6 @@ func (a *ArgoCDServer) newGRPCServer() (*grpc.Server, application.AppResourceTre
// NOTE: notice we do not configure the gRPC server here with TLS (e.g. grpc.Creds(creds))
// This is because TLS handshaking occurs in cmux handling
sOpts = append(sOpts, grpc.StreamInterceptor(grpc_middleware.ChainStreamServer(
otelgrpc.StreamServerInterceptor(), //nolint:staticcheck // TODO: ignore SA1019 for depreciation: see https://github.com/argoproj/argo-cd/issues/18258
grpc_logrus.StreamServerInterceptor(a.log),
grpc_prometheus.StreamServerInterceptor,
grpc_auth.StreamServerInterceptor(a.Authenticate),
Expand All @@ -801,7 +799,6 @@ func (a *ArgoCDServer) newGRPCServer() (*grpc.Server, application.AppResourceTre
)))
sOpts = append(sOpts, grpc.UnaryInterceptor(grpc_middleware.ChainUnaryServer(
bug21955WorkaroundInterceptor,
otelgrpc.UnaryServerInterceptor(), //nolint:staticcheck // TODO: ignore SA1019 for depreciation: see https://github.com/argoproj/argo-cd/issues/18258
grpc_logrus.UnaryServerInterceptor(a.log),
grpc_prometheus.UnaryServerInterceptor,
grpc_auth.UnaryServerInterceptor(a.Authenticate),
Expand All @@ -813,6 +810,8 @@ func (a *ArgoCDServer) newGRPCServer() (*grpc.Server, application.AppResourceTre
grpc_util.ErrorCodeGitUnaryServerInterceptor(),
grpc_util.PanicLoggerUnaryServerInterceptor(a.log),
)))
sOpts = append(sOpts, grpc.StatsHandler(otelgrpc.NewServerHandler()))

grpcS := grpc.NewServer(sOpts...)

versionpkg.RegisterVersionServiceServer(grpcS, a.serviceSet.VersionService)
Expand Down
33 changes: 0 additions & 33 deletions util/grpc/trace.go

This file was deleted.