Re: [Nagios-users] [Nagios-devel] Patches for inclusion to Nagios 4
On 2013-08-08 15:17, Ton Voon wrote: On 7 Aug 2013, at 08:51, Andreas Ericsson wrote: On 2013-08-06 19:16, Ton Voon wrote: Hi! We've published a list of patches for Nagios 4: http://www.opsview.com/whats-new/blog/opsview-patches-nagios-4 We'd be happy if you could review if these are acceptable for future inclusion or if anyone else finds them useful. I'd like to get patches with commit messages and proper author and signed-off-by info. Since we're using git for Nagios now, it'd go a long way in making sure everyone gets credit for the work they've done. The patches also need to apply cleanly to the latest master. You may want to clone the official Nagios repo, apply your patches on top of it and then send me a pull request for github or some such. git clone git://git.code.sf.net/p/nagios/nagios nagios-core should get you the very latest. If you apply your patches on top of 'master' and make sure to always do git pull --rebase when you want to get the latest and greatest you'll quickly see which patches either have been applied or which no longer *can* be applied. Then you can create a separate repository on github or some such and push the changes there. OK, we'll convert them into git changes based off master. However I'd like an assurance that the changes will be merged before we promise to convert. Style changes are fine, won't take much time and will not require retesting, but if we need to refactor object changes or make larger changes to logic, this will require retesting on our side. I'd like a reassurance that the time invested will result in a merge upstream, otherwise we're just wasting time for all of us. For instance, your assistance in the environment macros per-command was greatly appreciated and we've coded to the design agreed in the email conversations, but it hasn't been merged yet. It has, but it hasn't. The original patch no longer applies, due to extensive remodeling of the worker code. I still have it hanging around though, and I'm fairly inclined towards using it for services and hosts as well, so that environment macros can be set on a service level as well as on a command level. I've also been investigating the chances of doing this without resorting to setenv() (in essence, building a one-off block of the load-time environment macros, and then extending that whenever we hit an environment variable). But yea, I've got that one already. So I'd like to reverse the question and ask which changes are most likely to be accepted, based on the amount of changes required and we'll work through that list in order. Thread-safe calls It's harmless, and I'd apply it straight away if I had a sensible commit message for it, but I don't know why you need this so I can't really write one. While we're on the topic; Please write commit messages in imperative form and present tense, as if you're giving orders to the code for how it should change. Also give a short rationale for why it should change that way. The rules about not commenting out code you want to get rid of still applies though. It just looks terribly hackish when patches meant for upstream contains things like that. Slice services within hosts So long as it's configurable from cgi.cfg and the default stays the same as it is today I'll apply it immediately. It has no impact on loadable modules or other headaches, so that's a nobrainer, really. Check command by time period I feel this is somewhat lacking in efficiency and flexibility, and a much cleaner solution would be to add a filtering functionality to NERD so that checkresults can be shipped off to a third-party addon which can transform checkresults and plugin output as much as it likes. Failing that (which would be enormously cool but also a lot of work), I think it would most likely be best off as a separate module, with custom variables or separate configuration to support it. Supporting patches to run events when a timeperiod becomes active or inactive would still be welcome, obviously. Escalation via notification levels This is best off written as a module, using custom variables to configure it. If core support is required to block notifications to a particular contact, then such patches will naturally be accepted. The normal NEBCALLBACK_CANCEL return code signalling should work just fine for that. Synchronising state data Pretty invasive for quite a small benefit, and with enormous complexities to deal with to make this work properly. How do you handle reading the synced file while checks are in-flight and awaiting being returned to the mothership? They may have been spawned as a result of a bad checkresult on one node, but if the sync status returns them to OK and the in-flight result sets them as bad again, would that mean starting over on the retry-attempts? I see nothing in the patch that handles situations like that, so I'm forced to believe you haven't considered it. Considering the fact that full
Re: [Nagios-users] [Nagios-devel] Patches for inclusion to Nagios 4
On 2013-08-06 19:16, Ton Voon wrote: Hi! We've published a list of patches for Nagios 4: http://www.opsview.com/whats-new/blog/opsview-patches-nagios-4 We'd be happy if you could review if these are acceptable for future inclusion or if anyone else finds them useful. I'd like to get patches with commit messages and proper author and signed-off-by info. Since we're using git for Nagios now, it'd go a long way in making sure everyone gets credit for the work they've done. The patches also need to apply cleanly to the latest master. You may want to clone the official Nagios repo, apply your patches on top of it and then send me a pull request for github or some such. git clone git://git.code.sf.net/p/nagios/nagios nagios-core should get you the very latest. If you apply your patches on top of 'master' and make sure to always do git pull --rebase when you want to get the latest and greatest you'll quickly see which patches either have been applied or which no longer *can* be applied. Then you can create a separate repository on github or some such and push the changes there. On a first inspection though; * Don't comment out code. Bringing back dead code is what the VCS is for, and keeping it around is just plain dumb. If it shouldn't be in there, just remove it. In the same vein; Don't add commented-out code. * Avoid C++ comments. I know they're supported in C99, which I'm rooting for as the least version supported, but it's against the current style. * Don't mix spaces and tabs for indentation, unless it's continuation- indentation of a multi-line statement or condition. Stick to the style in the file you're editing, and *look* at the patch before you send it somewhere. * Avoid comments saying things like Opsview specific foobar if you want to have the patches included. If you *don't* want those patches included, don't send them to me or point me to where they are. It takes up a lot of time to remove crap like that, and I have no interest in cleaning up after anyone else. I'm messy enough as it is on my own. * Don't augment objects (such as hosts, services, commands) with new variables. Doing so means Nagios 4.1, and I can't take patches like that until Nagios 4 is at least released as stable. All objects have an 'id' field which means you can look up any extra data you want in O(1) time, provided you just initialize an array of size num_objects.$desired_object_type_in_plural before we enter the event loop. * Make patches the most scalable you can. For the check_time_period thing, you'd be far better off adding code to detect timeperiod changes, notice which timeperiods are used to change commands and make a one-off swap for all the affected commands as the desired timeperiod comes either in or out of effect. * Don't build on deprecated technology, such as external files for commands and/or check results. -- Andreas Ericsson andreas.erics...@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 Considering the successes of the wars on alcohol, poverty, drugs and terror, I think we should give some serious thought to declaring war on peace. -- Get 100% visibility into Java/.NET code with AppDynamics Lite! It's a free troubleshooting tool designed for production. Get down to code-level detail for bottlenecks, with 2% overhead. Download for free and get started troubleshooting in minutes. http://pubads.g.doubleclick.net/gampad/clk?id=48897031iu=/4140/ostg.clktrk ___ Nagios-users mailing list Nagios-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nagios-users ::: Please include Nagios version, plugin version (-v) and OS when reporting any issue. ::: Messages without supporting info will risk being sent to /dev/null
Re: [Nagios-users] [Nagios-devel] Patches for inclusion to Nagios 4
Looks like some good additions. Since I'm in the middle of doing some in-depth performance tuning/comparison between 3.2.3 and 4.0 I might to throw a couple of these on and see how things change. Thanks! Dan On Aug 6, 2013, at 12:16 PM, Ton Voon ton.v...@opsview.com wrote: Hi! We've published a list of patches for Nagios 4: http://www.opsview.com/whats-new/blog/opsview-patches-nagios-4 We'd be happy if you could review if these are acceptable for future inclusion or if anyone else finds them useful. Ton -- Get 100% visibility into Java/.NET code with AppDynamics Lite! It's a free troubleshooting tool designed for production. Get down to code-level detail for bottlenecks, with 2% overhead. Download for free and get started troubleshooting in minutes. http://pubads.g.doubleclick.net/gampad/clk?id=48897031iu=/4140/ostg.clktrk ___ Nagios-devel mailing list nagios-de...@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nagios-devel -- Get 100% visibility into Java/.NET code with AppDynamics Lite! It's a free troubleshooting tool designed for production. Get down to code-level detail for bottlenecks, with 2% overhead. Download for free and get started troubleshooting in minutes. http://pubads.g.doubleclick.net/gampad/clk?id=48897031iu=/4140/ostg.clktrk ___ Nagios-users mailing list Nagios-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nagios-users ::: Please include Nagios version, plugin version (-v) and OS when reporting any issue. ::: Messages without supporting info will risk being sent to /dev/null
