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

Update the tank reader to better handle FD leaks #76

Conversation

shriyanshk128T
Copy link

Description

Updating the error handling tank reader to handle FD leaks better

Testing

Before the change

This kept on increasing

[root@i-000cb472 centos]# ls -l /proc/23838/fd | wc -l
30
[root@i-000cb472 centos]# ls -l /proc/23838/fd
total 0
lrwx------ 1 root root 64 Jan  8 17:45 0 -> /dev/pts/4
lrwx------ 1 root root 64 Jan  8 17:45 1 -> /dev/pts/4
lr-x------ 1 root root 64 Jan  8 17:45 10 -> pipe:[66491592]
lr-x------ 1 root root 64 Jan  8 17:45 11 -> pipe:[66490785]
lr-x------ 1 root root 64 Jan  8 17:45 12 -> pipe:[66490867]
lr-x------ 1 root root 64 Jan  8 17:45 13 -> pipe:[66490972]
lr-x------ 1 root root 64 Jan  8 17:45 14 -> pipe:[66491060]
lr-x------ 1 root root 64 Jan  8 17:45 15 -> pipe:[66491158]
lr-x------ 1 root root 64 Jan  8 17:45 16 -> pipe:[66491228]
lr-x------ 1 root root 64 Jan  8 17:45 17 -> pipe:[66491334]
lr-x------ 1 root root 64 Jan  8 17:45 18 -> pipe:[66494506]
lr-x------ 1 root root 64 Jan  8 17:46 19 -> pipe:[66494582]
lrwx------ 1 root root 64 Jan  8 17:44 2 -> /dev/pts/4
lr-x------ 1 root root 64 Jan  8 17:46 20 -> pipe:[66494647]
lr-x------ 1 root root 64 Jan  8 17:46 21 -> pipe:[66494799]
lr-x------ 1 root root 64 Jan  8 17:46 22 -> pipe:[66494871]
lr-x------ 1 root root 64 Jan  8 17:46 23 -> pipe:[66494949]
lr-x------ 1 root root 64 Jan  8 17:46 24 -> pipe:[66495017]
lr-x------ 1 root root 64 Jan  8 17:46 25 -> pipe:[66495152]
lr-x------ 1 root root 64 Jan  8 17:46 26 -> pipe:[66495249]
lr-x------ 1 root root 64 Jan  8 17:46 27 -> pipe:[66495337]
lr-x------ 1 root root 64 Jan  8 17:46 28 -> pipe:[66495452]
lr-x------ 1 root root 64 Jan  8 17:46 29 -> pipe:[66496610]
lrwx------ 1 root root 64 Jan  8 17:45 3 -> socket:[66484181]
lr-x------ 1 root root 64 Jan  8 17:46 30 -> pipe:[66496661]
lrwx------ 1 root root 64 Jan  8 17:45 4 -> anon_inode:[eventpoll]
lr-x------ 1 root root 64 Jan  8 17:45 5 -> pipe:[66484179]
l-wx------ 1 root root 64 Jan  8 17:45 6 -> pipe:[66484179]
lr-x------ 1 root root 64 Jan  8 17:45 7 -> pipe:[66486157]
lr-x------ 1 root root 64 Jan  8 17:45 8 -> pipe:[66486256]
lr-x------ 1 root root 64 Jan  8 17:45 9 -> pipe:[66490435]

After the change

[root@i-000cb472 centos]# ls -l /proc/26223/fd
total 0
lrwx------ 1 root root 64 Jan  8 17:48 0 -> /dev/pts/4
lrwx------ 1 root root 64 Jan  8 17:48 1 -> /dev/pts/4
lrwx------ 1 root root 64 Jan  8 17:48 2 -> /dev/pts/4
lrwx------ 1 root root 64 Jan  8 17:48 3 -> socket:[66515225]
lrwx------ 1 root root 64 Jan  8 17:48 4 -> anon_inode:[eventpoll]
lr-x------ 1 root root 64 Jan  8 17:48 5 -> pipe:[66515224]
l-wx------ 1 root root 64 Jan  8 17:48 6 -> pipe:[66515224]
[root@i-000cb472 centos]# ls -l /proc/26223/fd | wc -l
8

Copy link
Collaborator

@gregschrock gregschrock left a comment

Choose a reason for hiding this comment

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

Did the t128_tank input function at all with this change? Visual inspection says it wouldn't work to process any data.

plugins/inputs/t128_tank/reader.go Outdated Show resolved Hide resolved
plugins/inputs/t128_tank/reader.go Outdated Show resolved Hide resolved
WAN-3597 #time 10m
@gregschrock
Copy link
Collaborator

Were you able to verify tank data can actually be read using the new implementation? I don't think the initial proposed code would have worked.

@shriyanshk128T
Copy link
Author

@gregschrock wrote a basic tank config to read events from the start and I was able to see the below as some samples

events,collector_id=auditd,host=i-000cb472,node=node0,subtype=ntp_adjustment,type=system event_detail="node=i-000cb472 type=SYSCALL msg=audit(1736438958.404:44515): arch=c000003e syscall=159 success=yes exit=0 a0=558b9260f980 a1=1 a2=0 a3=558b934cfc5c items=0 ppid=1 pid=11650 auid=4294967295 uid=38 gid=38 euid=38 suid=38 fsuid=38 egid=38 sgid=38 fsgid=38 tty=(none) ses=4294967295 comm=\"ntpd\" exe=\"/usr/sbin/ntpd\" key=\"128T\" old_time=2025-01-09T16:09:18.404000Z new_time=2025-01-09T16:09:18Z",new_date_time="2025-01-09T16:09:18Z",user="ntp" 1736438958404044515
events,collector_id=auditd,host=i-000cb472,node=node0,subtype=ntp_adjustment,type=system event_detail="node=i-000cb472 type=SYSCALL msg=audit(1736441019.442:44538): arch=c000003e syscall=159 success=yes exit=0 a0=558b9260f980 a1=1 a2=0 a3=558b934cf79c items=0 ppid=1 pid=11650 auid=4294967295 uid=38 gid=38 euid=38 suid=38 fsuid=38 egid=38 sgid=38 fsgid=38 tty=(none) ses=4294967295 comm=\"ntpd\" exe=\"/usr/sbin/ntpd\" key=\"128T\" old_time=2025-01-09T16:43:39.442000Z new_time=2025-01-09T16:43:39Z",new_date_time="2025-01-09T16:43:39Z",user="ntp" 1736441019442044538

@agrawalkaushik agrawalkaushik merged commit 11e50a6 into release-128tech-1.22 Jan 10, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants