refactor: make QueryLoggingResolver read the hostname on creation

Remove that code from util and the use of globals.

querylog/database_writer.go

@@ -176,7 +176,7 @@ func (d *DatabaseWriter) Write(entry *LogEntry) {
 		EffectiveTLDP: eTLD,
 		Answer:        entry.Answer,
 		ResponseCode:  entry.ResponseCode,
-		Hostname:      util.HostnameString(),
+		Hostname:      entry.BlockyInstance,
 	}
 
 	d.lock.Lock()

querylog/file_writer.go

@@ -115,7 +115,7 @@ func createQueryLogRow(logEntry *LogEntry) []string {
 		logEntry.ResponseCode,
 		logEntry.ResponseType,
 		logEntry.QuestionType,
-		util.HostnameString(),
+		logEntry.BlockyInstance,
 	}
 }
 

querylog/logger_writer.go

@@ -5,7 +5,6 @@ import (
 	"strings"
 
 	"github.com/0xERR0R/blocky/log"
-	"github.com/0xERR0R/blocky/util"
 	"github.com/sirupsen/logrus"
 )
 
@@ -40,7 +39,7 @@ func LogEntryFields(entry *LogEntry) logrus.Fields {
 		"question_type":   entry.QuestionType,
 		"answer":          entry.Answer,
 		"duration_ms":     entry.DurationMs,
-		"hostname":        util.HostnameString(),
+		"instance":        entry.BlockyInstance,
 	})
 }
 

querylog/logger_writer_test.go

@@ -51,7 +51,6 @@ var _ = Describe("LoggerWriter", func() {
 			Expect(fields).Should(HaveKeyWithValue("duration_ms", entry.DurationMs))
 			Expect(fields).Should(HaveKeyWithValue("question_type", entry.QuestionType))
 			Expect(fields).Should(HaveKeyWithValue("response_code", entry.ResponseCode))
-			Expect(fields).Should(HaveKey("hostname"))
 
 			Expect(fields).ShouldNot(HaveKey("client_names"))
 			Expect(fields).ShouldNot(HaveKey("question_name"))

querylog/writer.go

@@ -15,6 +15,7 @@ type LogEntry struct {
 	QuestionType   string
 	QuestionName   string
 	Answer         string
+	BlockyInstance string
 }
 
 type Writer interface {

resolver/query_logging_resolver.go

@@ -2,6 +2,10 @@ package resolver
 
 import (
 	"context"
+	"errors"
+	"fmt"
+	"os"
+	"strings"
 	"time"
 
 	"github.com/0xERR0R/blocky/config"
@@ -25,8 +29,9 @@ type QueryLoggingResolver struct {
 	NextResolver
 	typed
 
-	logChan chan *querylog.LogEntry
-	writer  querylog.Writer
+	logChan    chan *querylog.LogEntry
+	writer     querylog.Writer
+	instanceID string
 }
 
 func GetQueryLoggingWriter(ctx context.Context, cfg config.QueryLog) (querylog.Writer, error) {
@@ -58,7 +63,7 @@ func GetQueryLoggingWriter(ctx context.Context, cfg config.QueryLog) (querylog.W
 }
 
 // NewQueryLoggingResolver returns a new resolver instance
-func NewQueryLoggingResolver(ctx context.Context, cfg config.QueryLog) *QueryLoggingResolver {
+func NewQueryLoggingResolver(ctx context.Context, cfg config.QueryLog) (*QueryLoggingResolver, error) {
 	logger := log.PrefixedLog(queryLoggingResolverType)
 
 	var writer querylog.Writer
@@ -86,14 +91,20 @@ func NewQueryLoggingResolver(ctx context.Context, cfg config.QueryLog) *QueryLog
 		cfg.Type = config.QueryLogTypeConsole
 	}
 
+	instanceID, err := readInstanceID("/etc/hostname")
+	if err != nil {
+		return nil, err
+	}
+
 	logChan := make(chan *querylog.LogEntry, logChanCap)
 
 	resolver := QueryLoggingResolver{
 		configurable: withConfig(&cfg),
 		typed:        withType(queryLoggingResolverType),
 
-		logChan: logChan,
-		writer:  writer,
+		logChan:    logChan,
+		writer:     writer,
+		instanceID: instanceID,
 	}
 
 	go resolver.writeLog(ctx)
@@ -103,7 +114,7 @@ func NewQueryLoggingResolver(ctx context.Context, cfg config.QueryLog) *QueryLog
 		go resolver.periodicCleanUp(ctx)
 	}
 
-	return &resolver
+	return &resolver, nil
 }
 
 // triggers periodically cleanup of old log files
@@ -170,9 +181,10 @@ func (r *QueryLoggingResolver) createLogEntry(request *model.Request, response *
 	start time.Time, durationMs int64,
 ) *querylog.LogEntry {
 	entry := querylog.LogEntry{
-		Start:       start,
-		ClientIP:    "0.0.0.0",
-		ClientNames: []string{"none"},
+		Start:          start,
+		ClientIP:       "0.0.0.0",
+		ClientNames:    []string{"none"},
+		BlockyInstance: r.instanceID,
 	}
 
 	for _, f := range r.cfg.Fields {
@@ -227,3 +239,19 @@ func (r *QueryLoggingResolver) writeLog(ctx context.Context) {
 		}
 	}
 }
+
+func readInstanceID(file string) (string, error) {
+	// Prefer /etc/hostname over os.Hostname to allow easy differenciation in a Docker Swarm
+	// See details in https://github.com/0xERR0R/blocky/pull/756
+	hn, fErr := os.ReadFile(file)
+	if fErr == nil {
+		return strings.TrimSpace(string(hn)), nil
+	}
+
+	hostname, osErr := os.Hostname()
+	if osErr == nil {
+		return hostname, nil
+	}
+
+	return "", fmt.Errorf("cannot determine instance ID: %w", errors.Join(fErr, osErr))
+}

resolver/query_logging_resolver_test.go

@@ -42,6 +42,7 @@ var _ = Describe("QueryLoggingResolver", func() {
 	var (
 		sut        *QueryLoggingResolver
 		sutConfig  config.QueryLog
+		err        error
 		m          *mockResolver
 		tmpDir     *TmpFolder
 		mockRType  ResponseType
@@ -61,8 +62,6 @@ var _ = Describe("QueryLoggingResolver", func() {
 		ctx, cancelFn = context.WithCancel(context.Background())
 		DeferCleanup(cancelFn)
 
-		var err error
-
 		sutConfig, err = config.WithDefaults[config.QueryLog]()
 		Expect(err).Should(Succeed())
 
@@ -76,7 +75,8 @@ var _ = Describe("QueryLoggingResolver", func() {
 			sutConfig.SetDefaults() // not called when using a struct literal
 		}
 
-		sut = NewQueryLoggingResolver(ctx, sutConfig)
+		sut, err = NewQueryLoggingResolver(ctx, sutConfig)
+		Expect(err).Should(Succeed())
 
 		m = &mockResolver{
 			ResolveFn: func(context.Context, *Request) (*Response, error) {
@@ -441,6 +441,22 @@ var _ = Describe("QueryLoggingResolver", func() {
 			})
 		})
 	})
+
+	Describe("Hostname function tests", func() {
+		It("should use the given file if it exists", func() {
+			expected := "TestName"
+			tmpFile := NewTmpFolder("hostname").CreateStringFile("filetest1", expected+" \n")
+
+			Expect(readInstanceID(tmpFile.Path)).Should(Equal(expected))
+		})
+
+		It("should fallback to os.Hostname", func() {
+			expected, err := os.Hostname()
+			Expect(err).Should(Succeed())
+
+			Expect(readInstanceID("/var/empty/nonexistent")).Should(Equal(expected))
+		})
+	})
 })
 
 func readCsv(file string) ([][]string, error) {

server/server.go

@@ -317,12 +317,14 @@ func createQueryResolver(
 	upstreamTree, utErr := resolver.NewUpstreamTreeResolver(ctx, cfg.Upstreams, bootstrap)
 	blocking, blErr := resolver.NewBlockingResolver(ctx, cfg.Blocking, redisClient, bootstrap)
 	clientNames, cnErr := resolver.NewClientNamesResolver(ctx, cfg.ClientLookup, cfg.Upstreams, bootstrap)
+	queryLogging, qlErr := resolver.NewQueryLoggingResolver(ctx, cfg.QueryLog)
 	condUpstream, cuErr := resolver.NewConditionalUpstreamResolver(ctx, cfg.Conditional, cfg.Upstreams, bootstrap)
 	hostsFile, hfErr := resolver.NewHostsFileResolver(ctx, cfg.HostsFile, bootstrap)
 
 	err := multierror.Append(
 		multierror.Prefix(utErr, "upstream tree resolver: "),
 		multierror.Prefix(blErr, "blocking resolver: "),
+		multierror.Prefix(qlErr, "query logging resolver: "),
 		multierror.Prefix(cnErr, "client names resolver: "),
 		multierror.Prefix(cuErr, "conditional upstream resolver: "),
 		multierror.Prefix(hfErr, "hosts file resolver: "),
@@ -337,7 +339,7 @@ func createQueryResolver(
 		resolver.NewECSResolver(cfg.ECS),
 		clientNames,
 		resolver.NewEDEResolver(cfg.EDE),
-		resolver.NewQueryLoggingResolver(ctx, cfg.QueryLog),
+		queryLogging,
 		resolver.NewMetricsResolver(cfg.Prometheus),
 		resolver.NewRewriterResolver(cfg.CustomDNS.RewriterConfig, resolver.NewCustomDNSResolver(cfg.CustomDNS)),
 		hostsFile,