Skip to content
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

Add the logs subcommand to the CLI #2745

Merged
merged 6 commits into from
May 31, 2023
Merged

Add the logs subcommand to the CLI #2745

merged 6 commits into from
May 31, 2023

Conversation

rdner
Copy link
Member

@rdner rdner commented May 30, 2023

What does this PR do?

It adds the logs subcommand.

Following is supported:

  • Print the last -n lines of the logs (10 by default)
  • Follow the logs after printing
  • Filter logs by a component ID

Why is it important?

Improves development and troubleshooting experience.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
    - [ ] I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool
    - [ ] I have added an integration test or an E2E test

How to test this PR locally

  • Unit tests with a very good coverage (all the successful paths and edge cases are covered)
  • Install the agent built from this branch and then use elastic-agent logs -f

Related issues

Screenshots

Screen.Recording.2023-05-30.at.15.27.23.mov

Following is supported:

* Print the last `-n` lines of the logs (10 by default)
* Follow the logs after printing
* Filter logs by a component ID
@rdner rdner added enhancement New feature or request Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team Team:Elastic-Agent Label for the Agent team backport-skip labels May 30, 2023
@rdner rdner self-assigned this May 30, 2023
@elasticmachine
Copy link
Collaborator

elasticmachine commented May 30, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-05-30T13:08:36.941+0000

  • Duration: 21 min 4 sec

Test stats 🧪

Test Results
Failed 0
Passed 5803
Skipped 19
Total 5822

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages.

  • run integration tests : Run the Elastic Agent Integration tests.

  • run end-to-end tests : Generate the packages and run the E2E Tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Collaborator

elasticmachine commented May 30, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 98.611% (71/72) 👍
Files 68.4% (171/250) 👍 0.127
Classes 67.161% (317/472) 👍 0.14
Methods 53.902% (967/1794) 👍 0.224
Lines 39.781% (11200/28154) 👍 0.261
Conditionals 100.0% (0/0) 💚

filter = createComponentFilter(component)
}

logsDir := filepath.Join(paths.Home(), logger.DefaultLogDirectory)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First I tried to use:

// Logs returns the log directory for Agent
func Logs() string {
return logsPath
}

But (at least on Mac) it lead to /Library/Elastic/Agent which had some logs but they were not actively updated while the agent was running.

After some digging, I found this line and copied from here:

func MakeInternalFileOutput(cfg *Config) (zapcore.Core, error) {
// defaultCfg is used to set the defaults for the file rotation of the internal logging
// these settings cannot be changed by a user configuration
defaultCfg := logp.DefaultConfig(logp.DefaultEnvironment)
filename := filepath.Join(paths.Home(), DefaultLogDirectory, cfg.Beat)
al := zap.NewAtomicLevelAt(cfg.Level.ZapLevel())
internalLevelEnabler = &al // directly persisting struct will panic on accessing unitialized backing pointer

This seems to be the correct path for logs.

Can anyone explain why the logs are written in 2 different locations and confirm that this is the correct way to get the path?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to @blakerouse the current location is the right location to watch for logs.

@rdner rdner marked this pull request as ready for review May 30, 2023 13:45
@rdner rdner requested a review from a team as a code owner May 30, 2023 13:45
Copy link
Contributor

@pchila pchila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an optional small suggestion, otherwise LGTM

Comment on lines +102 to +106
if !filter(line) {
continue
}
_, _ = w.Write(line)
_, _ = w.Write([]byte{'\n'})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can flip the test here ?

Suggested change
if !filter(line) {
continue
}
_, _ = w.Write(line)
_, _ = w.Write([]byte{'\n'})
if filter(line) {
_, _ = w.Write(line)
_, _ = w.Write([]byte{'\n'})
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm going to keep it as it is. I always prefer less indentation with a terminal if.

@pchila
Copy link
Contributor

pchila commented May 31, 2023

Suggestion for future dev: could we add a --color option to have colored output (using something like https://github.com/fatih/color) for the different log levels? It could be useful to spot at a glance error/warning logs mixed with other log levels ;)

@belimawr belimawr self-requested a review May 31, 2023 15:07
@rdner rdner merged commit a35c498 into elastic:main May 31, 2023
22 checks passed
@michalpristas
Copy link
Contributor

tested on windows it works, all flags tested

rdner added a commit to elastic/ingest-docs that referenced this pull request Sep 4, 2023
rdner added a commit to elastic/ingest-docs that referenced this pull request Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip enhancement New feature or request Team:Elastic-Agent Label for the Agent team Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement a sub-command to show or follow the Elastic Agent logs
4 participants