Skip to content
This repository has been archived by the owner. It is now read-only.

[dev.icinga.com #1161] store cmd.cgi submissions in log #531

Closed
icinga-migration opened this issue Jan 27, 2011 · 18 comments
Closed

[dev.icinga.com #1161] store cmd.cgi submissions in log #531

icinga-migration opened this issue Jan 27, 2011 · 18 comments
Milestone

Comments

@icinga-migration
Copy link
Member

@icinga-migration icinga-migration commented Jan 27, 2011

This issue has been migrated from Redmine: https://dev.icinga.com/issues/1161

Created by mfriedrich on 2011-01-27 16:14:55 +00:00

Assignee: ricardo
Status: Resolved (closed on 2011-05-02 16:00:41 +00:00)
Target Version: 1.4
Last Update: 2014-12-08 09:39:55 +00:00 (in Redmine)


by opsview

Attachments

Changesets

2011-03-14 16:03:39 +00:00 by ricardo 6de855d

moved cgi log location from var/cgi_logs to share/log #1161

refs: #1161

2011-04-28 21:49:14 +00:00 by ricardo 35a5d6a

added lock checking on writing to cgi.log #1161

* also added remote address to every log entry

refs: #1161

2011-05-02 12:46:55 +00:00 by mfriedrich 57698ca

fix cgiutils.c cgi_log_archive_path instead of cgi_log_archive_dir, as shown in cgi.cfg

refs #1161

2011-05-02 12:51:32 +00:00 by mfriedrich c6e0c11

update icinga.spec with cgi logging feature #1161

refs #1161

2011-05-12 16:53:10 +00:00 by ricardo 2460d42

better handling of writing to cgi.log in cmd.cgi #1161

refs: #1161

2011-06-04 09:44:43 +00:00 by ricardo 7581398

better handling of writing to cgi.log in cmd.cgi #1161

refs: #1161

Relations:

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Feb 16, 2011

Updated by mfriedrich on 2011-02-16 07:53:22 +00:00

  • Target Version set to 1.4
@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Feb 25, 2011

Updated by ricardo on 2011-02-25 12:43:36 +00:00

  • Status changed from New to Feedback

I don't get this. Any Command you submit via cmd.cgi get's written into log file anyway.

Can someone explain the use of this?

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 8, 2011

Updated by ricardo on 2011-03-08 10:14:35 +00:00

closing this one?

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 8, 2011

Updated by mfriedrich on 2011-03-08 10:17:31 +00:00

no. the idea was to create an optional logfile where the complete cmd.cgi string is stored + the user who was doing that. this should be added als "cgi.log" or similar.
currently we don't have any (debug) log for the cgis, you can only guess what happened (mostly in combination with sent commands to the core pipe).

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 8, 2011

Updated by ricardo on 2011-03-08 11:35:26 +00:00

ok, this sounds different, because in the patch is the nagios.log mentioned.

What is with the "log_external_commands_user" option ??? This option will log the name of the user who submitted the command. and if you wanne know all submitted commands, you can use logfile filter from now on.

or we can put this in as debug code. so you have to switch it on if you need it (define #DEBUG).

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 8, 2011

Updated by mfriedrich on 2011-03-08 11:40:35 +00:00

the log_external_command_user option only works if you put the commands in a special syntax on the command pipe. the cgis don't support it, and other plugins/addons (like nsca) also don't (a change of the syntax, which breaks compatibility). so this option was always intentionally left disabled, and only given to the users who wanted to use that. mainly it creates more errors than success, so i'd rather kick this feature out of the core again than integrate it somewhere else.

furthermore the core shouldn't be bound too much on the cgis and vice versa. the overall idea was to take the patch and wrap that into a specific cgi log, and not icinga.log or similar. basically a feature request a while ago, where people where asking to log the actions their users do (because apache log won't tell exactly).

sorry for not making this more clear, but i kept that as a todo for myself on collecting patches and reviewing them afterwards.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 10, 2011

Updated by ricardo on 2011-03-10 21:57:30 +00:00

would like to solve https://dev.icinga.org/issues/610, with this as well.

It would be nice if all forced comments get stored in cgi.log as well.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 11, 2011

Updated by ricardo on 2011-03-11 14:20:54 +00:00

A consequence the apache would have write permission to log dir.

solution, own log dir for cgi log. Is this what we want?

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 11, 2011

Updated by mfriedrich on 2011-03-11 21:40:12 +00:00

opt-in, disabled by default. but when installing the cgis we'll also have an apache user at least for icinga.cmd so this would be a good point to require such things in the future.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 14, 2011

Updated by ricardo on 2011-03-14 13:30:34 +00:00

  • Assigned to set to ricardo
  • Done % changed from 0 to 90

see commit: dca6ed6

4 new cgi.cfg options (default):

use_logging=0
cgi_log_file=@localstatedir@/cgi_logs/icinga-cgi.log
cgi_log_rotation_method=d
cgi_log_archive_path=@localstatedir@/cgi_logs

Function "write_to_cgi_log" added.

The problem here is:

  1. There is no file locking. If two log actioin try to access the log file at the same time, one will fail.
  2. log_rotation gets checked on every request of write_logs because we have no daemon process to rotate the log. And it's not the job of icinga core to do that.
  3. write access gets checked on every request of cmd.cgi to show the user a warning if something is wrong.

And as usual, test, test, test

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 14, 2011

Updated by mfriedrich on 2011-03-14 20:59:09 +00:00

ad 1. should be resolvable by a try again loop? sth like

while(try_write()==ERROR)
   sleep 1;

normally i'll solve that like that, just imagine a thread waiting for a mutex lock. or a reader polling the socket where no data comes through.

ad 2. i would rotate the logs by size like ido2db debugfile.

ad 3. sounds good, cmd.cgi is the initial place where this log becomes interesting.

please post an RFC of the syntax in cgi.log either on icinga-devel and/or the wiki.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Mar 14, 2011

Updated by mfriedrich on 2011-03-14 21:01:26 +00:00

ad 1. again, only run that try again loop several times after the command being sent. maybe with an #define MAX_ERROR_CNT 10 and

while(try_write()==ERROR && cnt
@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Apr 27, 2011

Updated by mfriedrich on 2011-04-27 18:06:27 +00:00

any more thoughts on that?

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Apr 27, 2011

Updated by ricardo on 2011-04-27 23:10:28 +00:00

will implement the log rotation lock check this week or next wek

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Apr 28, 2011

Updated by ricardo on 2011-04-28 22:30:44 +00:00

  • Done % changed from 90 to 100
@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Apr 28, 2011

Updated by ricardo on 2011-04-28 22:32:35 +00:00

added remote address to every log entry as well.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented May 2, 2011

Updated by mfriedrich on 2011-05-02 16:00:41 +00:00

  • Status changed from Feedback to Resolved

pretty neat, veryfied all fine for icinga 1.4

[1304340317] EXTERNAL COMMAND: foo;127.0.0.1:eeec;ACKNOWLEDGE_SVC_PROBLEM;localhost;dep1;2;1;0;foo;test
@icinga-migration

This comment has been minimized.

Copy link
Member Author

@icinga-migration icinga-migration commented Dec 8, 2014

Updated by mfriedrich on 2014-12-08 09:39:55 +00:00

  • Project changed from 19 to Core, Classic UI, IDOUtils
  • Category set to Classic UI
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.