This patch makes the watchdog data easier to interpret/access. It does so by compiling "top x" lists. For example, it shows the most popular search terms, the most frequent 404s and the most frequent 403s.

This patch adds new menu items so you'll have to flush your cache.

CommentFileSizeAuthor
#6 improved-logs_0.patch5.21 KBwebchick
improved-logs.patch3.52 KBdries

Comments

Anonymous’s picture

Anonymous (not verified) commented

+1 from me.

tested on a fresh cvs install, and it works as advertised, and revealed a bug in the advanced search form, which i've submitted a fix for.

slightly off topic: is the $_SESSION['watchdog_overview_filter'] stuff a bug or a feature?

i found it a bit counter-intuitive that after visiting other sections of the site and coming back to admin/logs/watchdog the filter was whatever i last used. doesn't play nicely with multiple tabs either. just my 2 cents.

dries’s picture

dries commented

From a storage point of view, we could store these events in separate tables. A little bit more insight in the design decision.

The advantage would be (i) improved performance and (ii) less data duplication.

I choose not to do so for two reasons: (i) lack of time (sorry) and (ii) if we want to add a 'detailed view' to let people 'drill down' or investigate the results more closely, we'd still need the keep track of all the data.

For example, if we want to see the different referrers for 404s, or what users searched for certain keywords from what source pages, we would still have to keep track of all this data in the exact same way we are doing now in the watchdog table.

It's nice to see what pages don't exist, but it's also important to figure out what pages to fix. Right now, that information is still accessible in the watchdog table.

Anonymous’s picture

Anonymous (not verified) commented

should we add an index on the watchdog table type column for the new query in watchdog_top()?

killes@www.drop.org’s picture

killes@www.drop.org commented

@Dries: diff -u :p

@justin: adding an index to the watchdig table will slow down performance a lot. This is a table which has many inserts but few views, inserts woudl require the index to be rebuilt. However, it just so happens that I already prepared a patch which would allow us to add an index because it keeps the watchdog table small. :)

see http://drupal.org/node/78503

dries’s picture

dries commented

Anyone? (I probably reviewed several patches of you, now review mine. ;-))

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Needs work
StatusFileSize
new improved-logs_0.patch5.21 KB

No longer applied. Here is a re-roll. Seems like a good addition.

It might be useful for the message to link to the last instance in the watchdog table where this happened (admin/logs/event/##), or to add another column with this information. This way, things like referrer could be used to further track down *why* the error was happening, rather than just being informed that it was.

Steven’s picture

Steven commented

Cool, the watchdog could use some tweaks indeed... some comments:

  • Recent events seems a bit too obfuscated... it is the 'master' watchdog log after all, i.e. what administrators should check regularly. Can we get it a more attention-grabbing title or clearer description? Mentioning stuff like 'in reverse chronological order' seems a bit silly (the table is sortable after all). Perhaps the emphasis should be on 'keeping tabs on your site's activity' (the task/purpose) instead.
  • There is no help on the log-report pages. Perhaps we could put some links to drupal.org with information about dealing with 404s, 403s ... Having nothing but a bare table on the page is a bit spartan.
  • It seems a bit odd that log-report pages need to be explicitly added by a module. Couldn't we generate such summary tables for any type of watchdog message? It would seem the current approach is only really advantageous if the modules do more specific things, but they all just present the same table now.
  • In a way there is a bit of schizophrenia with the pages in this patch. There is a separate page for 404, 403 and search, which aggregates the messages. On the other hand, you can also just go to 'recent events' (the time log) and filter it by 404, 403 or search. These two pages are not the same, but it seems like they should be part of the same thing rather than split higher up.
  • This patch highlights a problem with "/admin" in my opinion. If I want to see the watchdog, my instinct is to click "administer" in the menu, and then "logs" in the menu as well (because it's closer to where my last click was, than the panels in the /admin page). This leads me to the very useless full-page version of the "logs" panel on "/admin". Then I need to read all the text and see that it's 'recent events' that I really want (instead of just any 'logs').
dries’s picture

dries commented

* Linking to the last event is not helpful. Eventually, we need an event browser for that. At least, that is what other systems provide. That will also fix Steven's remark: "They should be part of the same".

* I think it makes sense to focus on 404, 403 and search. These are the most useful, especially 404s and search. It wouldn't work to sort by other events. There would be no value.

I think we just need to add some help here, commit it, and refine it incrementally.

dries’s picture

dries commented

Committed a modified version to CVS HEAD.

dries’s picture

dries commented
Status: Needs work » Fixed
Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)