From a15f21ca1cd0acabf77a4e9750dd77b2b870a6f4 Mon Sep 17 00:00:00 2001
From: Eugene Bujak <hmage@hmage.net>
Date: Mon, 8 Oct 2018 20:02:09 +0300
Subject: [PATCH] code review -- move constants into named constants

---
 coredns_plugin/querylog.go      | 20 +++++++++++++-------
 coredns_plugin/querylog_file.go |  9 ++-------
 coredns_plugin/querylog_top.go  | 18 +++++++++---------
 3 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/coredns_plugin/querylog.go b/coredns_plugin/querylog.go
index 0ff676f8..9e45315f 100644
--- a/coredns_plugin/querylog.go
+++ b/coredns_plugin/querylog.go
@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"log"
+	"net"
 	"net/http"
 	"os"
 	"path"
@@ -19,9 +20,14 @@ import (
 )
 
 const (
-	logBufferCap      = 5000 // maximum capacity of logBuffer before it's flushed to disk
-	queryLogCacheSize = 1000 // maximum API response for /querylog
-	queryLogCacheTime = time.Minute
+	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     // 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)
diff --git a/coredns_plugin/querylog_file.go b/coredns_plugin/querylog_file.go
index fffd050a..7025fcd3 100644
--- a/coredns_plugin/querylog_file.go
+++ b/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)
diff --git a/coredns_plugin/querylog_top.go b/coredns_plugin/querylog_top.go
index 89426e14..e2bfa53e 100644
--- a/coredns_plugin/querylog_top.go
+++ b/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)
-	*/
+	}
 }