now copy log before gzipping it (#131)

* now copy log before gziping it
* matching on true for SKIP_INTEGRATION_TEST
* updated change logs

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## [0.7.3]
+### Fixed
+
+* now copy, archive, and delete copy instead of archive in place log files #130
+* unexpected use of tmp path #123
+* clean rest API URL by adding check for trailing slash #127 
+
 ## [0.7.2]
 
 ### Fixed
@@ -365,6 +372,9 @@
 
 - able to capture logs, configuration and diagnostic data from dremio clusters deployed on Kubernetes and on-prem
 
+[0.7.3]: https://github.com/dremio/dremio-diagnostic-collector/compare/v0.7.2...v0.7.3
+[0.7.2]: https://github.com/dremio/dremio-diagnostic-collector/compare/v0.7.1...v0.7.2
+[0.7.1]: https://github.com/dremio/dremio-diagnostic-collector/compare/v0.7.0...v0.7.1
 [0.7.0]: https://github.com/dremio/dremio-diagnostic-collector/compare/v0.6.2...v0.7.0
 [0.6.2]: https://github.com/dremio/dremio-diagnostic-collector/compare/v0.6.1...v0.6.2
 [0.6.1]: https://github.com/dremio/dremio-diagnostic-collector/compare/v0.6.0...v0.6.1

cmd/local/jvmcollect/ttopcollect_test.go

@@ -21,11 +21,9 @@ import (
 	"path/filepath"
 	"reflect"
 	"runtime"
-	"strings"
 	"testing"
 	"time"
 
-	"github.com/dremio/dremio-diagnostic-collector/cmd/local/conf"
 	"github.com/dremio/dremio-diagnostic-collector/cmd/local/jvmcollect"
 )
 
@@ -115,63 +113,6 @@ func TestTtopCollects(t *testing.T) {
 	}
 }
 
-func TestTtopCollectsIntegrationTest(t *testing.T) {
-	jarLoc := filepath.Join("testdata", "demo.jar")
-	cmd := exec.Command("java", "-jar", jarLoc)
-	if err := cmd.Start(); err != nil {
-		t.Fatalf("cmd.Start() failed with %s\n", err)
-	}
-	defer func() {
-		//in windows we may need a bit more time to kill the process
-		if runtime.GOOS == "windows" {
-			time.Sleep(500 * time.Millisecond)
-		}
-		if err := cmd.Process.Kill(); err != nil {
-			t.Fatalf("failed to kill process: %s", err)
-		} else {
-			t.Log("Process killed successfully.")
-		}
-	}()
-	pid := cmd.Process.Pid
-
-	tmpDir := t.TempDir()
-	ddcYaml := filepath.Join(tmpDir, "ddc.yaml")
-
-	if err := os.WriteFile(ddcYaml, []byte(fmt.Sprintf(`
-dremio-pid: %v
-dremio-ttop-freq-seconds: 1
-dremio-ttop-time-seconds: 2
-node-name: node1
-tmp-output-dir: %v
-
-`, pid, tmpDir)), 0600); err != nil {
-		t.Fatal(err)
-	}
-	ttopOutDir := filepath.Join(tmpDir, "ttop", "node1")
-	if err := os.MkdirAll(ttopOutDir, 0700); err != nil {
-		t.Fatal(err)
-	}
-	overrides := make(map[string]string)
-	c, err := conf.ReadConf(overrides, tmpDir)
-	if err != nil {
-		t.Fatalf("Unable to read conf: %v", err)
-	}
-	if err := jvmcollect.RunTtopCollect(c); err != nil {
-		t.Fatalf("unable to collect %v", err)
-	}
-
-	ttopFile := filepath.Join(ttopOutDir, "ttop.txt")
-	b, err := os.ReadFile(ttopFile)
-	if err != nil {
-		t.Error(err)
-	}
-	txt := string(b)
-	expected := "Monitoring threads ..."
-	if !strings.Contains(txt, expected) {
-		t.Errorf("expected text '%v' to be found in '%v'", expected, txt)
-	}
-}
-
 func TestTtopExec(t *testing.T) {
 	ttop := &jvmcollect.Ttop{}
 	jarLoc := filepath.Join("testdata", "demo.jar")
@@ -203,7 +144,7 @@ func TestTtopExec(t *testing.T) {
 	if text, err := ttop.KillTtop(); err != nil {
 		t.Errorf(err.Error())
 	} else {
-		t.Logf("text for top was `%v`", text)
+		t.Logf("text for ttop was `%v`", text)
 	}
 }
 

cmd/local/local_test.go

@@ -201,9 +201,11 @@ is-dremio-cloud: false
 	if !found {
 		t.Errorf("did not find jvm_settings.txt in entries '%v'", strings.Join(items, ", "))
 	}
-	expectedText := "demo.jar -Dtestflag=1"
-	if text != expectedText {
-		t.Errorf("expected '%q' but was '%q'", expectedText, text)
+	containsDemoJar := strings.Contains(text, "demo.jar")
+	containsFlag := strings.Contains(text, "-Dtestflag=1")
+	successful := containsFlag && containsDemoJar
+	if !successful {
+		t.Errorf("expected '-Dtestflag=1' and 'demo.jar' in the flags but was '%q'", text)
 	}
 }
 

cmd/local/logcollect/logcollect.go

@@ -63,7 +63,7 @@ func (l *Collector) RunCollectDremioServerLog() error {
 		errs = append(errs, fmt.Errorf("unable to copy %v to %v due to error %v", src, dest, err))
 	}
 	if len(errs) > 1 {
-		return fmt.Errorf("serveral errors while copying dremio server logs: %v", errors.Join(errs...))
+		return fmt.Errorf("several errors while copying dremio server logs: %v", errors.Join(errs...))
 	} else if (len(errs)) == 1 {
 		return errs[0]
 	}
@@ -112,7 +112,7 @@ func (l *Collector) RunCollectGcLogs() error {
 		}
 	}
 	if len(errs) > 1 {
-		return fmt.Errorf("serveral errors while copying dremio server logs: %v", errors.Join(errs...))
+		return fmt.Errorf("several errors while copying dremio server logs: %v", errors.Join(errs...))
 	} else if (len(errs)) == 1 {
 		return errs[0]
 	}
