From 510c64ef0649df4905cfe4c8353b148d8bcf9c85 Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Mon, 12 Mar 2018 16:32:35 -0500
Subject: [PATCH 01/10] Move diagnostics into package pilosa

---
 broadcast.go                                  |   8 +-
 diagnostics/diagnostics.go => diagnostics.go  | 103 +++++++++---------
 ...diagnostics_test.go => diagnostics_test.go |  41 +++----
 gc.go                                         |   4 +-
 server.go                                     |  43 +-------
 5 files changed, 77 insertions(+), 122 deletions(-)
 rename diagnostics/diagnostics.go => diagnostics.go (73%)
 rename diagnostics/diagnostics_test.go => diagnostics_test.go (83%)

diff --git a/broadcast.go b/broadcast.go
index a4e6403c1..de43f3b85 100644
--- a/broadcast.go
+++ b/broadcast.go
@@ -63,17 +63,17 @@ var NopBroadcaster Broadcaster
 
 type nopBroadcaster struct{}
 
-// SendSync A no-op implemenetation of Broadcaster SendSync method.
+// SendSync A no-op implementation of Broadcaster SendSync method.
 func (n *nopBroadcaster) SendSync(pb proto.Message) error {
 	return nil
 }
 
-// SendAsync A no-op implemenetation of Broadcaster SendAsync method.
+// SendAsync A no-op implementation of Broadcaster SendAsync method.
 func (n *nopBroadcaster) SendAsync(pb proto.Message) error {
 	return nil
 }
 
-// SendTo is a no-op implemenetation of Broadcaster SendTo method.
+// SendTo is a no-op implementation of Broadcaster SendTo method.
 func (c *nopBroadcaster) SendTo(to *Node, pb proto.Message) error {
 	return nil
 }
@@ -112,7 +112,7 @@ var NopGossiper Gossiper
 
 type nopGossiper struct{}
 
-// SendAsync A no-op implemenetation of Gossiper SendAsync method.
+// SendAsync A no-op implementation of Gossiper SendAsync method.
 func (n *nopGossiper) SendAsync(pb proto.Message) error {
 	return nil
 }
