Skip to content

Add Collector log#14309

Merged
thiagoftsm merged 38 commits into
netdata:masterfrom
thiagoftsm:collector_log
Jan 25, 2023
Merged

Add Collector log#14309
thiagoftsm merged 38 commits into
netdata:masterfrom
thiagoftsm:collector_log

Conversation

@thiagoftsm
Copy link
Copy Markdown
Contributor

@thiagoftsm thiagoftsm commented Jan 22, 2023

Summary

Fixes #13823
Fixes #6038

This PR has the main goal to separate collector logs from error.log simplifying netdata debug, development, and mainly helping users to find proper issues with collectors.

Some of streaming logs are going to be redirect for new collector.log, this is because our stream works like a collector (it uses the same protocol to communicate), but I did not redirect everything with this PR, because I understand this is not the main goal.

Test Plan
  1. Compile this branch
  2. Start netdata
  3. Verify that errors delivered by collectors are going to new collector.log.
Additional Information

This PR is moving logs from external and internal collectors to collector.log.

For users: How does this change affect me? Describe the PR affects users: - Which area of Netdata is affected by the change? collectors - Can they see the change or is it an under the hood? If they can see it, where? Yes, they will have separated logs for collectors. - How is the user impacted by the change? Simplify search for errors. - What are there any benefits of the change? Users have separated logs to report error for us.

@thiagoftsm
Copy link
Copy Markdown
Contributor Author

@thiagoftsm can we move proc_open (or whatever the func name) logs too
I found a solution with latest commit.

@vkalintiris
Copy link
Copy Markdown
Contributor

This PR is moving logs from external and internal collectors to collector.log.

@thiagoftsm, is it easy to add an option that'll keep everything in error.log?

I'm worried that from now on, we will have to check two different files in order to check for any errors during development, and we might miss some because of this change.

@ilyam8
Copy link
Copy Markdown
Member

ilyam8 commented Jan 25, 2023

@vkalintiris Will cat or tail + *.log solve the problem for you? I hope yes!

@vkalintiris
Copy link
Copy Markdown
Contributor

@vkalintiris Will cat or tail + *.log solve the problem for you? I hope yes!

Can live with that but we'll lose ordering of events, unless we start tracking timestamps :|

@andrewm4894
Copy link
Copy Markdown
Contributor

would having this as a config param be overkill? default to splitting but ability for anyone who might want to for whatever reason to just chose one single log.

@ilyam8
Copy link
Copy Markdown
Member

ilyam8 commented Jan 25, 2023

We have

        # debug = /opt/netdata/var/log/netdata/debug.log
        # error = /opt/netdata/var/log/netdata/error.log
        # access = /opt/netdata/var/log/netdata/access.log
        # health = /opt/netdata/var/log/netdata/health.log

I guess we can use these options to redirect logs to the same file (but not sure, never tried).

@MrZammler
Copy link
Copy Markdown
Contributor

We have

        # debug = /opt/netdata/var/log/netdata/debug.log
        # error = /opt/netdata/var/log/netdata/error.log
        # access = /opt/netdata/var/log/netdata/access.log
        # health = /opt/netdata/var/log/netdata/health.log

I guess we can use these options to redirect logs to the same file (but not sure, never tried).

Was about to post this. Agree with @ilyam8 if we can get this to work (maybe it already works) then that would be the cleanest solution.

Comment thread daemon/README.md Outdated
@Ferroin
Copy link
Copy Markdown
Member

Ferroin commented Jan 25, 2023

@Ferroin about docker documentation, we never described in our documentation nothing about logs, on the other hand our Dockerfile have commands to link them, are we missing information in our documentation? Or was this intentional?

We need to add a line to link the new log file as well, it can literally just copy the other lines.

Other than that there should be nothing else required on the packaging side of things.

@thiagoftsm
Copy link
Copy Markdown
Contributor Author

We have

        # debug = /opt/netdata/var/log/netdata/debug.log
        # error = /opt/netdata/var/log/netdata/error.log
        # access = /opt/netdata/var/log/netdata/access.log
        # health = /opt/netdata/var/log/netdata/health.log

I guess we can use these options to redirect logs to the same file (but not sure, never tried).

Was about to post this. Agree with @ilyam8 if we can get this to work (maybe it already works) then that would be the cleanest solution.

Yes, this PR brings a new collector option that can be defined inside netdata.conf.

Comment thread daemon/README.md Outdated
@thiagoftsm
Copy link
Copy Markdown
Contributor Author

@ilyam8 and @vkalintiris I tested now a scenario where both error and collector had /var/log/netdata/error.log and everything were concentrated in one log:

root@hades:/home/thiago/Netdata/netdata# ls -l /var/log/netdata/
total 856
-rw-r--r-- 1 netdata netdata   2917 Jan 25 17:28 access.log
-rw-r--r-- 1 netdata netdata      0 Jan 25 17:27 debug.log
-rw-r--r-- 1 netdata netdata 863535 Jan 25 17:28 error.log
-rw-r--r-- 1 root    root      5525 Jan 25 17:28 health.log

And when I commented the collector everything was split again:

root@hades:/home/thiago/Netdata/netdata# killall netdata
root@hades:/home/thiago/Netdata/netdata# vim /etc/netdata/netdata.conf 
root@hades:/home/thiago/Netdata/netdata# ps aux| grep netdata
root     22214  0.0  0.0   6832  2824 pts/1    S+   17:28   0:00 grep netdata
root@hades:/home/thiago/Netdata/netdata# rm -rf /var/log/netdata/*
root@hades:/home/thiago/Netdata/netdata# netdata
root@hades:/home/thiago/Netdata/netdata# ls -l /var/log/netdata/
total 308
-rw-r--r-- 1 netdata netdata      0 Jan 25 17:29 access.log
-rw-r--r-- 1 netdata netdata 248931 Jan 25 17:29 collector.log
-rw-r--r-- 1 netdata netdata      0 Jan 25 17:29 debug.log
-rw-r--r-- 1 root    root     60989 Jan 25 17:29 error.log
-rw-r--r-- 1 root    root       290 Jan 25 17:29 health.log

so the necessity to have another option is not necessary, please, let me know if you have another request.

MrZammler
MrZammler previously approved these changes Jan 25, 2023
ilyam8
ilyam8 previously approved these changes Jan 25, 2023
@thiagoftsm thiagoftsm dismissed stale reviews from ilyam8 and MrZammler via 08607c1 January 25, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feat]: Log to seperate log files per plugin/collector Lot of ERROR messages.

7 participants