👌 Apply review feedback

.vscode/settings.json

@@ -3,12 +3,5 @@
     "go.lintTool": "golangci-lint",
     "go.lintFlags": [
         "--fast"
-    ],
-    "cSpell.words": [
-        "Debugf",
-        "gonic",
-        "Infof",
-        "mapstructure",
-        "Warningf"
     ]
 }

cmd/main.go

@@ -3,17 +3,23 @@ package main
 
 import (
 	"context"
+	"errors"
 	"flag"
+	"fmt"
+	"os"
+	"os/signal"
 	"path"
+	"strings"
 	"sync"
+	"syscall"
 
 	"github.com/LiskHQ/op-fault-detector/pkg/api"
 	"github.com/LiskHQ/op-fault-detector/pkg/config"
 	"github.com/LiskHQ/op-fault-detector/pkg/log"
-	"github.com/LiskHQ/op-fault-detector/pkg/utils"
+	"github.com/spf13/viper"
 )
 
-var wg = sync.WaitGroup{}
+var apiServer *api.HTTPServerWrapper
 
 func main() {
 	ctx, cancel := context.WithCancel(context.Background())
@@ -24,24 +30,84 @@ func main() {
 		panic(err)
 	}
 
-	defaultConfigFilename := "config.yaml"
-	defaultConfigDir := path.Join(utils.GetCurrentDirectory(0), "..")
-	configFilepath := flag.String("config", path.Join(defaultConfigDir, defaultConfigFilename), "Path to the config file")
+	configFilepath := flag.String("config", "./config.yaml", "Path to the config file")
 	flag.Parse()
-	config, err := config.GetAppConfig(ctx, logger, *configFilepath)
+	config, err := getAppConfig(logger, *configFilepath)
 	if err != nil {
 		panic(err)
 	}
 
+	wg := sync.WaitGroup{}
+
+	doneChan := make(chan struct{})
+	signalChan := make(chan os.Signal, 1)
+	signal.Notify(signalChan, os.Interrupt, syscall.SIGTERM)
+
+	// Start Fault Detector
+
+	// Start API Server
+	serverChan := make(chan error, 1)
+	apiServer = api.NewHTTPServer(ctx, logger, &wg, config, serverChan)
 	wg.Add(1)
+	go apiServer.Start()
+
 	go func() {
-		err := api.StartServer(ctx, &wg, logger, config)
-		if err != nil {
-			logger.Errorf("Failed to start the server due to: %v", err.Error())
-			panic(err)
-		}
+		wg.Wait()
+		close(doneChan)
 	}()
 
-	// Wait for all the goroutines to finish
-	wg.Wait()
+	for {
+		select {
+		case <-doneChan:
+			performCleanup(logger)
+			return
+
+		case <-signalChan:
+			performCleanup(logger)
+			return
+
+		case err := <-serverChan:
+			logger.Errorf("Received error of %v", err)
+			return
+		}
+	}
+}
+
+// getAppConfig is the function that takes in the absolute path to the config file, parses the content and returns it.
+func getAppConfig(logger log.Logger, configFilepath string) (*config.Config, error) {
+	configDir := path.Dir(configFilepath)
+	configFilenameWithExt := path.Base(configFilepath)
+
+	splits := strings.FieldsFunc(configFilenameWithExt, func(r rune) bool { return r == '.' })
+	configType := splits[len(splits)-1] // Config file extension
+
+	viper.AddConfigPath(".")
+	viper.AddConfigPath("..")
+	viper.AddConfigPath("$HOME/.op-fault-detector")
+	viper.AddConfigPath(configDir)
+	viper.SetConfigName(configFilenameWithExt)
+	viper.SetConfigType(configType)
+	err := viper.ReadInConfig()
+	if err != nil {
+		return nil, fmt.Errorf("failed to load the config from disk: %w", err)
+	}
+
+	var config config.Config
+	err = viper.Unmarshal(&config)
+	if err != nil {
+		return nil, errors.New("failed to unmarshal config. Verify the 'Config' struct definition in 'pkg/config/config.go'")
+	}
+
+	if err := config.Validate(); err != nil {
+		return nil, err
+	}
+
+	return &config, nil
+}
+
+func performCleanup(logger log.Logger) {
+	err := apiServer.Stop()
+	if err != nil {
+		logger.Error("Server shutdown not successful: %w", err)
+	}
 }