diff --git a/diagnostics/diagnostics.go b/diagnostics.go
similarity index 73%
rename from diagnostics/diagnostics.go
rename to diagnostics.go
index 60248d6bb..602e910e0 100644
--- a/diagnostics/diagnostics.go
+++ b/diagnostics.go
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package diagnostics
+package pilosa
 
 import (
 	"bytes"
@@ -36,7 +36,7 @@ import (
 
 // Default version check URL.
 const (
-	DefaultVersionCheckURL = "https://diagnostics.pilosa.com/v0/version"
+	defaultVersionCheckURL = "https://diagnostics.pilosa.com/v0/version"
 )
 
 type versionResponse struct {
@@ -44,11 +44,9 @@ type versionResponse struct {
 	Message string `json:"message"`
 }
 
-// Diagnostics represents a client to the Pilosa cluster.
-type Diagnostics struct {
+// DiagnosticsCollector represents a collector/sender of diagnostics data
+type DiagnosticsCollector struct {
 	mu          sync.Mutex
-	wg          sync.WaitGroup
-	closing     chan struct{}
 	host        string
 	VersionURL  string
 	version     string
@@ -65,13 +63,12 @@ type Diagnostics struct {
 	logOutput io.Writer
 }
 
-// New returns a pointer to a new Diagnostics Client given an addr in the format "hostname:port".
-func New(host string) *Diagnostics {
+// New returns a pointer to a new DiagnosticsCollector Client given an addr in the format "hostname:port".
+func NewDiagnosticsCollector(host string) *DiagnosticsCollector {
 
-	return &Diagnostics{
-		closing:    make(chan struct{}),
+	return &DiagnosticsCollector{
 		host:       host,
-		VersionURL: DefaultVersionCheckURL,
+		VersionURL: defaultVersionCheckURL,
 		startTime:  time.Now().Unix(),
 		start:      time.Now(),
 		client:     http.DefaultClient,
@@ -81,37 +78,21 @@ func New(host string) *Diagnostics {
 }
 
 // SetVersion of locally running Pilosa Cluster to check against master.
-func (d *Diagnostics) SetVersion(v string) {
+func (d *DiagnosticsCollector) SetVersion(v string) {
 	d.version = v
 	d.Set("Version", v)
 }
 
 // SetInterval of the diagnostic go routine and match with the circuit breaker timeout.
-func (d *Diagnostics) SetInterval(i time.Duration) {
+func (d *DiagnosticsCollector) SetInterval(i time.Duration) {
 	d.interval = i
 }
 
-// schedule start the diagnostics service ticker.
-func (d *Diagnostics) schedule() {
-	ticker := time.NewTicker(d.interval)
-	defer ticker.Stop()
-
-	for {
-		select {
-		case <-d.closing:
-			return
-		case <-ticker.C:
-			d.CheckVersion()
-			d.Flush()
-		}
-	}
-}
-
 // Flush sends the current metrics.
-func (d *Diagnostics) Flush() error {
+func (d *DiagnosticsCollector) Flush() error {
 	d.mu.Lock()
 	d.metrics["Uptime"] = (time.Now().Unix() - d.startTime)
-	buf, _ := d.Encode()
+	buf, _ := d.encode()
 	d.mu.Unlock()
 
 	_, err := d.cb.Execute(func() (interface{}, error) {
@@ -135,7 +116,7 @@ func (d *Diagnostics) Flush() error {
 }
 
 // Open configures the circuit breaker used by the HTTP client.
-func (d *Diagnostics) Open() {
+func (d *DiagnosticsCollector) Open() {
 	var st gobreaker.Settings
 	if d.interval > 0 {
 		st.Timeout = d.interval * 2
@@ -145,15 +126,8 @@ func (d *Diagnostics) Open() {
 	d.logger().Printf("Pilosa is currently configured to send small diagnostics reports to our team every hour. More information here: https://www.pilosa.com/docs/latest/administration/#diagnostics")
 }
 
-// Close notify goroutine to stop.
-func (d *Diagnostics) Close() error {
-	close(d.closing)
-	d.wg.Wait()
-	return nil
-}
-
 // CheckVersion of the local build against Pilosa master.
-func (d *Diagnostics) CheckVersion() error {
+func (d *DiagnosticsCollector) CheckVersion() error {
 	var rsp versionResponse
 	req, err := http.NewRequest("GET", d.VersionURL, nil)
 	resp, err := d.client.Do(req)
@@ -174,15 +148,15 @@ func (d *Diagnostics) CheckVersion() error {
 	}
 
 	d.lastVersion = rsp.Version
-	if err := d.CompareVersion(rsp.Version); err != nil {
+	if err := d.compareVersion(rsp.Version); err != nil {
 		d.logger().Printf("%s\n", err.Error())
 	}
 
 	return nil
 }
 
-// CompareVersion check version strings.
-func (d *Diagnostics) CompareVersion(value string) error {
+// compareVersion check version strings.
+func (d *DiagnosticsCollector) compareVersion(value string) error {
 	currentVersion := VersionSegments(value)
 	localVersion := VersionSegments(d.version)
 
@@ -198,29 +172,29 @@ func (d *Diagnostics) CompareVersion(value string) error {
 }
 
 // Encode metrics maps into the json message format.
-func (d *Diagnostics) Encode() ([]byte, error) {
+func (d *DiagnosticsCollector) encode() ([]byte, error) {
 	return json.Marshal(d.metrics)
 }
 
 // Set adds a key value metric.
-func (d *Diagnostics) Set(name string, value interface{}) {
+func (d *DiagnosticsCollector) Set(name string, value interface{}) {
 	d.mu.Lock()
 	defer d.mu.Unlock()
 	d.metrics[name] = value
 }
 
 // SetLogger Set the logger output type.
-func (d *Diagnostics) SetLogger(logger io.Writer) {
+func (d *DiagnosticsCollector) SetLogger(logger io.Writer) {
 	d.logOutput = logger
 }
 
 // logger returns a logger that writes to LogOutput.
-func (d *Diagnostics) logger() *log.Logger {
+func (d *DiagnosticsCollector) logger() *log.Logger {
 	return log.New(d.logOutput, "", log.LstdFlags)
 }
 
 // EnrichWithOSInfo adds OS information to the diagnostics payload.
-func (d *Diagnostics) EnrichWithOSInfo() {
+func (d *DiagnosticsCollector) EnrichWithOSInfo() {
 	osInfo, err := host.Info()
 	if err != nil {
 		d.logOutput.Write([]byte(err.Error()))
@@ -243,7 +217,7 @@ func (d *Diagnostics) EnrichWithOSInfo() {
 }
 
 // EnrichWithMemoryInfo adds memory information to the diagnostics payload.
-func (d *Diagnostics) EnrichWithMemoryInfo() {
+func (d *DiagnosticsCollector) EnrichWithMemoryInfo() {
 	memory, err := mem.VirtualMemory()
 	if err != nil {
 		d.logOutput.Write([]byte(err.Error()))
@@ -254,6 +228,37 @@ func (d *Diagnostics) EnrichWithMemoryInfo() {
 
 }
 
+// EnrichWithSchemaProperties adds schema info to the diagnostics payload.
+func (d *DiagnosticsCollector) EnrichWithSchemaProperties(holder *Holder) {
+	var numSlices uint64
+	numFrames := 0
+	numIndexes := 0
+	bsiFieldCount := 0
+	timeQuantumEnabled := false
+
+	for _, index := range holder.Indexes() {
+		numSlices += index.MaxSlice() + 1
+		numIndexes += 1
+		for _, frame := range index.Frames() {
+			numFrames += 1
+			if frame.rangeEnabled {
+				if fields, err := frame.GetFields(); err == nil {
+					bsiFieldCount += len(fields)
+				}
+			}
+			if frame.TimeQuantum() != "" {
+				timeQuantumEnabled = true
+			}
+		}
+	}
+
+	d.Set("NumIndexes", numIndexes)
+	d.Set("NumFrames", numFrames)
+	d.Set("NumSlices", numSlices)
+	d.Set("BSIFieldCount", bsiFieldCount)
+	d.Set("TimeQuantumEnabled", timeQuantumEnabled)
+}
+
 // VersionSegments returns the numeric segments of the version as a slice of ints.
 func VersionSegments(segments string) []int {
 	segments = strings.Trim(segments, "v")
diff --git a/diagnostics/diagnostics_test.go b/diagnostics_test.go
similarity index 83%
rename from diagnostics/diagnostics_test.go
rename to diagnostics_test.go
index 8f85a57db..9d8e35d76 100644
--- a/diagnostics/diagnostics_test.go
+++ b/diagnostics_test.go
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package diagnostics_test
+package pilosa
 
 import (
 	"encoding/json"
@@ -23,25 +23,21 @@ import (
 	"runtime"
 	"strings"
 	"testing"
-
-	"github.com/pilosa/pilosa/diagnostics"
 )
 
 func TestDiagnosticsClient(t *testing.T) {
 	// Mock server.
 	server := httptest.NewServer(nil)
-	defer server.Close()
 
 	// Create a new client.
-	d := diagnostics.New(server.URL)
+	d := NewDiagnosticsCollector(server.URL)
 	d.SetLogger(ioutil.Discard)
 	d.Open()
-	defer d.Close()
 
 	d.Set("gg", 10)
 	d.Set("ss", "ss")
 
-	data, err := d.Encode()
+	data, err := d.encode()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -58,7 +54,7 @@ func TestDiagnosticsClient(t *testing.T) {
 
 	// Test the metrics after a flush.
 	d.Flush()
-	data, err = d.Encode()
+	data, err = d.encode()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -74,7 +70,7 @@ func TestDiagnosticsClient(t *testing.T) {
 
 func TestDiagnosticsVersion_Parse(t *testing.T) {
 	version := "0.1.1"
-	vs := diagnostics.VersionSegments(version)
+	vs := VersionSegments(version)
 
 	output := []int{0, 1, 1}
 	if !reflect.DeepEqual(vs, output) {
@@ -83,35 +79,34 @@ func TestDiagnosticsVersion_Parse(t *testing.T) {
 }
 
 func TestDiagnosticsVersion_Compare(t *testing.T) {
-	d := diagnostics.New("localhost:10101")
+	d := NewDiagnosticsCollector("localhost:10101")
 	d.Open()
-	defer d.Close()
 
 	version := "v0.1.1"
 	d.SetVersion(version)
 
-	err := d.CompareVersion("v1.7.0")
+	err := d.compareVersion("v1.7.0")
 	if !strings.Contains(err.Error(), "A newer version") {
 		t.Fatalf("Expected a newer version is available, actual error: %s", err)
 	}
-	err = d.CompareVersion("1.7.0")
+	err = d.compareVersion("1.7.0")
 	if !strings.Contains(err.Error(), "A newer version") {
 		t.Fatalf("Expected a newer version is available, actual error: %s", err)
 	}
-	err = d.CompareVersion("0.7.0")
+	err = d.compareVersion("0.7.0")
 	if !strings.Contains(err.Error(), "The latest Minor release is") {
 		t.Fatalf("Expected Minor Version Missmatch, actual error: %s", err)
 	}
-	err = d.CompareVersion("0.1.2")
+	err = d.compareVersion("0.1.2")
 	if !strings.Contains(err.Error(), "There is a new patch release of Pilosa") {
 		t.Fatalf("Expected Patch Version Missmatch, actual error: %s", err)
 	}
-	err = d.CompareVersion("0.1.1")
+	err = d.compareVersion("0.1.1")
 	if err != nil {
 		t.Fatalf("Versions should match")
 	}
 	d.SetVersion("v1.7.0")
-	err = d.CompareVersion("0.7.2")
+	err = d.compareVersion("0.7.2")
 	if err != nil {
 		t.Fatalf("Local version is greater")
 	}
@@ -125,11 +120,9 @@ func TestDiagnosticsVersion_Check(t *testing.T) {
 			Version: "1.1.1",
 		})
 	}))
-	defer server.Close()
 
 	// Create a new client.
-	d := diagnostics.New("localhost:10101")
-	defer d.Close()
+	d := NewDiagnosticsCollector("localhost:10101")
 
 	version := "0.1.1"
 	d.SetVersion(version)
@@ -138,10 +131,6 @@ func TestDiagnosticsVersion_Check(t *testing.T) {
 	d.CheckVersion()
 }
 
-type versionResponse struct {
-	Version string `json:"version"`
-}
-
 func compareJSON(a, b []byte) (bool, error) {
 	var j1, j2 interface{}
 	if err := json.Unmarshal(a, &j1); err != nil {
@@ -156,12 +145,10 @@ func compareJSON(a, b []byte) (bool, error) {
 func BenchmarkDiagnostics(b *testing.B) {
 	// Mock server.
 	server := httptest.NewServer(nil)
-	defer server.Close()
 
 	// Create a new client.
-	d := diagnostics.New(server.URL)
+	d := NewDiagnosticsCollector(server.URL)
 	d.SetLogger(ioutil.Discard)
-	defer d.Close()
 
 	prev := runtime.GOMAXPROCS(4)
 	defer runtime.GOMAXPROCS(prev)
diff --git a/gc.go b/gc.go
index a260cc0b5..3f036bdcd 100644
--- a/gc.go
+++ b/gc.go
@@ -29,10 +29,10 @@ var NopGCNotifier GCNotifier
 
 type nopGCNotifier struct{}
 
-// Close is a no-op implemenetation of GCNotifier Close method.
+// Close is a no-op implementation of GCNotifier Close method.
 func (n *nopGCNotifier) Close() {}
 
-// AfterGC is a no-op implemenetation of GCNotifier AfterGC method.
+// AfterGC is a no-op implementation of GCNotifier AfterGC method.
 func (c *nopGCNotifier) AfterGC() <-chan struct{} {
 	return nil
 }
diff --git a/server.go b/server.go
index 531609278..82ca2db2d 100644
--- a/server.go
+++ b/server.go
@@ -32,7 +32,6 @@ import (
 	"time"
 
 	"github.com/gogo/protobuf/proto"
-	"github.com/pilosa/pilosa/diagnostics"
 	"github.com/pilosa/pilosa/internal"
 
 	"golang.org/x/sync/errgroup"
@@ -70,7 +69,7 @@ type Server struct {
 	NodeID      string
 	URI         URI
 	Cluster     *Cluster
-	diagnostics *diagnostics.Diagnostics
+	diagnostics *DiagnosticsCollector
 
 	GCNotifier GCNotifier
 
@@ -100,7 +99,7 @@ func NewServer() *Server {
 		Handler:           NewHandler(),
 		Broadcaster:       NopBroadcaster,
 		BroadcastReceiver: NopBroadcastReceiver,
-		diagnostics:       diagnostics.New(DefaultDiagnosticServer),
+		diagnostics:       NewDiagnosticsCollector(DefaultDiagnosticServer),
 
 		Network: "tcp",
 
@@ -622,13 +621,13 @@ func (s *Server) monitorDiagnostics() {
 
 	// Flush the diagnostics metrics at startup, then on each tick interval
 	flush := func() {
-		enrichDiagnosticsWithSchemaProperties(s.diagnostics, s.Holder)
 		openFiles, err := CountOpenFiles()
 		if err == nil {
 			s.diagnostics.Set("OpenFiles", openFiles)
 		}
 		s.diagnostics.Set("GoRoutines", runtime.NumGoroutine())
 		s.diagnostics.EnrichWithMemoryInfo()
+		s.diagnostics.EnrichWithSchemaProperties(s.Holder)
 		s.diagnostics.CheckVersion()
 		s.diagnostics.Flush()
 	}
@@ -725,39 +724,3 @@ type StatusHandler interface {
 	ClusterStatus() (proto.Message, error)
 	HandleRemoteStatus(proto.Message) error
 }
-
-type diagnosticsFrameProperties struct {
-	BSIFieldCount      int
-	TimeQuantumEnabled bool
-}
-
-func enrichDiagnosticsWithSchemaProperties(d *diagnostics.Diagnostics, holder *Holder) {
-	// NOTE: this function is not in the diagnostics package, since circular imports are not allowed.
-	var numSlices uint64
-	numFrames := 0
-	numIndexes := 0
-	bsiFieldCount := 0
-	timeQuantumEnabled := false
-
-	for _, index := range holder.Indexes() {
-		numSlices += index.MaxSlice() + 1
-		numIndexes += 1
-		for _, frame := range index.Frames() {
-			numFrames += 1
-			if frame.rangeEnabled {
-				if fields, err := frame.GetFields(); err == nil {
-					bsiFieldCount += len(fields)
-				}
-			}
-			if frame.TimeQuantum() != "" {
-				timeQuantumEnabled = true
-			}
-		}
-	}
-
-	d.Set("NumIndexes", numIndexes)
-	d.Set("NumFrames", numFrames)
-	d.Set("NumSlices", numSlices)
-	d.Set("BSIFieldCount", bsiFieldCount)
-	d.Set("TimeQuantumEnabled", timeQuantumEnabled)
-}

From f4c1e0c492f772b979279f591461f825c7980151 Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Tue, 13 Mar 2018 11:41:45 -0500
Subject: [PATCH 02/10] Refactor gopsutil into SystemInfo interface/subpackage
 for dependency injection.

---
 diagnostics.go         | 131 ++++++++++++++++++++++++++++++++---------
 gc.go                  |   3 +
 gopsutil/systeminfo.go | 116 ++++++++++++++++++++++++++++++++++++
 server.go              |   5 +-
 server/server.go       |   2 +
 5 files changed, 227 insertions(+), 30 deletions(-)
 create mode 100644 gopsutil/systeminfo.go

diff --git a/diagnostics.go b/diagnostics.go
index 602e910e0..a4caf84cd 100644
--- a/diagnostics.go
+++ b/diagnostics.go
@@ -27,13 +27,9 @@ import (
 	"sync"
 	"time"
 
-	"github.com/shirou/gopsutil/host"
-	"github.com/shirou/gopsutil/mem"
 	"github.com/sony/gobreaker"
 )
 
-// TODO: unique Cluster ID
-
 // Default version check URL.
 const (
 	defaultVersionCheckURL = "https://diagnostics.pilosa.com/v0/version"
@@ -61,6 +57,8 @@ type DiagnosticsCollector struct {
 
 	cb        *gobreaker.CircuitBreaker
 	logOutput io.Writer
+
+	server *Server
 }
 
 // New returns a pointer to a new DiagnosticsCollector Client given an addr in the format "hostname:port".
@@ -193,50 +191,64 @@ func (d *DiagnosticsCollector) logger() *log.Logger {
 	return log.New(d.logOutput, "", log.LstdFlags)
 }
 
+// logErr logs the error and returns true if an error exists
+func (d *DiagnosticsCollector) logErr(err error) bool {
+	if err != nil {
+		d.logOutput.Write([]byte(err.Error()))
+		return true
+	}
+	return false
+}
+
 // EnrichWithOSInfo adds OS information to the diagnostics payload.
 func (d *DiagnosticsCollector) EnrichWithOSInfo() {
-	osInfo, err := host.Info()
-	if err != nil {
-		d.logOutput.Write([]byte(err.Error()))
+	uptime, err := d.server.SystemInfo.Uptime()
+	if !d.logErr(err) {
+		d.Set("HostUptime", uptime)
 	}
-	d.Set("HostUptime", osInfo.Uptime)
-
-	platform, family, version, err := host.PlatformInformation()
-	if err != nil {
-		d.logOutput.Write([]byte(err.Error()))
+	platform, err := d.server.SystemInfo.Platform()
+	if !d.logErr(err) {
+		d.Set("OSPlatform", platform)
 	}
-	d.Set("OSPlatform", platform)
-	d.Set("OSFamily", family)
-	d.Set("OSVersion", version)
-
-	kernelVersion, err := host.KernelVersion()
-	if err != nil {
-		d.logOutput.Write([]byte(err.Error()))
+	family, err := d.server.SystemInfo.Family()
+	if !d.logErr(err) {
+		d.Set("OSFamily", family)
+	}
+	version, err := d.server.SystemInfo.OSVersion()
+	if !d.logErr(err) {
+		d.Set("OSVersion", version)
+	}
+	kernelVersion, err := d.server.SystemInfo.KernelVersion()
+	if !d.logErr(err) {
+		d.Set("OSKernelVersion", kernelVersion)
 	}
-	d.Set("OSKernelVersion", kernelVersion)
 }
 
 // EnrichWithMemoryInfo adds memory information to the diagnostics payload.
 func (d *DiagnosticsCollector) EnrichWithMemoryInfo() {
-	memory, err := mem.VirtualMemory()
-	if err != nil {
-		d.logOutput.Write([]byte(err.Error()))
+	memFree, err := d.server.SystemInfo.MemFree()
+	if !d.logErr(err) {
+		d.Set("MemFree", memFree)
+	}
+	memTotal, err := d.server.SystemInfo.MemTotal()
+	if !d.logErr(err) {
+		d.Set("MemTotal", memTotal)
+	}
+	memUsed, err := d.server.SystemInfo.MemUsed()
+	if !d.logErr(err) {
+		d.Set("MemUsed", memUsed)
 	}
-	d.Set("MemFree", memory.Free)
-	d.Set("MemTotal", memory.Total)
-	d.Set("MemUsed", memory.Used)
-
 }
 
 // EnrichWithSchemaProperties adds schema info to the diagnostics payload.
-func (d *DiagnosticsCollector) EnrichWithSchemaProperties(holder *Holder) {
+func (d *DiagnosticsCollector) EnrichWithSchemaProperties() {
 	var numSlices uint64
 	numFrames := 0
 	numIndexes := 0
 	bsiFieldCount := 0
 	timeQuantumEnabled := false
 
-	for _, index := range holder.Indexes() {
+	for _, index := range d.server.Holder.Indexes() {
 		numSlices += index.MaxSlice() + 1
 		numIndexes += 1
 		for _, frame := range index.Frames() {
@@ -270,3 +282,64 @@ func VersionSegments(segments string) []int {
 	}
 	return segmentSlice
 }
+
+// SystemInfo collects information about the host OS
+type SystemInfo interface {
+	Uptime() (uint64, error)
+	Platform() (string, error)
+	Family() (string, error)
+	OSVersion() (string, error)
+	KernelVersion() (string, error)
+	MemFree() (uint64, error)
+	MemTotal() (uint64, error)
+	MemUsed() (uint64, error)
+}
+
+// NewNopSystemInfo creates a no-op implementation of SystemInfo
+func NewNopSystemInfo() *NopSystemInfo {
+	return &NopSystemInfo{}
+}
+
+// NopSystemInfo is a no-op implementation of SystemInfo
+type NopSystemInfo struct {
+}
+
+// Uptime is a no-op implementation of SystemInfo.Uptime
+func (n *NopSystemInfo) Uptime() (uint64, error) {
+	return 0, nil
+}
+
+// Platform is a no-op implementation of SystemInfo.Platform
+func (n *NopSystemInfo) Platform() (string, error) {
+	return "", nil
+}
+
+// Family is a no-op implementation of SystemInfo.Family
+func (n *NopSystemInfo) Family() (string, error) {
+	return "", nil
+}
+
+// OSVersion is a no-op implementation of SystemInfo.OSVersion
+func (n *NopSystemInfo) OSVersion() (string, error) {
+	return "", nil
+}
+
+// KernelVersion is a no-op implementation of SystemInfo.KernelVersion
+func (n *NopSystemInfo) KernelVersion() (string, error) {
+	return "", nil
+}
+
+// MemFree is a no-op implementation of SystemInfo.MemFree
+func (n *NopSystemInfo) MemFree() (uint64, error) {
+	return 0, nil
+}
+
+// MemTotal is a no-op implementation of SystemInfo.MemTotal
+func (n *NopSystemInfo) MemTotal() (uint64, error) {
+	return 0, nil
+}
+
+// MemUsed is a no-op implementation of SystemInfo.MemUsed
+func (n *NopSystemInfo) MemUsed() (uint64, error) {
+	return 0, nil
+}
diff --git a/gc.go b/gc.go
index 3f036bdcd..23dd0f0d0 100644
--- a/gc.go
+++ b/gc.go
@@ -14,6 +14,9 @@
 
 package pilosa
 
+// Ensure nopGCNotifier implements interface.
+var _ GCNotifier = &nopGCNotifier{}
+
 // GCNotifier represents an interface for garbage collection notificationss.
 type GCNotifier interface {
 	Close()
diff --git a/gopsutil/systeminfo.go b/gopsutil/systeminfo.go
new file mode 100644
index 000000000..12c300ba5
--- /dev/null
+++ b/gopsutil/systeminfo.go
@@ -0,0 +1,116 @@
+package gopsutil
+
+import (
+	"github.com/pilosa/pilosa"
+	"github.com/shirou/gopsutil/host"
+	"github.com/shirou/gopsutil/mem"
+)
+
+var _ pilosa.SystemInfo = NewSystemInfo()
+
+// SystemInfo is an implementation of pilosa.SystemInfo that uses gopsutil to collect information about the host OS
+type SystemInfo struct {
+	hostInfo  *host.InfoStat
+	memInfo   *mem.VirtualMemoryStat
+	platform  string
+	family    string
+	osVersion string
+}
+
+// Uptime returns the system uptime in seconds
+func (s *SystemInfo) Uptime() (uptime uint64, err error) {
+	if s.hostInfo == nil {
+		s.hostInfo, err = host.Info()
+		if err != nil {
+			return 0, err
+		}
+	}
+	return s.hostInfo.Uptime, nil
+}
+
+// Uptime returns the system platform
+func (s *SystemInfo) Platform() (string, error) {
+	err := s.collectPlatformInfo()
+	if err != nil {
+		return "", err
+	}
+	return s.platform, nil
+}
+
+// Family returns the system family
+func (s *SystemInfo) Family() (string, error) {
+	err := s.collectPlatformInfo()
+	if err != nil {
+		return "", err
+	}
+	return s.family, err
+}
+
+// OSVersion returns the OS Version
+func (s *SystemInfo) OSVersion() (string, error) {
+	err := s.collectPlatformInfo()
+	if err != nil {
+		return "", err
+	}
+	return s.osVersion, err
+}
+
+// collectPlatformInfo fetches and caches system platform information
+func (s *SystemInfo) collectPlatformInfo() error {
+	var err error
+	if s.platform == "" {
+		s.platform, s.family, s.osVersion, err = host.PlatformInformation()
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// collectMemoryInfo fetches and caches memory stats
+func (s *SystemInfo) collectMemoryInfo() (err error) {
+	if s.memInfo == nil {
+		s.memInfo, err = mem.VirtualMemory()
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// MemFree returns the amount of free memory in bytes
+func (s *SystemInfo) MemFree() (uint64, error) {
+	err := s.collectMemoryInfo()
+	if err != nil {
+		return 0, err
+	}
+	return s.memInfo.Free, err
+}
+
+// MemFree returns the amount of total memory in bytes
+func (s *SystemInfo) MemTotal() (uint64, error) {
+	err := s.collectMemoryInfo()
+	if err != nil {
+		return 0, err
+	}
+	return s.memInfo.Total, err
+}
+
+// MemFree returns the amount of used memory in bytes
+func (s *SystemInfo) MemUsed() (uint64, error) {
+	err := s.collectMemoryInfo()
+	if err != nil {
+		return 0, err
+	}
+	return s.memInfo.Used, err
+}
+
+// KernelVersion returns the kernel version as a string
+func (s *SystemInfo) KernelVersion() (string, error) {
+	return host.KernelVersion()
+}
+
+// NewSystemInfo is a constructor for the gopsutil implementation of SystemInfo
+func NewSystemInfo() *SystemInfo {
+	return &SystemInfo{}
+}
diff --git a/server.go b/server.go
index 82ca2db2d..bd0611041 100644
--- a/server.go
+++ b/server.go
@@ -70,6 +70,7 @@ type Server struct {
 	URI         URI
 	Cluster     *Cluster
 	diagnostics *DiagnosticsCollector
+	SystemInfo  SystemInfo
 
 	GCNotifier GCNotifier
 
@@ -100,6 +101,7 @@ func NewServer() *Server {
 		Broadcaster:       NopBroadcaster,
 		BroadcastReceiver: NopBroadcastReceiver,
 		diagnostics:       NewDiagnosticsCollector(DefaultDiagnosticServer),
+		SystemInfo:        NewNopSystemInfo(),
 
 		Network: "tcp",
 
@@ -114,6 +116,7 @@ func NewServer() *Server {
 	s.logger = log.New(s.LogOutput, "", log.LstdFlags)
 
 	s.Handler.Holder = s.Holder
+	s.diagnostics.server = s
 	return s
 }
 
@@ -627,7 +630,7 @@ func (s *Server) monitorDiagnostics() {
 		}
 		s.diagnostics.Set("GoRoutines", runtime.NumGoroutine())
 		s.diagnostics.EnrichWithMemoryInfo()
-		s.diagnostics.EnrichWithSchemaProperties(s.Holder)
+		s.diagnostics.EnrichWithSchemaProperties()
 		s.diagnostics.CheckVersion()
 		s.diagnostics.Flush()
 	}
diff --git a/server/server.go b/server/server.go
index 240b8b16b..eac98121d 100644
--- a/server/server.go
+++ b/server/server.go
@@ -34,6 +34,7 @@ import (
 
 	"github.com/pilosa/pilosa"
 	"github.com/pilosa/pilosa/gcnotify"
+	"github.com/pilosa/pilosa/gopsutil"
 	"github.com/pilosa/pilosa/gossip"
 	"github.com/pilosa/pilosa/statsd"
 )
@@ -152,6 +153,7 @@ func (m *Command) SetupServer() error {
 	if m.Config.Metric.Diagnostics {
 		m.Server.DiagnosticInterval = time.Duration(DefaultDiagnosticsInterval)
 	}
+	m.Server.SystemInfo = gopsutil.NewSystemInfo()
 	m.Server.GCNotifier = gcnotify.NewActiveGCNotifier()
 	m.Server.Holder.Stats, err = NewStatsClient(m.Config.Metric.Service, m.Config.Metric.Host)
 	if err != nil {

From c2444870c6835c6e5d4cf785f001f23610391617 Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Tue, 13 Mar 2018 12:27:49 -0500
Subject: [PATCH 03/10] Remove gobreaker dep and add HTTP timeout

---
 Gopkg.lock          |  6 -----
 diagnostics.go      | 60 ++++++++++++++-------------------------------
 diagnostics_test.go |  2 --
 server.go           | 12 ++++++---
 4 files changed, 27 insertions(+), 53 deletions(-)

diff --git a/Gopkg.lock b/Gopkg.lock
index c77dc951a..af6174c08 100644
--- a/Gopkg.lock
+++ b/Gopkg.lock
@@ -205,12 +205,6 @@
   packages = ["."]
   revision = "bb4de0191aa41b5507caa14b0650cdbddcd9280b"
 
-[[projects]]
-  name = "github.com/sony/gobreaker"
-  packages = ["."]
-  revision = "e9556a45379ef1da12e54847edb2fb3d7d566f36"
-  version = "0.3.0"
-
 [[projects]]
   branch = "master"
   name = "github.com/spf13/afero"
diff --git a/diagnostics.go b/diagnostics.go
index a4caf84cd..1abcdd144 100644
--- a/diagnostics.go
+++ b/diagnostics.go
@@ -26,8 +26,6 @@ import (
 	"strings"
 	"sync"
 	"time"
-
-	"github.com/sony/gobreaker"
 )
 
 // Default version check URL.
@@ -52,10 +50,8 @@ type DiagnosticsCollector struct {
 
 	metrics map[string]interface{}
 
-	client   *http.Client
-	interval time.Duration
+	client *http.Client
 
-	cb        *gobreaker.CircuitBreaker
 	logOutput io.Writer
 
 	server *Server
@@ -69,7 +65,7 @@ func NewDiagnosticsCollector(host string) *DiagnosticsCollector {
 		VersionURL: defaultVersionCheckURL,
 		startTime:  time.Now().Unix(),
 		start:      time.Now(),
-		client:     http.DefaultClient,
+		client:     &http.Client{Timeout: 10 * time.Second},
 		metrics:    make(map[string]interface{}),
 		logOutput:  ioutil.Discard,
 	}
@@ -81,47 +77,29 @@ func (d *DiagnosticsCollector) SetVersion(v string) {
 	d.Set("Version", v)
 }
 
-// SetInterval of the diagnostic go routine and match with the circuit breaker timeout.
-func (d *DiagnosticsCollector) SetInterval(i time.Duration) {
-	d.interval = i
-}
-
 // Flush sends the current metrics.
 func (d *DiagnosticsCollector) Flush() error {
 	d.mu.Lock()
+	defer d.mu.Unlock()
 	d.metrics["Uptime"] = (time.Now().Unix() - d.startTime)
-	buf, _ := d.encode()
-	d.mu.Unlock()
-
-	_, err := d.cb.Execute(func() (interface{}, error) {
-		req, err := http.NewRequest("POST", d.host, bytes.NewReader(buf))
-		req.Header.Set("Content-Type", "application/json")
-		resp, err := d.client.Do(req)
-		if err != nil {
-			return nil, err
-		}
-		defer resp.Body.Close()
-
-		// TODO verify response
-		body, err := ioutil.ReadAll(resp.Body)
-		if err != nil {
-			return nil, err
-		}
-		return body, nil
-	})
-
-	return err
-}
-
-// Open configures the circuit breaker used by the HTTP client.
-func (d *DiagnosticsCollector) Open() {
-	var st gobreaker.Settings
-	if d.interval > 0 {
-		st.Timeout = d.interval * 2
+	buf, err := d.encode()
+	if err != nil {
+		return err
 	}
-	d.cb = gobreaker.NewCircuitBreaker(st)
+	req, err := http.NewRequest("POST", d.host, bytes.NewReader(buf))
+	req.Header.Set("Content-Type", "application/json")
+	resp, err := d.client.Do(req)
+	if err != nil {
+		return err
+	}
+	defer resp.Body.Close()
 
-	d.logger().Printf("Pilosa is currently configured to send small diagnostics reports to our team every hour. More information here: https://www.pilosa.com/docs/latest/administration/#diagnostics")
+	// TODO verify response
+	_, err = ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return err
+	}
+	return nil
 }
 
 // CheckVersion of the local build against Pilosa master.
diff --git a/diagnostics_test.go b/diagnostics_test.go
index 9d8e35d76..7b66d8e18 100644
--- a/diagnostics_test.go
+++ b/diagnostics_test.go
@@ -32,7 +32,6 @@ func TestDiagnosticsClient(t *testing.T) {
 	// Create a new client.
 	d := NewDiagnosticsCollector(server.URL)
 	d.SetLogger(ioutil.Discard)
-	d.Open()
 
 	d.Set("gg", 10)
 	d.Set("ss", "ss")
@@ -80,7 +79,6 @@ func TestDiagnosticsVersion_Parse(t *testing.T) {
 
 func TestDiagnosticsVersion_Compare(t *testing.T) {
 	d := NewDiagnosticsCollector("localhost:10101")
-	d.Open()
 
 	version := "v0.1.1"
 	d.SetVersion(version)
diff --git a/server.go b/server.go
index bd0611041..dd8811463 100644
--- a/server.go
+++ b/server.go
@@ -605,15 +605,16 @@ func (s *Server) mergeRemoteStatus(ns *internal.NodeStatus) error {
 
 // monitorDiagnostics periodically polls the Pilosa Indexes for cluster info.
 func (s *Server) monitorDiagnostics() {
-	if s.DiagnosticInterval <= 0 {
+	// Do not send more than once a minute
+	if s.DiagnosticInterval < time.Minute {
 		s.Logger().Printf("diagnostics disabled")
 		return
+	} else {
+		s.Logger().Printf("Pilosa is currently configured to send small diagnostics reports to our team every hour. More information here: https://www.pilosa.com/docs/latest/administration/#diagnostics")
 	}
 
 	s.diagnostics.SetLogger(s.LogOutput)
 	s.diagnostics.SetVersion(Version)
-	s.diagnostics.SetInterval(s.DiagnosticInterval)
-	s.diagnostics.Open()
 	s.diagnostics.Set("Host", s.URI.host)
 	s.diagnostics.Set("Cluster", strings.Join(s.Cluster.NodeIDs(), ","))
 	s.diagnostics.Set("NumNodes", len(s.Cluster.Nodes))
@@ -632,7 +633,10 @@ func (s *Server) monitorDiagnostics() {
 		s.diagnostics.EnrichWithMemoryInfo()
 		s.diagnostics.EnrichWithSchemaProperties()
 		s.diagnostics.CheckVersion()
-		s.diagnostics.Flush()
+		err = s.diagnostics.Flush()
+		if err != nil {
+			s.Logger().Printf("Diagnostics error: %s", err)
+		}
 	}
 
 	ticker := time.NewTicker(s.DiagnosticInterval)

From 88471e94f197e5c6064042bc03eb8f91b9bd2ed7 Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Wed, 14 Mar 2018 15:34:33 -0500
Subject: [PATCH 04/10] Remove caching (the lib code is fast) and add tests

---
 gopsutil/systeminfo.go      | 55 ++++++++++++-------------------
 gopsutil/systeminfo_test.go | 64 +++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 35 deletions(-)
 create mode 100644 gopsutil/systeminfo_test.go

diff --git a/gopsutil/systeminfo.go b/gopsutil/systeminfo.go
index 12c300ba5..433aacc30 100644
--- a/gopsutil/systeminfo.go
+++ b/gopsutil/systeminfo.go
@@ -10,8 +10,6 @@ var _ pilosa.SystemInfo = NewSystemInfo()
 
 // SystemInfo is an implementation of pilosa.SystemInfo that uses gopsutil to collect information about the host OS
 type SystemInfo struct {
-	hostInfo  *host.InfoStat
-	memInfo   *mem.VirtualMemoryStat
 	platform  string
 	family    string
 	osVersion string
@@ -19,13 +17,23 @@ type SystemInfo struct {
 
 // Uptime returns the system uptime in seconds
 func (s *SystemInfo) Uptime() (uptime uint64, err error) {
-	if s.hostInfo == nil {
-		s.hostInfo, err = host.Info()
+	hostInfo, err := host.Info()
+	if err != nil {
+		return 0, err
+	}
+	return hostInfo.Uptime, nil
+}
+
+// collectPlatformInfo fetches and caches system platform information
+func (s *SystemInfo) collectPlatformInfo() error {
+	var err error
+	if s.platform == "" {
+		s.platform, s.family, s.osVersion, err = host.PlatformInformation()
 		if err != nil {
-			return 0, err
+			return err
 		}
 	}
-	return s.hostInfo.Uptime, nil
+	return nil
 }
 
 // Uptime returns the system platform
@@ -55,54 +63,31 @@ func (s *SystemInfo) OSVersion() (string, error) {
 	return s.osVersion, err
 }
 
-// collectPlatformInfo fetches and caches system platform information
-func (s *SystemInfo) collectPlatformInfo() error {
-	var err error
-	if s.platform == "" {
-		s.platform, s.family, s.osVersion, err = host.PlatformInformation()
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-// collectMemoryInfo fetches and caches memory stats
-func (s *SystemInfo) collectMemoryInfo() (err error) {
-	if s.memInfo == nil {
-		s.memInfo, err = mem.VirtualMemory()
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 // MemFree returns the amount of free memory in bytes
 func (s *SystemInfo) MemFree() (uint64, error) {
-	err := s.collectMemoryInfo()
+	memInfo, err := mem.VirtualMemory()
 	if err != nil {
 		return 0, err
 	}
-	return s.memInfo.Free, err
+	return memInfo.Free, err
 }
 
 // MemFree returns the amount of total memory in bytes
 func (s *SystemInfo) MemTotal() (uint64, error) {
-	err := s.collectMemoryInfo()
+	memInfo, err := mem.VirtualMemory()
 	if err != nil {
 		return 0, err
 	}
-	return s.memInfo.Total, err
+	return memInfo.Total, err
 }
 
 // MemFree returns the amount of used memory in bytes
 func (s *SystemInfo) MemUsed() (uint64, error) {
-	err := s.collectMemoryInfo()
+	memInfo, err := mem.VirtualMemory()
 	if err != nil {
 		return 0, err
 	}
-	return s.memInfo.Used, err
+	return memInfo.Used, err
 }
 
 // KernelVersion returns the kernel version as a string
diff --git a/gopsutil/systeminfo_test.go b/gopsutil/systeminfo_test.go
new file mode 100644
index 000000000..41b58ec1c
--- /dev/null
+++ b/gopsutil/systeminfo_test.go
@@ -0,0 +1,64 @@
+package gopsutil_test
+
+import (
+	"log"
+	"runtime"
+	"testing"
+
+	"github.com/pilosa/pilosa"
+	"github.com/pilosa/pilosa/gopsutil"
+)
+
+func TestSystemInfo(t *testing.T) {
+	var systemInfo pilosa.SystemInfo = gopsutil.NewSystemInfo()
+
+	//	Uptime()(uint64, error)
+	//	Platform()(string, error)
+	//	Family()(string, error)
+	//	OSVersion()(string, error)
+	//	KernelVersion()(string, error)
+	//	MemFree()(uint64, error)
+	//	MemTotal()(uint64, error)
+	//	MemUsed()(uint64, error)
+	//
+	uptime, err := systemInfo.Uptime()
+	if err != nil || uptime == 0 {
+		t.Fatalf("Error collecting uptime (error: %v)", err)
+	}
+
+	platform, err := systemInfo.Platform()
+	if err != nil || platform != runtime.GOOS {
+		t.Fatalf("Platform must be %s. (error: %v)", runtime.GOOS, err)
+	}
+
+	family, err := systemInfo.Family()
+	if err != nil {
+		t.Fatalf("Error getting OS family. (family: %v, error: %v)", family, err)
+	}
+
+	osversion, err := systemInfo.OSVersion()
+	if err != nil {
+		t.Fatalf("Error getting OS version. (osversion: %v, error: %v)", osversion, err)
+	}
+
+	kernelversion, err := systemInfo.KernelVersion()
+	if err != nil {
+		t.Fatalf("Error getting kernel version. (kernelversion: %v, error: %v)", kernelversion, err)
+	}
+
+	memfree, err := systemInfo.MemFree()
+	if err != nil {
+		t.Fatalf("Error getting memfree. (memfree: %v, error: %v)", memfree, err)
+	}
+
+	memused, err := systemInfo.MemUsed()
+	if err != nil {
+		t.Fatalf("Error getting memused. (memused: %v, error: %v)", memused, err)
+	}
+
+	memtotal, err := systemInfo.MemTotal()
+	log.Println(memtotal)
+	if err != nil {
+		t.Fatalf("Error getting memtotal. (memtotal: %v, error: %v)", memtotal, err)
+	}
+}

From 20dc1212f8d62f8e7db7eca49abc046ce85811f6 Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Wed, 14 Mar 2018 16:51:40 -0500
Subject: [PATCH 05/10] Address code review (mostly comments)

---
 diagnostics.go                                | 37 +++++++++----------
 ...cs_test.go => diagnostics_internal_test.go |  2 +-
 gopsutil/systeminfo.go                        | 22 +++++------
 3 files changed, 30 insertions(+), 31 deletions(-)
 rename diagnostics_test.go => diagnostics_internal_test.go (99%)

diff --git a/diagnostics.go b/diagnostics.go
index 1abcdd144..710f66c2d 100644
--- a/diagnostics.go
+++ b/diagnostics.go
@@ -38,7 +38,7 @@ type versionResponse struct {
 	Message string `json:"message"`
 }
 
-// DiagnosticsCollector represents a collector/sender of diagnostics data
+// DiagnosticsCollector represents a collector/sender of diagnostics data.
 type DiagnosticsCollector struct {
 	mu          sync.Mutex
 	host        string
@@ -57,9 +57,8 @@ type DiagnosticsCollector struct {
 	server *Server
 }
 
-// New returns a pointer to a new DiagnosticsCollector Client given an addr in the format "hostname:port".
+// NewDiagnosticsCollector returns a new DiagnosticsCollector given an addr in the format "hostname:port".
 func NewDiagnosticsCollector(host string) *DiagnosticsCollector {
-
 	return &DiagnosticsCollector{
 		host:       host,
 		VersionURL: defaultVersionCheckURL,
@@ -118,7 +117,7 @@ func (d *DiagnosticsCollector) CheckVersion() error {
 		return fmt.Errorf("json decode: %s", err)
 	}
 
-	// Same a version as last test
+	// If version has not changed since the last check, return
 	if rsp.Version == d.lastVersion {
 		return nil
 	}
@@ -133,8 +132,8 @@ func (d *DiagnosticsCollector) CheckVersion() error {
 
 // compareVersion check version strings.
 func (d *DiagnosticsCollector) compareVersion(value string) error {
-	currentVersion := VersionSegments(value)
-	localVersion := VersionSegments(d.version)
+	currentVersion := versionSegments(value)
+	localVersion := versionSegments(d.version)
 
 	if localVersion[0] < currentVersion[0] { //Major
 		return fmt.Errorf("Warning: You are running Pilosa %s. A newer version (%s) is available: https://github.com/pilosa/pilosa/releases", d.version, value)
@@ -249,8 +248,8 @@ func (d *DiagnosticsCollector) EnrichWithSchemaProperties() {
 	d.Set("TimeQuantumEnabled", timeQuantumEnabled)
 }
 
-// VersionSegments returns the numeric segments of the version as a slice of ints.
-func VersionSegments(segments string) []int {
+// versionSegments returns the numeric segments of the version as a slice of ints.
+func versionSegments(segments string) []int {
 	segments = strings.Trim(segments, "v")
 	segments = strings.Split(segments, "-")[0]
 	s := strings.Split(segments, ".")
@@ -261,7 +260,7 @@ func VersionSegments(segments string) []int {
 	return segmentSlice
 }
 
-// SystemInfo collects information about the host OS
+// SystemInfo collects information about the host OS.
 type SystemInfo interface {
 	Uptime() (uint64, error)
 	Platform() (string, error)
@@ -273,51 +272,51 @@ type SystemInfo interface {
 	MemUsed() (uint64, error)
 }
 
-// NewNopSystemInfo creates a no-op implementation of SystemInfo
+// NewNopSystemInfo creates a no-op implementation of SystemInfo.
 func NewNopSystemInfo() *NopSystemInfo {
 	return &NopSystemInfo{}
 }
 
-// NopSystemInfo is a no-op implementation of SystemInfo
+// NopSystemInfo is a no-op implementation of SystemInfo.
 type NopSystemInfo struct {
 }
 
-// Uptime is a no-op implementation of SystemInfo.Uptime
+// Uptime is a no-op implementation of SystemInfo.Uptime.
 func (n *NopSystemInfo) Uptime() (uint64, error) {
 	return 0, nil
 }
 
-// Platform is a no-op implementation of SystemInfo.Platform
+// Platform is a no-op implementation of SystemInfo.Platform.
 func (n *NopSystemInfo) Platform() (string, error) {
 	return "", nil
 }
 
-// Family is a no-op implementation of SystemInfo.Family
+// Family is a no-op implementation of SystemInfo.Family.
 func (n *NopSystemInfo) Family() (string, error) {
 	return "", nil
 }
 
-// OSVersion is a no-op implementation of SystemInfo.OSVersion
+// OSVersion is a no-op implementation of SystemInfo.OSVersion.
 func (n *NopSystemInfo) OSVersion() (string, error) {
 	return "", nil
 }
 
-// KernelVersion is a no-op implementation of SystemInfo.KernelVersion
+// KernelVersion is a no-op implementation of SystemInfo.KernelVersion.
 func (n *NopSystemInfo) KernelVersion() (string, error) {
 	return "", nil
 }
 
-// MemFree is a no-op implementation of SystemInfo.MemFree
+// MemFree is a no-op implementation of SystemInfo.MemFree.
 func (n *NopSystemInfo) MemFree() (uint64, error) {
 	return 0, nil
 }
 
-// MemTotal is a no-op implementation of SystemInfo.MemTotal
+// MemTotal is a no-op implementation of SystemInfo.MemTotal.
 func (n *NopSystemInfo) MemTotal() (uint64, error) {
 	return 0, nil
 }
 
-// MemUsed is a no-op implementation of SystemInfo.MemUsed
+// MemUsed is a no-op implementation of SystemInfo.MemUsed.
 func (n *NopSystemInfo) MemUsed() (uint64, error) {
 	return 0, nil
 }
diff --git a/diagnostics_test.go b/diagnostics_internal_test.go
similarity index 99%
rename from diagnostics_test.go
rename to diagnostics_internal_test.go
index 7b66d8e18..eb2498297 100644
--- a/diagnostics_test.go
+++ b/diagnostics_internal_test.go
@@ -69,7 +69,7 @@ func TestDiagnosticsClient(t *testing.T) {
 
 func TestDiagnosticsVersion_Parse(t *testing.T) {
 	version := "0.1.1"
-	vs := VersionSegments(version)
+	vs := versionSegments(version)
 
 	output := []int{0, 1, 1}
 	if !reflect.DeepEqual(vs, output) {
diff --git a/gopsutil/systeminfo.go b/gopsutil/systeminfo.go
index 433aacc30..e6285ade8 100644
--- a/gopsutil/systeminfo.go
+++ b/gopsutil/systeminfo.go
@@ -8,14 +8,14 @@ import (
 
 var _ pilosa.SystemInfo = NewSystemInfo()
 
-// SystemInfo is an implementation of pilosa.SystemInfo that uses gopsutil to collect information about the host OS
+// SystemInfo is an implementation of pilosa.SystemInfo that uses gopsutil to collect information about the host OS.
 type SystemInfo struct {
 	platform  string
 	family    string
 	osVersion string
 }
 
-// Uptime returns the system uptime in seconds
+// Uptime returns the system uptime in seconds.
 func (s *SystemInfo) Uptime() (uptime uint64, err error) {
 	hostInfo, err := host.Info()
 	if err != nil {
@@ -24,7 +24,7 @@ func (s *SystemInfo) Uptime() (uptime uint64, err error) {
 	return hostInfo.Uptime, nil
 }
 
-// collectPlatformInfo fetches and caches system platform information
+// collectPlatformInfo fetches and caches system platform information.
 func (s *SystemInfo) collectPlatformInfo() error {
 	var err error
 	if s.platform == "" {
@@ -36,7 +36,7 @@ func (s *SystemInfo) collectPlatformInfo() error {
 	return nil
 }
 
-// Uptime returns the system platform
+// Platform returns the system platform.
 func (s *SystemInfo) Platform() (string, error) {
 	err := s.collectPlatformInfo()
 	if err != nil {
@@ -45,7 +45,7 @@ func (s *SystemInfo) Platform() (string, error) {
 	return s.platform, nil
 }
 
-// Family returns the system family
+// Family returns the system family.
 func (s *SystemInfo) Family() (string, error) {
 	err := s.collectPlatformInfo()
 	if err != nil {
@@ -54,7 +54,7 @@ func (s *SystemInfo) Family() (string, error) {
 	return s.family, err
 }
 
-// OSVersion returns the OS Version
+// OSVersion returns the OS Version.
 func (s *SystemInfo) OSVersion() (string, error) {
 	err := s.collectPlatformInfo()
 	if err != nil {
@@ -63,7 +63,7 @@ func (s *SystemInfo) OSVersion() (string, error) {
 	return s.osVersion, err
 }
 
-// MemFree returns the amount of free memory in bytes
+// MemFree returns the amount of free memory in bytes.
 func (s *SystemInfo) MemFree() (uint64, error) {
 	memInfo, err := mem.VirtualMemory()
 	if err != nil {
@@ -72,7 +72,7 @@ func (s *SystemInfo) MemFree() (uint64, error) {
 	return memInfo.Free, err
 }
 
-// MemFree returns the amount of total memory in bytes
+// MemTotal returns the amount of total memory in bytes.
 func (s *SystemInfo) MemTotal() (uint64, error) {
 	memInfo, err := mem.VirtualMemory()
 	if err != nil {
@@ -81,7 +81,7 @@ func (s *SystemInfo) MemTotal() (uint64, error) {
 	return memInfo.Total, err
 }
 
-// MemFree returns the amount of used memory in bytes
+// MemUsed returns the amount of used memory in bytes.
 func (s *SystemInfo) MemUsed() (uint64, error) {
 	memInfo, err := mem.VirtualMemory()
 	if err != nil {
@@ -90,12 +90,12 @@ func (s *SystemInfo) MemUsed() (uint64, error) {
 	return memInfo.Used, err
 }
 
-// KernelVersion returns the kernel version as a string
+// KernelVersion returns the kernel version as a string.
 func (s *SystemInfo) KernelVersion() (string, error) {
 	return host.KernelVersion()
 }
 
-// NewSystemInfo is a constructor for the gopsutil implementation of SystemInfo
+// NewSystemInfo is a constructor for the gopsutil implementation of SystemInfo.
 func NewSystemInfo() *SystemInfo {
 	return &SystemInfo{}
 }

From 2d425e32e91aa9385631bbba7a5f6b6f8ca3286e Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Wed, 14 Mar 2018 17:33:54 -0500
Subject: [PATCH 06/10] Log platform

---
 gopsutil/systeminfo_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gopsutil/systeminfo_test.go b/gopsutil/systeminfo_test.go
index 41b58ec1c..4131942a2 100644
--- a/gopsutil/systeminfo_test.go
+++ b/gopsutil/systeminfo_test.go
@@ -28,7 +28,7 @@ func TestSystemInfo(t *testing.T) {
 
 	platform, err := systemInfo.Platform()
 	if err != nil || platform != runtime.GOOS {
-		t.Fatalf("Platform must be %s. (error: %v)", runtime.GOOS, err)
+		t.Fatalf("Platform must be %s. (platform: %v, error: %v)", platform, runtime.GOOS, err)
 	}
 
 	family, err := systemInfo.Family()

From 680acf4e3dc175a6199f4cefdcb4afdd06440c93 Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Wed, 14 Mar 2018 17:41:44 -0500
Subject: [PATCH 07/10] Fix error on linux

---
 gopsutil/systeminfo_test.go | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/gopsutil/systeminfo_test.go b/gopsutil/systeminfo_test.go
index 4131942a2..5d96a499c 100644
--- a/gopsutil/systeminfo_test.go
+++ b/gopsutil/systeminfo_test.go
@@ -2,7 +2,6 @@ package gopsutil_test
 
 import (
 	"log"
-	"runtime"
 	"testing"
 
 	"github.com/pilosa/pilosa"
@@ -27,8 +26,8 @@ func TestSystemInfo(t *testing.T) {
 	}
 
 	platform, err := systemInfo.Platform()
-	if err != nil || platform != runtime.GOOS {
-		t.Fatalf("Platform must be %s. (platform: %v, error: %v)", platform, runtime.GOOS, err)
+	if err != nil {
+		t.Fatalf("Error getting platform. (platform: %v, error: %v)", platform, err)
 	}
 
 	family, err := systemInfo.Family()

From 754de2e057e53026d6f09faf94cd3d8da733faf7 Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Thu, 15 Mar 2018 15:26:36 -0500
Subject: [PATCH 08/10] Add correct diagnostics interval to startup message.

---
 server.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server.go b/server.go
index dd8811463..54bacda1d 100644
--- a/server.go
+++ b/server.go
@@ -610,7 +610,7 @@ func (s *Server) monitorDiagnostics() {
 		s.Logger().Printf("diagnostics disabled")
 		return
 	} else {
-		s.Logger().Printf("Pilosa is currently configured to send small diagnostics reports to our team every hour. More information here: https://www.pilosa.com/docs/latest/administration/#diagnostics")
+		s.Logger().Printf("Pilosa is currently configured to send small diagnostics reports to our team every %v. More information here: https://www.pilosa.com/docs/latest/administration/#diagnostics", s.DiagnosticInterval)
 	}
 
 	s.diagnostics.SetLogger(s.LogOutput)

From 85b33f1bfb11835b830bcef8fd291cbc9185c614 Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Thu, 15 Mar 2018 15:27:01 -0500
Subject: [PATCH 09/10] Remove unused code and TODO and clarify with comment.

---
 diagnostics.go | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/diagnostics.go b/diagnostics.go
index 710f66c2d..08d77a03c 100644
--- a/diagnostics.go
+++ b/diagnostics.go
@@ -91,13 +91,8 @@ func (d *DiagnosticsCollector) Flush() error {
 	if err != nil {
 		return err
 	}
+	// Intentionally ignoring response body, as user does not need to be notified of error.
 	defer resp.Body.Close()
-
-	// TODO verify response
-	_, err = ioutil.ReadAll(resp.Body)
-	if err != nil {
-		return err
-	}
 	return nil
 }
 

From be70bbfed2f5b3032d166bac549e7dec4d713135 Mon Sep 17 00:00:00 2001
From: Cody Soyland <cody@soyland.org>
Date: Thu, 15 Mar 2018 15:27:48 -0500
Subject: [PATCH 10/10] Fix bug: backend won't store empty strings.

---
 diagnostics.go | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/diagnostics.go b/diagnostics.go
index 08d77a03c..3ad07ed42 100644
--- a/diagnostics.go
+++ b/diagnostics.go
@@ -148,6 +148,13 @@ func (d *DiagnosticsCollector) encode() ([]byte, error) {
 
 // Set adds a key value metric.
 func (d *DiagnosticsCollector) Set(name string, value interface{}) {
+	switch v := value.(type) {
+	case string:
+		if v == "" {
+			// Do not set empty string
+			return
+		}
+	}
 	d.mu.Lock()
 	defer d.mu.Unlock()
 	d.metrics[name] = value