feature: Limit log history to prevent browser lag and grpc encode failures (#507)

* feature: Limit log history to prevent browser lag and grpc encode failures

* cicd: Simplify if/else chain to switch

* cicd: Lint failure in JS because of a trailing space

* cicd: Coderabbit fixes on PR

* cicd: Move config sanitize into correct place

* cicd: Handle negative startOffset, negative endOffset and empty logs

* Update internal/executor/executor.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* cicd: Fix getLogs segfault

* cicd: Fix code rabbit nitpicks

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

OliveTin.proto

@@ -102,7 +102,9 @@ message StartActionByGetAndWaitResponse {
 	LogEntry log_entry = 1;
 }
 
-message GetLogsRequest{};
+message GetLogsRequest{
+  int64 start_offset = 1;
+};
 
 message LogEntry {
 	string datetime_started = 1;
@@ -120,10 +122,13 @@ message LogEntry {
 	bool execution_started = 14;
 	bool execution_finished = 15;
 	bool blocked = 16;
+	int64 datetime_index = 17;
 }
 
 message GetLogsResponse {
 	repeated LogEntry logs = 1;
+	int64 count_remaining = 2;
+	int64 page_size = 3;
 }
 
 message ValidateArgumentTypeRequest {

internal/config/config.go

@@ -96,6 +96,7 @@ type Config struct {
 	ExternalRestAddress             string
 	LogLevel                        string
 	LogDebugOptions                 LogDebugOptions
+	LogHistoryPageSize              int64
 	Actions                         []*Action             `mapstructure:"actions"`
 	Entities                        []*EntityFile         `mapstructure:"entities"`
 	Dashboards                      []*DashboardComponent `mapstructure:"dashboards"`
@@ -212,6 +213,7 @@ func DefaultConfigWithBasePort(basePort int) *Config {
 	config.EnableCustomJs = false
 	config.ExternalRestAddress = "."
 	config.LogLevel = "INFO"
+	config.LogHistoryPageSize = 10
 	config.CheckForUpdates = false
 	config.DefaultPermissions.Exec = true
 	config.DefaultPermissions.View = true

internal/config/config_reloader.go

@@ -36,14 +36,6 @@ func Reload(cfg *Config) {
 		os.Exit(1)
 	}
 
-	if cfg.AuthRequireGuestsToLogin {
-		log.Infof("AuthRequireGuestsToLogin is enabled. All defaultPermissions will be set to false")
-
-		cfg.DefaultPermissions.View = false
-		cfg.DefaultPermissions.Exec = false
-		cfg.DefaultPermissions.Logs = false
-	}
-
 	metricConfigReloadedCount.Inc()
 	metricConfigActionCount.Set(float64(len(cfg.Actions)))
 

internal/config/sanitize.go

@@ -41,6 +41,28 @@ func (action *Action) sanitize(cfg *Config) {
 	for idx := range action.Arguments {
 		action.Arguments[idx].sanitize()
 	}
+
+	sanitizeAuthRequireGuestsToLogin(cfg)
+	sanitizeLogHistoryPageSize(cfg)
+}
+
+func sanitizeAuthRequireGuestsToLogin(cfg *Config) {
+	if cfg.AuthRequireGuestsToLogin {
+		log.Infof("AuthRequireGuestsToLogin is enabled. All defaultPermissions will be set to false")
+
+		cfg.DefaultPermissions.View = false
+		cfg.DefaultPermissions.Exec = false
+		cfg.DefaultPermissions.Logs = false
+	}
+}
+
+func sanitizeLogHistoryPageSize(cfg *Config) {
+	if cfg.LogHistoryPageSize < 10 {
+		log.Warnf("LogsHistoryLimit is too low, setting it to 10")
+		cfg.LogHistoryPageSize = 10
+	} else if cfg.LogHistoryPageSize > 100 {
+		log.Warnf("LogsHistoryLimit is high, you can do this, but expect browser lag.")
+	}
 }
 
 func getActionID(action *Action) string {

internal/executor/executor.go

@@ -37,8 +37,9 @@ type ActionBinding struct {
 // Executor represents a helper class for executing commands. It's main method
 // is ExecRequest
 type Executor struct {
-	logs           map[string]*InternalLogEntry
-	LogsByActionId map[string][]*InternalLogEntry
+	logs                  map[string]*InternalLogEntry
+	logsTrackingIdsByDate []string
+	LogsByActionId        map[string][]*InternalLogEntry
 
 	logmutex sync.RWMutex
 
@@ -85,6 +86,7 @@ type InternalLogEntry struct {
 	ExecutionTrackingID string
 	Process             *os.Process
 	Username            string
+	Index               int64
 
 	/*
 		The following 3 properties are obviously on Action normally, but it's useful
@@ -104,6 +106,7 @@ func DefaultExecutor(cfg *config.Config) *Executor {
 	e := Executor{}
 	e.Cfg = cfg
 	e.logs = make(map[string]*InternalLogEntry)
+	e.logsTrackingIdsByDate = make([]string, 0)
 	e.LogsByActionId = make(map[string][]*InternalLogEntry)
 	e.MapActionIdToBinding = make(map[string]*ActionBinding)
 
@@ -135,18 +138,62 @@ func (e *Executor) AddListener(m listener) {
 	e.listeners = append(e.listeners, m)
 }
 
-func (e *Executor) GetLogsCopy() map[string]*InternalLogEntry {
+// getPagingStartIndex calculates the starting index for log pagination.
+// Parameters:
+//
+//	startOffset: The offset from the most recent log (0 means start from the most recent)
+//	totalLogCount: Total number of logs available
+//	count: Number of logs to retrieve
+//
+// Returns: The calculated starting index for pagination
+func getPagingStartIndex(startOffset int64, totalLogCount int64, count int64) int64 {
+	var startIndex int64
+
+	if startOffset <= 0 {
+		startIndex = totalLogCount
+	} else {
+		startIndex = (totalLogCount - startOffset)
+
+		if startIndex < 0 {
+			startIndex = 1
+		}
+	}
+
+	return startIndex - 1
+}
+
+func (e *Executor) GetLogTrackingIds(startOffset int64, pageCount int64) ([]*InternalLogEntry, int64) {
 	e.logmutex.RLock()
 
-	copy := make(map[string]*InternalLogEntry)
+	totalLogCount := int64(len(e.logsTrackingIdsByDate))
 
-	for k, v := range e.logs {
-		copy[k] = v
+	startIndex := getPagingStartIndex(startOffset, totalLogCount, pageCount)
+
+	pageCount = min(totalLogCount, pageCount)
+
+	endIndex := max(0, (startIndex-pageCount)+1)
+
+	log.WithFields(log.Fields{
+		"startOffset": startOffset,
+		"pageCount":   pageCount,
+		"total":       totalLogCount,
+		"startIndex":  startIndex,
+		"endIndex":    endIndex,
+	}).Infof("GetLogTrackingIds")
+
+	trackingIds := make([]*InternalLogEntry, 0, pageCount)
+
+	if totalLogCount > 0 {
+		for i := endIndex; i <= startIndex; i++ {
+			trackingIds = append(trackingIds, e.logs[e.logsTrackingIdsByDate[i]])
+		}
 	}
 
 	e.logmutex.RUnlock()
 
-	return copy
+	remainingLogs := endIndex
+
+	return trackingIds, remainingLogs
 }
 
 func (e *Executor) GetLog(trackingID string) (*InternalLogEntry, bool) {
@@ -176,7 +223,10 @@ func (e *Executor) GetLogsByActionId(actionId string) []*InternalLogEntry {
 func (e *Executor) SetLog(trackingID string, entry *InternalLogEntry) {
 	e.logmutex.Lock()
 
+	entry.Index = int64(len(e.logsTrackingIdsByDate))
+
 	e.logs[trackingID] = entry
+	e.logsTrackingIdsByDate = append(e.logsTrackingIdsByDate, trackingID)
 
 	e.logmutex.Unlock()
 }

internal/executor/executor_test.go

@@ -109,3 +109,73 @@ func TestArgumentNameSnakeCase(t *testing.T) {
 	assert.Equal(t, "echo 'Tickling Fred'", out)
 	assert.Nil(t, err)
 }
+
+func TestGetLogsEmpty(t *testing.T) {
+	e, cfg := testingExecutor()
+
+	assert.Equal(t, int64(10), cfg.LogHistoryPageSize, "Logs page size should be 10")
+
+	logs, remaining := e.GetLogTrackingIds(0, 10)
+
+	assert.NotNil(t, logs, "Logs should not be nil")
+	assert.Equal(t, 0, len(logs), "No logs yet")
+	assert.Equal(t, int64(0), remaining, "There should be no remaining logs")
+}
+
+func TestGetLogsLessThanPageSize(t *testing.T) {
+	e, cfg := testingExecutor()
+
+	cfg.Actions = append(cfg.Actions, &config.Action{
+		Title: "blat",
+		Shell: "date",
+	})
+
+	assert.Equal(t, int64(10), cfg.LogHistoryPageSize, "Logs page size should be 10")
+
+	logEntries, remaining := e.GetLogTrackingIds(0, 10)
+
+	assert.Equal(t, 0, len(logEntries), "There should be 0 logs")
+	assert.Zero(t, remaining, "There should be no remaining logs")
+
+	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, "blat", cfg)
+
+	logEntries, remaining = e.GetLogTrackingIds(0, 10)
+
+	assert.Equal(t, 7, len(logEntries), "There should be 7 logs")
+	assert.Zero(t, remaining, "There should be no remaining logs")
+
+	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, "blat", cfg)
+	execNewReqAndWait(e, "blat", cfg)
+
+	logEntries, remaining = e.GetLogTrackingIds(0, 10)
+
+	assert.Equal(t, 10, len(logEntries), "There should be 10 logs")
+	assert.Equal(t, int64(2), remaining, "There should be 1 remaining logs")
+}
+
+func execNewReqAndWait(e *Executor, title string, cfg *config.Config) {
+	req := &ExecutionRequest{
+		ActionTitle: title,
+		Cfg:         cfg,
+	}
+
+	wg, _ := e.ExecRequest(req)
+	wg.Wait()
+}
+
+func TestGetPagingIndexes(t *testing.T) {
+	assert.Zero(t, getPagingStartIndex(5, 0, 5), "Testing start index from empty list")
+	assert.Equal(t, int64(4), getPagingStartIndex(5, 10, 5), "Testing start index from mid point")
+	assert.Equal(t, int64(9), getPagingStartIndex(-1, 10, 5), "Testing start index with negative offset")
+	assert.Equal(t, int64(0), getPagingStartIndex(15, 10, 5), "Testing start index with large offset")
+	assert.Equal(t, int64(9), getPagingStartIndex(0, 10, 0), "Testing start index with zero count")
+}

internal/grpcapi/grpcApi.go

@@ -14,7 +14,6 @@ import (
 
 	"errors"
 	"net"
-	"sort"
 
 	acl "github.com/OliveTin/OliveTin/internal/acl"
 	config "github.com/OliveTin/OliveTin/internal/config"
@@ -209,6 +208,7 @@ func internalLogEntryToPb(logEntry *executor.InternalLogEntry) *pb.LogEntry {
 		ActionId:            logEntry.ActionId,
 		DatetimeStarted:     logEntry.DatetimeStarted.Format("2006-01-02 15:04:05"),
 		DatetimeFinished:    logEntry.DatetimeFinished.Format("2006-01-02 15:04:05"),
+		DatetimeIndex:       logEntry.Index,
 		Output:              logEntry.Output,
 		TimedOut:            logEntry.TimedOut,
 		Blocked:             logEntry.Blocked,
@@ -335,24 +335,20 @@ func (api *oliveTinAPI) GetLogs(ctx ctx.Context, req *pb.GetLogsRequest) (*pb.Ge
 
 	ret := &pb.GetLogsResponse{}
 
-	// TODO Limit to 10 entries or something to prevent browser lag.
+	logEntries, countRemaining := api.executor.GetLogTrackingIds(req.StartOffset, cfg.LogHistoryPageSize)
 
-	for trackingId, logEntry := range api.executor.GetLogsCopy() {
+	for _, logEntry := range logEntries {
 		action := cfg.FindAction(logEntry.ActionTitle)
 
 		if action == nil || acl.IsAllowedLogs(cfg, user, action) {
 			pbLogEntry := internalLogEntryToPb(logEntry)
-			pbLogEntry.ExecutionTrackingId = trackingId
 
 			ret.Logs = append(ret.Logs, pbLogEntry)
 		}
 	}
 
-	sorter := func(i, j int) bool {
-		return ret.Logs[i].DatetimeStarted < ret.Logs[j].DatetimeStarted
-	}
-
-	sort.Slice(ret.Logs, sorter)
+	ret.CountRemaining = countRemaining
+	ret.PageSize = cfg.LogHistoryPageSize
 
 	return ret, nil
 }

webui.dev/index.html

@@ -63,7 +63,7 @@
 						</button>
 					</label>
 				</div>
-				<table title = "Logs">
+				<table id = "logsTable" title = "Logs" hidden>
 					<thead>
 						<tr title = "untitled">
 							<th>Timestamp</th>
@@ -74,6 +74,10 @@
 					</thead>
 					<tbody id = "logTableBody" />
 				</table>
+
+				<p id = "logsTableEmpty">There are no logs to display. <a href = "/">Return to index</a></p>
+
+				<p><strong>Note:</strong> The server is configured to only send <strong id = "logs-server-page-size">?</strong> log entries at a time. The search box at the top of this page only searches this current page of logs.</p>
 			</section>
 
 			<section id = "contentDiagnostics" title = "Diagnostics" class = "box-shadow" hidden>

webui.dev/js/marshaller.js

@@ -646,6 +646,19 @@ function marshalDirectory (item, section) {
 }
 
 export function marshalLogsJsonToHtml (json) {
+  // This function is called internally with a "fake" server response, that does
+  // not have pageSize set. So we need to check if it's set before trying to use it.
+  if (json.pageSize !== undefined) {
+    document.getElementById('logs-server-page-size').innerText = json.pageSize
+  }
+
+  if (json.logs != null && json.logs.length > 0) {
+    document.getElementById('logsTable').hidden = false
+    document.getElementById('logsTableEmpty').hidden = true
+  } else {
+    return
+  }
+
   for (const logEntry of json.logs) {
     let row = document.getElementById('log-' + logEntry.executionTrackingId)