A server running multiple Drupal sites, all using syslog for watchdog, gets all messages in the log prefixed by "drupal:". I think it is a bug that there is no way to override this string ('cause if it is a feature then it is too late to change :-).

This simple patch allows each site to specify a different identity to the openlog() function.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JacobSingh’s picture

Status: Needs review » Reviewed & tested by the community

This looks like a non-evil patch. Ideally, I would like to see this type of altering happening in an altering hook or a single function which is globally defined for this purpose. I can see cases where you want different facilities, etc depending on the content of the message, other vars, etc..

But for now, an improvement and worth adding.

Dries’s picture

Should we make a recommendation as how to pick a good Syslog identity? I'm not sure people will get it based on the current form description. Thoughts?

jbrown’s picture

I think the site URL should be in there somewhere...

bjaspan’s picture

re #3: The site URL is already in there. Here is a sample entry from my local notebook. I set the identity field to "headtest" and the base_url() is http://head.local:8888:

Aug 12 13:50:36 barry-jaspans-macbook-pro-10 headtest[23671]: http://head.local:8888|1281635436|cron|127.0.0.1|http://head.local:8888/admin/reports/status/run-cron?render=overlay|http://head.local:8888/|0||Cron run completed.

re #2: New language in patch.

Already marked RTBC and I just changed a string, to leaving it RTBC.

Dries’s picture

OK, I committed to CVS HEAD. It makes sense to make it configurable (according to the specification), but people could already differentiate between sites using the URL field, not?

bjaspan’s picture

Status: Reviewed & tested by the community » Fixed

The problem with using the URL field is that there may be many different domains served by a single site, so it could be tricky, slow, or annoying to construct a grep regexp to match exactly the right set of log entries.

Marking fixed since the patch is committed.

bjaspan’s picture

Version: 7.x-dev » 6.x-dev
Status: Fixed » Reviewed & tested by the community
FileSize
1.51 KB

The same improvement is useful for D6.

Dave Reid’s picture

Version: 6.x-dev » 7.x-dev
Priority: Normal » Major
Status: Reviewed & tested by the community » Needs work

The patch is missing an variable_del() on uninstall for this addition.

bjaspan’s picture

Version: 7.x-dev » 6.x-dev
Priority: Major » Normal
Status: Needs work » Reviewed & tested by the community

You are correct that the patch committed to D7 does not have a variable_del() for the new variable, but in fact syslog.module already had two other variables and has no .install file or uninstall hook at all. So, it's a bug, but it is not really a bug with this patch. Open a separate issue if you feel strongly. :-)

bjaspan’s picture

Version: 6.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Needs work

Actually, there is another bug in syslog.module: The syslog_format form element is inside the block testing if LOG_LOCAL0 exists, when it should not. That is also not really related to this patch, though this patch makes the existing problem "worse" by adding yet another form element incorrectly inside the block. So, fine, I'll fix both and submit a new patch.

bjaspan’s picture

Version: 7.x-dev » 6.x-dev
Status: Needs work » Needs review
FileSize
4.34 KB

New patch. Adds syslog.install with an uninstall hook for all variables and fix the if LOG_LOCAL0 test I mentioned in the previous comment.

bjaspan’s picture

Version: 6.x-dev » 7.x-dev

Ooops, this is a D7 patch.

bjaspan’s picture

Title: syslog.module should support openlog() identity argument » syslog.module: openlog() identity argument and unrelated bug fixes
Dries’s picture

Status: Needs review » Fixed

Looks good. Committed to CVS HEAD.

bjaspan’s picture

Title: syslog.module: openlog() identity argument and unrelated bug fixes » syslog.module should support openlog() identity argument
Version: 7.x-dev » 6.x-dev
Status: Fixed » Reviewed & tested by the community

Putting this issue back to RTBC for D6. The patch in #7 is still valid. It does not add a variable_del() for the new variable on uninstall, because like D7, D6 syslog.module has no uninstall. It seems less important to fix this oversight for D6 than for D7. If necessary though the syslog.install file from the patch in #11 should work for D6 too.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed the install fragment from #11 and the patch from #7.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.