Add Log helper function to output package (#6356)

* add Log helper function

* remove comment

* add comment

pkg/skaffold/build/cache/retrieve.go

@@ -62,7 +62,7 @@ func (c *cache) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, ar
 	var alreadyBuilt []graph.Artifact
 	for i, artifact := range artifacts {
 		eventV2.CacheCheckInProgress(artifact.ImageName)
-		out, _ := output.WithEventContext(out, constants.Build, artifact.ImageName)
+		out := output.WithEventContext(out, constants.Build, artifact.ImageName)
 		output.Default.Fprintf(out, " - %s: ", artifact.ImageName)
 
 		result := results[i]

pkg/skaffold/build/scheduler.go

@@ -108,7 +108,7 @@ func (s *scheduler) build(ctx context.Context, tags tag.ImageTags, i int) error
 	}
 	defer closeFn()
 
-	w, _ = output.WithEventContext(w, constants.Build, a.ImageName)
+	w = output.WithEventContext(w, constants.Build, a.ImageName)
 	finalTag, err := performBuild(ctx, w, tags, a, s.artifactBuilder)
 	if err != nil {
 		event.BuildFailed(a.ImageName, err)

pkg/skaffold/deploy/deploy_mux.go

@@ -102,7 +102,7 @@ func (m DeployerMux) RegisterLocalImages(images []graph.Artifact) {
 func (m DeployerMux) Deploy(ctx context.Context, w io.Writer, as []graph.Artifact) error {
 	for i, deployer := range m.deployers {
 		eventV2.DeployInProgress(i)
-		w, _ = output.WithEventContext(w, constants.Deploy, strconv.Itoa(i))
+		w = output.WithEventContext(w, constants.Deploy, strconv.Itoa(i))
 		ctx, endTrace := instrumentation.StartTrace(ctx, "Deploy")
 
 		if err := deployer.Deploy(ctx, w, as); err != nil {

pkg/skaffold/kubernetes/portforward/entry_manager.go

@@ -115,7 +115,7 @@ func NewEntryManager(entryForwarder EntryForwarder) *EntryManager {
 }
 
 func (b *EntryManager) forwardPortForwardEntry(ctx context.Context, out io.Writer, entry *portForwardEntry) {
-	out, _ = output.WithEventContext(out, constants.PortForward, fmt.Sprintf("%s/%s", entry.resource.Type, entry.resource.Name))
+	out = output.WithEventContext(out, constants.PortForward, fmt.Sprintf("%s/%s", entry.resource.Type, entry.resource.Name))
 
 	// Check if this resource has already been forwarded
 	if _, ok := b.forwardedResources.Load(entry.key()); ok {

pkg/skaffold/kubernetes/status/status_check.go

@@ -287,7 +287,7 @@ func (s *Monitor) printStatusCheckSummary(out io.Writer, r *resource.Deployment,
 	}
 	event.ResourceStatusCheckEventCompleted(r.String(), ae)
 	eventV2.ResourceStatusCheckEventCompleted(r.String(), sErrors.V2fromV1(ae))
-	out, _ = output.WithEventContext(out, constants.Deploy, r.String())
+	out = output.WithEventContext(out, constants.Deploy, r.String())
 	status := fmt.Sprintf("%s %s", tabHeader, r)
 	if ae.ErrCode != proto.StatusCode_STATUSCHECK_SUCCESS {
 		if str := r.ReportSinceLastUpdated(s.muteLogs); str != "" {
@@ -333,7 +333,7 @@ func (s *Monitor) printStatus(deployments []*resource.Deployment, out io.Writer)
 			ae := r.Status().ActionableError()
 			event.ResourceStatusCheckEventUpdated(r.String(), ae)
 			eventV2.ResourceStatusCheckEventUpdated(r.String(), sErrors.V2fromV1(ae))
-			out, _ := output.WithEventContext(out, constants.Deploy, r.String())
+			out := output.WithEventContext(out, constants.Deploy, r.String())
 			fmt.Fprintln(out, trimNewLine(str))
 		}
 	}

pkg/skaffold/output/output.go

@@ -32,6 +32,8 @@ const TimestampFormat = "2006-01-02 15:04:05"
 type skaffoldWriter struct {
 	MainWriter  io.Writer
 	EventWriter io.Writer
+	task        constants.Phase
+	subtask     string
 
 	timestamps bool
 }
@@ -101,14 +103,35 @@ func GetUnderlyingWriter(out io.Writer) io.Writer {
 
 // WithEventContext will return a new skaffoldWriter with the given parameters to be used for the event writer.
 // If the passed io.Writer is not a skaffoldWriter, then it is simply returned.
-func WithEventContext(out io.Writer, phase constants.Phase, subtaskID string) (io.Writer, *logrus.Logger) {
+func WithEventContext(out io.Writer, phase constants.Phase, subtaskID string) io.Writer {
 	if sw, isSW := out.(skaffoldWriter); isSW {
 		return skaffoldWriter{
 			MainWriter:  sw.MainWriter,
 			EventWriter: eventV2.NewLogger(phase, subtaskID),
+			task:        phase,
+			subtask:     subtaskID,
 			timestamps:  sw.timestamps,
-		}, nil
+		}
+	}
+
+	return out
+}
+
+// Log takes an io.Writer (ideally of type output.skaffoldWriter) and constructs
+// a logrus.Entry from it, adding fields for task and subtask information
+func Log(out io.Writer) *logrus.Entry {
+	sw, isSW := out.(skaffoldWriter)
+	if isSW {
+		return logrus.WithFields(logrus.Fields{
+			"task":    sw.task,
+			"subtask": sw.subtask,
+		})
 	}
 
-	return out, nil
+	// Use constants.DevLoop as the default task, as it's the highest level task we
+	// can default to if one isn't specified.
+	return logrus.WithFields(logrus.Fields{
+		"task":    constants.DevLoop,
+		"subtask": eventV2.SubtaskIDNone,
+	})
 }

pkg/skaffold/output/output_test.go

@@ -152,8 +152,8 @@ func TestWithEventContext(t *testing.T) {
 
 	for _, test := range tests {
 		testutil.Run(t, test.name, func(t *testutil.T) {
-			got, _ := WithEventContext(test.writer, test.phase, test.subtaskID)
-			t.CheckDeepEqual(test.expected, got, cmpopts.IgnoreTypes(false))
+			got := WithEventContext(test.writer, test.phase, test.subtaskID)
+			t.CheckDeepEqual(test.expected, got, cmpopts.IgnoreTypes(false, "", constants.DevLoop))
 		})
 	}
 }
@@ -217,3 +217,36 @@ func TestWriteWithTimeStamps(t *testing.T) {
 		})
 	}
 }
+
+func TestLog(t *testing.T) {
+	tests := []struct {
+		name            string
+		writer          io.Writer
+		expectedTask    constants.Phase
+		expectedSubtask string
+	}{
+		{
+			name: "arbitrary task and subtask from writer",
+			writer: skaffoldWriter{
+				task:    constants.Build,
+				subtask: "test",
+			},
+			expectedTask:    constants.Build,
+			expectedSubtask: "test",
+		},
+		{
+			name:            "non skaffoldWriter",
+			writer:          ioutil.Discard,
+			expectedTask:    constants.DevLoop,
+			expectedSubtask: eventV2.SubtaskIDNone,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := Log(test.writer)
+			testutil.CheckDeepEqual(t, test.expectedTask, got.Data["task"])
+			testutil.CheckDeepEqual(t, test.expectedSubtask, got.Data["subtask"])
+		})
+	}
+}

pkg/skaffold/runner/build.go

@@ -65,7 +65,7 @@ func (r *Builder) GetBuilds() []graph.Artifact {
 // Build builds a list of artifacts.
 func (r *Builder) Build(ctx context.Context, out io.Writer, artifacts []*latestV1.Artifact) ([]graph.Artifact, error) {
 	eventV2.TaskInProgress(constants.Build, "Build containers")
-	out, _ = output.WithEventContext(out, constants.Build, eventV2.SubtaskIDNone)
+	out = output.WithEventContext(out, constants.Build, eventV2.SubtaskIDNone)
 
 	// Use tags directly from the Kubernetes manifests.
 	if r.runCtx.DigestSource() == NoneDigestSource {
@@ -172,7 +172,7 @@ func (r *Builder) imageTags(ctx context.Context, out io.Writer, artifacts []*lat
 
 	for i, artifact := range artifacts {
 		imageName := artifact.ImageName
-		out, _ := output.WithEventContext(out, constants.Build, imageName)
+		out := output.WithEventContext(out, constants.Build, imageName)
 		output.Default.Fprintf(out, " - %s -> ", imageName)
 
 		select {

pkg/skaffold/runner/v1/deploy.go

@@ -69,7 +69,7 @@ func (r *SkaffoldRunner) Deploy(ctx context.Context, out io.Writer, artifacts []
 	}
 	defer r.deployer.GetStatusMonitor().Reset()
 
-	out, _ = output.WithEventContext(out, constants.Deploy, eventV2.SubtaskIDNone)
+	out = output.WithEventContext(out, constants.Deploy, eventV2.SubtaskIDNone)
 
 	output.Default.Fprintln(out, "Tags used in deployment:")