Skip to content

Commit

Permalink
refactor: make QueryLoggingResolver read the hostname on creation
Browse files Browse the repository at this point in the history
Remove that code from util and the use of globals.

Partly move away from calling the value a "hostname" since it's more of
an instance ID, especially if you run more than one per host.
  • Loading branch information
ThinkChaos committed Aug 30, 2024
1 parent 1f8bc4f commit a4f853a
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 106 deletions.
2 changes: 1 addition & 1 deletion querylog/database_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion querylog/file_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func createQueryLogRow(logEntry *LogEntry) []string {
logEntry.ResponseCode,
logEntry.ResponseType,
logEntry.QuestionType,
util.HostnameString(),
logEntry.BlockyInstance,
}
}

Expand Down
3 changes: 1 addition & 2 deletions querylog/logger_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"strings"

"github.com/0xERR0R/blocky/log"
"github.com/0xERR0R/blocky/util"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -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,
})
}

Expand Down
1 change: 0 additions & 1 deletion querylog/logger_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
1 change: 1 addition & 0 deletions querylog/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type LogEntry struct {
QuestionType string
QuestionName string
Answer string
BlockyInstance string
}

type Writer interface {
Expand Down
46 changes: 37 additions & 9 deletions resolver/query_logging_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ package resolver

import (
"context"
"errors"
"fmt"
"os"
"strings"
"time"

"github.com/0xERR0R/blocky/config"
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 differentiation 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))
}
22 changes: 19 additions & 3 deletions resolver/query_logging_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ var _ = Describe("QueryLoggingResolver", func() {
var (
sut *QueryLoggingResolver
sutConfig config.QueryLog
err error
m *mockResolver
tmpDir *TmpFolder
mockRType ResponseType
Expand All @@ -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())

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,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: "),
Expand All @@ -324,7 +326,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,
Expand Down
40 changes: 0 additions & 40 deletions util/hostname.go

This file was deleted.

48 changes: 0 additions & 48 deletions util/hostname_test.go

This file was deleted.

0 comments on commit a4f853a

Please sign in to comment.