Add loger, tally and renames and decouple the server from controllers

Author: behinddwalls

.envrc

@@ -1,7 +1,4 @@
-GOPATH=$(pwd)
-export GOPATH
 PATH_add tools
 
-
-WORKSPACE_ROOT=$(pwd)
-export WORKSPACE_ROOT
+REPO_ROOT=$(pwd)
+export REPO_ROOT

.github/workflows/build_and_test.yml

@@ -9,7 +9,6 @@ on:
       - opened
       - reopened
       - synchronize
-      - ready_for_review
 
 jobs:
   build-and-test:

.gitignore

@@ -5,6 +5,7 @@ MODULE.bazel.lock
 .vscode/
 .idea/
 .claude/
+.ijwb/
 
 
 # Built binaries

MODULE.bazel

@@ -37,8 +37,10 @@ go_deps.from_file(go_mod = "//:go.mod")
 use_repo(
     go_deps,
     "com_github_gogo_protobuf",
+    "com_github_uber_go_tally_v4",
     "org_golang_google_grpc",
     "org_golang_google_protobuf",
     "org_uber_go_fx",
     "org_uber_go_yarpc",
+    "org_uber_go_zap",
 )

README.md

@@ -50,6 +50,23 @@ For detailed instructions, see [examples/README.md](examples/README.md).
 
 See [docs/architecture/STRUCTURE.md](docs/architecture/STRUCTURE.md) for a detailed breakdown of the project structure.
 
