Currently the watchdog() function is logging events using REQUEST_TIME constant which is just $_SERVER['REQUEST_TIME']. This works fine in most cases but processes that take an extended period of time all get logged at the time the process began, and not when the event actually happened. For example when running data migration scripts that can take much longer to process all of the errors get logged at the time the process began. It seems that watchdog would be more accurate if it logged the time with time() instead.

CommentFileSizeAuthor
#19 watchdog-time.patch731 bytesjbrown
#12 1239410-12.patch667 bytesswentel
#8 drupal-1239410-8.x.patch660 bytescangeceiro
#5 drupal-1239410-8.x.patch660 bytescangeceiro
#3 drupal-1239410-7.x.patch569 bytescangeceiro
#2 drupal-1239410-8.x.patch589 bytescangeceiro
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented
Version: 8.x-dev » 7.x-dev

same for d7, this causes unwanted issues with drupal queue and long running cronjobs. One could also log the REQUEST_TIME in an additional field, but from my point of view timestamp should contain the time the function has been called.

cangeceiro’s picture

cangeceiro CreditAttribution: cangeceiro commented
Version: 7.x-dev » 8.x-dev
Status: Active » Needs review
FileSize
drupal-1239410-8.x.patch589 bytes

attached is a patch for d8

cangeceiro’s picture

cangeceiro CreditAttribution: cangeceiro commented
FileSize
drupal-1239410-7.x.patch569 bytes

and a backport to d7

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs review » Needs work

Makes sense, but we should add a comment so this doesn't get reverted again later.

cangeceiro’s picture

cangeceiro CreditAttribution: cangeceiro commented
FileSize
drupal-1239410-8.x.patch660 bytes

like so?

cangeceiro’s picture

cangeceiro CreditAttribution: cangeceiro commented
Status: Needs work » Needs review

status change

Status: Needs review » Needs work

The last submitted patch, drupal-1239410-8.x.patch, failed testing.

cangeceiro’s picture

cangeceiro CreditAttribution: cangeceiro commented
FileSize
drupal-1239410-8.x.patch660 bytes

reroll of patch.

cangeceiro’s picture

cangeceiro CreditAttribution: cangeceiro commented
Status: Needs work » Needs review
ramlev’s picture

ramlev CreditAttribution: ramlev commented
Status: Needs review » Reviewed & tested by the community

Tested an working.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Needs work 
+      'timestamp'   => time(), // Request time isn't accurate for long processes.  Use time() instead

This comment doesn't confirm to the coding standards, it should be above the line and be a full sentence with a period at the end. See http://drupal.org/coding-standards

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Needs work » Needs review
FileSize
1239410-12.patch667 bytes

Move the comment above the line.

Status: Needs review » Needs work

The last submitted patch, 1239410-12.patch, failed testing.

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Needs work » Needs review

#12: 1239410-12.patch queued for re-testing.

Andre-B’s picture

Andre-B
Primary language German
Location Offenburg, Germany
CreditAttribution: Andre-B commented

#12: 1239410-12.patch queued for re-testing.

jbrown’s picture

jbrown CreditAttribution: jbrown commented

#12: 1239410-12.patch queued for re-testing.

jbrown’s picture

jbrown CreditAttribution: jbrown commented
Category: feature » bug
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Needs backport to D7

I think this makes sense, as a valid use of time(). Thanks!

Committed and pushed to 8.x. Probably makes sense to backport to 7.x, too.

jbrown’s picture

jbrown CreditAttribution: jbrown commented
Status: Patch (to be ported) » Needs review
FileSize
watchdog-time.patch731 bytes
jbrown’s picture

jbrown CreditAttribution: jbrown commented
Status: Needs review » Reviewed & tested by the community

Same patch as D8.

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein commented
Status: Reviewed & tested by the community » Fixed

Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/c987bbd

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