config.yaml

@@ -1,11 +1,15 @@
-# Server configurations
-server:
-  host: "127.0.0.1"
-  port: 8080
-  gin_mode: "release" # Set to "debug" for development
+# General system configurations
+system:
+  log_level: "info"
 
-# API configurations
+# API related configurations
 api:
+  server:
+    host: "127.0.0.1"
+    port: 8080
   base_path: "/api"
   register_versions:
     - v1
+
+# Faultdetector configurations
+fault_detector:

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/gin-gonic/gin v1.9.1
 	github.com/magiconair/properties v1.8.7
 	github.com/spf13/viper v1.18.2
+	go.uber.org/multierr v1.10.0
 	go.uber.org/zap v1.26.0
 )
 
@@ -57,7 +58,6 @@ require (
 	github.com/tklauser/numcpus v0.6.1 // indirect
 	github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
 	github.com/ugorji/go/codec v1.2.12 // indirect
-	go.uber.org/multierr v1.10.0 // indirect
 	golang.org/x/arch v0.7.0 // indirect
 	golang.org/x/crypto v0.18.0 // indirect
 	golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa // indirect

pkg/api/routes/registration.go

@@ -7,16 +7,19 @@ import (
 	"github.com/gin-gonic/gin"
 )
 
+// RegisterHandlers is responsible to register all handlers for routes without any base path.
 func RegisterHandlers(logger log.Logger, router *gin.Engine) {
 	router.GET("/ping", handlers.GetPing)
 }
 
+// RegisterHandlersByGroup is responsible to register all the handlers for routes that are prefixed under a specified base path as a routerGroup.
 func RegisterHandlersByGroup(logger log.Logger, routerGroup *gin.RouterGroup, versions []string) {
 	for _, version := range versions {
 		RegisterHandlersForVersion(logger, routerGroup, version)
 	}
 }
 
+// RegisterHandlersForVersion is responsible to register API version specific route handlers.
 func RegisterHandlersForVersion(logger log.Logger, routerGroup *gin.RouterGroup, version string) {
 	group := routerGroup.Group(version)
 
@@ -25,6 +28,6 @@ func RegisterHandlersForVersion(logger log.Logger, routerGroup *gin.RouterGroup,
 		group.GET("/status", v1.GetStatus)
 
 	default:
-		logger.Warningf("No routes and handlers defined for version %v. Please verify the API config.", version)
+		logger.Warningf("No routes and handlers defined for version %s. Please verify the API config.", version)
 	}
 }

pkg/api/server.go

@@ -4,7 +4,9 @@ package api
 import (
 	"context"
 	"fmt"
+	"net/http"
 	"sync"
+	"time"
 
 	"github.com/LiskHQ/op-fault-detector/pkg/api/middlewares"
 	"github.com/LiskHQ/op-fault-detector/pkg/api/routes"
@@ -13,26 +15,49 @@ import (
 	"github.com/gin-gonic/gin"
 )
 
-// StartServer starts the HTTP API server on invocation and stops it when the parent context is killed.
-func StartServer(ctx context.Context, wg *sync.WaitGroup, logger log.Logger, config *config.Config) error {
-	defer wg.Done()
-
-	for {
-		select {
-		case <-ctx.Done():
-			return nil
-
-		default:
-			err := registerHandlersAndStartAPIServer(ctx, logger, config)
-			if err != nil {
-				return err
-			}
-		}
+// HTTPServerWrapper embeds the http.Server along with the various other properties.
+type HTTPServerWrapper struct {
+	server    *http.Server
+	ctx       context.Context
+	logger    log.Logger
+	wg        *sync.WaitGroup
+	errorChan chan error
+}
+
+// Start starts the HTTP API server.
+func (w *HTTPServerWrapper) Start() {
+	defer w.wg.Done()
+
+	w.logger.Infof("Starting the HTTP server on %s.", w.server.Addr)
+	err := w.server.ListenAndServe()
+	if err != nil {
+		w.errorChan <- err
+	}
+}
+
+// Stop gracefully shuts down the HTTP API server.
+func (w *HTTPServerWrapper) Stop() error {
+	err := w.server.Shutdown(w.ctx)
+	if err == nil {
+		w.logger.Infof("Successfully stopped the HTTP server.")
 	}
+
+	return err
 }
 
-func registerHandlersAndStartAPIServer(ctx context.Context, logger log.Logger, config *config.Config) error {
-	gin.SetMode(config.Server.GinMode)
+func getGinModeFromSysLogLevel(sysLogLevel string) string {
+	ginMode := gin.DebugMode // Default mode
+
+	if sysLogLevel != "debug" && sysLogLevel != "trace" {
+		ginMode = gin.ReleaseMode
+	}
+
+	return ginMode
+}
+
+// NewHTTPServer creates a router instance and sets up the necessary routes/handlers.
+func NewHTTPServer(ctx context.Context, logger log.Logger, wg *sync.WaitGroup, config *config.Config, errorChan chan error) *HTTPServerWrapper {
+	gin.SetMode(getGinModeFromSysLogLevel(config.System.LogLevel))
 
 	router := gin.Default()
 
@@ -46,13 +71,24 @@ func registerHandlersAndStartAPIServer(ctx context.Context, logger log.Logger, c
 	// Register handlers for routes following the base path
 	basePath := config.Api.BasePath
 	baseGroup := router.Group(basePath)
-	logger.Debugf("Registering handlers for endpoints under %v.", basePath)
+	logger.Debugf("Registering handlers for endpoints under path '%s'.", basePath)
 	routes.RegisterHandlersByGroup(logger, baseGroup, config.Api.RegisterVersions)
 
-	// Start the HTTP server and serve API requests
-	host := config.Server.Host
-	port := config.Server.Port
-	addr := fmt.Sprintf("%v:%v", host, port)
-	logger.Infof("Starting the HTTP API server on %v:%v", host, port)
-	return router.Run(addr)
+	host := config.Api.Server.Host
+	port := config.Api.Server.Port
+	addr := fmt.Sprintf("%s:%d", host, port)
+
+	server := &HTTPServerWrapper{
+		&http.Server{
+			Addr:              addr,
+			Handler:           router,
+			ReadHeaderTimeout: 10 * time.Second, // TODO: Check appropriate value
+		},
+		ctx,
+		logger,
+		wg,
+		errorChan,
+	}
+
+	return server
 }

pkg/api/server_test.go

@@ -0,0 +1,56 @@
+package api
+
+import (
+	"testing"
+
+	"github.com/LiskHQ/op-fault-detector/pkg/log"
+	"github.com/gin-gonic/gin"
+	"github.com/magiconair/properties/assert"
+)
+
+func TestGetGinModeFromSysLogLevel(t *testing.T) {
+	testCases := []struct {
+		name        string
+		logLevel    string
+		wantGinMode string
+	}{
+		{
+			name:        "should return debug gin_mode with trace log_level",
+			logLevel:    log.LevelTrace,
+			wantGinMode: gin.DebugMode,
+		},
+		{
+			name:        "should return debug gin_mode with debug log_level",
+			logLevel:    log.LevelDebug,
+			wantGinMode: gin.DebugMode,
+		},
+		{
+			name:        "should return release gin_mode with info log_level",
+			logLevel:    log.LevelInfo,
+			wantGinMode: gin.ReleaseMode,
+		},
+		{
+			name:        "should return release gin_mode with warn log_level",
+			logLevel:    log.LevelWarn,
+			wantGinMode: gin.ReleaseMode,
+		},
+		{
+			name:        "should return release gin_mode with error log_level",
+			logLevel:    log.LevelError,
+			wantGinMode: gin.ReleaseMode,
+		},
+		{
+			name:        "should return release gin_mode with fatal log_level",
+			logLevel:    log.LevelFatal,
+			wantGinMode: gin.ReleaseMode,
+		},
+	}
+
+	t.Parallel()
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			gotGinMode := getGinModeFromSysLogLevel(tc.logLevel)
+			assert.Equal(t, gotGinMode, tc.wantGinMode)
+		})
+	}
+}