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

Status / Monitoring - Last Sample Zero II #145

Conversation

NOYB
Copy link
Contributor

@NOYB NOYB commented Jun 7, 2016

Monitoring graphs last sample being zero

If there is no RRD data sample for the current minute then the graph, and last and minimum data values will be zero, and average skewed, in certain circumstances. Such as resolution 1 minute and graph loaded prior to the current minute RRD sample being made. Similar for the first minute of other resolutions and custom time periods.

This commit corrects this by restricting the 'end' option to no later than the RRD last sample time.

@jdillard
Copy link
Contributor

jdillard commented Jun 7, 2016

What do you mean by "Similar for the first minute of other resolutions" and does a93f57d fix that?

I'll take a look at the custom time period parts tomorrow, that makes sense to me.

@NOYB
Copy link
Contributor Author

NOYB commented Jun 7, 2016

For instance custom time periods are on hour boundaries. The first minute of each hour results in a zero value.

a93f57d for sure does not fix it for custom time periods. Don't recall now but maybe that was applicable to custom time periods.

P.S. shouldn't this "60" => "-2min" be "-1min"?

Also, don't have my old code in place to test with, but think only '-1min' was necessary to cover all resolutions. Think it just needs to make sure the end time reference is not beyond the last sample time and RRD workings takes care of it from there.

@jdillard
Copy link
Contributor

jdillard commented Jun 7, 2016

Ah ok, I thought you meant for the preset time periods as well, makes more sense.

That commit was only intended to fix the preset time periods. I set it to 2min because it was hit or miss on a trailing zero value with only 1min. You set yours to 1min, but combined with the hack I removed (that chopped off the last sample value) it equated to two minutes (2 sample points).

@jdillard
Copy link
Contributor

jdillard commented Jun 7, 2016

P.S. for the top part of your commit that gets the LST you can use the code at line 149 to line 151 instead:

$rrd_info_array = rrd_info($rrd_location . $left . ".rrd");
$left_last_updated = $rrd_info_array['last_update'];

and that code doesn't' have to stay there, probably more useful up top.

@NOYB NOYB force-pushed the Status_/_Monitoring_-_Last_Sample_Zero_II branch from 5889cef to 32c5735 Compare June 7, 2016 07:49
@jdillard
Copy link
Contributor

jdillard commented Jun 7, 2016

I was working on this last night and got a little carried away. aaf5e5c has what I did for the top part and 20bde27 has the logic. does that fix look good?

rrd_fetch still adjusts itself for a best fit and can end up with weird start/end times, but it is a step closer and cleaner.

@NOYB NOYB force-pushed the Status_/_Monitoring_-_Last_Sample_Zero_II branch from 2f2d994 to 3d5e745 Compare June 8, 2016 09:28
@NOYB
Copy link
Contributor Author

NOYB commented Jun 8, 2016

Time period is 1 resolution too long.
Using larger (later) of left/right updated, instead of smaller (earlier), can result in zero ending value of the other.

Works but think this works better: 3d5e745

@jdillard
Copy link
Contributor

Time period is 1 resolution too long.

Yup, you're right. I had -2min, because when I did -1min last time I was testing it would occasionally have a last zero value, but it must have been caused by something else.

Using larger (later) of left/right updated, instead of smaller (earlier), can result in zero ending value of the other.

makes sense.

@NOYB
Copy link
Contributor Author

NOYB commented Jun 13, 2016

rrdtool fetch sometimes does return 1 or sometimes 2 nan responses. Can reproduce with a command line.
rrdtool fetch /var/db/rrd/WAN_DHCP-quality.rrd AVERAGE -r 60 -s end-1h

By using the last updated minus resolution, the nan responses are avoided.
Then also adjust start by adding resolution to retain the correct time period duration.

The 1 vs. 2 nan responses may have something to do with the relationship between time of the last sample data point relative to the last update time.

Appears to work well for both "canned" and custom graphs.
@NOYB NOYB force-pushed the Status_/_Monitoring_-_Last_Sample_Zero_II branch from 3d5e745 to 5b20ed3 Compare June 13, 2016 22:14
@jdillard
Copy link
Contributor

I haven't forgotten about these @NOYB, just been busy. I should be able to get back to working on Status > Monitoring soon.

@gonzopancho
Copy link
Member

ping @jdillard let's get to some resolution on this.

@netgate-git-updates netgate-git-updates merged commit 5b20ed3 into pfsense:devel Dec 16, 2016
netgate-git-updates pushed a commit that referenced this pull request Aug 16, 2022
ChangeLog: https://www.nlnetlabs.nl/news/2022/Aug/15/ldns-1.8.3-released/

1.8.3   2022-08-15
         * bugfix #183: Assertion failure with OPT record without rdata.
         * Fix for syntax error in pyldns

1.8.2   2022-08-12
         * bugfix #147: Allow for tabs in whitespace before quoted rdata
           fields.
         * bugfix #149: Add some missing [out] annotations to doxygen
           parameters.
         * Fix build error on Solaris 10 with inet_ntop redeclaration
           error.
         * Fix -U flag with ldns-signzone.
         * Enable compile of SVCB and HTTPS support by default.
         * bugfix #179: Free line memory even if zone file parsing fails
         * bugfix #166: Grow buffer when writing chars and fixed size
           strings when converting to presentation format, preventing
           potential assersion errors.
         * bugfix #46: Print network errors when secure tracing.
         * EDNS0 Option handling and conversion into presentation format.
         * bugfix #145: ldns-verify-zone should not call occluded records
           glue.

PR:	265859
Reported by:	[email protected] (maintainer)
netgate-git-updates pushed a commit that referenced this pull request Feb 26, 2023
ChangeLog:
Core

Using vlucas/valitron for user input validation
Bumped FontAwesome to version 6.2.0 (#141)
PHP versions
7.4 is now the minimal supported versions, older versions are not supported anymore (#143)
extended support for PHP 8.1 (#147)
Separated some templates into application/views/templates/partials folder (#144)
Removed Composer lock file from git repository
To avoid any potential issues for users using different version of PHP, composer.lock has been removed from the Git repo
Fixed how MVC is implemented by using psr-15 http-handler (#145)
Added router and user authentication middlewares
Using single pass psr-15 middleware for application routing and user authentication
Disabling user authentication does not display a blank page anymore (#140)
Improved how exceptions and errors are handled (#145)
PHP errors and exception handler and renderer has been refactored (#148)
Instantiate Session instance from the Core Controller (#149)
Disabling users authentication does not create a fatalog error nor blank page anymore (#135)
Dashboard

Breadcrumb navigation is now hidden on home page (Dashboard)
Jobs report

Fixed error with elapsed time when a job haven't been started yet if a job
is in pending status, elapsed time column will display 'n/a'
Docker image

Provided Docker image on Docker Hub (#153)
Documentation

Update documentation about deprecated version and general security information (#142)
Updated / fixed documentation
The FAQ has been fixed / updated
Security

Added security policy and documented know security vulnerabilities (#135 and #140)
Fixed
New feature(s)
Thanks to @sruckh, @skidoo23 and all community users for their feedback, tests, help and bug reports
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.

4 participants