From 7f1dea72f27a3c06be3240dc0cd13030f59e4f93 Mon Sep 17 00:00:00 2001
From: Milos Gajdos <milosgajdos83@gmail.com>
Date: Thu, 28 Nov 2019 18:08:48 +0000
Subject: [PATCH] Simplified Logs RPC. Cleaned up code. Added comments.

---
 debug/buffer/buffer.go    |  8 +++++---
 debug/handler/debug.go    | 19 ++++++++++++++-----
 debug/log/default.go      | 22 +++++++++++++++++-----
 debug/log/default_test.go | 10 +++++-----
 debug/log/options.go      |  9 +++++++++
 debug/service/service.go  |  1 +
 6 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/debug/buffer/buffer.go b/debug/buffer/buffer.go
index 50c511d4..881e832a 100644
--- a/debug/buffer/buffer.go
+++ b/debug/buffer/buffer.go
@@ -6,12 +6,14 @@ import (
 	"time"
 )
 
+// Buffer is ring buffer
 type Buffer struct {
 	size int
 	sync.RWMutex
 	vals []*Entry
 }
 
+// Entry is ring buffer data entry
 type Entry struct {
 	Value     interface{}
 	Timestamp time.Time
@@ -43,14 +45,14 @@ func (b *Buffer) Put(v interface{}) {
 
 // Get returns the last n entries
 func (b *Buffer) Get(n int) []*Entry {
+	b.RLock()
+	defer b.RUnlock()
+
 	// reset any invalid values
 	if n > b.size || n < 0 {
 		n = b.size
 	}
 
-	b.RLock()
-	defer b.RUnlock()
-
 	// create a delta
 	delta := b.size - n
 
diff --git a/debug/handler/debug.go b/debug/handler/debug.go
index 144be48d..d5a9ea6c 100644
--- a/debug/handler/debug.go
+++ b/debug/handler/debug.go
@@ -1,3 +1,4 @@
+// Pacjage handler implements service debug handler
 package handler
 
 import (
@@ -46,15 +47,23 @@ func (d *Debug) Stats(ctx context.Context, req *proto.StatsRequest, rsp *proto.S
 }
 
 func (d *Debug) Logs(ctx context.Context, req *proto.LogRequest, stream proto.Debug_LogsStream) error {
-	var records []log.Record
+	var options []log.ReadOption
+
 	since := time.Unix(0, req.Since)
 	if !since.IsZero() {
-		records = d.log.Read(log.Since(since))
-	} else {
-		records = d.log.Read(log.Count(int(req.Count)))
+		options = append(options, log.Since(since))
 	}
 
-	// TODO: figure out the stream later on
+	count := int(req.Count)
+	if count > 0 {
+		options = append(options, log.Count(count))
+	}
+
+	// get the log records
+	records := d.log.Read(options...)
+
+	// TODO: figure out the stream
+
 	for _, record := range records {
 		metadata := make(map[string]string)
 		for k, v := range record.Metadata {
diff --git a/debug/log/default.go b/debug/log/default.go
index 9e546cb4..35b11fe4 100644
--- a/debug/log/default.go
+++ b/debug/log/default.go
@@ -50,13 +50,24 @@ func (l *defaultLog) Read(opts ...ReadOption) []Record {
 	// if Since options ha sbeen specified we honor it
 	if !options.Since.IsZero() {
 		entries = l.Buffer.Since(options.Since)
-	} else {
-		// otherwie return last count entries
-		entries = l.Buffer.Get(options.Count)
 	}
 
-	// TODO: if both Since and Count are set should we return?
-	// last Count from the returned time scoped entries?
+	// only if we specified valid count constraint
+	// do we do some serious if-else kung-fu
+	// if since has been given we return *count* number of
+	// logs since the requested timestamp;
+	// otherwise we retourn last count number of logs
+	if options.Count > 0 {
+		switch len(entries) > 0 {
+		case true:
+			// if we request fewer logs than what since constraint gives us
+			if options.Count < len(entries) {
+				entries = entries[0:options.Count]
+			}
+		default:
+			entries = l.Buffer.Get(options.Count)
+		}
+	}
 
 	records := make([]Record, 0, len(entries))
 	for _, entry := range entries {
@@ -66,5 +77,6 @@ func (l *defaultLog) Read(opts ...ReadOption) []Record {
 		}
 		records = append(records, record)
 	}
+
 	return records
 }
diff --git a/debug/log/default_test.go b/debug/log/default_test.go
index 0ef6e903..e0f08a7c 100644
--- a/debug/log/default_test.go
+++ b/debug/log/default_test.go
@@ -9,21 +9,21 @@ func TestLogger(t *testing.T) {
 	// set size to some value
 	size := 100
 	// override the global logger
-	logger = NewLog(Size(size))
+	DefaultLog = NewLog(Size(size))
 	// make sure we have the right size of the logger ring buffer
-	if logger.(*defaultLog).Size() != size {
-		t.Errorf("expected buffer size: %d, got: %d", size, logger.(*defaultLog).Size())
+	if DefaultLog.(*defaultLog).Size() != size {
+		t.Errorf("expected buffer size: %d, got: %d", size, DefaultLog.(*defaultLog).Size())
 	}
 
 	// Log some cruft
 	Info("foobar")
 	// increase the log level
-	level = LevelDebug
+	DefaultLevel = LevelDebug
 	Debugf("foo %s", "bar")
 
 	// Check if the logs are stored in the logger ring buffer
 	expected := []string{"foobar", "foo bar"}
-	entries := logger.Read(Count(len(expected)))
+	entries := DefaultLog.Read(Count(len(expected)))
 	for i, entry := range entries {
 		if !reflect.DeepEqual(entry.Value, expected[i]) {
 			t.Errorf("expected %s, got %s", expected[i], entry.Value)
diff --git a/debug/log/options.go b/debug/log/options.go
index 2320b71e..03dece38 100644
--- a/debug/log/options.go
+++ b/debug/log/options.go
@@ -31,6 +31,8 @@ type ReadOptions struct {
 	Since time.Time
 	// Count specifies number of logs to return
 	Count int
+	// Stream requests continuous log stream
+	Stream bool
 }
 
 // ReadOption used for reading the logs
@@ -49,3 +51,10 @@ func Count(c int) ReadOption {
 		o.Count = c
 	}
 }
+
+// Stream requests continuous log stream
+func Stream(s bool) ReadOption {
+	return func(o *ReadOptions) {
+		o.Stream = s
+	}
+}
diff --git a/debug/service/service.go b/debug/service/service.go
index 5089058e..aa3fd2e7 100644
--- a/debug/service/service.go
+++ b/debug/service/service.go
@@ -26,6 +26,7 @@ func NewDebug(name string) *Debug {
 	}
 }
 
+// Logs queries the service logs and returns a channel to read the logs from
 func (d *Debug) Logs(opts ...log.ReadOption) (<-chan log.Record, error) {
 	options := log.ReadOptions{}
 	// initialize the read options