Skip to content

Commit

Permalink
feat: migrate from home-rolled to standard gRPC middleware
Browse files Browse the repository at this point in the history
The home-rolled middleware was introduced before the standard
`otelgrpc.NewClientHandler()` and `otelgrpc.NewServerHandler()` were
available.

The standard middleware measures the same things as the home-rolled one
(request count and request duration) and a few other metrics
(such as request/response size histograms).
  • Loading branch information
odsod committed Aug 3, 2024
1 parent 301aeff commit 815bcba
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 168 deletions.
2 changes: 0 additions & 2 deletions cloudmonitoring/doc.go

This file was deleted.

148 changes: 0 additions & 148 deletions cloudmonitoring/metricmiddleware.go

This file was deleted.

4 changes: 1 addition & 3 deletions dialservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,9 @@ func DialService(ctx context.Context, target string, opts ...grpc.DialOption) (*
target,
append(
[]grpc.DialOption{
grpc.WithStatsHandler(otelgrpc.NewClientHandler()),
grpc.WithDefaultServiceConfig(run.config.Client.AsServiceConfigJSON()),
grpc.WithChainUnaryInterceptor(
//nolint:staticcheck // package is deprecated, replace when possible
otelgrpc.UnaryClientInterceptor(),
run.metricMiddleware.GRPCUnaryClientInterceptor,
run.requestLoggerMiddleware.GRPCUnaryClientInterceptor,
run.clientMiddleware.GRPCUnaryClientInterceptor,
),
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ require (
go.opentelemetry.io/contrib/instrumentation/runtime v0.53.0
go.opentelemetry.io/otel v1.28.0
go.opentelemetry.io/otel/bridge/opencensus v1.28.0
go.opentelemetry.io/otel/metric v1.28.0
go.opentelemetry.io/otel/sdk v1.28.0
go.opentelemetry.io/otel/sdk/metric v1.28.0
go.uber.org/zap v1.27.0
Expand Down Expand Up @@ -64,6 +63,7 @@ require (
github.com/tklauser/numcpus v0.8.0 // indirect
github.com/yusufpapurcu/wmi v1.2.4 // indirect
go.opencensus.io v0.24.0 // indirect
go.opentelemetry.io/otel/metric v1.28.0 // indirect
go.opentelemetry.io/otel/trace v1.28.0 // indirect
go.uber.org/multierr v1.10.0 // indirect
golang.org/x/crypto v0.25.0 // indirect
Expand Down
11 changes: 3 additions & 8 deletions grpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,16 @@ func NewGRPCServer(ctx context.Context, opts ...grpc.ServerOption) *grpc.Server
panic("cloudrunner.NewGRPCServer: must be called with a context from cloudrunner.Run")
}
serverOptions := []grpc.ServerOption{
grpc.StatsHandler(otelgrpc.NewServerHandler()),
grpc.ChainUnaryInterceptor(
//nolint:staticcheck // package is deprecated, replace when possible
otelgrpc.UnaryServerInterceptor(),
run.loggerMiddleware.GRPCUnaryServerInterceptor, // adds context logger
run.traceMiddleware.GRPCServerUnaryInterceptor, // needs the context logger
run.metricMiddleware.GRPCUnaryServerInterceptor,
run.loggerMiddleware.GRPCUnaryServerInterceptor, // adds context logger
run.traceMiddleware.GRPCServerUnaryInterceptor, // needs the context logger
run.requestLoggerMiddleware.GRPCUnaryServerInterceptor, // needs to run after trace
run.serverMiddleware.GRPCUnaryServerInterceptor, // needs to run after request logger
),
grpc.ChainStreamInterceptor(
//nolint:staticcheck // package is deprecated, replace when possible
otelgrpc.StreamServerInterceptor(),
run.loggerMiddleware.GRPCStreamServerInterceptor,
run.traceMiddleware.GRPCStreamServerInterceptor,
run.metricMiddleware.GRPCStreamServerInterceptor,
run.requestLoggerMiddleware.GRPCStreamServerInterceptor,
run.serverMiddleware.GRPCStreamServerInterceptor,
),
Expand Down
6 changes: 0 additions & 6 deletions run.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

"go.einride.tech/cloudrunner/cloudclient"
"go.einride.tech/cloudrunner/cloudconfig"
"go.einride.tech/cloudrunner/cloudmonitoring"
"go.einride.tech/cloudrunner/cloudotel"
"go.einride.tech/cloudrunner/cloudprofiler"
"go.einride.tech/cloudrunner/cloudrequestlog"
Expand Down Expand Up @@ -101,10 +100,6 @@ func Run(fn func(context.Context) error, options ...Option) (err error) {
if run.requestLoggerMiddleware.MessageTransformer == nil {
run.requestLoggerMiddleware.MessageTransformer = protosensitive.Redact
}
run.metricMiddleware, err = cloudmonitoring.NewMetricMiddleware()
if err != nil {
return fmt.Errorf("cloudrunner.Run: %w", err)
}
ctx = withRunContext(ctx, &run)
ctx = cloudruntime.WithConfig(ctx, run.config.Runtime)
logger, err := cloudzap.NewLogger(run.config.Logger)
Expand Down Expand Up @@ -182,7 +177,6 @@ type runContext struct {
clientMiddleware cloudclient.Middleware
requestLoggerMiddleware cloudrequestlog.Middleware
traceMiddleware cloudtrace.Middleware
metricMiddleware cloudmonitoring.MetricMiddleware
securityHeadersMiddleware cloudserver.SecurityHeadersMiddleware
}

Expand Down

0 comments on commit 815bcba

Please sign in to comment.