Skip to content

Commit

Permalink
Fix components in diag collection from Kibana (#3295)
Browse files Browse the repository at this point in the history
* Fix diag collection from Kibana for components: we didn't collect components in the Handler, only in the cmd command

---------

Co-authored-by: Paolo Chila <[email protected]>
(cherry picked from commit d3d510d)

# Conflicts:
#	internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go
  • Loading branch information
pierrehilbert authored and mergify[bot] committed Aug 28, 2023
1 parent 8baa62e commit 9ec67a8
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 2 deletions.
32 changes: 32 additions & 0 deletions changelog/fragments/1691647290-kibana-diag-collection.yaml
Original file line number Diff line number Diff line change
@@ -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: Add components in the diagnostic collection when initialized from Kibana.

# 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:

# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
component: elastic-agent

# 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/3295

# 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/3294
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/diagnostics"
"github.com/elastic/elastic-agent/internal/pkg/fleetapi"
"github.com/elastic/elastic-agent/internal/pkg/fleetapi/acker"
"github.com/elastic/elastic-agent/pkg/component"

"golang.org/x/time/rate"
)
Expand All @@ -39,6 +40,7 @@ type Uploader interface {
type diagnosticsProvider interface {
DiagnosticHooks() diagnostics.Hooks
PerformDiagnostics(ctx context.Context, req ...runtime.ComponentUnitDiagnosticRequest) []runtime.ComponentUnitDiagnostic
PerformComponentDiagnostics(ctx context.Context, additionalMetrics []cproto.AdditionalDiagnosticRequest, req ...component.Component) ([]runtime.ComponentDiagnostic, error)
}

// abstractLogger represents a logger implementation
Expand Down Expand Up @@ -131,11 +133,14 @@ func (h *Diagnostics) collectDiag(ctx context.Context, action *fleetapi.ActionDi
h.log.Debug("Gathering unit diagnostics.")
uDiag := h.diagUnits(ctx)

h.log.Debug("Gathering component diagnostics.")
cDiag := h.diagComponents(ctx)

var r io.Reader
// attempt to create the a temporary diagnostics file on disk in order to avoid loading a
// potentially large file in memory.
// if on-disk creation fails an in-memory buffer is used.
f, s, err := h.diagFile(aDiag, uDiag)
f, s, err := h.diagFile(aDiag, uDiag, cDiag)
if err != nil {
var b bytes.Buffer
h.log.Warnw("Diagnostics action unable to use temporary file, using buffer instead.", "error.message", err)
Expand All @@ -145,7 +150,11 @@ func (h *Diagnostics) collectDiag(ctx context.Context, action *fleetapi.ActionDi
h.log.Warn(str)
}
}()
<<<<<<< HEAD

Check failure on line 153 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / macos

syntax error: unexpected <<, expecting }

Check failure on line 153 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / CodeCoverage

syntax error: unexpected <<, expecting }

Check failure on line 153 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

expected statement, found '<<' (typecheck)
err := diagnostics.ZipArchive(&wBuf, &b, aDiag, uDiag)
=======

Check failure on line 155 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / macos

syntax error: unexpected ==, expecting }

Check failure on line 155 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / CodeCoverage

