/var/log still filling up

[borpin (Brian Orpin)]

@djh

And the OP did it using mosquitto_sub, not by using emonhub logs, IIUC.

Yes but the emonhub log was important as it showed emonhub was sending the message, just the Mosquitto broker was not receiving it.

However, under normal conditions having the log just running at warn or error and setting message level appropriately would reduce number of messages. However, usually, the situation is 'emonhub stopped 10 hrs ago` and in that case being able to back into detailed logs may identify why. I prefer verbose logging, it is a small overhead to deal with them really. YMMV.

[djh (Dave Howorth)]

@borpin

Yes but the emonhub log was important as it showed emonhub was sending the message

I’m confused. I just looked at the thread again and don’t see anywhere the emonhub log is mentioned.

[borpin (Brian Orpin)]

@djh

I just looked at the thread again and don’t see anywhere the emonhub log is mentioned.

Very first post quotes emonhub log (second code entry)…

[djh (Dave Howorth)]

You mean the one directly after this description?

In the emoncms.log it aappears that the data is published:

I now understand why I was confused. You’re right, it is an emonhub.log not an emoncms.log

But it just shows the same thing as the mosquitto_sub logs above, so doesn’t add any value.

[borpin (Brian Orpin)]

I could go on but I won’t; life is too short.

[TrystanLea]

I have applied the above changes, which all appear to be working as expected:

1. –logfile parameter in emonhub service: Update emonhub.service · openenergymonitor/emonhub@8cc8e0e · GitHub

2. rc.local update to create emonhub folder:
   Update rc.local · openenergymonitor/emonpi@813fb97 · GitHub

3. emonhub update script modification to create emonhub log folder:
   Update emonhub.sh · openenergymonitor/emonpi@b74caff · GitHub

4. emonhub log viewer change in config module:
   Update config_controller.php · emoncms/config@5d66119 · GitHub

[pb66 (Paul)]

@TrystanLea

I think I would prefer to do this than just disabling the systemd service installation, it would also fix existing systems that have already run an update. Would you be happy if I proceed with the above @pb66 @borpin?

The rc.local mod is of course temporary, to be replaced with our non rc.local requirements as part of the new emonSD image & scripts in development.

Yeah the PR to put the change on hold was purely that! Just to stop more users falling into the issue that we didn’t have an immediate agreed fix in place for, especially before the emoncms v10 was released, but that ship has sailed and the “quick fix” didn’t get any attention in almost a week! ( I know you were away, but you aren’t the only one with access).

1, 3 and 4, great, I had pretty much done the same thing in my forks but realised there was little point in doing any PR’s by that time.

Regards the logfile creation in rc.local, IMO it doesn’t matter how temporary, temporary is, it’s not temporary enough. I explored getting the service unit to create the logdir as the init.d script has done so reliably for so long. I edited my service unit to include

ExecStart=/usr/share/emonhub/emonhub.py --config-file=/etc/emonhub/emonhub.conf --logfile=/var/log/emonhub/emonhub.log
PermissionsStartOnly=true
ExecStartPre=-/bin/mkdir -p /var/log/emonhub/
ExecStartPre=-/bin/chgrp -R emonhub /var/log/emonhub/
ExecStartPre=-/bin/chmod -R 777 /var/log/emonhub/

which works fine, it also includes some minor permission changes to allow the user to manipulate logfiles without using sudo (eg truncating or removing sensitive info before posting) and to set just the group ownership so the folder ownership is more in keeping with other package maintained log folders (ie root owned).

PS I also do not think the PIDFILE is used (unless type=forking) and the word “description” doesn’t need to be in the service unit description, when you look at the service log entries, “restarted the emonhub service description” doesn’t make much sense. So I now have

[Unit]
Description=emonHub service
DefaultDependencies=no
Before=shutdown.target
Conflicts=shutdown.target
Requires=local-fs.target
After=sysinit.target syslog.target local-fs.target

[Service]
User=emonhub
ExecStart=/usr/share/emonhub/emonhub.py --config-file=/etc/emonhub/emonhub.conf --logfile=/var/log/emonhub/emonhub.log
PermissionsStartOnly=true
ExecStartPre=-/bin/mkdir -p /var/log/emonhub/
ExecStartPre=-/bin/chgrp -R emonhub /var/log/emonhub/
ExecStartPre=-/bin/chmod -R 777 /var/log/emonhub/

Type=simple
Restart=always

[Install]
WantedBy=multi-user.target

and an entry in rc.local is not needed.

This don’t need to be a temporary “fix” either. IMO making the service unit(s) create their own log folder (and files?) makes for a much more robust set up, even if L2R is used, a service that can recreate it’s own log folder (at the cost of a couple of lines of code) is belt and braces, it would mean that even if users accidentally deleted emonhub.log, a simple restart would fix it.

Whilst it is not a good idea to start meddling with all the various packages (mysql redis etc) service units, I think there is value in ensuring that OEM services, intended for use on a sdcard (not exclusively) are able to create their own logfolders. There are now at least 2 emoncms services that will be logging to emoncms.log, so even the core emoncms install can have it’s log folder created before any web pages or requests are served. This will also shorten install steps and ensure any accidental changes to permissions or ownership are corrected automatically.

@TrystanLea

Im not sure if I need to make any logrotate modifications yet to fix this on existing systems.

I’ve looked through the logrotate install/update and it seems to be currently complete (as intended on previous emonSD images but absent on the Oct2018 image) but I wonder about how “multi-run safe” that script would be, surely the backed up files will get overwritten on every run as symlinks will pass the -f test. Also there is some confusion around the hourly vs daily cron (linking then moving?) so I was going to propose these changes Comparing master...logrotate · emoncms/emoncms · GitHub which will backup either daily or hourly cron files (but not symlinked copies) and forces a new cron symlink, just in case an existing incorrect symlink blocks a correct one being written.

I had also added the logrotate log folder check and create to the cron so that the folder is always found (ie not required in rc.local) and so that permissions are corrected even if it is there since the rc.local tries to change the ownership to pi:pi, which I didn’t understand as pi doesn’t have the authority to run logrotate properly unless using sudo and then root ownership is fine!

There is also a problem with the recent addition of a /var/log/* wildcard being added that will cause issues with rotating rotated files as if they were independent logfiles (not as previous rotations). Plus the “rotate 2” was changed to “rotate 1” at the same time, IMO this should be put back (check to see if logs require rotation hourly · emoncms/emoncms@ddb0854 · GitHub).

Plus this “fix” seems to be on/off/on Update logrotate.conf · emoncms/emoncms@7cc85b7 · GitHub which I do not think is needed if logrotate is run as root (by cron), this maybe linked to the pi:pi ownership in rc.local?

I still think there are improvements to be had to both the way logrotate is implemented and the logrotate rules thenselves as part of the L2R roll-out, but a return to emonhub.log and the old intended logrotate is a good temporary position, although not perfect, it has been ok over the last couple of years, certainly good enough to see us through to roll out of a better solution.

[borpin (Brian Orpin)]

@pb66

Regards the logfile creation in rc.local, IMO it doesn’t matter how temporary, temporary is, it’s not temporary enough. I explored getting the service unit to create the logdir as the init.d script has done so reliably for so long.

I suggest using systemd-tmpfiles as that is exactly what it is designed for (managing non persistent files). You could put a pre command in the service file to wait until the folder has been created but the start order means that (I think) the tmpfiles are created as soon as the FS is mounted.

[borpin (Brian Orpin)]

@pb66

I still think there are improvements to be had to both the way logrotate is implemented and the logrotate rules thenselves as part of the L2R roll-out, but a return to emonhub.log and the old intended logrotate is a good temporary position, although not perfect, it has been ok over the last couple of years, certainly good enough to see us through to roll out of a better solution.

@pb66 and I don’t agree on this, but I feel we should first revert the logrotate to a standard config so all the various packages pull in their own configurations (where set). The only change to the standard setup would be to change it from a daily trigger via cron, to an hourly (and that could just be for SDCard environments). In addition then, include a specific logrotate config for the emon systems to manage & rotate their logs.

This then fits with the ‘not just for an SDCard install’ philosophy of the new scripts.

This scheme could easily sit alongside L2R other than the olddir directive, as the issue there is where 2 packages have the same error.log file specified in different folders.