-
Notifications
You must be signed in to change notification settings - Fork 148
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
Log watcher properly #2518
Log watcher properly #2518
Conversation
This pull request does not have a backport label. Could you fix it @michalpristas? 🙏
NOTE: |
🌐 Coverage report
|
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.
Didn't test it yet but the change looks good
I don't see any automated testing for this, would it be possible to write some?
} | ||
|
||
// using Data() + "/logs", for some reason default paths/Logs() is the home dir... | ||
logPath := filepath.Join(paths.Home(), "logs") + string(filepath.Separator) | ||
logPath := filepath.Join(pathsHome, "logs") + string(filepath.Separator) |
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.
Not introduced in this change but I am just curious: why do we need an extra separator ad the end of 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.
dunno, probably something with a zip writer
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.
Change looks good. Hopefully we can cover this code which is hard to unit test with an upgrade integration test.
You know what to add to your upgrade integration test then :-) |
The zip file did not contain a directory entry for the elastic-agent-{commit} directory. Relates elastic#2518
This calls filepath.ToSlash on any names used in zip entries. Relates elastic#2518
…2523) * internal/pkg/diagnostics - Fix creation of sub-directories in logs/ Prior to this change sub-directories within logs/ were not created properly and resulted in unexpected directories within diagnostics files. A path separator was missing. Before the change the new test was failing with - Name: (string) (len=13) "logs/sub-dir/", + Name: (string) (len=12) "logssub-dir/", * Update test to include the versioned sub-directory I made the test pass, but this these exposes a problem with the newly added elastic-agent-{commit} directory in that the zip file is missing a directory entry for it. * Add missing directory entry for logs/elastic-agent-{commit} The zip file did not contain a directory entry for the elastic-agent-{commit} directory. Relates #2518 * All files within the zip must use forward slash This calls filepath.ToSlash on any names used in zip entries. Relates #2518
…2523) * internal/pkg/diagnostics - Fix creation of sub-directories in logs/ Prior to this change sub-directories within logs/ were not created properly and resulted in unexpected directories within diagnostics files. A path separator was missing. Before the change the new test was failing with - Name: (string) (len=13) "logs/sub-dir/", + Name: (string) (len=12) "logssub-dir/", * Update test to include the versioned sub-directory I made the test pass, but this these exposes a problem with the newly added elastic-agent-{commit} directory in that the zip file is missing a directory entry for it. * Add missing directory entry for logs/elastic-agent-{commit} The zip file did not contain a directory entry for the elastic-agent-{commit} directory. Relates #2518 * All files within the zip must use forward slash This calls filepath.ToSlash on any names used in zip entries. Relates #2518 (cherry picked from commit 91a3122)
@michalpristas Is it possible to change the monitoring configuration to ingest these watcher log files? |
What does this PR do?
This PR makes watcher log into
logs
directory.Logs are name in a similar manner as normal logs but with
watcher
suffix (see picture)When rollback is performed everything except logs directory is removed. this can then be collected later.
On successful upgrade full cleanup is performed and everything except latest agent is removed.
This PR also modifies diagnostics collection.
Now logs are stored into
logs/elastic-agent-{hash}
So in case of rollback watcher logs will be present in
oldhash
dir and agent logs will be store innewhash
directoryWhy is it important?
Debug-ability of agent during rollback
Checklist
./changelog/fragments
using the changelog toolFixes: #2262