The accesslog in the statistics module isn't the most elegant thing.
What it does is, each page view it writes to the database these items:

  • Page title
  • Path
  • url
  • hostname (IP addess)
  • uid
  • Session id
  • Page load time via timer_read
  • timestamp

Many of these can be gathered by free external tools such as google analytics. These services often offer custom variable tracking, so a contrib module could add this functionality.

I will work on a patch to strip out the accesslog features from statistics module.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

timmillwood’s picture

I would still like to see this make it's way into Drupal 8 as I really feel the 'access log' gives little value, most of the value from the statistics module comes from the 'node counter'.

Dave Reid’s picture

Agreed. Let's support the basic use case of counters for all entity types, but anything advanced should probably not be for core.

iamEAP’s picture

The node counter functionality of statistics is great for site builders / administrators (for, e.g., building a View of the most "popular" content).

While the access log is not immediately useful for the same audience, it's extremely useful (at least in my opinion) for developers.

  • Unless you're running server monitoring, it's the only window into PHP's performance.
  • Unless you have a custom variable in GA dedicated to tracking whether a user is logged in or not, access log is the only way to know what percentage of your traffic is anonymous.
  • You might be able to infer from watchdog, but it's very useful for verifying malicious IPs
  • Related to the previous item, GA only tracks hits from users running JavaScript; access log is the only way to track hits from users not running JS (probably not users, but more likely bots).
timmillwood’s picture

@iamEAP - I don't feel these useful items you mentioned really need to be in core and by moving them to contrib allows for better expansion on features and functionality. I really hope you could be one of many to help lead this effort with your already widely used contrib module.

Also to cover your last point about javascript. The node counter has already moved to JavaScript in Drupal 8 to allow it to give accurate results when using reverse proxy caching. If we were to keep the accesslog I would like to do the same with that and move it to JavaScript.

iamEAP’s picture

I suppose accesslog isn't necessarily appropriate for core, especially with the direction node/entity statistics is going / has gone.

If you remove the accesslog, I'm happy to adopt its schema/table in an 8.x branch of my contrib module.

timmillwood’s picture

Status: Active » Needs review
FileSize
13.38 KB

Starting with removing tests

Status: Needs review » Needs work

The last submitted patch, 1446956-6.patch, failed testing.

timmillwood’s picture

Status: Needs work » Needs review
FileSize
14.54 KB
timmillwood’s picture

Status: Needs review » Needs work

So that's the tests sorted, now to do the rest.

timmillwood’s picture

Status: Needs work » Needs review
FileSize
48.5 KB

Removed the access log schema and settings form.

Little by little!

timmillwood’s picture

FileSize
39.15 KB

Bad patch in #10

also removed most things now.

Status: Needs review » Needs work

The last submitted patch, 1446956-11.patch, failed testing.

catch’s picture

timmillwood’s picture

I am really keen to remove the accesslog, but there is a lot of code to remove and by the time I find it all, and test it all, core has changed so much that my patch doesn't work.

Changing the access log to use ajax is near impossible because of the kind of data it gathers, it would need to have a php built gif on the page that gathers the information much like Google analytics or other such tools.

iamEAP’s picture

Just out of curiosity Tim, what advantage do you have in mind for a PHP-built gif over a regular AJAX request?

iamEAP’s picture

Kicking this back off again. Re-roll of #11.

iamEAP’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal-remove_accesslog-1446956-16.patch, failed testing.

iamEAP’s picture

Status: Needs work » Needs review
FileSize
40.55 KB

Missed a merge conflict in the re-roll. Also getting rid of another test that's not relevant anymore.

iamEAP’s picture

Assigned: timmillwood » Unassigned

Un-assigning Tim for wider review.

mondrake’s picture

Hi, I tested and it looks ok to me.

I just wonder whether we should have an explicit dp_drop_table for accesslog in the update hook?

Cheers

timmillwood’s picture

Eric,
Thanks for looking into this. All looks good to me, I am going to send as re-test to the testbot now before marking RTBC.

Tim

timmillwood’s picture

timmillwood’s picture

Status: Needs review » Reviewed & tested by the community

Looking good!
Really excited to get this committed.

Berdir’s picture

