From be0dd22594088d80a743b44a37b3e21f452a5e92 Mon Sep 17 00:00:00 2001 From: fearful-symmetry Date: Tue, 28 Nov 2023 13:35:04 -0800 Subject: [PATCH 1/4] add custom MarshalYAML to component --- pkg/component/component.go | 11 ++++++++++- pkg/component/component_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/pkg/component/component.go b/pkg/component/component.go index 18060b645fd..b4354ed64f5 100644 --- a/pkg/component/component.go +++ b/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,11 @@ type Component struct { ShipperRef *ShipperReference `yaml:"shipper,omitempty"` } +func (c Component) MarshalYAML() (interface{}, error) { + c.ErrMsg = c.Err.Error() + return c, nil +} + // Type returns the type of the component. func (c *Component) Type() string { if c.InputSpec != nil { diff --git a/pkg/component/component_test.go b/pkg/component/component_test.go index 00c4d1c63cb..8d42ab64427 100644 --- a/pkg/component/component_test.go +++ b/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{ From 79a4a5438e75744ea26096168fb49323b67260ca Mon Sep 17 00:00:00 2001 From: fearful-symmetry Date: Tue, 28 Nov 2023 13:54:35 -0800 Subject: [PATCH 2/4] add changelog --- ...315-custom-yaml-marshal-for-component.yaml | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 changelog/fragments/1701208315-custom-yaml-marshal-for-component.yaml diff --git a/changelog/fragments/1701208315-custom-yaml-marshal-for-component.yaml b/changelog/fragments/1701208315-custom-yaml-marshal-for-component.yaml new file mode 100644 index 00000000000..d6c6834ced5 --- /dev/null +++ b/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 From b1e1297276c87c6fd9af7d1c55f2712bfcec337d Mon Sep 17 00:00:00 2001 From: fearful-symmetry Date: Tue, 28 Nov 2023 14:32:09 -0800 Subject: [PATCH 3/4] add nil check --- pkg/component/component.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/component/component.go b/pkg/component/component.go index b4354ed64f5..d9100a6dd98 100644 --- a/pkg/component/component.go +++ b/pkg/component/component.go @@ -192,7 +192,9 @@ type Component struct { } func (c Component) MarshalYAML() (interface{}, error) { - c.ErrMsg = c.Err.Error() + if c.Err != nil { + c.ErrMsg = c.Err.Error() + } return c, nil } From ae2415a0eb6d7e2bcc530bfdafcee751a0bf0486 Mon Sep 17 00:00:00 2001 From: fearful-symmetry Date: Tue, 28 Nov 2023 14:54:23 -0800 Subject: [PATCH 4/4] update tests --- .../pkg/agent/application/coordinator/diagnostics_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/pkg/agent/application/coordinator/diagnostics_test.go b/internal/pkg/agent/application/coordinator/diagnostics_test.go index 1e12dd38141..58cadb9c22a 100644 --- a/internal/pkg/agent/application/coordinator/diagnostics_test.go +++ b/internal/pkg/agent/application/coordinator/diagnostics_test.go @@ -383,12 +383,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: