Bug 13074 - Enhancement: Move collectd configuration in subdir so Addons can include monitoring files
Summary: Enhancement: Move collectd configuration in subdir so Addons can include moni...
Status: MODIFIED
Alias: None
Product: IPFire
Classification: Unclassified
Component: --- (show other bugs)
Version: 2
Hardware: all Unspecified
: - Unknown - Aesthetic Issue
Assignee: Adolf Belka
QA Contact:
URL: https://community.ipfire.org/t/surica...
Keywords:
Depends on:
Blocks:
 
Reported: 2023-03-30 09:57 UTC by dnl
Modified: 2024-09-23 12:03 UTC (History)
3 users (show)

See Also:


Attachments
Example of white / grey colors (250.52 KB, image/png)
2023-05-01 18:06 UTC, Jon
Details

Note You need to log in before you can comment on or make changes to this bug.
Description dnl 2023-03-30 09:57:38 UTC
Currently collectd configuration all exists in files in /etc

To monitor a new metric (for example one associated with an Addon) the main configuration file /etc/collectd.conf must be modified to add an "include" entry.

If this configuration was changed to use an etc configuration directory (such as  /etc/collectd.d or /etc/collectd.conf.d) then Addons would only need to include a unique configuration file for that directory.


IMPLEMENTATION
The end of the /etc/collect.conf file currently contains:
```
include "/etc/collectd.thermal"
include "/etc/collectd.custom"
include "/etc/collectd.vpn"
```

This could all be replaced by:
```
include "/etc/collectd.d/*"
```
and all custom configuration files moved in to /etc/collectd.d

I have tested this and as long as at least one file exits in /etc/collectd.d then it will work.  The file in the directory can contain only comments, such as collectd.custom does by default, there just has to be one file in the directory.
Comment 1 Michael Tremer 2023-03-30 15:48:50 UTC
Hello,

I think this is a great idea. Would you like to submit patches for this?
Comment 2 dnl 2023-04-01 06:31:57 UTC
Yes, I'll have a go!
Comment 3 dnl 2023-04-02 05:38:44 UTC
Hi again,

Two issues:

1. I can see how to change the structure in the existing codebase from git, but where do I write a script to migrate existing IPFire systems to this new structure?  I think I'd need to modify `ipfire-2.x/lfs/collectd` but I am unclear as to how.  Can you think of any examples which I could copy please?

2. I'm afraid I won't be able to contribute this myself as I avoid using my real name online and this breaks your git tags sign off rule.  This change I'm doing is trivial, so perhaps it might still be helpful for me to prepare the work for someone else to submit it?  It's up to you.

