Cacti / cacti

Cacti ™
http://www.cacti.net
GNU General Public License v2.0
1.61k stars 399 forks source link

rrdtool is executed with locale set for information requests #1121

Closed pierrewillenbrock closed 6 years ago

pierrewillenbrock commented 6 years ago

rrdtool is executed with locale set when cacti is trying to retrieve information about rra files. with a locale set, rrdtool outputs numeric data according to the given locale. If the locale happens to use comma (,) as a decimal separator, cacti(or rather php) fails to parse these. This shows up as incorrect bandwidth sums in traffic graphs, but also when using "Data Source Info Mode"(causes erroneous mismatch messages for minimum, maximum and XFF values).

On the other hand, for generating graphs, passing the locale seems to be reasonable, so the numeric values are formatted accordingly(even though cacti generated numbers in texts do not follow the locale, leading to the use of different decimal separators in the same graph).

This is using cacti 1.1.20, rrdtool 1.5.5. In settings, "Language support" is "Enabled", "Language" is "English", "Auto Language Detection" is "Enabled". The browser is probably reporting a german locale.

putenv("LANG") in __rrd_execute(lib/rrd.php) fixes bandwidth summation and "Data Source Info Mode", but also removes localisation of numeric data from graphs. As far as i can see, LANG is not set by the web server(edit: turns out i am wrong, LANG is set outside of php).

ronytomen commented 6 years ago

Well, that is quite an oversight.

cigamit commented 6 years ago

Can you please upload some screen shots of what needs fixing and your suggestion?

pierrewillenbrock commented 6 years ago

Behaviour of unmodified cacti 1.1.20: bad-lang-for-rrdtool cacti 1.1.20 with removing LANG environment variable around rrdtool invocation: good-lang-for-rrdtool

For full localisation the following would be needed:

Leaving the graphs unlocalised, with points for decimal separators only involves calling rrdtool without LANG, irrespective of whether a graph is generated or data is pulled from rrdtool.

I am still trying to understand where the german LANG environment variable is introduced. My browser sends Accept-Language:"en-US,en;q=0.8,de-DE;q=0.5,de;q=0.3" as header, the rest of cacti is localised to english. The rest of the cacti UI is not localised to german until i disable "Auto Language Detection" and set "Language" to "German". This does not change any of the decimal separators in the graphs, though.

cigamit commented 6 years ago

Boy talk about shedding light on a subject. It's now crystal clear to me what the issue is now. Easy fix.

cigamit commented 6 years ago

Update your graph_variables.php from the develop branch and let us know everything is working as expected. Close if resolved.

pierrewillenbrock commented 6 years ago

That fixes the decimal points on the totals(when german is actually requested), but leaves the numbers incorrect.

In the meantime, i figured out that the LANG environment is actually set outside cacti. You may want to protect against that or declare it a configuration error. (I will edit the initial report to indicate that.)

I don't know if cacti is doing anything about the decimal points of the rrdtool generated numbers in the graphs, but i am planning to find out.

cigamit commented 6 years ago

Upload the updated graphs doing the Arlo Guthrie thing to the images again.

pierrewillenbrock commented 6 years ago

Dropping LANG from the process' environment turns out to be surprisingly hard, had to resort to dropping it before starting the web-server. The decimal separators of the totals stay german(comma), but the other numbers use english (point).

pierrewillenbrock commented 6 years ago

Looks like this

mixed-decimal-separator-no-lang-for-rrdtool

I configured german language, no auto-detect. LANG has been taken out of the equation by having the web server startup remove that.

Please note that i'm more interested in getting acceptable values into the graphs than getting the decimal separators correct, although that's also an issue.

cigamit commented 6 years ago

Update lib/rrd.php from develop and then return your findings.

pierrewillenbrock commented 6 years ago

The decimal separators are fine now, and totals started to be plausible after updating graph_variables.php.

I think it should also filter for "info" in addition to "fetch", or inversely check for "graphv".

Anyway, the original issue has been resolved, and, as far as i can tell, in a way that does not require stripping LANG out of the web servers environment. Thank you for fixing this problem.

cigamit commented 6 years ago

Can you review CSV export and Data Source Info for issues. If you find them, can you please post images here?

eojot commented 6 years ago

Could this 'fix' break the sum function when I'm not using auto as power of 10 divisor? I.e. |sum:auto:current:2:auto| works but |sum:3:current:2:auto| is broken and always shows 0 instead of e.g. 5.7 Reverting rrd.php and graph_variables.php back to 1.1.28 fixes this for me. Unfortunately I have datasources which are not bits/bytes and I need a sum of and the sum function returns values like 10.06 or 22.91 - at least up to 1.1.28

cigamit commented 6 years ago

lib/rrd.php had nothing to do with the power output but rather the internationalization of rrdtool fetch information that needed to be in the english locale for the math to work inside of PHP.

So, I have a few questions:

1) What is the web servers locale? 2) Run rrdtool fetch from the command line as the root user and post some output. 3) Run php -a and type into it the following:

echo (1000,2 + 1000,2) . PHP_EOL;

Post that output.

eojot commented 6 years ago

apaches LANG is set to C (debian default), tried changing to de_AT.UTF8 but that somehow results in completely wrong (way too high) values for the sum even with 1.1.28 and still nothing with 1.1.29

RRDtool 1.6.0, default shell LANG=de_AT.UTF-8 rrdtool fetch /usr/share/cacti/site/rra/kwzhler1_m3zaehler_169.rrd LAST m3Zaehler 1514884500: 1,0044816054e-01 1514884800: 1,9933333333e-01 1514885100: 1,6688888889e-01 with LANG set to C rrdtool uses . instead of ,

php (7.0.26-1) seems to dislike the , even when I set the locales to de_AT.utf8

cigamit commented 6 years ago

Okay, so in lib/rrd.php, what if you change the lang to C instead of en_EN.UTF8?

Second point, what power were you thinking of? I would like to also know the use case. It may require a change in the syntax of the Nth ptile or bandwidth summation syntax.

eojot commented 6 years ago

Uhm, I changed something completely different in graph_variables.php and it's working again: line 500, 1.1.29, broken: $summation /= pow($pow, intval($regexp_match_array[1])); replace with the line from 1.1.28: $summation /= pow(10, intval($regexp_match_array[1])); makes it work again. I dont see $pow being set in variable_bandwidth_summation anywhere, copy/pasting the few lines to set pow from variable_nth_percentile fixes this as well. So it's actually not a language issue but an issue with the updated summation function. Setting LANG to C (even unconditionally) changed nothing. Still wonder why it showed at least 'something' when I was fooling around with apaches locale settings. I'm (ab)using cacti to visualize my power/gas/water meters as well so using 'auto' and the resulting 'B' it automatically prints in the output is not really what I want. I'm using e.g. 3 to just print kWh/m3 etc. and add the proper value in the comment text.

cigamit commented 6 years ago

This is now fixed in develop. It just missed the cut for 1.1.30. So, just use graph_variables.php from develop and you will be all set.