Merge #36922

36922: goroutinedumper: Gzip files r=tbg a=giorgosp

Fixes #36846

Release note: None

Co-authored-by: George Papadrosou <[email protected]>

pkg/cli/zip.go

@@ -398,7 +398,8 @@ func runDebugZip(cmd *cobra.Command, args []string) error {
 					return z.createError("/goroutines", err)
 				}
 				for _, file := range goroutinesResp.Files {
-					name := prefix + "/goroutines/" + file.Name + ".txt"
+					// NB: the files have a .txt.gz suffix already.
+					name := prefix + "/goroutines/" + file.Name
 					if err := z.createRawOrError(name, file.Contents, err); err != nil {
 						return err
 					}

pkg/server/goroutinedumper/goroutinedumper.go

@@ -15,6 +15,7 @@
 package goroutinedumper
 
 import (
+	"compress/gzip"
 	"context"
 	"fmt"
 	"io/ioutil"
@@ -175,14 +176,22 @@ func gc(ctx context.Context, dir string, sizeLimit int64) {
 }
 
 func takeGoroutineDump(dir string, filename string) error {
+	filename = filename + ".txt.gz"
 	path := filepath.Join(dir, filename)
 	f, err := os.Create(path)
 	if err != nil {
 		return errors.Wrapf(err, "error creating file %s for goroutine dump", path)
 	}
 	defer f.Close()
-	if err = pprof.Lookup("goroutine").WriteTo(f, 2); err != nil {
+	w := gzip.NewWriter(f)
+	if err = pprof.Lookup("goroutine").WriteTo(w, 2); err != nil {
 		return errors.Wrapf(err, "error writing goroutine dump to %s", path)
 	}
-	return nil
+	// Flush and write the gzip header. It doesn't close the underlying writer.
+	if err := w.Close(); err != nil {
+		return errors.Wrapf(err, "error closing gzip writer for %s", path)
+	}
+	// Return f.Close() too so that we don't miss a potential error if everything
+	// else succeeded.
+	return f.Close()
 }

pkg/server/goroutinedumper/goroutinedumper_test.go

@@ -316,11 +316,11 @@ func TestTakeGoroutineDump(t *testing.T) {
 	t.Run("fails because dump already exists as a directory", func(t *testing.T) {
 		tempDir, dirCleanupFn := testutils.TempDir(t)
 		defer dirCleanupFn()
-		filename := "goroutine_dump"
-		path := filepath.Join(tempDir, filename)
+		path := filepath.Join(tempDir, "goroutine_dump.txt.gz")
 		err := os.Mkdir(path, 0755)
 		assert.NoError(t, err, "failed to make dump directory %s", path)
 
+		filename := "goroutine_dump"
 		err = takeGoroutineDump(tempDir, filename)
 		assert.Error(t, err)
 		assert.Contains(

pkg/server/status_test.go

@@ -310,7 +310,7 @@ func TestStatusGetFiles(t *testing.T) {
 	t.Run("goroutines", func(t *testing.T) {
 		const testFilesNo = 3
 		for i := 0; i < testFilesNo; i++ {
-			testFile := filepath.Join(storeSpec.Path, "logs", goroutinesDir, fmt.Sprintf("goroutine_dump%d.txt", i))
+			testFile := filepath.Join(storeSpec.Path, "logs", goroutinesDir, fmt.Sprintf("goroutine_dump%d.txt.gz", i))
 			if err := ioutil.WriteFile(testFile, []byte(fmt.Sprintf("Goroutine dump %d", i)), 0644); err != nil {
 				t.Fatal(err)
 			}
@@ -328,7 +328,7 @@ func TestStatusGetFiles(t *testing.T) {
 		}
 
 		for i, file := range response.Files {
-			expectedFileName := fmt.Sprintf("goroutine_dump%d.txt", i)
+			expectedFileName := fmt.Sprintf("goroutine_dump%d.txt.gz", i)
 			if file.Name != expectedFileName {
 				t.Fatalf("expected file name %s, found %s", expectedFileName, file.Name)
 			}