Fallback when using mysql or file as query log (#318) (#336)

querylog/database_writer.go

@@ -1,6 +1,7 @@
 package querylog
 
 import (
+	"fmt"
 	"strings"
 	"sync"
 	"time"
@@ -38,11 +39,12 @@ type DatabaseWriter struct {
 	dbFlushPeriod    time.Duration
 }
 
-func NewDatabaseWriter(target string, logRetentionDays uint64, dbFlushPeriod time.Duration) *DatabaseWriter {
+func NewDatabaseWriter(target string, logRetentionDays uint64, dbFlushPeriod time.Duration) (*DatabaseWriter, error) {
 	return newDatabaseWriter(mysql.Open(target), logRetentionDays, dbFlushPeriod)
 }
 
-func newDatabaseWriter(target gorm.Dialector, logRetentionDays uint64, dbFlushPeriod time.Duration) *DatabaseWriter {
+func newDatabaseWriter(target gorm.Dialector, logRetentionDays uint64,
+	dbFlushPeriod time.Duration) (*DatabaseWriter, error) {
 	db, err := gorm.Open(target, &gorm.Config{
 		Logger: logger.New(
 			log.Log(),
@@ -55,12 +57,13 @@ func newDatabaseWriter(target gorm.Dialector, logRetentionDays uint64, dbFlushPe
 	})
 
 	if err != nil {
-		util.FatalOnError("can't create database connection", err)
-		return nil
+		return nil, fmt.Errorf("can't create database connection: %w", err)
 	}
 
 	// Migrate the schema
-	util.FatalOnError("can't perform auto migration", db.AutoMigrate(&logEntry{}))
+	if err := db.AutoMigrate(&logEntry{}); err != nil {
+		return nil, fmt.Errorf("can't perform auto migration: %w", err)
+	}
 
 	w := &DatabaseWriter{
 		db:               db,
@@ -69,7 +72,7 @@ func newDatabaseWriter(target gorm.Dialector, logRetentionDays uint64, dbFlushPe
 
 	go w.periodicFlush()
 
-	return w
+	return w, nil
 }
 
 func (d *DatabaseWriter) periodicFlush() {

querylog/database_writer_test.go

@@ -3,8 +3,6 @@ package querylog
 import (
 	"time"
 
-	"github.com/0xERR0R/blocky/helpertest"
-
 	"github.com/0xERR0R/blocky/model"
 	"github.com/0xERR0R/blocky/util"
 	"github.com/miekg/dns"
@@ -22,7 +20,8 @@ var _ = Describe("DatabaseWriter", func() {
 		When("New log entry was created", func() {
 			It("should be persisted in the database", func() {
 				sqlite := sqlite.Open("file::memory:")
-				writer := newDatabaseWriter(sqlite, 7, 1)
+				writer, err := newDatabaseWriter(sqlite, 7, 1)
+				Expect(err).Should(Succeed())
 				request := &model.Request{
 					Req: util.NewMsgWithQuestion("google.de.", dns.TypeA),
 					Log: logrus.NewEntry(logrus.New()),
@@ -56,7 +55,9 @@ var _ = Describe("DatabaseWriter", func() {
 		When("There are log entries with timestamp exceeding the retention period", func() {
 			It("these old entries should be deleted", func() {
 				sqlite := sqlite.Open("file::memory:")
-				writer := newDatabaseWriter(sqlite, 1, 1)
+				writer, err := newDatabaseWriter(sqlite, 1, 1)
+				Expect(err).Should(Succeed())
+
 				request := &model.Request{
 					Req: util.NewMsgWithQuestion("google.de.", dns.TypeA),
 					Log: logrus.NewEntry(logrus.New()),
@@ -108,10 +109,9 @@ var _ = Describe("DatabaseWriter", func() {
 
 		When("connection parameters wrong", func() {
 			It("should be log with fatal", func() {
-				helpertest.ShouldLogFatal(func() {
-					NewDatabaseWriter("wrong param", 7, 1)
-				})
-
+				_, err := NewDatabaseWriter("wrong param", 7, 1)
+				Expect(err).Should(HaveOccurred())
+				Expect(err.Error()).Should(HavePrefix("can't create database connection"))
 			})
 		})
 	})

querylog/file_writer.go

@@ -27,16 +27,16 @@ type FileWriter struct {
 	logRetentionDays uint64
 }
 
-func NewCSVWriter(target string, perClient bool, logRetentionDays uint64) *FileWriter {
+func NewCSVWriter(target string, perClient bool, logRetentionDays uint64) (*FileWriter, error) {
 	if _, err := os.Stat(target); target != "" && err != nil && os.IsNotExist(err) {
-		log.PrefixedLog(loggerPrefixFileWriter).Fatalf("query log directory '%s' does not exist or is not writable", target)
+		return nil, fmt.Errorf("query log directory '%s' does not exist or is not writable", target)
 	}
 
 	return &FileWriter{
 		target:           target,
 		perClient:        perClient,
 		logRetentionDays: logRetentionDays,
-	}
+	}, nil
 }
 
 func (d *FileWriter) Write(entry *Entry) {

querylog/file_writer_test.go

@@ -11,7 +11,6 @@ import (
 	"path/filepath"
 	"time"
 
-	"github.com/0xERR0R/blocky/helpertest"
 	"github.com/0xERR0R/blocky/log"
 
 	"github.com/0xERR0R/blocky/model"
@@ -29,23 +28,21 @@ var _ = Describe("FileWriter", func() {
 		Expect(err).Should(Succeed())
 	})
 	AfterEach(func() {
-		Expect(err).Should(Succeed())
 		_ = os.RemoveAll(tmpDir)
 	})
 
 	Describe("CSV writer", func() {
 		When("target dir does not exist", func() {
-			It("should log with fatal", func() {
-				helpertest.ShouldLogFatal(func() {
-					NewCSVWriter("wrongdir", false, 0)
-				})
+			It("should return error", func() {
+				_, err = NewCSVWriter("wrongdir", false, 0)
+				Expect(err).Should(HaveOccurred())
 			})
 		})
 		When("New log entry was created", func() {
 			It("should be logged in one file", func() {
 				tmpDir, err = ioutil.TempDir("", "queryLoggingResolver")
 				Expect(err).Should(Succeed())
-				writer := NewCSVWriter(tmpDir, false, 0)
+				writer, _ := NewCSVWriter(tmpDir, false, 0)
 				res, err := util.NewMsgWithAnswer("example.com", 123, dns.TypeA, "123.124.122.122")
 
 				Expect(err).Should(Succeed())
@@ -92,7 +89,7 @@ var _ = Describe("FileWriter", func() {
 			It("should be logged in separate files per client", func() {
 				tmpDir, err = ioutil.TempDir("", "queryLoggingResolver")
 				Expect(err).Should(Succeed())
-				writer := NewCSVWriter(tmpDir, true, 0)
+				writer, _ := NewCSVWriter(tmpDir, true, 0)
 				res, err := util.NewMsgWithAnswer("example.com", 123, dns.TypeA, "123.124.122.122")
 
 				Expect(err).Should(Succeed())
@@ -143,7 +140,7 @@ var _ = Describe("FileWriter", func() {
 			It("should delete old files", func() {
 				tmpDir, err = ioutil.TempDir("", "queryLoggingResolver")
 				Expect(err).Should(Succeed())
-				writer := NewCSVWriter(tmpDir, false, 1)
+				writer, _ := NewCSVWriter(tmpDir, false, 1)
 				res, err := util.NewMsgWithAnswer("example.com", 123, dns.TypeA, "123.124.122.122")
 
 				Expect(err).Should(Succeed())

resolver/query_logging_resolver.go

@@ -22,32 +22,44 @@ type QueryLoggingResolver struct {
 	logRetentionDays uint64
 	logChan          chan *querylog.Entry
 	writer           querylog.Writer
+	logType          config.QueryLogType
 }
 
 // NewQueryLoggingResolver returns a new resolver instance
 func NewQueryLoggingResolver(cfg config.QueryLogConfig) ChainedResolver {
 	var writer querylog.Writer
 
-	switch cfg.Type {
+	var err error
+
+	logType := cfg.Type
+	switch logType {
 	case config.QueryLogTypeCsv:
-		writer = querylog.NewCSVWriter(cfg.Target, false, cfg.LogRetentionDays)
+		writer, err = querylog.NewCSVWriter(cfg.Target, false, cfg.LogRetentionDays)
 	case config.QueryLogTypeCsvClient:
-		writer = querylog.NewCSVWriter(cfg.Target, true, cfg.LogRetentionDays)
+		writer, err = querylog.NewCSVWriter(cfg.Target, true, cfg.LogRetentionDays)
 	case config.QueryLogTypeMysql:
-		writer = querylog.NewDatabaseWriter(cfg.Target, cfg.LogRetentionDays, 30*time.Second)
+		writer, err = querylog.NewDatabaseWriter(cfg.Target, cfg.LogRetentionDays, 30*time.Second)
 	case config.QueryLogTypeConsole:
 		writer = querylog.NewLoggerWriter()
 	case config.QueryLogTypeNone:
 		writer = querylog.NewNoneWriter()
 	}
 
+	if err != nil {
+		logger(queryLoggingResolverPrefix).Error("can't create query log writer, using console as fallback: ", err)
+
+		writer = querylog.NewLoggerWriter()
+		logType = config.QueryLogTypeConsole
+	}
+
 	logChan := make(chan *querylog.Entry, logChanCap)
 
 	resolver := QueryLoggingResolver{
 		target:           cfg.Target,
 		logRetentionDays: cfg.LogRetentionDays,
 		logChan:          logChan,
 		writer:           writer,
+		logType:          logType,
 	}
 
 	go resolver.writeLog()
@@ -118,16 +130,9 @@ func (r *QueryLoggingResolver) writeLog() {
 
 // Configuration returns the current resolver configuration
 func (r *QueryLoggingResolver) Configuration() (result []string) {
-	if r.target != "" {
-		result = append(result, fmt.Sprintf("target: \"%s\"", r.target))
-		result = append(result, fmt.Sprintf("logRetentionDays: %d", r.logRetentionDays))
-
-		if r.logRetentionDays == 0 {
-			result = append(result, "log cleanup deactivated")
-		}
-	} else {
-		result = []string{"deactivated"}
-	}
+	result = append(result, fmt.Sprintf("type: \"%s\"", r.logType))
+	result = append(result, fmt.Sprintf("target: \"%s\"", r.target))
+	result = append(result, fmt.Sprintf("logRetentionDays: %d", r.logRetentionDays))
 
 	return
 }

resolver/query_logging_resolver_test.go

@@ -11,7 +11,6 @@ import (
 	"path/filepath"
 	"time"
 
-	"github.com/0xERR0R/blocky/helpertest"
 	"github.com/0xERR0R/blocky/querylog"
 
 	"github.com/0xERR0R/blocky/config"
@@ -169,7 +168,9 @@ var _ = Describe("QueryLoggingResolver", func() {
 	Describe("Slow writer", func() {
 		When("writer is too slow", func() {
 			BeforeEach(func() {
-				sutConfig = config.QueryLogConfig{}
+				sutConfig = config.QueryLogConfig{
+					Type: config.QueryLogTypeNone,
+				}
 			})
 			It("should drop messages", func() {
 				mockWriter := &SlowMockWriter{}
@@ -202,54 +203,9 @@ var _ = Describe("QueryLoggingResolver", func() {
 				Expect(len(c) > 1).Should(BeTrue())
 			})
 		})
-
-		When("resolver is disabled", func() {
-			BeforeEach(func() {
-				sutConfig = config.QueryLogConfig{}
-			})
-			It("should return 'disabled'", func() {
-				c := sut.Configuration()
-				Expect(c).Should(HaveLen(1))
-				Expect(c).Should(Equal([]string{"deactivated"}))
-			})
-		})
 	})
 
 	Describe("Clean up of query log directory", func() {
-		When("Log directory does not exist", func() {
-
-			It("should exit with error", func() {
-				defer func() { Log().ExitFunc = nil }()
-
-				var fatal bool
-
-				Log().ExitFunc = func(int) { fatal = true }
-				_ = NewQueryLoggingResolver(config.QueryLogConfig{
-					Target: "notExists",
-					Type:   config.QueryLogTypeCsv,
-				})
-
-				Expect(fatal).Should(BeTrue())
-			})
-		})
-		When("not existing log directory is configured, log retention is enabled", func() {
-			It("should exit with error", func() {
-				defer func() { Log().ExitFunc = nil }()
-
-				var fatal bool
-
-				Log().ExitFunc = func(int) { fatal = true }
-
-				sut := NewQueryLoggingResolver(config.QueryLogConfig{
-					Target:           "wrongDir",
-					Type:             config.QueryLogTypeCsv,
-					LogRetentionDays: 7,
-				}).(*QueryLoggingResolver)
-
-				sut.doCleanUp()
-				Expect(fatal).Should(BeTrue())
-			})
-		})
 		When("fallback logger is enabled, log retention is enabled", func() {
 			It("should do nothing", func() {
 
@@ -297,14 +253,14 @@ var _ = Describe("QueryLoggingResolver", func() {
 
 var _ = Describe("Wrong target configuration", func() {
 	When("database path is wrong", func() {
-		It("should log fatal", func() {
-			helpertest.ShouldLogFatal(func() {
-				sutConfig := config.QueryLogConfig{
-					Target: "dummy",
-					Type:   config.QueryLogTypeMysql,
-				}
-				NewQueryLoggingResolver(sutConfig)
-			})
+		It("should use fallback", func() {
+			sutConfig := config.QueryLogConfig{
+				Target: "dummy",
+				Type:   config.QueryLogTypeMysql,
+			}
+			resolver := NewQueryLoggingResolver(sutConfig)
+			loggingResolver := resolver.(*QueryLoggingResolver)
+			Expect(loggingResolver.logType).Should(Equal(config.QueryLogTypeConsole))
 		})
 
 	})