+## Architecture
+
+The project follows clean architecture principles with clear separation of concerns:
+
+- **Controllers** (`core/controller/`): Pure business logic, independent of transport layer
+  - Only depend on logger, metrics, and protobuf types
+  - Example: `PingController` handles ping business logic
+
+- **Server Adapters** (`examples/server/`): gRPC transport layer
+  - Wrap controllers and implement gRPC service interfaces
+  - Handle protocol-specific concerns (e.g., `UnimplementedServiceServer`)
+
+- **Observability**: Built-in logging and metrics
+  - Structured logging with [Zap](https://github.com/uber-go/zap)
+  - Metrics collection with [Tally](https://github.com/uber-go/tally)
+  - Development servers use human-readable console logging
+
 ## Development
 
 ### Prerequisites
@@ -226,7 +243,8 @@ message PingResponse {
     string message = 1;
     string service_name = 2;
     int64 timestamp = 3;
-    string hostname = 4;  // New field added
+    string hostname = 4;
+    string new_field = 5;  // New field added
 }
 ```
 
@@ -240,8 +258,8 @@ make proto
 # - gateway/protopb/gateway_grpc.pb.go
 # - gateway/protopb/gateway.pb.yarpc.go
 
-# Now update the service implementation
-# Edit gateway/core/controller/ping.go to populate the hostname field
+# Now update the controller implementation
+# Edit gateway/core/controller/ping.go to populate the new field in the PingResponse
 ```
 
 ### Testing
@@ -313,17 +331,44 @@ make test
    make proto
    ```
 
-3. **Implement the method in the service:**
+3. **Implement the method in the controller:**
+   ```go
+   // In gateway/core/controller/ (create a new controller file)
+   type NewController struct {
+       logger       *zap.Logger
+       metricsScope tally.Scope
+   }
+
+   func NewNewController(logger *zap.Logger, scope tally.Scope) *NewController {
+       if logger == nil {
+           logger = zap.NewNop()
+       }
+       if scope == nil {
+           scope = tally.NoopScope
+       }
+       return &NewController{logger: logger, metricsScope: scope}
+   }
+
+   func (c *NewController) NewMethod(ctx context.Context, req *pb.NewRequest) (*pb.NewResponse, error) {
+       // Business logic here
+       c.logger.Info("new method called")
+       c.metricsScope.Counter("new_method_total").Inc(1)
+       // ...
+   }
+   ```
+
+4. **Create server wrapper in example:**
    ```go
-   // In gateway/core/controller/ping.go (or create a new file)
-   func (s *PingServiceImpl) NewMethod(ctx context.Context, req *pb.NewRequest) (*pb.NewResponse, error) {
-       // Implementation here
+   // In examples/server/gateway/main.go
+   // Add method delegation to GatewayServer struct:
+   func (s *GatewayServer) NewMethod(ctx context.Context, req *pb.NewRequest) (*pb.NewResponse, error) {
+       return s.newController.NewMethod(ctx, req)
    }
    ```
 
-4. **Update clients to call the new method**
+5. **Update clients to call the new method**
 
-5. **Rebuild and test:**
+6. **Rebuild and test:**
    ```bash
    make build
    ```

examples/server/gateway/BUILD.bazel

@@ -8,8 +8,10 @@ go_library(
     deps = [
         "//gateway/core/controller",
         "//gateway/protopb",
+        "@com_github_uber_go_tally_v4//:tally",
         "@org_golang_google_grpc//:grpc",
         "@org_golang_google_grpc//reflection",
+        "@org_uber_go_zap//:zap",
     ],
 )
 

examples/server/gateway/main.go

@@ -1,18 +1,33 @@
 package main
 
 import (
+	"context"
 	"fmt"
 	"net"
 	"os"
 	"os/signal"
 	"syscall"
+	"time"
 
+	"github.com/uber-go/tally/v4"
 	"github.com/uber/submitqueue/gateway/core/controller"
 	pb "github.com/uber/submitqueue/gateway/protopb"
+	"go.uber.org/zap"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/reflection"
 )
 
+// GatewayServer wraps the controller and implements the gRPC service interface
+type GatewayServer struct {
+	pb.UnimplementedGatewayServiceServer
+	controller *controller.PingController
+}
+
+// Ping delegates to the controller
+func (s *GatewayServer) Ping(ctx context.Context, req *pb.PingRequest) (*pb.PingResponse, error) {
+	return s.controller.Ping(ctx, req)
+}
+
 func main() {
 	if err := run(); err != nil {
 		fmt.Fprintf(os.Stderr, "Failed to start gateway server: %v\n", err)
@@ -21,12 +36,37 @@ func main() {
 }
 
 func run() error {
+	// Initialize development logger (human-readable console output)
+	logger, err := zap.NewDevelopment()
+	if err != nil {
+		return fmt.Errorf("failed to create logger: %w", err)
+	}
+	defer logger.Sync()
+
+	// Initialize metrics scope
+	scope := tally.NewTestScope("gateway", nil)
+	go func() {
+		ticker := time.NewTicker(10 * time.Second)
+		defer ticker.Stop()
+		for range ticker.C {
+			snapshot := scope.Snapshot()
+			logger.Info("metrics snapshot",
+				zap.Any("counters", snapshot.Counters()),
+				zap.Any("gauges", snapshot.Gauges()),
+				zap.Any("timers", snapshot.Timers()),
+			)
+		}
+	}()
+
 	// Create gRPC server
 	grpcServer := grpc.NewServer()
 
-	// Register the ping service
-	pingService := controller.NewPingService()
-	pb.RegisterGatewayServiceServer(grpcServer, pingService)
+	// Create ping controller and wrap it for gRPC
+	pingController := controller.NewPingController(logger, scope)
+	gatewayServer := &GatewayServer{
+		controller: pingController,
+	}
+	pb.RegisterGatewayServiceServer(grpcServer, gatewayServer)
 
 	// Register reflection service for debugging with grpcurl
 	reflection.Register(grpcServer)

examples/server/orchestrator/BUILD.bazel

@@ -8,8 +8,10 @@ go_library(
     deps = [
         "//orchestrator/core/controller",
         "//orchestrator/protopb",
+        "@com_github_uber_go_tally_v4//:tally",
         "@org_golang_google_grpc//:grpc",
         "@org_golang_google_grpc//reflection",
+        "@org_uber_go_zap//:zap",
     ],
 )
 

examples/server/orchestrator/main.go

@@ -1,18 +1,33 @@
 package main
 
 import (
+	"context"
 	"fmt"
 	"net"
 	"os"
 	"os/signal"
 	"syscall"
+	"time"
 
+	"github.com/uber-go/tally/v4"
 	"github.com/uber/submitqueue/orchestrator/core/controller"
 	pb "github.com/uber/submitqueue/orchestrator/protopb"
+	"go.uber.org/zap"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/reflection"
 )
 
+// OrchestratorServer wraps the controller and implements the gRPC service interface
+type OrchestratorServer struct {
+	pb.UnimplementedOrchestratorServiceServer
+	controller *controller.PingController
+}
+
+// Ping delegates to the controller
+func (s *OrchestratorServer) Ping(ctx context.Context, req *pb.PingRequest) (*pb.PingResponse, error) {
+	return s.controller.Ping(ctx, req)
+}
+
 func main() {
 	if err := run(); err != nil {
 		fmt.Fprintf(os.Stderr, "Failed to start orchestrator server: %v\n", err)
@@ -21,12 +36,37 @@ func main() {
 }
 
 func run() error {
+	// Initialize development logger (human-readable console output)
+	logger, err := zap.NewDevelopment()
+	if err != nil {
+		return fmt.Errorf("failed to create logger: %w", err)
+	}
+	defer logger.Sync()
+
+	// Initialize metrics scope
+	scope := tally.NewTestScope("orchestrator", nil)
+	go func() {
+		ticker := time.NewTicker(10 * time.Second)
+		defer ticker.Stop()
+		for range ticker.C {
+			snapshot := scope.Snapshot()
+			logger.Info("metrics snapshot",
+				zap.Any("counters", snapshot.Counters()),
+				zap.Any("gauges", snapshot.Gauges()),
+				zap.Any("timers", snapshot.Timers()),
+			)
+		}
+	}()
+
 	// Create gRPC server
 	grpcServer := grpc.NewServer()
 
-	// Register the ping service
-	pingService := controller.NewPingService()
-	pb.RegisterOrchestratorServiceServer(grpcServer, pingService)
+	// Create ping controller and wrap it for gRPC
+	pingController := controller.NewPingController(logger, scope)
+	orchestratorServer := &OrchestratorServer{
+		controller: pingController,
+	}
+	pb.RegisterOrchestratorServiceServer(grpcServer, orchestratorServer)
 
 	// Register reflection service for debugging with grpcurl
 	reflection.Register(grpcServer)

examples/server/speculator/BUILD.bazel

@@ -8,8 +8,10 @@ go_library(
     deps = [
         "//speculator/core/controller",
         "//speculator/protopb",
+        "@com_github_uber_go_tally_v4//:tally",
         "@org_golang_google_grpc//:grpc",
         "@org_golang_google_grpc//reflection",
+        "@org_uber_go_zap//:zap",
     ],
 )