Allow collection of access statistics asynchronously

I think the general idea will be the same, though in the spirit of the module, it will be easily overridden. Perhaps something like the following:

• By default, just ping a callback URL via AJAX that runs through the normal PHP execution,
• Allow modules to declare callbacks in JS for their fields (to get custom, client-side data). This data would then be appended in some way to the main AJAX callback, which would write through the client-side data instead of the server-side data.

Definitely appreciate the thoughts, mondrake.

Not sure I'm 100% sold on a scope control variable, but I definitely agree more context could be provided in that hook invocation. Unfortunately, adding it would break the API.

I think the primary use case that I'd like to solve is related to what you were alluding to at the end there: allowing some level of access statistics collection for statically cached pages (e.g. Varnish, boost, etc). That being said, I think we can't only support pages rendered by JS-enabled clients; it would largely defeat one of the use-cases for the Core Statistics module (e.g. easily finding malicious bots' IPs for blocking). Maybe a compromise would be allowing the module to run in three modes:

1. Classic: basically as it functions now,
2. Client Side: no hook_exit() invocation; only invoked via client-side async request (useful if you don't care about the core use-cases for the stats module),
3. Mixed Mode: hook_exit() invoked only for pages not meant to be parsed by a browser, otherwise invoked via client-side async request (useful if you still care about core use-cases for the stats module, or use Varnish/Boost/etc and don't mind if the data might be a little inconsistent).

High level architecture for the async callback:

• Always fully bootstrapped, never cached (for dx/convenience, not very scalable, but at least fully bootstrapped, so it's alterable to a point where you could make it scalable if you needed)
• By default, always collect stats as they're collected now in hook_exit (though in the primary menu callback, rather than hook_exit, obviously),
• Allow fields to be overridden via POST parameters (populated via JS). If the JS data is present, always use it; otherwise, degrade to the PHP generated values

High level architecture for the JS API:

• For each field definition, add an optional "JS" property which would include all the details necessary for a drupal_add_js() call. Up to API implementors to determine whether they want to split fields up into separate JS files, or just use one file and include for all fields (or, for that matter, inline it).
• Each JS file would bind an event handler to a custom jQuery event (e.g. BetterStatistics.accesslog). The event handler would handle data collection and append it to the object passed in by reference.
• Better Statistics would, add some async JS as late as possible that would trigger the aforementioned jQuery event, then POST the resulting data to a callback URL (which would be handled as specified above; degrading to PHP for fields that do not specify JS).

Nowhere near done, but getting there. Taking a long time because I am definitely not a frontend developer. Attaching the patch to get a feel for what TestBot thinks. No new tests yet.

Done so far

• Extending the statistics_enable_access_log variable to be on/off + the modes I described above (client-side only and mixed mode),
• Introducing a global Better Statistics object/facade bs (variable name is technically configurable),
• A simple queue stub (inspired by Google Analytics) is added early in the JS stack, and triggers a request to load the full API definition asynchronously,
• All statistics commands will be piped through the facade in JS, like so:
  • For sending pageviews/accesslog entries: bs('accesslog');
  • For sending node hits: bs('entityView', 'node', 1);
• Methods can be added/replaced by other modules like so:
  • To replace an existing implementation: BetterStatistics.prototype.accesslog = function() {/* my new function */};
  • To add a new method: BetterStatistics.prototype.newMethod = function() {/* my brand new method */};, which would be invoked like so: bs('newMethod');

Still todo

• Implement (on the PHP side) the code to include JS supplied by hook_better_statistics_fields() implementors,
• Implement a callback URL to handle data from JS API POST requests,
• Implement bs('accesslog') and bs('entityView' ...) to POST to the callback,
• Add event triggers before the aforementioned HTTP requests to gather data,
• Implement event listeners for the aforementioned triggers to supply the data (see core_fields.js and custom_fields.js).

Tests for the additional server-side functionality from #5. If they pass, I'll start hammering on the todos.

Tests were passing, but there were exceptions due to calling system/ajax via GET and with no params. I found another instance of core that didn't return HTML and swapped it in place (rss.xml). Also added assertions on HTML-based pages that ensure the API JS is included, as well as the queue stub.

Working on this during DrupalCon Portland Sprints.

Another pile of work as well as tests to ensure the AJAX callback handles JSON data correctly.

Done in this chunk

• Added code to include JS supplied by hook_better_statistics_fields() implementors. This is done with a new 'js' key on the returned array which is itself an array with at least 'data' and 'type' keys, but can include any other option passed to drupal_add_js().
• Added a callback URL to handle data from JS API POST requests at /statistics/ajax/TYPE, where TYPE is the type of statistical data being collected,
• Implemented bs('accesslog') to POST to the callback,
• Added jQuery event trigger for accesslog data collection; other scripts can bind to this like: $(document).bind('statistics.accesslog', function(e, data) {...});,
• Added an event listener for the core accesslog fields to supply their override data.

Still todo

• Document the new 'js' key that can be provided in hook_better_statistics_fields(),
• Add custom JS field implementations (cache, user-agent, peak memory),
• Finish implementing default core/custom field event bindings,
• Need a hook_update_N() function to ensure core stats accesslog fields on existing installs get the newly provided 'js' key in the active store,
• Implement bs('entityView')

More progress; discovered a bug where cached pages would get 2 accesslog hits because hook_exit() had no idea that JS existed on the page (which would not occur w/Varnish or Boost). Added some code in prelog to prevent that situation (as well as a test). Still needs some more tests work

Done in this chunk

• Documented the 'js' key provided in hook_better_statistics_fields(), added it to the list of keys able to be written through on cache flush / configuration change / cron,
• Worked out core/custom field event bindings for the obvious fields; still one field in question, see below,
• Implemented bs('entity_view'), as well as its PHP callback.

Still todo

• Determine a strategy to get the cache status from the client-side (js/fields/custom.js),
• Need a hook_update_N() function to ensure core stats accesslog fields on existing installs get the newly provided 'js' key in the active store; this may be as simple as one of the existing update functions, though potentially not (need to investigate)
• Write tests for AJAX data collection of bs('entity_view') implementation.

Per the remaining todos, I'm moving this back to needs work. That being said, if patches had release candidates, #12 would be the first.

Gave up on the cache status from client-side issue. Don't believe it's possible to solve. Also removed the need for an update hook by very slightly refactoring the field update function.

Request for API Feedback

After looking at the code, I've discovered there's actually very little in the way of dependencies, save for the jQuery event triggering/binding. Wondering from an API perspective whether or not we should kill off that jQuery dependency and write a short, standard JavaScript event triggering method for BetterStatistics. The difference for JS developers would be...

$(document).bind('statistics.accesslog', function(e, data) {
  data.custom_field = 'foo';
});

Vs.

document.addEventListener('statistics.accesslog', function(event) {
  event.data.custom_field = 'foo';
});

On the one hand, this makes it so that it's totally portable and can be run on page with 0 dependencies. On the other hand, it's very likely that anyone doing advanced Stats/Analytics implementations, especially on a Drupal install, is likely to already be using jQuery. I tend to lean toward the former. Would love to hear feedback from anyone else listening in on this thread.

Done in this chunk

• Giving up on determining cache status from the client-side. Just passing an "UNKN" string to ensure as little data corruption/pollution there as possible,
• Very subtle changes to _better_statistics_update_fields() that allow changes to the Core Statistics/default fields to be written through (and therefore, removes the need for yet another update hook).

Still todo

• Write tests for AJAX data collection of bs('entity_view') implementation,
• Decide whether to explicitly depend on jQuery for event triggering, or write a simple event firing system.

#14: better_statistics-collect_stats_client_side-1960704-14.patch queued for re-testing.

#14: better_statistics-collect_stats_client_side-1960704-14.patch queued for re-testing.

Still an opportunity to change it, but I decided to go with the non-jQuery trigger for maximum portability.

All that's left is to add tests for the entity_view.

Added tests for bs('entity_view')'s callback. Consider this patch a solid release candidate.

Looking for any and all feedback (on the API, the functionality, etc). A quick recap:

To add a JS-implementation of an existing Better Statistics Field, add this to your hook_better_statistics_fields():

function MY_MODULE_better_statistics_fields() {
  $fields['my_field'] = array(
    'schema' => array(...),
    'callback' => 'my_module_statistics_callback',
    'views_field' => array(...),
    'js' => array(
      'data' => 'path/to/my_module/js/my_field.js',
      'type' => 'file',
    ),
  );

  return $fields;
}

Which will auto-include your my_field.js file when needed. In that file, you would do something like this to register your data as it is determined on the client-side:

document.addEventListener('statistics.accesslog', function(event) {
  event.data.my_field = 'My Field whose value was determined on the client side.';
}), false);

Plenty of other goodies and ways to extend Better Statistics, but I'll save those for a doc page (the module is complex enough to warrant one now).

#20: better_statistics-collect_stats_client_side-1960704-20.patch queued for re-testing.

#20: better_statistics-collect_stats_client_side-1960704-20.patch queued for re-testing.

#20: better_statistics-collect_stats_client_side-1960704-20.patch queued for re-testing.

Making better_statistics.js a Library entry via hook_library() to simplify inclusion in other contexts. Also making the bs JS global truly swappable.

Mean to attach the interdiff the second time...

#27: better_statistics-collect_stats_client_side-1960704-27.patch queued for re-testing.

Good catch, mondrake. And excellent usage of the API in #33! It's fundamental enough that I might want to add it in the hook_library instantiation, though. e.g.:

     w[v]('set', 'i', 1 * new Date());
+    w[v]('set', 'basePath', '" . base_path() . "');

I found a more fundamental flaw with the API, though...

I started abstracting accesslog and entity_view (and their prototype definitions) into separate files and separate hook_library entries so you could run drupal_add_library('better_statistics', 'bsjsapi-accesslog') and drupal_add_library('better_statistics', 'bsjsapi-entity_view') independently and have it resolve the dependency for you).

The problem is that due to the async call to better_statistics.js, it's possible that the BetterStatistics global function/object is undefined when one of these files (e.g. better_statistics.accesslog.js) attempts to define a prototype method on BetterStatistics. In many (I've found most) cases, the async call hasn't finished processing yet, so there's an error (like "can't define prototype.accesslog of undefined" or something).

Relatedly, there's little stopping someone from firing bs('custom_callback') between when it runs through all queued requests and when BetterStatistics.prototype.custom_callback is defined.

As such, it's nearly impossible to define useful custom callbacks (or override existing callbacks).

Tons of changes here, building off of #33 and attempting to solve the problems from #35. I'll post an interdiff shortly to make things clear (getting very messy with thousand line patches)...

Done

• The biggest conceptual change is to replace the hook_library() implementation with a set of a few other functions. To add a new Statistics method (e.g. accesslog, entity_view, what have you), you'd call better_statistics_add_method(), with a file to your method's JS implementation. That function statically holds on to all added methods. On every page, Better Statistics will attempt to load all added methods. If methods have been added, it will take them and wrap them between better_statistics.init.js and better_statistics.exec.js (where the BetterStatistics.prototype.accesslog, etc. definitions existed previously), and build a cached JS file as Drupal normally does when JS preprocessing is turned on. This file is then used as the BS JS API file. Convoluted, but at least has an easy-to-understand interface for developers; also, only the methods that are needed on a given page are added (this may not be desirable, now that I think about it; see the first "to be done" item below).
• Additionally, I broke out the node hit statistics from within the accesslog conditional so that they could be invoked independent of the accesslog. Added a radio override to it in the admin.inc.
• Also, @mondrake, I moved your bs('set') method from #33 to the new location, as I mentioned in #34.

To be done

• Rather than better_statistics_add_method(), should it be more like a hook_statistics_method_library, that generates a single cached JS file for all pages? Seems weird if MY_MODULE defines bs('my_module'), and it's available on some pages and not others. Not the most intuitive behavior.
• Seems like the other bs('set') method in the preprocess HTML should be more tightly coupled with the path field (e.g. added in its hook_better_statistics_fields() JS entry), which would require the ability to add an arbitrary number of script entries per field. Not crucial now, since the core fields can't be disabled, but it's possible other modules will want to use similar functionality.
• Add tests for content hit statistics admin modifications
• Add hook_uninstall() changes for content hit statistics (like the ones already added for accesslog),

Interdiff of #36, for reference.

Done

• Decided to go with a hook_better_statistics_methods() approach rather than a better_statistics_add_method() approach. The BS JS API will be available on all pages with all declared methods, now. All the settings do is trigger their default usages.
• Fixed tests checking for BS JS API presence.

To be done

• Add tests for content statistics administration changes
• Add hook_uninstall goodies for content statistics variable

Okay. Cleaning up uninstall and tests. Here's another "release candidate" patch.

From a browser that doesn't implement Navigation Timing:

ReferenceError: Can't find variable: performance

How about...

window.performance = window.performance || {};

Thanks, mondrake.

re #1: Totally agree. Maybe something like the following?

• Do not log access statistics
• Log all requests to Drupal (server-side)
• Log HTML pageviews only (client-side only)
• Log HTML pageviews and other requests to Drupal (mixed mode)

Where the above is true if we assume "to Drupal" means "to the Drupal codebase and not a reverse proxy cache or a statically cached HTML page."

You might also think of it in terms of capabilities, but these would be unsuitable for a checkbox label. Maybe as description text below?

• No access statistics available in Drupal
• Suitable for simple Drupal installations; not suitable for tracking page visits if you use Boost or a reverse proxy cache like Varnish
• Suitable for tracking page visits; not suitable for tracking and blocking abusive robots
• Can track abusive bots AND page visits, but data can be less consistent

I think in a 2.x version of the API, I'll want to break off page views from the accesslog as its own API method.

re #2: An interesting thought, but I think it poses a few problems. Namely, as soon as the cookie is set, I believe strange things will start happening with caching (since Drupal varies on cookie). Additionally, this assumes the request makes it back to Drupal; what about Varnish/Boost/etc?

Might be acceptable. How about this?

As per #2, since I would very much like it, how about we open up a follow-up / feature request to try and get it working?

Excellent! As always, thanks very much for your feedback and reviews, mondrake!

Committed 1603dac.

Follow-up created here: #2019937: Detect cache status from the client-side