Thank you!
Comment 4 Michael Tremer 2023-04-11 12:52:45 UTC
(In reply to dnl from comment #3)
> Hi again,
> 
> Two issues:
> 
> 1. I can see how to change the structure in the existing codebase from git,
> but where do I write a script to migrate existing IPFire systems to this new
> structure?  I think I'd need to modify `ipfire-2.x/lfs/collectd` but I am
> unclear as to how.  Can you think of any examples which I could copy please?

I would say that it is a good start to create the changes in the build environment for a fresh install. Step two should then be looking at what we need to do in the updater.

> 2. I'm afraid I won't be able to contribute this myself as I avoid using my
> real name online and this breaks your git tags sign off rule.  This change
> I'm doing is trivial, so perhaps it might still be helpful for me to prepare
> the work for someone else to submit it?  It's up to you.

This is indeed a problem. It is not only about any name being on top of the commit. It is about authorship and the copyright that you are holding on your patches.
Comment 5 Adolf Belka 2023-04-19 08:46:47 UTC
Would it be helpful if I picked up this bug and put a patch together for it?
Comment 6 Michael Tremer 2023-04-19 09:22:37 UTC
(In reply to Adolf Belka from comment #5)
> Would it be helpful if I picked up this bug and put a patch together for it?

I am happy with this :)
Comment 7 dnl 2023-04-19 09:45:48 UTC
(In reply to Michael Tremer from comment #4)
> This is indeed a problem. It is not only about any name being on top of the
> commit. It is about authorship and the copyright that you are holding on
> your patches.

Thanks Michael.  I understand - that makes sense.


(In reply to Adolf Belka from comment #5)
> Would it be helpful if I picked up this bug and put a patch together for it?

Yes please!  I actually don't have much free time yet anyway so that would be super helpful.

I can give you a git diff but I'm not familiar with patchwork.  Would a diff be helpful?

The include line in /etc/collectd.conf needs to be 'include "/etc/collectd.d/*"' but otherwise this is totally straightforward.
Comment 8 dnl 2023-04-19 09:47:27 UTC
PS: I'm still not sure if I prefer /etc/collectd.d or /etc/collectd.conf.d myself.
The latter is very long but makes it clear that the '.conf.d' part refers to a directory for configuration files rater than a mistaken extra .d after collectd (in which the d presumably stands for daemon).
Comment 9 Michael Tremer 2023-04-19 10:17:55 UTC
(In reply to dnl from comment #8)
> PS: I'm still not sure if I prefer /etc/collectd.d or /etc/collectd.conf.d
> myself.
> The latter is very long but makes it clear that the '.conf.d' part refers to
> a directory for configuration files rater than a mistaken extra .d after
> collectd (in which the d presumably stands for daemon).

Normally "collectd.d" is the standardised way to do this. See init.d, rc.d and so on.
Comment 10 Jon 2023-04-29 17:31:03 UTC
Hey Adolf!  I found this BZ yesterday.  Have you gotten very far on this?  

If not, then I have some past notes I can share.
Comment 11 Adolf Belka 2023-05-01 10:00:36 UTC
Hi @Jon.

No I haven't gotten round to this bug yet. I have some others that I am still working on.
Comment 12 Jon 2023-05-01 18:06:33 UTC
Created attachment 1155 [details]
Example of white / grey colors

Most of my notes are background-ish (gathering info) related.

Obvious stuff:
- not every add-on should get auto-added to collectd (and Status > Services WebGUI).
- some add-ons have little to no info the gather (e.g. apcupsd, avahi-daemon).
- adding too many add-ons will clutter graphs.
- decided what add-on gets a graph is difficult.

Side note: graphs.pl has a limited # of colors and many are currently white/lite grey.  See attached image.

Adding a new add-on to the collectd (and the graph) is easy.  This could go in the addon install.sh

```
cat <<EOF > "/etc/collectd.d/${addonName}.conf"
<Plugin processes>
	Process "${addonName}"
</Plugin>
EOF

/etc/rc.d/init.d/collectd restart
```

As we learned with messagebus and dbus there could be different names for add-ons and their processes.  e.g. pmacct vs pmacctd
Comment 13 Adolf Belka 2024-07-08 12:22:43 UTC
Sorry that it has taken so long but I am now getting round to this.

My proposal would be to do two stages to this. The first being the addition of the /etc/collectd.d/ directory, moving existing collectd files that are currently included, into the collectd.d directory.

This change would then enable users to add any additional collectd.xyz files into the system for use with an addon.

A second step would then be to raise a new bug for an addon that had data available that would be generally required by all users so that the appropriate config file could be created and the associated plugin if required.

A separate bug report would then be created for each addon that required a config file so that as each is completed that bug report can be closed.
Comment 14 Adolf Belka 2024-08-06 10:25:37 UTC
Patch to create the collectd.d directory and adjust existing collectd profiles such as precache and thermal has been submitted.

https://lists.ipfire.org/hyperkitty/list/development@lists.ipfire.org/thread/AWHFVRWB2CARCEKM25PXAGRHNVRSATRH/

https://patchwork.ipfire.org/project/ipfire/list/?series=4389
Comment 16 Adolf Belka 2024-09-23 12:03:38 UTC
The patch has been submitted into next (will be CU190)