-
Notifications
You must be signed in to change notification settings - Fork 2
Add Prometheus metrics endpoint #222
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package adapter | ||
|
|
||
| import "github.com/prometheus/client_golang/prometheus" | ||
|
|
||
| var ( | ||
| opCounter = prometheus.NewCounterVec(prometheus.CounterOpts{ | ||
| Name: "elastickv_operations_total", | ||
| Help: "Total number of KV operations", | ||
| }, []string{"op", "client"}) | ||
| ) | ||
|
|
||
| func init() { | ||
| prometheus.MustRegister(opCounter) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,6 +97,7 @@ func (r *RedisServer) ping(conn redcon.Conn, _ redcon.Command) { | |
| } | ||
|
|
||
| func (r *RedisServer) set(conn redcon.Conn, cmd redcon.Command) { | ||
| opCounter.WithLabelValues("set", "redis").Inc() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the gRPC handlers, there's code duplication here for incrementing the metrics counter. This can be improved by using a higher-order function to wrap your command handlers. This would centralize the metrics logic and make the code cleaner and easier to maintain. You could define a wrapper and use it in For example: // withMetrics wraps a redcon handler to increment an operation counter.
func (r *RedisServer) withMetrics(op string, handler func(redcon.Conn, redcon.Command)) func(redcon.Conn, redcon.Command) {
return func(conn redcon.Conn, cmd redcon.Command) {
opCounter.WithLabelValues(op, "redis").Inc()
handler(conn, cmd)
}
}
// In NewRedisServer():
r.route["SET"] = r.withMetrics("set", r.set)
r.route["GET"] = r.withMetrics("get", r.get)
// ... and so onWith this change, you would remove the manual |
||
| res, err := r.redisTranscoder.SetToRequest(cmd.Args[1], cmd.Args[2]) | ||
| if err != nil { | ||
| conn.WriteError(err.Error()) | ||
|
|
@@ -113,6 +114,7 @@ func (r *RedisServer) set(conn redcon.Conn, cmd redcon.Command) { | |
| } | ||
|
|
||
| func (r *RedisServer) get(conn redcon.Conn, cmd redcon.Command) { | ||
| opCounter.WithLabelValues("get", "redis").Inc() | ||
| v, err := r.store.Get(context.Background(), cmd.Args[1]) | ||
| if err != nil { | ||
| conn.WriteNull() | ||
|
|
@@ -123,6 +125,7 @@ func (r *RedisServer) get(conn redcon.Conn, cmd redcon.Command) { | |
| } | ||
|
|
||
| func (r *RedisServer) del(conn redcon.Conn, cmd redcon.Command) { | ||
| opCounter.WithLabelValues("del", "redis").Inc() | ||
| res, err := r.redisTranscoder.DeleteToRequest(cmd.Args[1]) | ||
| if err != nil { | ||
| conn.WriteError(err.Error()) | ||
|
|
@@ -139,6 +142,7 @@ func (r *RedisServer) del(conn redcon.Conn, cmd redcon.Command) { | |
| } | ||
|
|
||
| func (r *RedisServer) exists(conn redcon.Conn, cmd redcon.Command) { | ||
| opCounter.WithLabelValues("exists", "redis").Inc() | ||
| ok, err := r.store.Exists(context.Background(), cmd.Args[1]) | ||
| if err != nil { | ||
| conn.WriteError(err.Error()) | ||
|
|
@@ -153,6 +157,7 @@ func (r *RedisServer) exists(conn redcon.Conn, cmd redcon.Command) { | |
| } | ||
|
|
||
| func (r *RedisServer) keys(conn redcon.Conn, cmd redcon.Command) { | ||
| opCounter.WithLabelValues("keys", "redis").Inc() | ||
|
|
||
| // If an asterisk (*) is not included, the match will be exact, | ||
| // so check if the key exists. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import ( | |
| "fmt" | ||
| "log" | ||
| "net" | ||
| "net/http" | ||
| "os" | ||
| "path/filepath" | ||
|
|
||
|
|
@@ -17,8 +18,10 @@ import ( | |
| pb "github.com/bootjp/elastickv/proto" | ||
| "github.com/bootjp/elastickv/store" | ||
| "github.com/cockroachdb/errors" | ||
| "github.com/grpc-ecosystem/go-grpc-prometheus" | ||
| "github.com/hashicorp/raft" | ||
| boltdb "github.com/hashicorp/raft-boltdb/v2" | ||
| "github.com/prometheus/client_golang/prometheus/promhttp" | ||
| "golang.org/x/sync/errgroup" | ||
| "google.golang.org/grpc" | ||
| "google.golang.org/grpc/credentials/insecure" | ||
|
|
@@ -31,6 +34,7 @@ var ( | |
| raftId = flag.String("raft_id", "", "Node id used by Raft") | ||
| raftDir = flag.String("raft_data_dir", "data/", "Raft data dir") | ||
| raftBootstrap = flag.Bool("raft_bootstrap", false, "Whether to bootstrap the Raft cluster") | ||
| metricsAddr = flag.String("metrics_address", ":2112", "TCP host+port for Prometheus metrics") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test failure you noted ( To fix this, you should configure tests to use a dynamic port. A common technique is to use |
||
| ) | ||
|
|
||
| func main() { | ||
|
|
@@ -62,7 +66,10 @@ func main() { | |
| log.Fatalf("failed to start raft: %v", err) | ||
| } | ||
|
|
||
| gs := grpc.NewServer() | ||
| gs := grpc.NewServer( | ||
| grpc.UnaryInterceptor(grpc_prometheus.UnaryServerInterceptor), | ||
| grpc.StreamInterceptor(grpc_prometheus.StreamServerInterceptor), | ||
| ) | ||
| trx := kv.NewTransaction(r) | ||
| coordinate := kv.NewCoordinator(trx, r) | ||
| pb.RegisterRawKVServer(gs, adapter.NewGRPCServer(s, coordinate)) | ||
|
|
@@ -73,19 +80,26 @@ func main() { | |
| leaderhealth.Setup(r, gs, []string{"RawKV", "Example"}) | ||
| raftadmin.Register(gs, r) | ||
| reflection.Register(gs) | ||
| grpc_prometheus.Register(gs) | ||
|
|
||
| redisL, err := lc.Listen(ctx, "tcp", *redisAddr) | ||
| if err != nil { | ||
| log.Fatalf("failed to listen: %v", err) | ||
| } | ||
|
|
||
| mux := http.NewServeMux() | ||
| mux.Handle("/metrics", promhttp.Handler()) | ||
|
|
||
| eg := errgroup.Group{} | ||
| eg.Go(func() error { | ||
| return errors.WithStack(gs.Serve(grpcSock)) | ||
| }) | ||
| eg.Go(func() error { | ||
| return errors.WithStack(adapter.NewRedisServer(redisL, s, coordinate).Run()) | ||
| }) | ||
| eg.Go(func() error { | ||
| return errors.WithStack(http.ListenAndServe(*metricsAddr, mux)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [golangci] reported by reviewdog 🐶 |
||
| }) | ||
|
|
||
| err = eg.Wait() | ||
| if err != nil { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While adding metrics is great, manually incrementing the counter in every gRPC method leads to code duplication and is error-prone. If a new method is added, one might forget to add the metrics line.
A better approach is to use a custom gRPC unary interceptor to handle this logic centrally. The interceptor can extract the method name, format it, and increment the counter. This would remove boilerplate from all the service methods and make the code more maintainable.
For example, you could create an interceptor in the
adapterpackage:Then, in
main.go, you would chain this interceptor with the existing one fromgo-grpc-prometheus, for example by using a library likegrpc-middleware.