+++ b/core/modules/statistics/lib/Drupal/statistics/Tests/StatisticsAdminTest.phpundefined
@@ -62,19 +62,13 @@ function setUp() {
-    $this->assertFalse($config->get('count_content_views'), 'Count content view log is disabled by default.');
+    $this->assertFalse($config->get('count_content_views'), t('Count content view log is disabled by default.'));
...
-    $this->assertTrue($config->get('count_content_views'), 'Count content view log is enabled.');
+    $this->assertTrue($config->get('count_content_views'), t('Count content view log is enabled.'));

+++ b/core/modules/statistics/lib/Drupal/statistics/Tests/StatisticsLoggingTest.phpundefined
@@ -81,10 +79,7 @@ function testLogging() {
+    $this->assertIdentical($this->drupalGetHeader('X-Drupal-Cache'), 'MISS', t('Testing an uncached page.'));

@@ -92,10 +87,7 @@ function testLogging() {
+    $this->assertIdentical($this->drupalGetHeader('X-Drupal-Cache'), 'HIT', t('Testing a cached page.'));

These shouldn't use t()

iamEAP’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
40.02 KB

Good catch, Berdir. Fixing those here.

@mondrake In past upgrade paths, I believe accesslog has been truncated (maybe I'm imagining this); though there've also been cases (in Taxonomy, for example) where a table's been left behind for contrib to pick up. In any event, I think it's fine to leave it for now and address it when the upgrade path is being worked on.

mondrake’s picture

OK, thanks

Good to leave it there for contrib to pick up.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

Now those lines are all unchanged. Looks good, was RTBC.

webchick’s picture

Assigned: Unassigned » Dries

Hm. Assigning to Dries, since this seems to remove basically all of the usefulness of this module.

timmillwood’s picture

node counter is the most useful part of this module, it counts all node views allowing for "Most viewed content" views to be generated etc. The access log just duplicates what any analytics or server access log does. It offers little or no functionality just statistical data that is better sourced else where. Access log doesn't work with reverse proxy cache and adds extra database load.

webchick’s picture

Yes, but I don't run external analytics due to privacy concerns, it is much easier to simply go to admin/reports than grep my Apache logs or install a third-party log visualization script, and I don't use a reverse proxy cache (along with 90%+ of the rest of the Drupal websites out there) so I don't care about that. :)

Anyway, let's see what Dries says.

webchick’s picture

Issue summary: View changes

typo

NITEMAN’s picture

I don't know if accesslog should remain in core or if it should be decoupled from node counter functionality but for me and many others provides invaluable information.

Right now it's one of the bests sources of information for server side performance monitoring and tuning... so, please, if it's finally removed provide a clean path to a contrib alternative.

Best regards

timmillwood’s picture

I would suggest a tool such as New Relic for performance monitoring and tuning, the accesslog in statistics module is going to cause more performance issues. You should see a faster site by disabling it.

iamEAP’s picture

I understand the concerns mentioned by webchick and NITEMAN; I expressed similar concerns in #3.

I maintain Better Statistics in contrib. In D7, it support the default accesslog functionality, but also an API to allow collection of arbitrary data. On the roadmap is pluggable storage (e.g. a syslog for Statistics).

I could honestly go in either direction:

  • Remove accesslog from core and let contrib pick it up.
  • Keep accesslog in core, but make it more pluggable.

But as Tim Mentions now, it's not currently doing its job very well.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Needs work

This patch doesn't look ready yet. Some quick things I noticed:

  1. statistics_help() seems to be providing lots of help text about things the module no longer does.
  2. The "access statistics" permission is still defined but doesn't seem to do anything anymore.
  3. I have to wonder, given what's left after this patch and how small/focused it is on node statistics, whether the following actually makes any sense anymore...
    name = Statistics
    description = Logs access statistics for your site.
    

***

Also, I have no strong opinion on whether this should be in core, but it doesn't sound like there's a clear plan yet for maintaining this in contrib .... Is someone planning to maintain it as a standalone module?

I don't think it's accurate to say that the accesslog duplicates what analytics or server access logs do; maybe that's partially the case for anonymous users, but definitely not for authenticated users. The module provides deep integration of those statistics within your Drupal site itself. Example from the patch...

-  $items['user/%user/track/navigation'] = array(
-    'title' => 'Page visits',
-    'page callback' => 'statistics_user_tracker',
...
- * Page callback: Displays statistics for a user.
iamEAP’s picture

Status: Needs work » Needs review
FileSize
8.17 KB
46.88 KB

Here's a patch that..

  • Takes a stab at making statistics_help() reflect what the module does (which is now mostly just a content popularity module; a good next step after this would be generalizing Statistics to arbitrary entities, rather than just nodes),
  • Removes the unnecessary permission,
  • Cleans up other instances of "access statistics" around the module.

I mentioned earlier that I'd be willing to, at a minimum, maintain the functionality that we're ripping out here in the 8.x version of the Better Statistics module (I'd attempt to improve upon it there as well, obviously).

NITEMAN’s picture

Every measurement tool has its performance impact (¿Heisenberg?), acceslog doesn't impact so much in a well configured dedicated InnoDB.

I know New relic... but IMHO we can't seriously solve everything using external black box services.

Best Regards

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

Lets get this done, we have a critical waiting on this..anyone who wants this functionality in d8 will be able to find it in contrib in Better Statistics module

catch’s picture

NewRelic has nothing to do with this issue.

If you want performance measurement of page loads from within Drupal you can use http://drupal.org/project/performance http://drupal.org/project/xhprof and others.

Dries’s picture

Assigned: Dries » Unassigned
Status: Reviewed & tested by the community » Needs work

I agree that this no longer is core worthy so I committed this to 8.x. Not to mention it is often a source of problems. I do think we should add a 'CHANGELOG.txt' entry so I'm marking it 'needs work' on that basis.

iamEAP’s picture

Status: Needs work » Needs review
FileSize
540 bytes

CHANGELOG.txt entry, per Dries' note. Also made a change record: http://drupal.org/node/1900384

dawehner’s picture

Don't we need a changelog entry for that?

effulgentsia’s picture

@dawehner: what do you mean in #42? Do we now have some other process for making changelog.txt changes other than submitting a patch as #41 has done?

dawehner’s picture

Yeah good question what I talked about, missed somehow the link on http://drupal.org/node/1900384 . I'm sorry.

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community

#41 looks good. Thanks.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed the follow-up, thanks!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

is change to it
What it does is