feat: emit metrics for exec, verify and render (#9078)

* feat: configure verify and exec commands to emit metrics (#9013)

* feat: configure render, verify, and exec commands to emit metrics

* fix: remove render command from tracking due to corrupted output data

* feat: enable skaffold render to track telemetry (#9020)

* feat: making survey, update, and tracking messages to print to stderr instead of stdout

* feat: enable `skaffold render` to track telemetry

* feat: remove tests checking if a stadout is set, now we are using stderr

* feat: make stderr used to print survey, update, and metricts promp, coloreable to keep same behaviour as stdin

* feat: removing IsStdout function and related tests

* fix: change baseRef to point to v2.6 release

cmd/skaffold/app/cmd/cmd.go

@@ -87,7 +87,8 @@ func NewSkaffoldCommand(out, errOut io.Writer) *cobra.Command {
 			if cmd.Name() != cobra.ShellCompRequestCmd && cmd.Name() != cobra.ShellCompNoDescRequestCmd {
 				instrumentation.SetCommand(cmd.Name())
 				out := output.GetWriter(context.Background(), out, defaultColor, forceColors, timestamps)
-				cmd.Root().SetOutput(out)
+				cmd.Root().SetOut(out)
+				cmd.Root().SetErr(errOut)
 
 				// Setup logs
 				if err := setUpLogs(errOut, v, timestamps); err != nil {
@@ -131,21 +132,21 @@ func NewSkaffoldCommand(out, errOut io.Writer) *cobra.Command {
 				if err := config.UpdateMsgDisplayed(opts.GlobalConfig); err != nil {
 					log.Entry(context.TODO()).Debugf("could not update the 'last-prompted' config for 'update-config' section due to %s", err)
 				}
-				fmt.Fprintf(cmd.OutOrStderr(), "%s\n", msg)
+				fmt.Fprintf(cmd.ErrOrStderr(), "%s\n", msg)
 			default:
 			}
 			// check if survey prompt needs to be displayed
 			select {
 			case promptSurveyID := <-surveyPrompt:
 				if promptSurveyID != "" {
-					if err := s.DisplaySurveyPrompt(cmd.OutOrStdout(), promptSurveyID); err != nil {
+					if err := s.DisplaySurveyPrompt(output.NewColorWriter(cmd.ErrOrStderr()), promptSurveyID); err != nil {
 						fmt.Fprintf(cmd.OutOrStderr(), "%v\n", err)
 					}
 				}
 			default:
 			}
 			if metricsPrompt {
-				if err := prompt.DisplayMetricsPrompt(opts.GlobalConfig, cmd.OutOrStdout()); err != nil {
+				if err := prompt.DisplayMetricsPrompt(opts.GlobalConfig, output.NewColorWriter(cmd.ErrOrStderr())); err != nil {
 					fmt.Fprintf(cmd.OutOrStderr(), "%v\n", err)
 				}
 			}

cmd/skaffold/app/cmd/exec.go

@@ -39,6 +39,7 @@ func NewCmdExec() *cobra.Command {
 		WithExample("Execute a defined action that uses an image built from Skaffold. First, build the images", "build --file-output=build.json").
 		WithExample("Then use the built artifacts", "exec <action-name> --build-artifacts=build.json").
 		WithCommonFlags().
+		WithHouseKeepingMessages().
 		WithArgs(func(cmd *cobra.Command, args []string) error {
 			if len(args) != 1 {
 				log.Entry(context.TODO()).Errorf("`exec` requires exactly one action to execute")

cmd/skaffold/app/cmd/render.go

@@ -46,6 +46,7 @@ func NewCmdRender() *cobra.Command {
 			// This "--output" flag replaces the --render-output flag, which is deprecated.
 			{Value: &opts.RenderOutput, Name: "output", Shorthand: "o", DefValue: "", Usage: "File to write rendered manifests to"},
 		}).
+		WithHouseKeepingMessages().
 		NoArgs(doRender)
 }
 

hack/versions/pkg/schema/check.go

@@ -28,7 +28,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/v2/hack/versions/pkg/diff"
 )
 
-const baseRef = "origin/main"
+const baseRef = "origin/release/v2.6"
 
 func RunSchemaCheckOnChangedFiles() error {
 	git, err := newGit(baseRef)

integration/housekeeping_messages_test.go

@@ -33,7 +33,8 @@ func TestHouseKeepingMessagesNotShownForDiagnose(t *testing.T) {
 func TestHouseKeepingMessagesShownForDev(t *testing.T) {
 	MarkIntegrationTest(t, CanRunWithoutGcp)
 	file := testutil.TempFile(t, "config", nil)
-	out := skaffold.Run("-c", file, "--update-check=true").InDir("examples/getting-started").RunOrFailOutput(t)
+	out, err := skaffold.Run("-c", file, "--update-check=true").InDir("examples/getting-started").RunWithCombinedOutput(t)
+	testutil.CheckError(t, false, err)
 	testutil.CheckContains(t, "Help improve Skaffold", string(out))
 	skaffold.Delete().InDir("examples/getting-started").Run(t)
 }

pkg/skaffold/instrumentation/meter.go

@@ -66,7 +66,7 @@ var (
 )
 
 func init() {
-	MeteredCommands.Insert("apply", "build", "delete", "deploy", "dev", "debug", "filter", "generate_pipeline", "render", "run", "test")
+	MeteredCommands.Insert("apply", "build", "delete", "deploy", "dev", "debug", "filter", "generate_pipeline", "render", "run", "test", "verify", "exec")
 	doesBuild.Insert("build", "render", "dev", "debug", "run")
 	doesDeploy.Insert("apply", "deploy", "dev", "debug", "run")
 }

pkg/skaffold/instrumentation/prompt/prompt.go

@@ -32,7 +32,6 @@ You may choose to opt out of this collection by running the following command:
 
 var (
 	// for testing
-	isStdOut     = output.IsStdout
 	updateConfig = config.UpdateGlobalCollectMetrics
 	getConfig    = config.GetConfigForCurrentKubectx
 	setStatus    = instrumentation.SetOnlineStatus
@@ -53,9 +52,6 @@ func ShouldDisplayMetricsPrompt(configfile string) bool {
 }
 
 func DisplayMetricsPrompt(configFile string, out io.Writer) error {
-	if isStdOut(out) {
-		output.Green.Fprintf(out, Prompt)
-		return updateConfig(configFile, true)
-	}
-	return nil
+	output.Green.Fprintf(out, Prompt)
+	return updateConfig(configFile, true)
 }

pkg/skaffold/instrumentation/prompt/prompt_test.go

@@ -19,7 +19,6 @@ package prompt
 import (
 	"bytes"
 	"fmt"
-	"io"
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/config"
@@ -74,23 +73,16 @@ func TestShouldDisplayMetricsPrompt(t *testing.T) {
 
 func TestDisplayMetricsPrompt(t *testing.T) {
 	tests := []struct {
-		name       string
-		mockStdOut bool
-		expected   string
+		name     string
+		expected string
 	}{
 		{
-			name:       "std out",
-			mockStdOut: true,
-			expected:   Prompt,
-		},
-		{
-			name: "not std out",
+			name:     "std out",
+			expected: Prompt,
 		},
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.name, func(t *testutil.T) {
-			mock := func(io.Writer) bool { return test.mockStdOut }
-			t.Override(&isStdOut, mock)
 			t.Override(&updateConfig, func(_ string, _ bool) error { return nil })
 			var buf bytes.Buffer
 			err := DisplayMetricsPrompt("", &buf)

pkg/skaffold/output/output.go

@@ -19,7 +19,6 @@ package output
 import (
 	"context"
 	"io"
-	"os"
 	"time"
 
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/constants"
@@ -76,18 +75,6 @@ func GetWriter(ctx context.Context, out io.Writer, defaultColor int, forceColors
 	}
 }
 
-func IsStdout(out io.Writer) bool {
-	sw, isSW := out.(skaffoldWriter)
-	if isSW {
-		out = sw.MainWriter
-	}
-	cw, isCW := out.(colorableWriter)
-	if isCW {
-		out = cw.Writer
-	}
-	return out == os.Stdout
-}
-
 // GetUnderlyingWriter returns the underlying writer if out is a colorableWriter
 func GetUnderlyingWriter(out io.Writer) io.Writer {
 	sw, isSW := out.(skaffoldWriter)

pkg/skaffold/output/output_test.go

@@ -30,52 +30,6 @@ import (
 	"github.com/GoogleContainerTools/skaffold/v2/testutil"
 )
 
-func TestIsStdOut(t *testing.T) {
-	tests := []struct {
-		description string
-		out         io.Writer
-		expected    bool
-	}{
-		{
-			description: "std out passed",
-			out:         os.Stdout,
-			expected:    true,
-		},
-		{
-			description: "out nil",
-			out:         nil,
-		},
-		{
-			description: "out bytes buffer",
-			out:         new(bytes.Buffer),
-		},
-		{
-			description: "colorable std out passed",
-			out: skaffoldWriter{
-				MainWriter: NewColorWriter(os.Stdout),
-			},
-			expected: true,
-		},
-		{
-			description: "colorableWriter passed",
-			out:         NewColorWriter(os.Stdout),
-			expected:    true,
-		},
-		{
-			description: "invalid colorableWriter passed",
-			out: skaffoldWriter{
-				MainWriter: NewColorWriter(io.Discard),
-			},
-			expected: false,
-		},
-	}
-	for _, test := range tests {
-		testutil.Run(t, test.description, func(t *testutil.T) {
-			t.CheckDeepEqual(test.expected, IsStdout(test.out))
-		})
-	}
-}
-
 func TestGetUnderlyingWriter(t *testing.T) {
 	tests := []struct {
 		description string

pkg/skaffold/survey/survey.go

@@ -44,7 +44,6 @@ Tip: To permanently disable the survey prompt, run:
 
 var (
 	// for testing
-	isStdOut             = output.IsStdout
 	open                 = browser.OpenURL
 	updateSurveyPrompted = sConfig.UpdateGlobalSurveyPrompted
 	parseConfig          = schema.ParseConfigAndUpgrade
@@ -109,9 +108,6 @@ func recentlyPrompted(gc *sConfig.SurveyConfig) bool {
 }
 
 func (s *Runner) DisplaySurveyPrompt(out io.Writer, id string) error {
-	if !isStdOut(out) {
-		return nil
-	}
 	if sc, ok := getSurvey(id); ok {
 		output.Green.Fprintf(out, sc.prompt())
 		return updateSurveyPrompted(s.configFile)

pkg/skaffold/survey/survey_test.go

@@ -19,7 +19,6 @@ package survey
 import (
 	"bytes"
 	"fmt"
-	"io"
 	"testing"
 	"time"
 
@@ -34,26 +33,16 @@ func TestDisplaySurveyForm(t *testing.T) {
 		description        string
 		mockSurveyPrompted func(_ string) error
 		expected           string
-		mockStdOut         bool
 	}{
 		{
 			description:        "std out",
-			mockStdOut:         true,
 			mockSurveyPrompted: func(_ string) error { return nil },
 			expected: `Help improve Skaffold with our 2-minute anonymous survey: run 'skaffold survey'
 `,
 		},
-		{
-			description: "not std out",
-			mockSurveyPrompted: func(_ string) error {
-				return fmt.Errorf("not expected to call")
-			},
-		},
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
-			mock := func(io.Writer) bool { return test.mockStdOut }
-			t.Override(&isStdOut, mock)
 			mockOpen := func(string) error { return nil }
 			t.Override(&open, mockOpen)
 			t.Override(&updateSurveyPrompted, test.mockSurveyPrompted)