@@ -153,7 +153,7 @@ func (l *Collector) RunCollectDremioAccessLogs() error {
 func (l *Collector) RunCollectDremioAuditLogs() error {
 	simplelog.Debug("Collecting audit logs from Coordinator(s) ...")
 	if err := l.exportArchivedLogs(l.dremioLogDir, "audit.json", "audit", l.dremioLogsNumDays); err != nil {
-		return fmt.Errorf("unable to archive audit.jsons due to error %v", err)
+		return fmt.Errorf("unable to archive audit.json files due to error %v", err)
 	}
 	simplelog.Debug("... collecting audit logs from Coordinator(s) COMPLETED")
 
@@ -165,7 +165,7 @@ func (l *Collector) RunCollectAccelerationLogs() error {
 	if err := l.exportArchivedLogs(l.dremioLogDir, "acceleration.log", "acceleration", l.dremioLogsNumDays); err != nil {
 		return fmt.Errorf("unable to archive acceleration.logs due to error %v", err)
 	}
-	simplelog.Debug("... collecting acceleragtion logs from Coordinator(s) COMPLETED")
+	simplelog.Debug("... collecting acceleration logs from Coordinator(s) COMPLETED")
 
 	return nil
 }
@@ -181,19 +181,29 @@ func (l *Collector) RunCollectQueriesJSON() error {
 	return nil
 }
 
-func (l *Collector) exportArchivedLogs(srcLogDir string, unarchivedFile string, logPrefix string, archiveDays int) error {
+func (l *Collector) exportArchivedLogs(srcLogDir string, unzippedFile string, logPrefix string, archiveDays int) error {
 	var errs []error
-	src := path.Join(srcLogDir, unarchivedFile)
+	src := path.Join(srcLogDir, unzippedFile)
 	var outDir string
 	if logPrefix == "queries" {
 		outDir = l.queriesOutDir
 	} else {
 		outDir = l.logsOutDir
 	}
-	dest := path.Join(outDir, unarchivedFile)
-	//instead of copying it we just archive it to a new location
-	if err := ddcio.GzipFile(path.Clean(src), path.Clean(dest+".gz")); err != nil {
-		errs = append(errs, fmt.Errorf("archiving of log file %v failed due to error %v", unarchivedFile, err))
+	unzippedFileDest := path.Join(outDir, unzippedFile)
+	//we must copy before archival to avoid races around the archiving features of logging (which also use gzip)
+	if err := ddcio.CopyFile(path.Clean(src), path.Clean(unzippedFileDest)); err != nil {
+		errs = append(errs, fmt.Errorf("copying of log file %v failed due to error %v", unzippedFile, err))
+	} else {
+		// if this is successful go ahead and gzip it
+		if err := ddcio.GzipFile(path.Clean(unzippedFileDest), path.Clean(unzippedFileDest+".gz")); err != nil {
+			errs = append(errs, fmt.Errorf("archiving of log file %v failed due to error %v", unzippedFile, err))
+		} else {
+			//if we've successfully gzipped the file we can safely delete the source
+			if err := os.Remove(path.Clean(unzippedFileDest)); err != nil {
+				errs = append(errs, fmt.Errorf("cleanup of old log file %v failed due to error %v", unzippedFile, err))
+			}
+		}
 	}
 
 	today := time.Now()
@@ -210,17 +220,23 @@ func (l *Collector) exportArchivedLogs(srcLogDir string, unarchivedFile string,
 				simplelog.Debugf("Copying archive file for %v:%v", processingDate, f.Name())
 				src := filepath.Join(srcLogDir, "archive", f.Name())
 				dst := filepath.Join(outDir, f.Name())
-				if strings.HasSuffix(f.Name(), ".gz") {
-					if err := ddcio.CopyFile(path.Clean(src), path.Clean(dst)); err != nil {
-						errs = append(errs, fmt.Errorf("unable to move file %v to %v due to error %v", src, dst, err))
-						continue
-					}
-				} else {
-					//instead of copying it we just archive it to a new location
-					if err := ddcio.GzipFile(path.Clean(src), path.Clean(dst+".gz")); err != nil {
+
+				//we must copy before archival to avoid races around the archiving features of logging (which also use gzip)
+				if err := ddcio.CopyFile(path.Clean(src), path.Clean(dst)); err != nil {
+					errs = append(errs, fmt.Errorf("unable to move file %v to %v due to error %v", src, dst, err))
+					continue
+				}
+				if !strings.HasSuffix(f.Name(), ".gz") {
+					//go ahead and archive the file since it's not already
+					if err := ddcio.GzipFile(path.Clean(dst), path.Clean(dst+".gz")); err != nil {
 						errs = append(errs, fmt.Errorf("unable to archive file %v to %v due to error %v", src, dst, err))
 						continue
 					}
+					//if we've successfully gzipped the file we can safely delete the source (the continue above will guard against executing this)
+					if err := os.Remove(path.Clean(dst)); err != nil {
+						errs = append(errs, fmt.Errorf("cleanup of old log file %v failed due to error %v", unzippedFile, err))
+					}
+
 				}
 			}
 		}

integrationtest/kube/collect_kube_test.go

@@ -143,6 +143,11 @@ var namespace string
 var dremioPATToken string
 
 func TestMain(m *testing.M) {
+	isIntegration := os.Getenv("SKIP_INTEGRATION_SETUP")
+	if isIntegration == "true" {
+		return
+	}
+
 	simplelog.InitLogger(4)
 	exitCode := func() (exitCode int) {
 		var err error
@@ -385,11 +390,17 @@ func TestMain(m *testing.M) {
 }
 
 func TestValidateTestWasCorrectlyRun(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping testing in short mode")
+	}
 	if !setupIsRun {
 		t.Error("integration tests broken, nothing was run, if all other tests are passing this means there is an error in setup")
 	}
 }
 func TestRemoteCollectOnK8s(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping testing in short mode")
+	}
 	var err error
 	transferDir := "/opt/dremio/data/ddc-transfer"
 	tmpOutputDir := "/opt/dremio/data/ddc-tmp-out"

integrationtest/ssh/collect_ssh_test.go

@@ -50,6 +50,9 @@ type SSHTestConf struct {
 }
 
 func TestSSHBasedRemoteCollect(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping testing in short mode")
+	}
 
 	b, err := os.ReadFile(filepath.Join("testdata", "ssh.json"))
 	if err != nil {

script/test

@@ -1,6 +1,6 @@
 #!/bin/sh
 
-# script/test: Run test suite for application.
+# script/test: Run all tests
 
 #set -e
 
@@ -11,4 +11,4 @@ cd "$(dirname "$0")/.."
 ./script/clean
 ./script/build
 
- go test -race  -coverpkg=./... -coverprofile=covprofile ./...
+ go test -race -coverpkg=./... -coverprofile=covprofile ./...

script/test-unit

@@ -0,0 +1,14 @@
+#!/bin/sh
+
+# script/test: Run all but the integration tests
+
+#set -e
+
+cd "$(dirname "$0")/.."
+
+[ -z "$DEBUG" ] || set -x
+
+./script/clean
+./script/build
+export SKIP_INTEGRATION_SETUP=true
+ go test -race -short -coverpkg=./... -coverprofile=covprofile ./...