Clean-up error handling and logging (#2590)

* Revert Close method

Signed-off-by: tzzed <zerouali.t@gmail.com>

* fix lint

Signed-off-by: tzzed <zerouali.t@gmail.com>

cmd/all-in-one/main.go

@@ -16,7 +16,6 @@
 package main
 
 import (
-	"fmt"
 	"io"
 	"log"
 	"os"
@@ -131,7 +130,9 @@ by default uses only in-memory database.`,
 				StrategyStore:  strategyStore,
 				HealthCheck:    svc.HC(),
 			})
-			c.Start(cOpts)
+			if err := c.Start(cOpts); err != nil {
+				log.Fatal(err)
+			}
 
 			// agent
 			// if the agent reporter grpc host:port was not explicitly set then use whatever the collector is listening on
@@ -161,19 +162,18 @@ by default uses only in-memory database.`,
 
 			svc.RunAndThen(func() {
 				agent.Stop()
-				cp.Close()
-				c.Close()
-				querySrv.Close()
+				_ = cp.Close()
+				_ = c.Close()
+				_ = querySrv.Close()
 				if closer, ok := spanWriter.(io.Closer); ok {
-					err := closer.Close()
-					if err != nil {
+					if err := closer.Close(); err != nil {
 						logger.Error("Failed to close span writer", zap.Error(err))
 					}
 				}
 				if err := storageFactory.Close(); err != nil {
 					logger.Error("Failed to close storage factory", zap.Error(err))
 				}
-				tracerCloser.Close()
+				_ = tracerCloser.Close()
 			})
 			return nil
 		},
@@ -197,8 +197,7 @@ by default uses only in-memory database.`,
 	)
 
 	if err := command.Execute(); err != nil {
-		fmt.Println(err.Error())
-		os.Exit(1)
+		log.Fatal(err)
 	}
 }
 

cmd/collector/app/collector.go

@@ -16,6 +16,7 @@ package app
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"net/http"
 	"time"
@@ -84,44 +85,45 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error {
 	c.spanProcessor = handlerBuilder.BuildSpanProcessor()
 	c.spanHandlers = handlerBuilder.BuildHandlers(c.spanProcessor)
 
-	if grpcServer, err := server.StartGRPCServer(&server.GRPCServerParams{
+	grpcServer, err := server.StartGRPCServer(&server.GRPCServerParams{
 		HostPort:      builderOpts.CollectorGRPCHostPort,
 		Handler:       c.spanHandlers.GRPCHandler,
 		TLSConfig:     builderOpts.TLS,
 		SamplingStore: c.strategyStore,
 		Logger:        c.logger,
-	}); err != nil {
-		c.logger.Fatal("could not start gRPC collector", zap.Error(err))
-	} else {
-		c.grpcServer = grpcServer
+	})
+	if err != nil {
+		return fmt.Errorf("could not start gRPC collector %w", err)
 	}
+	c.grpcServer = grpcServer
 
-	if httpServer, err := server.StartHTTPServer(&server.HTTPServerParams{
+	httpServer, err := server.StartHTTPServer(&server.HTTPServerParams{
 		HostPort:       builderOpts.CollectorHTTPHostPort,
 		Handler:        c.spanHandlers.JaegerBatchesHandler,
 		HealthCheck:    c.hCheck,
 		MetricsFactory: c.metricsFactory,
 		SamplingStore:  c.strategyStore,
 		Logger:         c.logger,
-	}); err != nil {
-		c.logger.Fatal("could not start the HTTP server", zap.Error(err))
-	} else {
-		c.hServer = httpServer
+	})
+	if err != nil {
+		return fmt.Errorf("could not start the HTTP server %w", err)
 	}
+	c.hServer = httpServer
 
 	c.tlsCloser = &builderOpts.TLS
-	if zkServer, err := server.StartZipkinServer(&server.ZipkinServerParams{
+	zkServer, err := server.StartZipkinServer(&server.ZipkinServerParams{
 		HostPort:       builderOpts.CollectorZipkinHTTPHostPort,
 		Handler:        c.spanHandlers.ZipkinSpansHandler,
 		HealthCheck:    c.hCheck,
 		AllowedHeaders: builderOpts.CollectorZipkinAllowedHeaders,
 		AllowedOrigins: builderOpts.CollectorZipkinAllowedOrigins,
 		Logger:         c.logger,
-	}); err != nil {
-		c.logger.Fatal("could not start the Zipkin server", zap.Error(err))
-	} else {
-		c.zkServer = zkServer
+	})
+	if err != nil {
+		return fmt.Errorf("could not start the Zipkin server %w", err)
 	}
+	c.zkServer = zkServer
+
 	c.publishOpts(builderOpts)
 
 	return nil
@@ -143,19 +145,17 @@ func (c *Collector) Close() error {
 	// HTTP server
 	if c.hServer != nil {
 		timeout, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-		err := c.hServer.Shutdown(timeout)
-		if err != nil {
-			c.logger.Error("failed to stop the main HTTP server", zap.Error(err))
+		if err := c.hServer.Shutdown(timeout); err != nil {
+			c.logger.Fatal("failed to stop the main HTTP server", zap.Error(err))
 		}
 		defer cancel()
 	}
 
 	// Zipkin server
 	if c.zkServer != nil {
 		timeout, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-		err := c.zkServer.Shutdown(timeout)
-		if err != nil {
-			c.logger.Error("failed to stop the Zipkin server", zap.Error(err))
+		if err := c.zkServer.Shutdown(timeout); err != nil {
+			c.logger.Fatal("failed to stop the Zipkin server", zap.Error(err))
 		}
 		defer cancel()
 	}