diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a192c2c..582f4f84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## [3.2.2] - 2024-09-03 + +## Changed + +* added note that the executor can be empty during the UI prompts + +## Added + +* remove host from executor list if it is present in the coordinator list +* remove a host from the list of nodes if it has been specified twice + ## [3.2.1] - 2024-08-26 ## Added diff --git a/cmd/local/queriesjson/queries.go b/cmd/local/queriesjson/queries.go index 49a1c628..042caf65 100644 --- a/cmd/local/queriesjson/queries.go +++ b/cmd/local/queriesjson/queries.go @@ -236,7 +236,7 @@ func parseLine(line string, i int) (QueriesRow, error) { dat := make(map[string]interface{}) err := json.Unmarshal([]byte(line), &dat) if err != nil { - return *new(QueriesRow), fmt.Errorf("queries.json line #%v: %v[...] - error: %v", i, strutils.LimitString(line, 50), err) + return *new(QueriesRow), fmt.Errorf("queries.json line #%v: %v[...] - error: %v", i, strutils.GetEndOfString(line, 50), err) } var row = new(QueriesRow) if val, ok := dat["queryId"]; ok { diff --git a/cmd/root.go b/cmd/root.go index 198aa88d..92876221 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -130,7 +130,6 @@ func startTicker() (stop func()) { close(quit) } } - func RemoteCollect(collectionArgs collection.Args, sshArgs ssh.Args, kubeArgs kubernetes.KubeArgs, fallbackEnabled bool, hook shutdown.Hook) error { patSet := collectionArgs.DremioPAT != "" consoleprint.UpdateRuntime( diff --git a/cmd/root/collection/capture.go b/cmd/root/collection/capture.go index 8793b93b..39fa6330 100644 --- a/cmd/root/collection/capture.go +++ b/cmd/root/collection/capture.go @@ -325,7 +325,7 @@ func StartCapture(c HostCaptureConfiguration, localDDCPath, localDDCYamlPath str StatusUX: "LOCAL-COLLECT", Result: consoleprint.ResultFailure, EndProcess: true, - Message: strutils.LimitString(strings.Join(allHostLog, " - "), 1024), + Message: strutils.GetEndOfString(strings.Join(allHostLog, " - "), 1024), } consoleprint.UpdateNodeState(nodeState) simplelog.HostLog(host, fmt.Sprintf("%#v", nodeState)) diff --git a/cmd/root/collection/collector.go b/cmd/root/collection/collector.go index b92b9d75..77503d36 100644 --- a/cmd/root/collection/collector.go +++ b/cmd/root/collection/collector.go @@ -22,6 +22,8 @@ import ( "io/fs" "os" "path/filepath" + "slices" + "sort" "strings" "sync" "time" @@ -84,6 +86,58 @@ type HostCaptureConfiguration struct { CollectionMode string } +func FilterCoordinators(coordinators []string) []string { + // use a map for the unique key property, so we handle duplicates in the list + filteredList := make(map[string]bool) + for _, e := range coordinators { + filteredList[e] = true + } + // convert back into a slice + // again we are dealing with not + // large enough sizes to warrant optimization + var result []string + for k := range filteredList { + result = append(result, k) + } + // sort for consistent behavior and testing + slices.Sort(result) + slices.Reverse(result) + return result +} + +func FilterExecutors(executors []string, coordinators []string) []string { + // use a map for the unique key property, so we handle duplicates in the list + filteredList := make(map[string]bool) + for _, e := range executors { + // we're not going to bother optimizing this: + // the list will not be long enough to matter + var dupe bool + // if it's a coordinator we don't need it + for _, c := range coordinators { + if c == e { + dupe = true + simplelog.Warningf("found %v in coordinator and executor list, removing from executor list", e) + consoleprint.AddWarningToConsole(fmt.Sprintf("%v was listed as executor and coordinator, choosing coordinator", e)) + break + } + } + if !dupe { + filteredList[e] = true + } + } + // convert back into a slice + // again we are dealing with not + // large enough sizes to warrant optimization + var result []string + for k := range filteredList { + result = append(result, k) + } + // sort for consistent behavior and testing + sort.Strings(result) + slices.Reverse(result) + return result +} + func Execute(c Collector, s CopyStrategy, collectionArgs Args, hook shutdown.Hook, clusterCollection ...func([]string)) error { start := time.Now().UTC() outputLoc := collectionArgs.OutputLoc @@ -117,10 +171,11 @@ func Execute(c Collector, s CopyStrategy, collectionArgs Args, hook shutdown.Hoo return err } - executors, err := c.GetExecutors() + executorsRaw, err := c.GetExecutors() if err != nil { return err } + executors := FilterExecutors(executorsRaw, coordinators) totalNodes := len(executors) + len(coordinators) if totalNodes == 0 { diff --git a/cmd/root/collection/collector_test.go b/cmd/root/collection/collector_test.go new file mode 100644 index 00000000..f381d940 --- /dev/null +++ b/cmd/root/collection/collector_test.go @@ -0,0 +1,61 @@ +// Copyright 2023 Dremio Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// collection package provides the interface for collection implementation and the actual collection execution +package collection + +import "testing" + +func TestFilterCoordinators(t *testing.T) { + t.Log("testing filtering duplicates") + firstItem := "192.168.1.20" + secondItem := "192.168.1.120" + filtered := FilterCoordinators([]string{firstItem, firstItem, secondItem}) + expectedItems := 2 + if len(filtered) != expectedItems { + t.Errorf("expected %v but got %v items", expectedItems, len(filtered)) + } + t.Log("items are sorted in desc order by default") + if filtered[0] != firstItem { + t.Errorf("expected %v but got %v", firstItem, filtered[1]) + } + if filtered[1] != secondItem { + t.Errorf("expected %v but got %v", secondItem, filtered[0]) + } +} + +func TestFilterExecutors(t *testing.T) { + t.Log("testing filtering duplicates") + firstItem := "192.168.1.20" + secondItem := "192.168.1.120" + filtered := FilterExecutors([]string{firstItem, firstItem, secondItem}, []string{}) + expectedItems := 2 + if len(filtered) != expectedItems { + t.Errorf("expected %v but got %v items", expectedItems, len(filtered)) + } + t.Log("items are sorted in desc order by default") + if filtered[0] != firstItem { + t.Errorf("expected %v but got %v", firstItem, filtered[1]) + } + if filtered[1] != secondItem { + t.Errorf("expected %v but got %v", secondItem, filtered[0]) + } + + t.Logf("now verify filter out coordinators") + filtered = FilterExecutors([]string{firstItem, firstItem, secondItem}, []string{firstItem, secondItem}) + expectedItems = 0 + if len(filtered) != expectedItems { + t.Errorf("expected %v but got %v items", expectedItems, len(filtered)) + } +} diff --git a/pkg/consoleprint/consoleprint.go b/pkg/consoleprint/consoleprint.go index 20669667..11dba164 100644 --- a/pkg/consoleprint/consoleprint.go +++ b/pkg/consoleprint/consoleprint.go @@ -24,6 +24,8 @@ import ( "strings" "sync" "time" + + "github.com/dremio/dremio-diagnostic-collector/v3/pkg/strutils" ) // NodeCaptureStats represents stats for a node capture. @@ -54,6 +56,7 @@ type CollectionStats struct { patSet bool startTime int64 endTime int64 + warnings []string mu sync.RWMutex // Mutex to protect access } @@ -157,6 +160,16 @@ func UpdateCollectionMode(collectionMode string) { c.collectionMode = collectionMode } +// AddWarningToConsole adds a trimed string to the list of warnings +// lines after the first line are also trimmed +func AddWarningToConsole(warning string) { + c.mu.Lock() + defer c.mu.Unlock() + tokens := strings.Split(warning, "\n") + trimmed := tokens[0] + c.warnings = append(c.warnings, strutils.TruncateString(trimmed, 120)) +} + var c *CollectionStats func init() { @@ -315,6 +328,13 @@ func PrintState() { if c.ddcVersion != "" { ddcVersion = c.ddcVersion } + var warningsBuilder strings.Builder + for i, w := range c.warnings { + _, err := warningsBuilder.WriteString(fmt.Sprintf("%v. %v\n", i+1, w)) + if err != nil { + fmt.Printf("unable to write string %v: (%v)", w, err) + } + } fmt.Printf( `================================= == Dremio Diagnostic Collector == @@ -338,10 +358,13 @@ Collect Duration : elapsed %v seconds Tarball : %v Result : %v +-- Warnings -- +%v + %v `, time.Now().Format(time.RFC1123), strings.TrimSpace(ddcVersion), c.ddcYaml, c.logFile, c.collectionType, strings.Join(c.enabled, ","), strings.Join(c.disabled, ","), strings.ToUpper(c.collectionMode), c.collectionArgs, patMessage, autodetectEnabled, c.TransfersComplete, total, - durationElapsed, c.tarball, c.result, nodes.String()) + durationElapsed, c.tarball, c.result, warningsBuilder.String(), nodes.String()) c.mu.Unlock() } diff --git a/pkg/simplelog/logger.go b/pkg/simplelog/logger.go index 70f83e5a..694b9f4a 100644 --- a/pkg/simplelog/logger.go +++ b/pkg/simplelog/logger.go @@ -243,42 +243,42 @@ func newLogger(f io.Writer, cleanup func()) *Logger { } func (l *Logger) Debug(format string) { - trimmed := strutils.LimitString(format, msgMax) + trimmed := strutils.GetEndOfString(format, msgMax) handleLogError(l.debugLogger.Output(2, trimmed), trimmed, "DEBUG") } func (l *Logger) Info(format string) { - trimmed := strutils.LimitString(format, msgMax) + trimmed := strutils.GetEndOfString(format, msgMax) handleLogError(l.infoLogger.Output(2, trimmed), trimmed, "INFO") } func (l *Logger) Warning(format string) { - trimmed := strutils.LimitString(format, msgMax) + trimmed := strutils.GetEndOfString(format, msgMax) handleLogError(l.warningLogger.Output(2, trimmed), trimmed, "WARNING") } func (l *Logger) Error(format string) { - trimmed := strutils.LimitString(format, msgMax) + trimmed := strutils.GetEndOfString(format, msgMax) handleLogError(l.errorLogger.Output(2, trimmed), trimmed, "ERROR") } func (l *Logger) Debugf(format string, v ...interface{}) { - msg := strutils.LimitString(fmt.Sprintf(format, v...), msgMax) + msg := strutils.GetEndOfString(fmt.Sprintf(format, v...), msgMax) handleLogError(l.debugLogger.Output(2, msg), msg, "DEBUGF") } func (l *Logger) Infof(format string, v ...interface{}) { - msg := strutils.LimitString(fmt.Sprintf(format, v...), msgMax) + msg := strutils.GetEndOfString(fmt.Sprintf(format, v...), msgMax) handleLogError(l.infoLogger.Output(2, msg), msg, "INFOF") } func (l *Logger) Warningf(format string, v ...interface{}) { - msg := strutils.LimitString(fmt.Sprintf(format, v...), msgMax) + msg := strutils.GetEndOfString(fmt.Sprintf(format, v...), msgMax) handleLogError(l.warningLogger.Output(2, msg), msg, "WARNINGF") } func (l *Logger) Errorf(format string, v ...interface{}) { - msg := strutils.LimitString(fmt.Sprintf(format, v...), msgMax) + msg := strutils.GetEndOfString(fmt.Sprintf(format, v...), msgMax) handleLogError(l.errorLogger.Output(2, msg), msg, "ERRORF") } @@ -287,56 +287,56 @@ func (l *Logger) Errorf(format string, v ...interface{}) { func Debug(format string) { ddcLogMut.Lock() defer ddcLogMut.Unlock() - trimmed := strutils.LimitString(format, msgMax) + trimmed := strutils.GetEndOfString(format, msgMax) handleLogError(logger.debugLogger.Output(2, trimmed), trimmed, "DEBUG") } func Info(format string) { ddcLogMut.Lock() defer ddcLogMut.Unlock() - trimmed := strutils.LimitString(format, msgMax) + trimmed := strutils.GetEndOfString(format, msgMax) handleLogError(logger.infoLogger.Output(2, trimmed), trimmed, "INFO") } func Warning(format string) { ddcLogMut.Lock() defer ddcLogMut.Unlock() - trimmed := strutils.LimitString(format, msgMax) + trimmed := strutils.GetEndOfString(format, msgMax) handleLogError(logger.warningLogger.Output(2, trimmed), trimmed, "WARNING") } func Error(format string) { ddcLogMut.Lock() defer ddcLogMut.Unlock() - trimmed := strutils.LimitString(format, msgMax) + trimmed := strutils.GetEndOfString(format, msgMax) handleLogError(logger.errorLogger.Output(2, trimmed), trimmed, "ERROR") } func Debugf(format string, v ...interface{}) { ddcLogMut.Lock() defer ddcLogMut.Unlock() - msg := strutils.LimitString(fmt.Sprintf(format, v...), msgMax) + msg := strutils.GetEndOfString(fmt.Sprintf(format, v...), msgMax) handleLogError(logger.debugLogger.Output(2, msg), msg, "DEBUGF") } func Infof(format string, v ...interface{}) { ddcLogMut.Lock() defer ddcLogMut.Unlock() - msg := strutils.LimitString(fmt.Sprintf(format, v...), msgMax) + msg := strutils.GetEndOfString(fmt.Sprintf(format, v...), msgMax) handleLogError(logger.infoLogger.Output(2, msg), msg, "INFOF") } func Warningf(format string, v ...interface{}) { ddcLogMut.Lock() defer ddcLogMut.Unlock() - msg := strutils.LimitString(fmt.Sprintf(format, v...), msgMax) + msg := strutils.GetEndOfString(fmt.Sprintf(format, v...), msgMax) handleLogError(logger.warningLogger.Output(2, msg), msg, "WARNINGF") } func Errorf(format string, v ...interface{}) { ddcLogMut.Lock() defer ddcLogMut.Unlock() - msg := strutils.LimitString(fmt.Sprintf(format, v...), msgMax) + msg := strutils.GetEndOfString(fmt.Sprintf(format, v...), msgMax) handleLogError(logger.errorLogger.Output(2, msg), msg, "ERRORF") } diff --git a/pkg/strutils/truncate.go b/pkg/strutils/truncate.go index a52c5a14..a2afba9c 100644 --- a/pkg/strutils/truncate.go +++ b/pkg/strutils/truncate.go @@ -19,7 +19,7 @@ import ( "unicode/utf8" ) -func LimitString(s string, maxLength int) string { +func GetEndOfString(s string, maxLength int) string { max := 0 if maxLength > 0 { max = maxLength @@ -29,12 +29,24 @@ func LimitString(s string, maxLength int) string { return s } - // Truncate the string to the desired length + // get the end of the string up to desired length runes := []rune(s) truncatedRunes := runes[len(runes)-max:] return string(truncatedRunes) } +func TruncateString(s string, maxLength int) string { + max := 0 + if maxLength > 0 { + max = maxLength + } + // Check if the string is already within the desired length + if utf8.RuneCountInString(s) <= max { + return s + } + return s[:max] +} + func GetLastLine(s string) string { index := strings.LastIndex(s, "\n") if index == -1 { diff --git a/pkg/strutils/truncate_test.go b/pkg/strutils/truncate_test.go index 8b267268..13c3f9b0 100644 --- a/pkg/strutils/truncate_test.go +++ b/pkg/strutils/truncate_test.go @@ -21,42 +21,42 @@ import ( ) func TestLimitStringTooLong(t *testing.T) { - a := strutils.LimitString("12345", 1) + a := strutils.GetEndOfString("12345", 1) if a != "5" { t.Errorf("expected '1' but got '%v'", a) } } func TestLimitStringWhenStringIsShorterThanLimit(t *testing.T) { - a := strutils.LimitString("12345", 100) + a := strutils.GetEndOfString("12345", 100) if a != "12345" { t.Errorf("expected '12345' but got '%v'", a) } } func TestLimitStringWhenStringIsEmpty(t *testing.T) { - a := strutils.LimitString("", 100) + a := strutils.GetEndOfString("", 100) if a != "" { t.Errorf("expected '' but got '%v'", a) } } func TestLimitStringWhenLimitAndStringAreDefault(t *testing.T) { - a := strutils.LimitString("", 0) + a := strutils.GetEndOfString("", 0) if a != "" { t.Errorf("expected '' but got '%v'", a) } } func TestLimitStringWhenLimitIsDefault(t *testing.T) { - a := strutils.LimitString("12345", 0) + a := strutils.GetEndOfString("12345", 0) if a != "" { t.Errorf("expected '' but got '%v'", a) } } func TestLimitStringWhenLimitINegative(t *testing.T) { - a := strutils.LimitString("12345", -1) + a := strutils.GetEndOfString("12345", -1) if a != "" { t.Errorf("expected '' but got '%v'", a) }