syntax error: unexpected ==, expecting }
err := diagnostics.ZipArchive(&wBuf, &b, aDiag, uDiag, cDiag)
>>>>>>> d3d510dbb1 (Fix components in diag collection from Kibana (#3295))

Check failure on line 157 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / macos

invalid character U+0023 '#'

Check failure on line 157 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / CodeCoverage

invalid character U+0023 '#'

Check failure on line 157 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

illegal character U+0023 '#' (typecheck)
if err != nil {
h.log.Errorw(
"diagnostics action handler failed generate zip archive",
Expand Down Expand Up @@ -240,8 +249,46 @@ func (h *Diagnostics) diagUnits(ctx context.Context) []client.DiagnosticUnitResu
return uDiag
}

// diagUnits gathers diagnostics from components.
func (h *Diagnostics) diagComponents(ctx context.Context) []client.DiagnosticComponentResult {
cDiag := make([]client.DiagnosticComponentResult, 0)
h.log.Debug("Performing component diagnostics")
startTime := time.Now()
defer func() {
h.log.Debugf("Component diagnostics complete. Took: %s", time.Since(startTime))
}()
rr, err := h.diagProvider.PerformComponentDiagnostics(ctx, []cproto.AdditionalDiagnosticRequest{})
if err != nil {
h.log.Errorf("Error fetching component-level diagnostics: %w", err)
}
h.log.Debug("Collecting results of component diagnostics")
for _, r := range rr {
diag := client.DiagnosticComponentResult{
ComponentID: r.Component.ID,
}
if r.Err != nil {
diag.Err = r.Err
} else {
results := make([]client.DiagnosticFileResult, 0, len(r.Results))
for _, res := range r.Results {
results = append(results, client.DiagnosticFileResult{
Name: res.Name,
Filename: res.Filename,
Description: res.Description,
ContentType: res.ContentType,
Content: res.Content,
Generated: res.Generated.AsTime(),
})
}
diag.Results = results
}
cDiag = append(cDiag, diag)
}
return cDiag
}

// diagFile will write the diagnostics to a temporary file and return the file ready to be read
func (h *Diagnostics) diagFile(aDiag []client.DiagnosticFileResult, uDiag []client.DiagnosticUnitResult) (*os.File, int64, error) {
func (h *Diagnostics) diagFile(aDiag []client.DiagnosticFileResult, uDiag []client.DiagnosticUnitResult, cDiag []client.DiagnosticComponentResult) (*os.File, int64, error) {
f, err := os.CreateTemp("", "elastic-agent-diagnostics")
if err != nil {
return nil, 0, err
Expand All @@ -254,7 +301,11 @@ func (h *Diagnostics) diagFile(aDiag []client.DiagnosticFileResult, uDiag []clie
h.log.Warn(str)
}
}()
<<<<<<< HEAD

Check failure on line 304 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / macos

syntax error: unexpected <<, expecting }

Check failure on line 304 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / CodeCoverage

syntax error: unexpected <<, expecting }

Check failure on line 304 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

expected statement, found '<<' (typecheck)
if err := diagnostics.ZipArchive(&wBuf, f, aDiag, uDiag); err != nil {
=======

Check failure on line 306 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

expected statement, found '==' (typecheck)
if err := diagnostics.ZipArchive(&wBuf, f, aDiag, uDiag, cDiag); err != nil {
>>>>>>> d3d510dbb1 (Fix components in diag collection from Kibana (#3295))

Check failure on line 308 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / macos

invalid character U+0023 '#'

Check failure on line 308 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / CodeCoverage

invalid character U+0023 '#'

Check failure on line 308 in internal/pkg/agent/application/actions/handlers/handler_action_diagnostics.go

View workflow job for this annotation

GitHub Actions / lint (ubuntu-latest)

expected statement, found '>>' (typecheck)
os.Remove(name)
return nil, 0, err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

"github.com/elastic/elastic-agent-client/v7/pkg/client"
"github.com/elastic/elastic-agent-client/v7/pkg/proto"

"github.com/elastic/elastic-agent/internal/pkg/agent/application/actions/handlers/mocks"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
Expand Down Expand Up @@ -89,6 +90,7 @@ func TestDiagnosticHandlerHappyPathWithLogs(t *testing.T) {

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{hook1})
mockDiagProvider.EXPECT().PerformDiagnostics(mock.Anything, mock.Anything).Return([]runtime.ComponentUnitDiagnostic{mockUnitDiagnostic})
mockDiagProvider.EXPECT().PerformComponentDiagnostics(mock.Anything, mock.Anything).Return([]runtime.ComponentDiagnostic{}, nil)

mockAcker := mocks.NewAcker(t)
mockAcker.EXPECT().Ack(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, a fleetapi.Action) error {
Expand Down Expand Up @@ -169,6 +171,7 @@ func TestDiagnosticHandlerUploaderErrorWithLogs(t *testing.T) {

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{})
mockDiagProvider.EXPECT().PerformDiagnostics(mock.Anything, mock.Anything).Return([]runtime.ComponentUnitDiagnostic{})
mockDiagProvider.EXPECT().PerformComponentDiagnostics(mock.Anything, mock.Anything).Return([]runtime.ComponentDiagnostic{}, nil)

// this error will be returbned by the uploader
uploaderError := errors.New("upload went wrong!")
Expand Down Expand Up @@ -209,6 +212,7 @@ func TestDiagnosticHandlerZipArchiveErrorWithLogs(t *testing.T) {

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{})
mockDiagProvider.EXPECT().PerformDiagnostics(mock.Anything, mock.Anything).Return([]runtime.ComponentUnitDiagnostic{})
mockDiagProvider.EXPECT().PerformComponentDiagnostics(mock.Anything, mock.Anything).Return([]runtime.ComponentDiagnostic{}, nil)

mockAcker := mocks.NewAcker(t)
mockAcker.EXPECT().Ack(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, a fleetapi.Action) error {
Expand Down Expand Up @@ -244,6 +248,7 @@ func TestDiagnosticHandlerAckErrorWithLogs(t *testing.T) {

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{})
mockDiagProvider.EXPECT().PerformDiagnostics(mock.Anything, mock.Anything).Return([]runtime.ComponentUnitDiagnostic{})
mockDiagProvider.EXPECT().PerformComponentDiagnostics(mock.Anything, mock.Anything).Return([]runtime.ComponentDiagnostic{}, nil)

mockAcker := mocks.NewAcker(t)
ackError := errors.New("acking went wrong")
Expand Down Expand Up @@ -282,6 +287,7 @@ func TestDiagnosticHandlerCommitErrorWithLogs(t *testing.T) {

mockDiagProvider.EXPECT().DiagnosticHooks().Return([]diagnostics.Hook{})
mockDiagProvider.EXPECT().PerformDiagnostics(mock.Anything, mock.Anything).Return([]runtime.ComponentUnitDiagnostic{})
mockDiagProvider.EXPECT().PerformComponentDiagnostics(mock.Anything, mock.Anything).Return([]runtime.ComponentDiagnostic{}, nil)

mockAcker := mocks.NewAcker(t)
mockAcker.EXPECT().Ack(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, a fleetapi.Action) error {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 9ec67a8

Please sign in to comment.