New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Collect Endpoint Security logs on elastic-agent diagnostics collect command #242
Conversation
run end-to-end tests |
1 similar comment
run end-to-end tests |
run end-to-end tests |
79f0218
to
6c1a9f0
Compare
@AndersonQ Looking at the fleet-ci/pr-merge this look like an issue with the golint module. Can you take a look? |
/test |
if err != nil { | ||
return closeHandlers(fmt.Errorf("unable to create log file in archive: %w", err), lf) | ||
|
||
name := strings.TrimPrefix(path, logPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are endpoint logs in a flat dir? or is there any nesting in subdirs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as far as I know, on a flat dir. Why? if there are nested dir it wouldn't work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the names will get mangled on Windows if it's nested. Using filepath.ToSlash(strings.TrimPrefix(path, logPath))
will ensure that windows separators are properly converted
run end-to-end tests |
…-agent into 105-collect-endpoit-logs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm; just fix the linting issues
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
The failing test is unrelated to the PR and it's failing for others as well. An issue was created to investigate it |
Fixes #105