Add production-grade Go gRPC scaffold service#1
Conversation
Reference implementation of a production-ready Go gRPC service on K8s, demonstrating current best practices: structured logging (slog), OTel metrics with Prometheus, gRPC channelz/reflection/health, rate limiting, auth interceptor, structured errors (google.rpc.Status + errdetails), feature flags (OpenFeature), and optional Postgres via pgx+sqlc. Also upgrades rules_go 0.56.1→0.60.0, gazelle 0.44.0→0.50.0, and Go SDK 1.24.6→1.24.13 to fix Bazel 8.5.0 compatibility.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 11 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
| adminPort = flag.Int("admin-port", envInt("ADMIN_PORT", 8081), "Admin HTTP port") | ||
| logLevel = flag.String("log-level", envStr("LOG_LEVEL", "info"), "Log level (debug, info, warn, error)") | ||
| databaseURL = flag.String("database-url", envStr("DATABASE_URL", ""), "PostgreSQL connection URL") | ||
| otelEndpoint = flag.String("otel-endpoint", envStr("OTEL_EXPORTER_OTLP_ENDPOINT", ""), "OTLP exporter endpoint") |
There was a problem hiding this comment.
OTLP endpoint flag parsed but never used
Medium Severity
The otelEndpoint flag and OTEL_EXPORTER_OTLP_ENDPOINT environment variable are defined but not passed to observability.Setup. This prevents the configuration of an OTLP exporter, so OpenTelemetry traces and metrics (including otelgrpc spans) are not shipped, leading to silent observability gaps.
Triggered by team rule: Config Completeness: Missing Env Vars, Hardcoded Values
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
| ) | ||
| } | ||
| }() | ||
| } |
There was a problem hiding this comment.
Unbounded detached goroutines for DB writes
Medium Severity
The SaveGreeting call runs in a detached goroutine with context.WithoutCancel(ctx). This allows unbounded goroutine accumulation under load and prevents graceful shutdown. It risks silent data loss, goroutine leaks, and shutdown stalls as database.Close(pool) may block or fail.
Triggered by team rule: Resource Leaks: Connections, Processes, Tasks
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
| if err := s.Serve(lis); err != nil { | ||
| logger.Error("gRPC server error", slog.String("error", err.Error())) | ||
| } | ||
| }() |
There was a problem hiding this comment.
Normal shutdown logged as admin and gRPC errors
Low Severity
During graceful shutdown, both the admin and gRPC servers log expected http.ErrServerClosed and grpc.ErrServerStopped errors as generic server errors. This creates noisy alerts and can hide genuine issues.
Triggered by team rule: Error Handling: Silent Failures, Swallowed Exceptions
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
| grpcrecovery.StreamServerInterceptor(), | ||
| auth.StreamServerInterceptor(), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Rate limiter throttles gRPC health check probes
High Severity
The limiter.UnaryServerInterceptor() runs before auth.UnaryServerInterceptor() and has no method allowlist, so /grpc.health.v1.Health/Check is subject to the same token bucket as normal traffic. During a legitimate burst at or above the configured RPS, kubelet gRPC probes (used by liveness/readiness/startup probes in deployment.yaml) receive ResourceExhausted, are treated as unhealthy by Kubernetes, and can trigger unnecessary pod restarts that amplify the incident.
Additional Locations (1)
Triggered by team rule: Stuck State & Data Loss
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
| grpcrecovery.StreamServerInterceptor(), | ||
| auth.StreamServerInterceptor(), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Rejected RPCs never produce structured log entries
Medium Severity
The logging interceptor is chained after the rate-limit and auth interceptors, so any request rejected by those earlier stages short-circuits before logging.UnaryServerInterceptor runs. Rate-limited, unauthenticated, and missing-token requests are therefore returned to clients without any log line, no request_id, and no peer information — making it effectively impossible to diagnose throttling waves or auth regressions from logs alone.
Additional Locations (1)
Triggered by team rule: Best Practices: Logging, Idempotency, Comments vs. Code
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
| logger.Info("shutdown signal received") | ||
|
|
||
| healthServer.SetServingStatus("", healthpb.HealthCheckResponse_NOT_SERVING) | ||
| time.Sleep(5 * time.Second) |
There was a problem hiding this comment.
Health status race during shutdown window
Medium Severity
The DB health-check goroutine and the shutdown path both write to healthServer.SetServingStatus("", ...). When shutdown begins, the goroutine's in-flight database.HealthCheck can complete after the main path writes NOT_SERVING, then overwrite it back to SERVING. During the 5-second drain window Kubernetes readiness can flap back to serving and traffic continues to route to a shutting-down pod.
Triggered by team rule: Concurrency: Race Conditions, Data Races, Atomicity
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
| grpcrecovery.StreamServerInterceptor(), | ||
| auth.StreamServerInterceptor(), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Auth and rate-limit errors bypass logging interceptor
Medium Severity
In ChainUnaryInterceptor the order is recovery → rate-limit → auth → logging → errors. When ratelimit or auth return an error, control never reaches logging or scerrors interceptors, so rejected requests (rate-limit, missing/invalid tokens) are never logged and never attached to a trace span. The stream chain additionally has no rate-limit or logging/errors interceptors at all.
Triggered by team rule: Error Handling: Silent Failures, Swallowed Exceptions
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
| "/grpc.channelz.v1.Channelz/GetChannel": true, | ||
| "/grpc.channelz.v1.Channelz/GetSubchannel": true, | ||
| "/grpc.channelz.v1.Channelz/GetSocket": true, | ||
| } |
There was a problem hiding this comment.
Channelz auth allowlist missing methods
Low Severity
The skipMethods allowlist registers channelz methods GetTopChannels, GetServers, GetChannel, GetSubchannel, GetSocket, but the channelz v1 service also exposes GetServer (singular) and GetServerSockets. Tools walking the channelz graph will hit Unauthenticated on those two RPCs, breaking the debugging workflow that service.RegisterChannelzServiceToServer is meant to enable.
Triggered by team rule: Authorization & Security
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
|
|
||
| database.Close(pool) | ||
| telemetry.Shutdown(shutdownCtx) | ||
| logger.Info("shutdown complete") |
There was a problem hiding this comment.
Shutdown error returns ignored
Low Severity
adminSrv.Shutdown(shutdownCtx) and telemetry.Shutdown(shutdownCtx) both return errors that are silently discarded. If the 5-second shutdown context expires before metrics/trace providers flush, or the admin server fails to close cleanly, operators have no visibility and metrics in flight are lost without any indication.
Triggered by team rule: Error Handling: Silent Failures, Swallowed Exceptions
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
| } | ||
| } | ||
| return fallback | ||
| } |
There was a problem hiding this comment.
envInt silently falls back on invalid values
Low Severity
envInt silently returns the fallback when the environment variable is present but not a valid integer. A typo like GRPC_PORT=abc does not fail startup or log anything; the service silently binds to the default port, which may differ from what the operator configured and leave the intended port unbound.
Triggered by team rule: Authorization & Security
Reviewed by Cursor Bugbot for commit 26c7b77. Configure here.
1 participant
Summary
scaffold/— a production-grade Go gRPC reference implementation for Kubernetes with all the patterns a real service needs out of the box@io_bazel_rules_go→@rules_gorepo name in proto BUILD after upgradeWhat's in the scaffold
cmd/greeter/main.gointernal/logging/internal/observability/internal/errors/internal/auth/internal/ratelimit/internal/featureflags/internal/database/internal/admin/internal/version/deploy/Test plan
bazel build //...— full monorepo builds successfullybazel test //...— all tests pass (greeterservice unit tests)grpc_example/builds unaffected by rules_go upgradebazel run //scaffold/cmd/greeter:greeter— verify gRPC server starts on :50051, admin on :8081grpcurl -plaintext localhost:50051 grpc.health.v1.Health/Check— returns SERVINGcurl localhost:8081/metrics— shows Prometheus metricscurl localhost:8081/version— shows build info JSONNote
Medium Risk
Adds a sizable new Go service scaffold (gRPC server, interceptors, metrics, DB hooks, and K8s/Docker deployment) and upgrades
rules_go/gazelle, which can affect builds and generated code across the repo. Risk is mainly build/toolchain compatibility and behavior changes in the new interceptors (auth/rate limit/error mapping) when the scaffold is used.Overview
Adds a new
scaffold/Go module with a deployablegreetergRPC service, including structured logging with request IDs, OpenTelemetry metrics/tracing with Prometheus export, admin HTTP endpoints (/metrics,/healthz,/version,pprof), auth and rate-limit interceptors, and optional PostgreSQL persistence viapgx+sqlc.Upgrades Bazel Go tooling (
rules_go0.56.1→0.60.0,gazelle0.44.0→0.50.0, Go SDK 1.24.6→1.24.13), refreshesMODULE.bazel.lock, addsdeps.bzl+WORKSPACEmacro for IDE/Gazelle, and adjustsgrpc_example/prototo use@protobufproto_libraryplusgo_grpc_v2compilers (with a small Rust BUILD deps reorder).Reviewed by Cursor Bugbot for commit 26c7b77. Bugbot is set up for automated code reviews on this repo. Configure here.