Add custom MarshalYAML() method to component to fix YAML marshaling o…

…f error values (#3835) (#3856)

* add custom MarshalYAML to component

* add changelog

* add nil check

* update tests

---------

Co-authored-by: Pierre HILBERT <[email protected]>
(cherry picked from commit 93f159a)

Co-authored-by: Alex K <[email protected]>
Co-authored-by: Pierre HILBERT <[email protected]>

changelog/fragments/1701208315-custom-yaml-marshal-for-component.yaml

@@ -0,0 +1,32 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: bug-fix
+
+# Change summary; a 80ish characters long description of the change.
+summary: custom-yaml-marshal-for-component
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+description: Create a custom MarshalYAML() method to properly handle error fields
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: component
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+pr: https://github.com/elastic/elastic-agent/pull/3835
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+issue: https://github.com/elastic/elastic-agent/issues/2940

internal/pkg/agent/application/coordinator/diagnostics_test.go

@@ -381,12 +381,10 @@ func TestDiagnosticComponentsActual(t *testing.T) {
 		},
 	}
 
-	// The error values here shouldn't really be empty, this is a known bug, see
-	// https://github.com/elastic/elastic-agent/issues/2940
 	expected := `
 components:
   - id: component-1
-    error: {}
+    error: "component error"
     input_type: "test-input"
     output_type: "test-output"
     units:

pkg/component/component.go

@@ -156,7 +156,11 @@ type Component struct {
 
 	// Err used when there is an error with running this input. Used by the runtime to alert
 	// the reason that all of these units are failed.
-	Err error `yaml:"error,omitempty"`
+	Err error `yaml:"-"`
+	// the YAML marshaller won't handle `error` values, since they don't implement MarshalYAML()
+	// the Component's own MarshalYAML method needs to handle this, and place any error values here instead of `Err`,
+	// so they can properly be rendered as a string
+	ErrMsg string `yaml:"error,omitempty"`
 
 	// InputSpec on how the input should run. (not set when ShipperSpec set)
 	InputSpec *InputRuntimeSpec `yaml:"input_spec,omitempty"`
@@ -187,6 +191,13 @@ type Component struct {
 	ShipperRef *ShipperReference `yaml:"shipper,omitempty"`
 }
 
+func (c Component) MarshalYAML() (interface{}, error) {
+	if c.Err != nil {
+		c.ErrMsg = c.Err.Error()
+	}
+	return c, nil
+}
+
 // Type returns the type of the component.
 func (c *Component) Type() string {
 	if c.InputSpec != nil {

pkg/component/component_test.go

@@ -36,6 +36,31 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// fake error type used for the test below
+type testErr struct {
+	data string
+}
+
+func (t testErr) Error() string {
+	return t.data
+}
+
+func TestComponentMarshalError(t *testing.T) {
+	testComponent := Component{
+		ID:  "test-device",
+		Err: testErr{data: "test error value"},
+	}
+	componentConfigs := []Component{testComponent}
+
+	outData, err := yaml.Marshal(struct {
+		Components []Component `yaml:"components"`
+	}{
+		Components: componentConfigs,
+	})
+	require.NoError(t, err)
+	require.Contains(t, string(outData), "test error value")
+}
+
 func TestToComponents(t *testing.T) {
 	linuxAMD64Platform := PlatformDetail{
 		Platform: Platform{