Skip to content

Commit

Permalink
chore(v2): add gRPC client call duration metric (#3717)
Browse files Browse the repository at this point in the history
  • Loading branch information
kolesnikovae authored Nov 27, 2024
1 parent 00f81b9 commit a5ec059
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 20 deletions.
11 changes: 3 additions & 8 deletions pkg/experiment/ingester/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import (
"github.com/grafana/dskit/ring"
ring_client "github.com/grafana/dskit/ring/client"
"github.com/grafana/dskit/services"
otgrpc "github.com/opentracing-contrib/go-grpc"
"github.com/opentracing/opentracing-go"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/sony/gobreaker/v2"
Expand Down Expand Up @@ -289,10 +287,7 @@ func newConnPool(

// The options (including interceptors) are shared by all client connections.
options = append(options, dialOpts...)
options = append(options,
grpc.WithUnaryInterceptor(otgrpc.OpenTracingClientInterceptor(opentracing.GlobalTracer())),
grpc.WithDefaultServiceConfig(grpcServiceConfig),
)
options = append(options, grpc.WithDefaultServiceConfig(grpcServiceConfig))

// Note that circuit breaker must be created per client conn.
factory := connpool.NewConnPoolFactory(func(desc ring.InstanceDesc) []grpc.DialOption {
Expand All @@ -304,8 +299,8 @@ func newConnPool(
"segment-writer",
ring_client.PoolConfig{
CheckInterval: poolCleanupPeriod,
// Note that no health checks are not used:
// gGRPC health-checking is done at the gRPC connection level.
// Note that health checks are not used: gGRPC health-checking
// is done at the gRPC connection level.
HealthCheckEnabled: false,
HealthCheckTimeout: 0,
MaxConcurrentHealthChecks: 0,
Expand Down
7 changes: 1 addition & 6 deletions pkg/experiment/metastore/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import (
"github.com/grafana/dskit/services"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/raft"
otgrpc "github.com/opentracing-contrib/go-grpc"
"github.com/opentracing/opentracing-go"
"google.golang.org/grpc"

metastorev1 "github.com/grafana/pyroscope/api/gen/proto/go/metastore/v1"
Expand Down Expand Up @@ -165,10 +163,7 @@ func dial(address string, grpcClientConfig grpcclient.Config, _ log.Logger) (*gr
return nil, err
}
// TODO: https://github.com/grpc/grpc-proto/blob/master/grpc/service_config/service_config.proto
options = append(options,
grpc.WithDefaultServiceConfig(grpcServiceConfig),
grpc.WithUnaryInterceptor(otgrpc.OpenTracingClientInterceptor(opentracing.GlobalTracer())),
)
options = append(options, grpc.WithDefaultServiceConfig(grpcServiceConfig))
return grpc.Dial(address, options...)
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/experiment/query_backend/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (

"github.com/grafana/dskit/grpcclient"
"github.com/grafana/dskit/services"
"github.com/opentracing-contrib/go-grpc"
"github.com/opentracing/opentracing-go"
"google.golang.org/grpc"

queryv1 "github.com/grafana/pyroscope/api/gen/proto/go/query/v1"
Expand Down Expand Up @@ -35,7 +33,6 @@ func dial(address string, grpcClientConfig grpcclient.Config) (*grpc.ClientConn,
}
// TODO: https://github.com/grpc/grpc-proto/blob/master/grpc/service_config/service_config.proto
options = append(options,
grpc.WithUnaryInterceptor(otgrpc.OpenTracingClientInterceptor(opentracing.GlobalTracer())),
grpc.WithDefaultServiceConfig(grpcServiceConfig),
grpc.WithMaxCallAttempts(500),
)
Expand Down
27 changes: 27 additions & 0 deletions pkg/phlare/modules_experimental.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@ package phlare
import (
"fmt"
"slices"
"time"

"github.com/go-kit/log"
"github.com/grafana/dskit/middleware"
"github.com/grafana/dskit/ring"
"github.com/grafana/dskit/services"
otgrpc "github.com/opentracing-contrib/go-grpc"
"github.com/opentracing/opentracing-go"
"github.com/prometheus/client_golang/prometheus"
"google.golang.org/grpc"
grpchealth "google.golang.org/grpc/health"

compactionworker "github.com/grafana/pyroscope/pkg/experiment/compactor"
Expand All @@ -19,6 +24,7 @@ import (
"github.com/grafana/pyroscope/pkg/experiment/metastore/discovery"
querybackend "github.com/grafana/pyroscope/pkg/experiment/query_backend"
querybackendclient "github.com/grafana/pyroscope/pkg/experiment/query_backend/client"
"github.com/grafana/pyroscope/pkg/util"
"github.com/grafana/pyroscope/pkg/util/health"
)

Expand Down Expand Up @@ -68,6 +74,7 @@ func (f *Phlare) initSegmentWriter() (services.Service, error) {
}

func (f *Phlare) initSegmentWriterClient() (_ services.Service, err error) {
f.Cfg.SegmentWriter.GRPCClientConfig.Middleware = f.grpcClientInterceptors()
// Validation of the config is not required since
// it's already validated in initSegmentWriterRing.
logger := log.With(f.logger, "component", "segment-writer-client")
Expand Down Expand Up @@ -138,6 +145,7 @@ func (f *Phlare) initMetastoreClient() (services.Service, error) {
return nil, fmt.Errorf("failed to create discovery: %w %s", err, f.Cfg.Metastore.Address)
}

f.Cfg.Metastore.GRPCClientConfig.Middleware = f.grpcClientInterceptors()
f.metastoreClient = metastoreclient.New(
f.logger,
f.Cfg.Metastore.GRPCClientConfig,
Expand Down Expand Up @@ -169,6 +177,7 @@ func (f *Phlare) initQueryBackendClient() (services.Service, error) {
if err := f.Cfg.QueryBackend.Validate(); err != nil {
return nil, err
}
f.Cfg.QueryBackend.GRPCClientConfig.Middleware = f.grpcClientInterceptors()
c, err := querybackendclient.New(
f.Cfg.QueryBackend.Address,
f.Cfg.QueryBackend.GRPCClientConfig,
Expand Down Expand Up @@ -214,3 +223,21 @@ func (f *Phlare) initHealthServer() (services.Service, error) {
f.healthServer = grpchealth.NewServer()
return nil, nil
}

func (f *Phlare) grpcClientInterceptors() []grpc.UnaryClientInterceptor {
requestDuration := util.RegisterOrGet(f.reg, prometheus.NewHistogramVec(prometheus.HistogramOpts{
Namespace: "pyroscope",
Subsystem: "grpc_client",
Name: "request_duration_seconds",
Help: "Time (in seconds) spent waiting for gRPC response.",
Buckets: prometheus.DefBuckets,
NativeHistogramBucketFactor: 1.1,
NativeHistogramMaxBucketNumber: 50,
NativeHistogramMinResetDuration: time.Hour,
}, []string{"method", "status_code"}))

return []grpc.UnaryClientInterceptor{
middleware.UnaryClientInstrumentInterceptor(requestDuration, middleware.ReportGRPCStatusOption),
otgrpc.OpenTracingClientInterceptor(opentracing.GlobalTracer()),
}
}
3 changes: 0 additions & 3 deletions pkg/phlare/phlare.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,6 @@ type Config struct {
ConfigExpandEnv bool `yaml:"-"`

// Experimental modules.
// TODO(kolesnikovae):
// - Generalized experimental features?
// - Better naming.
v2Experiment bool
SegmentWriter segmentwriter.Config `yaml:"segment_writer" doc:"hidden"`
Metastore metastore.Config `yaml:"metastore" doc:"hidden"`
Expand Down

0 comments on commit a5ec059

Please sign in to comment.