code review -- move constants into named constants

coredns_plugin/querylog.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
+	"net"
 	"net/http"
 	"os"
 	"path"
@@ -20,8 +21,13 @@ import (
 
 const (
 	logBufferCap           = 5000            // maximum capacity of logBuffer before it's flushed to disk
+	queryLogTimeLimit      = time.Hour * 24  // how far in the past we care about querylogs
+	queryLogRotationPeriod = time.Hour * 24  // rotate the log every 24 hours
+	queryLogFileName       = "querylog.json" // .gz added during compression
 	queryLogCacheSize      = 1000            // maximum API response for /querylog
-	queryLogCacheTime = time.Minute
+	queryLogCacheTime      = time.Minute     // if requested more often than this, give out cached response
+	queryLogTopSize        = 500             // Keep in memory only top N values
+	queryLogAPIPort        = "8618"          // 8618 is sha512sum of "querylog" then each byte summed
 )
 
 var (
@@ -112,7 +118,7 @@ func handleQueryLog(w http.ResponseWriter, r *http.Request) {
 		logBufferLock.RUnlock()
 
 		if len(values) < queryLogCacheSize {
-			values = appendFromLogFile(values, queryLogCacheSize, time.Hour*24)
+			values = appendFromLogFile(values, queryLogCacheSize, queryLogTimeLimit)
 		}
 		queryLogLock.Lock()
 		queryLogCache = values
@@ -223,14 +229,14 @@ func handleQueryLog(w http.ResponseWriter, r *http.Request) {
 	if err != nil {
 		errortext := fmt.Sprintf("Unable to write response json: %s", err)
 		log.Println(errortext)
-		http.Error(w, errortext, 500)
+		http.Error(w, errortext, http.StatusInternalServerError)
 	}
 }
 
 func startQueryLogServer() {
-	listenAddr := "127.0.0.1:8618" // 8618 is sha512sum of "querylog" then each byte summed
+	listenAddr := net.JoinHostPort("127.0.0.1", queryLogAPIPort)
 
-	go periodicQueryLogRotate(queryLogRotationPeriod)
+	go periodicQueryLogRotate()
 	go periodicHourlyTopRotate()
 
 	http.HandleFunc("/querylog", handleQueryLog)

coredns_plugin/querylog_file.go

@@ -13,11 +13,6 @@ import (
 	"github.com/go-test/deep"
 )
 
-const (
-	queryLogRotationPeriod = time.Hour * 24  // rotate the log every 24 hours
-	queryLogFileName       = "querylog.json" // .gz added during compression
-)
-
 var (
 	fileWriteLock sync.Mutex
 )
@@ -135,8 +130,8 @@ func rotateQueryLog() error {
 	return nil
 }
 
-func periodicQueryLogRotate(t time.Duration) {
-	for range time.Tick(t) {
+func periodicQueryLogRotate() {
+	for range time.Tick(queryLogRotationPeriod) {
 		err := rotateQueryLog()
 		if err != nil {
 			log.Printf("Failed to rotate querylog: %s", err)

coredns_plugin/querylog_top.go

@@ -5,6 +5,9 @@ import (
 	"fmt"
 	"log"
 	"net/http"
+	"os"
+	"path"
+	"runtime"
 	"sort"
 	"strconv"
 	"strings"
@@ -30,9 +33,9 @@ type hourTop struct {
 }
 
 func (top *hourTop) init() {
-	top.domains = gcache.New(500).LRU().Build()
-	top.blocked = gcache.New(500).LRU().Build()
-	top.clients = gcache.New(500).LRU().Build()
+	top.domains = gcache.New(topLRUsize).LRU().Build()
+	top.blocked = gcache.New(topLRUsize).LRU().Build()
+	top.clients = gcache.New(topLRUsize).LRU().Build()
 }
 
 type dayTop struct {
@@ -227,7 +230,7 @@ func loadTopFromFiles() error {
 	}
 
 	needMore := func() bool { return true }
-	err := genericLoader(onEntry, needMore, time.Hour*24)
+	err := genericLoader(onEntry, needMore, queryLogTimeLimit)
 	if err != nil {
 		log.Printf("Failed to load entries from querylog: %s", err)
 		return err
@@ -340,20 +343,17 @@ func (d *dayTop) hoursReadUnlock()   { tracelock(); d.hoursLock.RUnlock() }
 func (d *dayTop) loadedWriteLock()   { tracelock(); d.loadedLock.Lock() }
 func (d *dayTop) loadedWriteUnlock() { tracelock(); d.loadedLock.Unlock() }
 
-// func (d *dayTop) loadedReadLock()    { tracelock(); d.loadedLock.RLock() }
-// func (d *dayTop) loadedReadUnlock()  { tracelock(); d.loadedLock.RUnlock() }
-
 func (h *hourTop) Lock()    { tracelock(); h.mutex.Lock() }
 func (h *hourTop) RLock()   { tracelock(); h.mutex.RLock() }
 func (h *hourTop) RUnlock() { tracelock(); h.mutex.RUnlock() }
 func (h *hourTop) Unlock()  { tracelock(); h.mutex.Unlock() }
 
 func tracelock() {
-	/*
+	if false { // not commented out to make code checked during compilation
 		pc := make([]uintptr, 10) // at least 1 entry needed
 		runtime.Callers(2, pc)
 		f := path.Base(runtime.FuncForPC(pc[1]).Name())
 		lockf := path.Base(runtime.FuncForPC(pc[0]).Name())
 		fmt.Fprintf(os.Stderr, "%s(): %s\n", f, lockf)
-	*/
+	}
 }