Added an admin page to display Backtrace from watchdog
pounard - August 11, 2009 - 12:29
| Project: | DataSync |
| Version: | 6.x-1.0-alpha3 |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
I did a simple page display which get back the backtrace from a failed job (adds also a field into datasync_jobs table to store the backtrace if there is).
It displays the link to this page in the watchdog entry which says that the job failed.
| Attachment | Size |
|---|---|
| datasync-6.x-1.0-alpha3-patched-backtrace.patch | 4.05 KB |
#1
It was patched over a version with #536528: DataSync should catch exceptions and mark the job as failed and #536512: Datasync should not set the memory_limit itself patches applied, but I don't think it should break if you don't have the last one.
#2
This is a great idea and something I was thinking about from the beginning. There should definitely be a way to monitor failures (and get notified about them in different ways). However, I'm not sure I agree with the approach. Here's what I think:
I definitely want this functionality to get in. Thanks again for all your contributions Pierre
#3
1) I agree
2)
I don't think so. First implementation on my side was on a separate table. The fact it was unnecessary for only field.3) Ok, with an implementation with a hook your point of view on 2) is better.
The thing is, you have to do a delete operation on hook, when you delete a job, don't keep trace of backtrace, so, this need a datasync_job_delete() function implentation (like I did in drush integration file).
#4
About deletion, the $job->remove() method calls hook_datasync_job_removed(), so any deletion can be done there
#5
Ok, thanks
#6
Hello again!
I'm actually rewriting this feature as a new module. I wish you could review it when I'll upload it here. I wonder if you could create a contrib/ sub dir in your module repository and allow me to put this as a submodule, and the drush integration (#541784: Drush integration) with it?
The fact I'm not fond of creating a new module project for each of these single features.
What do you think?
#7
Note about datasync.job.inc
Lines 141 to 147:
<?phptry {
//THIS ACTUALLY RUNS THE PHASE
$result = $this->{$this->phase}();
}
catch (Exception $e) {
$result = t("Exception caught: %exception", array('%exception' => $e->getMessage()));
}
?>
This code should also export $e backtrace so I could get it in the hook_datasync_exception_thrown(). The main goal of this submodule is not debug DataSync itself, but debug the custom implementation.
#8
Please review and try this.
#9
#10
Pierre,
This is great. I've made a few changes - some changes to the copy, some drupal coding standards changes, and a couple bugfixes. The patch to DataSync is attached below.
I think it would be a killer feature if we extended this module beyond exception catching. Maybe we change the name to datasync_log and we implement many of the other hooks (hook_datasync_phase_failed, datasync_job_removed, datasync_job_added, datasync_phase_succeeded, datasync_phase_postponed, etc.). The user should be able to configure which events are logged. Because the events are logged with watchdog, they can have the information of whatever job events they want emailed to them, written to a logfile, etc. We then add a view like the /admin/settings/datasync/jobs page to show all the log messages.
What do you think?
Also, I see what you mean about the hook_datasync_exception_thrown hook being confusing and am going to create an issue for it
#11
I totally agree with the configuration granularity you are proposing. There is a lot to do with this submodule. I'll take some time as soon as I can to continue improvements on this module.
#12
When you are doing this:
<?phptry {
//THIS ACTUALLY RUNS THE PHASE
$result = $this->{$this->phase}();
}
catch (Exception $e) {
$result = t("Exception caught: %exception", array('%exception' => $e->getMessage()));
}
?>
You are catching every exception that may happen in the custom job implementation. In my case, I throw some custom exceptions (EntityException in my module) that I want to be catched by your module if I forgot the right catch statement in my job implementation (plus, some PHP low level exceptions could be thrown too).
The thing that is wrong in this code is you get back exception message but not the backtrace of the custom exception (remember that my exception is not a DataSync exception, but a custom exception which comes from an API which is not DataSync related).
I don't have my working laptop with me right now, so I can't do a quick and dirty patch to show you, but I hope this clarify what I try to explain.
What I want to get back here, is the custom exception backtrace, not just the error message, so when you throw the hook_datasync_exception_thrown() I can get it back in the submodule to log it.
Remember that I does this module to debug my custom jobs, not to debug DataSync itself. But the idea to do a granular multi-level logging is still a really good idea.
#13
Please try this one, and tell what do you think.
EDIT: oups, I'm tired, rename the file to .tar.gz (typo error)
Re EDIT: please avoid diff if you modify it, I did put the module into a different folder this is like hell to apply:)
#14
This version is quite buggy, I'll send a new one soon (I'm currently workig with, I really needed this feature).
#15
Here is an updated version, with no known bugs for me.
#16
@andrewlevine
Did you looked at the code update?