Skip to content

Commit

Permalink
refactor: log gathering
Browse files Browse the repository at this point in the history
  • Loading branch information
Ricardo Lüders committed Nov 22, 2023
1 parent c1a38b1 commit 909b7cf
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ func Test_GatherKubeControllerManagerLogs(t *testing.T) {
bufio.NewScanner(strings.NewReader(
tc.logline,
)),
msgFilter.MessagesToSearch,
messagesRegex,
common.WithRegexFilter(messagesRegex),
nil)

// Assert
Expand Down
2 changes: 1 addition & 1 deletion pkg/gatherers/clusterconfig/gather_node_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func nodeLogString(ctx context.Context, req *rest.Request, messagesRegex *regexp
scanner = bufio.NewScanner(r)
}

return common.FilterLogFromScanner(scanner, nil, messagesRegex, func(lines []string) []string {
return common.FilterLogFromScanner(scanner, common.WithRegexFilter(messagesRegex), func(lines []string) []string {
if len(lines) > logNodeMaxLines {
return lines[len(lines)-logNodeMaxLines:]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package clusterconfig

import (
"bufio"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -36,10 +35,6 @@ func Test_GatherOpenShiftAPIServerOperatorLogs(t *testing.T) {

// Given
msgFilter := getAPIServerOperatorLogsMessagesFilter()
var messagesRegex *regexp.Regexp
if msgFilter.IsRegexSearch {
messagesRegex = regexp.MustCompile(strings.Join(msgFilter.MessagesToSearch, "|"))
}

for _, testCase := range testCases {
tc := testCase
Expand All @@ -51,8 +46,7 @@ func Test_GatherOpenShiftAPIServerOperatorLogs(t *testing.T) {
bufio.NewScanner(strings.NewReader(
tc.logline,
)),
msgFilter.MessagesToSearch,
messagesRegex,
common.WithSubstringFilter(msgFilter.MessagesToSearch),
nil)

// Assert
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package clusterconfig

import (
"bufio"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -31,10 +30,6 @@ func Test_GatherOpenshiftAuthenticationLogs(t *testing.T) {

// Given
msgFilter := getOpenshiftAuthenticationLogsMessagesFilter()
var messagesRegex *regexp.Regexp
if msgFilter.IsRegexSearch {
messagesRegex = regexp.MustCompile(strings.Join(msgFilter.MessagesToSearch, "|"))
}

for _, testCase := range testCases {
tc := testCase
Expand All @@ -46,8 +41,7 @@ func Test_GatherOpenshiftAuthenticationLogs(t *testing.T) {
bufio.NewScanner(strings.NewReader(
tc.logline,
)),
msgFilter.MessagesToSearch,
messagesRegex,
common.WithSubstringFilter(msgFilter.MessagesToSearch),
nil)

// Assert
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ func Test_GatherOpenshiftSDNControllerLogs(t *testing.T) {
bufio.NewScanner(strings.NewReader(
tc.logline,
)),
msgFilter.MessagesToSearch,
messagesRegex,
common.WithRegexFilter(messagesRegex),
nil)

// Assert
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package clusterconfig

import (
"bufio"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -46,10 +45,6 @@ func Test_GatherOpenshiftSDNLogs(t *testing.T) {

// Given
msgFilter := getGatherOpenshiftSDNLogsMessageFilter()
var messagesRegex *regexp.Regexp
if msgFilter.IsRegexSearch {
messagesRegex = regexp.MustCompile(strings.Join(msgFilter.MessagesToSearch, "|"))
}

for _, testCase := range testCases {
tc := testCase
Expand All @@ -61,8 +56,7 @@ func Test_GatherOpenshiftSDNLogs(t *testing.T) {
bufio.NewScanner(strings.NewReader(
tc.logline,
)),
msgFilter.MessagesToSearch,
messagesRegex,
common.WithSubstringFilter(msgFilter.MessagesToSearch),
nil)

// Assert
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package clusterconfig

import (
"bufio"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -31,10 +30,6 @@ func Test_GatherSAPVsystemIptablesLogs(t *testing.T) {

// Given
msgFilter := getSAPLicenseManagementLogsMessageFilter()
var messagesRegex *regexp.Regexp
if msgFilter.IsRegexSearch {
messagesRegex = regexp.MustCompile(strings.Join(msgFilter.MessagesToSearch, "|"))
}

for _, testCase := range testCases {
tc := testCase
Expand All @@ -46,8 +41,7 @@ func Test_GatherSAPVsystemIptablesLogs(t *testing.T) {
bufio.NewScanner(strings.NewReader(
tc.logline,
)),
msgFilter.MessagesToSearch,
messagesRegex,
common.WithSubstringFilter(msgFilter.MessagesToSearch),
nil)

// Assert
Expand Down
8 changes: 1 addition & 7 deletions pkg/gatherers/clusterconfig/gather_scheduler_logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package clusterconfig

import (
"bufio"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -31,10 +30,6 @@ func Test_GatherSchedulerLogs(t *testing.T) {

// Given
msgFilter := getSchedulerLogsMessagesFilter()
var messagesRegex *regexp.Regexp
if msgFilter.IsRegexSearch {
messagesRegex = regexp.MustCompile(strings.Join(msgFilter.MessagesToSearch, "|"))
}

for _, testCase := range testCases {
tc := testCase
Expand All @@ -46,8 +41,7 @@ func Test_GatherSchedulerLogs(t *testing.T) {
bufio.NewScanner(strings.NewReader(
tc.logline,
)),
msgFilter.MessagesToSearch,
messagesRegex,
common.WithSubstringFilter(msgFilter.MessagesToSearch),
nil)

// Assert
Expand Down
71 changes: 48 additions & 23 deletions pkg/gatherers/common/gather_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ type LogMessagesFilter struct {
Previous bool `json:"previous,omitempty"`
}

// FilterLogCallbackFunc is used to apply extra filtering to log lines
type FilterLogCallbackFunc func(lines []string) []string

// FilterLogOption is used to apply the filter engine (e.g: Regex, Substring)
type FilterLogOption func(line string) bool

// CollectLogsFromContainers collects logs from containers
// - containerFilter allows you to specify
// - namespace in which to search for pods
Expand Down Expand Up @@ -125,7 +131,14 @@ func CollectLogsFromContainers( //nolint:gocyclo

request := coreClient.Pods(containersFilter.Namespace).GetLogs(pod.Name, podLogOptions(containerName, messagesFilter))

logs, err := filterLogs(ctx, request, messagesFilter.MessagesToSearch, messagesRegexp)
var filter FilterLogOption
if messagesFilter.IsRegexSearch {
filter = WithRegexFilter(messagesRegexp)
} else {
filter = WithSubstringFilter(messagesFilter.MessagesToSearch)
}

logs, err := FilterLogsFromRequest(ctx, request, filter)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -162,9 +175,32 @@ func podContainers(pod *corev1.Pod) []string {
return containerNames
}

func filterLogs(
ctx context.Context, request *restclient.Request, messagesToSearch []string, messagesRegexp *regexp.Regexp,
) (string, error) {
// WithRegexFilter filter the log line with the given regex
func WithRegexFilter(r *regexp.Regexp) FilterLogOption {
return func(line string) bool {
return r.MatchString(line)
}
}

// WithSubstringFilter filter the log line with the given messages
func WithSubstringFilter(messages []string) FilterLogOption {
return func(line string) bool {
if len(messages) == 0 {
return true
}

for _, m := range messages {
if strings.Contains(strings.ToLower(line), strings.ToLower(m)) {
return true
}
}
return false
}
}

// FilterLogsFromRequest filters the desired messages from log from given Request.
// If no filter is specified all log lines in the request would be included.
func FilterLogsFromRequest(ctx context.Context, request *restclient.Request, filter FilterLogOption) (string, error) {
stream, err := request.Stream(ctx)
if err != nil {
return "", err
Expand All @@ -178,34 +214,23 @@ func filterLogs(
}()

scanner := bufio.NewScanner(stream)
return FilterLogFromScanner(scanner, messagesToSearch, messagesRegexp, nil)
return FilterLogFromScanner(scanner, filter, nil)
}

// FilterLogFromScanner filters the desired messages from the log
func FilterLogFromScanner(scanner *bufio.Scanner, messagesToSearch []string, messagesRegexp *regexp.Regexp,
cb func(lines []string) []string) (string, error) {
// FilterLogFromScanner filters the desired messages from the log from given Scanner.
// If no filter is specified all log lines in the scanner would be included.
func FilterLogFromScanner(scanner *bufio.Scanner, filter FilterLogOption, cb FilterLogCallbackFunc,
) (string, error) {
var result []string

for scanner.Scan() {
line := scanner.Text()

if messagesRegexp != nil {
matches := messagesRegexp.MatchString(line)
if matches {
result = append(result, line)
}
continue
}

for _, messageToSearch := range messagesToSearch {
if strings.Contains(strings.ToLower(line), strings.ToLower(messageToSearch)) {
result = append(result, line)
}
if filter == nil {
result = append(result, line)
}

if len(messagesToSearch) == 0 {
if ok := filter(line); ok {
result = append(result, line)
continue
}
}

Expand Down
10 changes: 8 additions & 2 deletions pkg/gatherers/common/gather_logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,13 @@ func Test_FilterLogFromScanner(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reader := strings.NewReader(testText)
result, err := FilterLogFromScanner(bufio.NewScanner(reader), tt.messagesToSearch, tt.messagesRegex, nil)
var filter FilterLogOption
if tt.messagesRegex != nil {
filter = WithRegexFilter(tt.messagesRegex)
} else {
filter = WithSubstringFilter(tt.messagesToSearch)
}
result, err := FilterLogFromScanner(bufio.NewScanner(reader), filter, nil)
assert.NoError(t, err)
assert.Equal(t, tt.expectedOutput, result)
})
Expand All @@ -186,7 +192,7 @@ func Benchmark_FilterLogFromScanner(b *testing.B) {
reader := strings.NewReader(testText)
for i := 0; i <= b.N; i++ {
// nolint errcheck
FilterLogFromScanner(bufio.NewScanner(reader), messagesToSearch, messagesRegex, nil)
FilterLogFromScanner(bufio.NewScanner(reader), WithRegexFilter(messagesRegex), nil)
runtime.ReadMemStats(&m)
b.Logf("Size of allocated heap objects: %d MB, Size of heap in use: %d MB", m.Alloc/1024/1024, m.HeapInuse/1024/1024)
}
Expand Down

0 comments on commit 909b7cf

Please sign in to comment.