Description
Logman is a module which provides a better UI for searching dblog based watchdog messages and includes apache access log search.

The UI is intended to provide easy search of log messages with more information. The UI can be used to search both the watchdog and apache access log messages.The watchdog search tool provides searching ability on almost all fields of watchdog table. Similarly apache access log search tool provides searching facility on most of the common fields.

Module provides a UI on every page indicating count of number of dblog based watchdog messages grouped by severity.

Module also provides a statistics page with google column charts on dblog based watchdog and apache access log data.

Summary of features provided.
- Watchdog based log search
- Apache access log search
- Statistics page based on apache access log and watchdog log
- Page UI that appear on each (only for users with permission and can be disabled), as an alert for number of watchdog based log messages for that page grouped by severity.

This version of the module provides search and statistics facilities for dblog based watchdog messages, the future versions are intended to support gelf based watchdog messages.

Other similar modules and how Logman is different
Following are links to some the similar modules:
https://drupal.org/project/views_watchdog
https://drupal.org/project/logging_alerts
https://drupal.org/project/watchdog_filtering
https://drupal.org/project/better_watchdog_ui

The views_watchdog and logging_alerts are available with D6 core, but views_watchdog provides a basic watchdog log browsing whereas logman watchdog search tool will provide extensive search on log messages. The logging_alerts is entirely different from logman as it provides email and web server based logging.

The watchdog_fitering and better_watchdog_ui is only available with D7 core. Still comparing with logman watchdog_filtering provides again basic kind of log search and better_watchdog_ui does provides better watchdog log search, however the logman is still different from these two modules as well because it not only provides extensive search on watchdog log messages but also includes features like apache access log search, statistics and UI on each page as an alert based on watchdog log.

Project Page

Link to repository
git clone --branch 6.x-1.x http://git.drupal.org/sandbox/AbhijeetGhosh/2030383.git logman

Contributions with other projects
https://drupal.org/node/1957158 (Drupal 8 core)
https://drupal.org/node/1996644 (Drupal 8 core)

Reviews of project
https://drupal.org/node/2031701#comment-7633121
https://drupal.org/node/2088431#comment-7873571
https://drupal.org/node/2067533#comment-7874309

Reviews of other project:
https://drupal.org/node/2089695#comment-7883575
https://drupal.org/node/2094477#comment-7883681

Comments

abghosh82’s picture

Issue summary:View changes

Updated the link in reviews for project.

abghosh82’s picture

Issue summary:View changes

Added the link to repository.

abghosh82’s picture

Issue summary:View changes

Updated the link to repository.

abghosh82’s picture

Issue summary:View changes

Updated the description.

abghosh82’s picture

Status:Active» Needs review

Please review my module. Setting it for review.

PA robot’s picture

Status:Needs review» Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxAbhijeetGhosh2030383git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

abghosh82’s picture

Status:Needs work» Needs review

I have fixed the Drupal CS review issues. Setting to review again.

urwen’s picture

Hi my friend,

Help with some reviews, 3 at least and put yourself the bonus review tag so other developers will review type module.

Thank you

t14’s picture

Hi

I installed this module and took it for a test drive.
I like the idea. Below are the following issues I came across

Error message on page admin/reports/logman
Notice: Undefined offset: 14 in LogmanApacheSearch->readApacheLog() (line 192 of /sites/all/modules/logman/2030383/includes/lib/LogmanApacheSearch.php).

You need to create a hook_uninstall in your .install file and use variable_del. So that your users have the option to remove all of the variables stored in the databases by your module.

Hope that was helpful
T

abghosh82’s picture

Thanks for review t14, I agree should have added hook_uninstall(). Fixed the couple of issues you mentioned.

abghosh82’s picture

Issue summary:View changes

Updated the description.

abghosh82’s picture

Issue summary:View changes

Updated reviews of project.

abghosh82’s picture

Issue summary:View changes

Added contributions with other projects.

abghosh82’s picture

I have done 3 manual reviews so adding a review bonus tag.

klausi’s picture

Assigned:Unassigned» kscheirer
Status:Needs review» Reviewed & tested by the community
Issue tags:-PAReview: review bonus

Review of the 6.x-1.x branch:

  • DrupalPractice has found some issues with your code, but could be false positives.
    FILE: /home/klausi/pareview_temp/includes/lib/LogmanWatchdogSearch.php
    --------------------------------------------------------------------------------
    FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
    --------------------------------------------------------------------------------
    195 | WARNING | Do not call theme functions directly, use theme('pager', ...)
         |         | instead
    --------------------------------------------------------------------------------

    FILE: /home/klausi/pareview_temp/includes/logman_watchdog.inc
    --------------------------------------------------------------------------------
    FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
    --------------------------------------------------------------------------------
    242 | WARNING | Do not call theme functions directly, use theme('table', ...)
         |         | instead
    --------------------------------------------------------------------------------

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

  1. There are a lot of similar modules already - you described differences to existing modules, but is yet another new module really necessary? Please reconsider joining forces with an existing project before working further on this.
  2. logman_init(): this is run on every single page request, don't do that. Add your CSS and JS when you actually need it (in the page callabck that creates your output).
  3. logman_statistics_form(): doc block is wrong, this is not a hook. See https://drupal.org/node/1354#forms
  4. interface LogmanSearchInterface: please document all @param and @return variables.
  5. class LogmanWatchdogSearch: this looks very vulnerable to SQL injection although it seems like no dangerous user provided text is passed to that function. Always use placeholder in DB queries, make sure to read https://drupal.org/writing-secure-code again. searchLog() looks also dangerous, but it seems pager_query() takes care of variable substitution and I could not exploit anything.
  6. notice: Undefined index: values in /home/klausi/workspace/drupal-6/sites/all/modules/logman/logman.module on line 345. Please enable PHP notices in your development environment.

Otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to kscheirer as he might have time to take a final look at this.

abghosh82’s picture

Thanks a lot klausi for your review and a sort of positive feedback for RTBC.

For the PAReview issues not sure, can theme_table and theme_page not be used, which I thought was a valid drupal 6 function?

As I have mentioned in issue description this module is different to others, since in this module there is apache access log search and statistics for both watchdog and apache access log and statistics/alert on each page, the part which is not there in any on the other similar modules, also planning to support gelf in this module, so would like to keep this separate.

For logman_init(), I have used this for the page statistics/alert that is visible at the bottom of each page. The statistics UI sits on the hook_footer (there is no menu callback for it) where I cannot use drupal_add_css or drupal_add_js, as I think it takes affect only in menu callback or hook_init. Also I have added the visibility for this purpose only, so if user chooses not to display the UI, thus the UI and corresponding js and css will not be added to that page. Let me know how should I use it in this case.

Fixed the doc block in logman_statistics_form().

Fixed the comment issues in LogmanSearchInterface.

For LogmanWatchdogSearch sql queries vulnerable to SQL injection, I went to the link you suggested, and checked the code I have used database abstraction and have used substitutions for all fields except timestamps, which I think should get filtered out with the use of strtotime in calling function in logman_watchdog.inc. For statistics I have not used substitutions but the value which it depends on is drupal site's page URI and timestamps generated by the code. Please suggest me how would you like me to fix this.

Fixed the php notice.

abghosh82’s picture

Issue summary:View changes

Updated reviews of project.

abghosh82’s picture

Issue summary:View changes

Updated reviews of other project.

kscheirer’s picture

Assigned:kscheirer» Unassigned
Status:Reviewed & tested by the community» Fixed

Thanks for addressing the similar modules on your project page.

  • In logman_watchdog_search_result() why are you setting a date_default_timezone_set("Europe/London"); ?
  • In LogmanWatchdogSearch::getStatistics() and other functions, instead of using your own tokens with substitution, you can use db_query() to handle that for you.
  • I may have missed it, but I didn't see anyplace you checked that the apache log file is readable?
  • For the theme functions, klausi meant that you should call them like theme('pager') instead of directly as theme_pager(). This is done so overrides have a chance to intercept those calls. If you call theme_pager directly, no override can happen.
  • Do you plan to release a D7 version of this module?

Very nicely written module!

Thanks for your contribution, abghosh82!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

----
Top Shelf Modules - Crafted, Curated, Contributed.

abghosh82’s picture

Thanks a lot kscheirer, for your review and giving me RTBC.

I would fix the issues you and klausi have mentioned and also thanks for explaining me the difference between theme_pager and theme('pager'). I would fix this as well.

Setting default timezone may not be required, I might remove it, after doing some tests.

Yes I am planning a D7 version of this module with gelf support, I will also enhance this D6 version with gelf support.

abghosh82’s picture

Fix the theme_table and theme_pager issues, and made SQL queries more secure. Removed date_default_timezone_set("Europe/London"); as not required.

Now I will release this module to full project.

Thanks for all reviews.

Status:Fixed» Closed (fixed)

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

Anonymous’s picture

Issue summary:View changes

Added one more project review.