Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sparklines and stat for Page views, Downloads and Outlinks separately #1454

Closed
mattab opened this issue Jun 29, 2010 · 53 comments
Closed
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone

Comments

@mattab
Copy link
Member

mattab commented Jun 29, 2010

Currently Piwik reports total number of actions, which includes page views, downloads and outlinks.

While this stat is useful, it would also make sense to report specifically about each action, and have a sparkline+stat for each of the following

  • Page views
  • Downloads
  • Clicks on outlinks
@anonymous-matomo-user
Copy link

It will by useful!

@mattab
Copy link
Member Author

mattab commented Nov 24, 2010

Very useful indeed, comparing pages such as marsa-alam.htm and standard downloads / clicks is useful.

@mattab
Copy link
Member Author

mattab commented Mar 18, 2011

Also in the API we should return nb_actions and nb_downloads nb_outlinks nb_hits

@mattab
Copy link
Member Author

mattab commented Sep 17, 2011

Pretty useful and must have!

@timo-bes
Copy link
Member

While working on this, I found a tracker bug which was visible in the ecommerceOrderWithItems testcase. I'll document it now and commit the fix/tests later.

The test case works as follows (only relevant parts):

  • Visitor views 4 pages in a visit
  • In a second visit, he views 6 pages (3 above the loop + 3 in the first loop with offset=0)
  • In a third visit, he views 3 pages (in the loop with offset=5)
  • In a fourth visit, he views 3 more pages (in the loop with offset=24)
  • After the loop, an order is placed with a time offset that leads to a fifth visit. Another order is placed in that visit.
  • Then, an order is placed for another idSite, which leads to a sixth visit

Number of actions should be 4 / 6 / 3 / 3 / 0 / 0, since actions are the sum of pageviews, outlinks and downloads. The computed actions are 4 / 6 / 3 / 4 / 1 / 1. This is because the fourth visit starts with doTrackEcommerceOrder and the fifth and sixth start with doTrackEcommerceOrder. All the others start with doTrackPageView. When a new visit is created, visit_total_actions is set to 1, which is only correct if the first thing that is tracked is an action.

The fix is to check whether we are tracking an action and set visit_total_actions to 0 or 1. I have this fix in my working copy. It will be commited with the other tests for this ticket.

@timo-bes
Copy link
Member

This ticket requires some schema changes: visit_total_(hits|downloads|outlinks) in log_visit.

I added them to core/Db/Schema/Myisam.php and created an Update 1.6-rc2.php in core/Updates. This doesn't trigger the updater. What do I have to do?

@mattab
Copy link
Member Author

mattab commented Oct 19, 2011

To trigger update, change core/Version.php to a new version number, then create the update file for this version number. It will trigger the update.

But in this case you dont need new fields in the schema, which creates performance overhead. To process page/downloads/outlinks, there is a simpler way and also that is more efficient. You can get the total number of downloads/oulinks/page views after processing the datatables in: https://github.com/piwik/piwik/blob/master/plugins/Actions/Actions.php#L463

This is similar to what is done in the Referers plugins to process the number of referers from each type (campaign, websites,etc.): https://github.com/piwik/piwik/blob/master/plugins/Referers/Referers.php#L477

@mattab
Copy link
Member Author

mattab commented Oct 19, 2011

The fix is to check whether we are tracking an action and set visit_total_actions to 0 or 1. I have this fix in my working copy. It will be commited with the other tests for this ticket.

It sounds good!

@mattab
Copy link
Member Author

mattab commented Oct 19, 2011

To get the downloads/outlinks/actions sum, for each table, you could do something like

$downloads = array_sum($dataTable->getColumn(Piwik_Archive::INDEX_PAGE_NB_HITS)) ?

@timo-bes
Copy link
Member

Thanks for the feedback. I still think it would be good to add hits/downloads/outlinks to the schema.

  1. It doesn't really create performance overhead. Increasing the counts is done in the same query as increasing the number of actions.
  2. It's faster to aggregate. The aggregation for archiving takes place in the same query as the other VisitsSummary metrics. On the other hand, the sum of rows (one row being one url of which there can be a lot) will be calculated for 30 days to display a sparkline.
  3. Using the Actions archives would be a break in concept. All the other VisitsSummary metrics are calculated in one query and don't depend on the other plugins (since they are the core metrics). Loading the Actions report in the VisitsSummary API would look a little clumsy, wouldn't it?

These are the reasons why I would stick with this approach. If you're still conviced it would be better another way, I'll refactor. A compromise could be to add another report to the Actions plugin that returns the sums. This would partition related metrics in two reports (the ones we are talking about and the VisitsSummary) but it's still better than loading the Actions report in the VisitsSummary plugin. Nevertheless, I'd stick with counting the hits/downloads/outlinks in the database.

@mattab
Copy link
Member Author

mattab commented Oct 20, 2011

The performance overhead is because we add 3 fields to the log_visit table, which cause overhead of 8b * N visits. Not much, but log_* tables must stay as small as possible.

Of course you are right that it is not ideal because VisitsSummary would then required Actions archives to display all sparklines. I think this limitation is preferred than adding fields to log_visit.

The Downloads/outlinks/pages metrics would be aggregated during archiving (as a "numeric" archive). So, for a sparkline of 30 days, the performance is the same as if you would process these metrics in VisitsSummary. Indeed it is important that all metrics are pre-processed. So the array_sum() would be during archiving only (when the datatable have just been processed). Then VisitsSummary controller would call Actions_API::getDownloadsCount for example. Does it sound good?

@timo-bes
Copy link
Member

How do we add this to the UI? We could just add three more sparklines in the visitors overview or we could introduce some kind of groups in the overview. Any ideas?

Also, it would be cool to display pageviews and unique pageviews (same for downloads and outlinks) in one graph. I think we don't need a separate sparkline for the unique metrics, this would probably be too much.

@timo-bes
Copy link
Member

(In [5386]) refs #1454 fix for tracking of nb_actions: only track pageviews, outlinks and downloads

@timo-bes
Copy link
Member

(In [5387]) refs #1454 archiving (unique) pageviews, (unique) downloads and (unique) outlinks

@timo-bes
Copy link
Member

The unit tests don't run on my machine so I can't add the new expected API output. Is this only happening locally or are the tests broken in trunk?

@timo-bes
Copy link
Member

(In [5388]) refs #1454 adding pageviews, downloads and outlinks to the ui

@timo-bes
Copy link
Member

[5388] is a first attempt at the UI. I would like to make it similar to row evolution but that's hard to do for 12 metrics... Maybe someone has an idea!?

@timo-bes
Copy link
Member

(In [5389]) refs #1454 ui improvements

@timo-bes
Copy link
Member

(In [5390]) refs #1454 ui improvements (this time from the right working copy)

@timo-bes
Copy link
Member

(In [5391]) refs #1454 ui improvements (revertig 5389)

@timo-bes
Copy link
Member

In [I commited the updated files from the wrong working copy. The changes are reverted in 5391. The actual UI improvements are in [5390]: I made the visits overview more compact by combining metrics.

@mattab
Copy link
Member Author

mattab commented Oct 30, 2011

(In [5392]) refs #1454 Updating expected test files after adding few fields in the VisitsSummary output

@mattab
Copy link
Member Author

mattab commented Oct 30, 2011

Feedback:

  • Great idea to display and plot both metrics :)
  • Backward compatibility UI: the "pageviews" metrics should display the old "Actions" count if the pageviews count is not available (ie. for dates before the upgrade to piwik 1.7). I realize this is NOT consistent and partly incorrect (but only when there were downloads/outlinks). It is much better than display zeros for all previous data. Also, if we do not do that, there would be no way to access the previous page views count, which is a very basic metric that we must make available :)
  • Test should work. On my box, it takes 800s to run tests/integration/Main.test.php - this is VERY painful. However, it should work. Hopefully at some point I will spend some time trying to make tests faster so it's easier for developers...

@timo-bes
Copy link
Member

Regarding backward compatibility: could we manipulate the existing archives to add the new metrics during the update? We would just have to sum the values for all archived action tables and would not need to use the logs.

@mattab
Copy link
Member Author

mattab commented Oct 30, 2011

Probably doable, but would potentially take a long time to select all archives (potentially thousands) and update it...

I think it's OK to leave the old data as it is but just make sure the UI uses Nb_Actions when available, if nb_pageviews is not available for a given day?

@timo-bes
Copy link
Member

(In [5393]) refs #1454 new action metrics are only accessed by actions plugin, bounce rate fix, changed color of second line chart series, updated expected test files

@timo-bes
Copy link
Member

(In [5394]) refs #1454 backward compatibility: show nb_actions if finer metrics are not available, german translation

@mattab
Copy link
Member Author

mattab commented Oct 31, 2011

(In [5395]) refs #1454

  • rename API to Actions.get
  • EN translation fix
  • tests update, fix build?

@timo-bes
Copy link
Member

(In [5396]) refs #1454 making german and english translations consistent

@mattab
Copy link
Member Author

mattab commented Nov 1, 2011

(In [5399]) Fixing more test refs #1454

@timo-bes
Copy link
Member

timo-bes commented Nov 4, 2011

(In [5406]) refs #1454 different csv encoding on the build server - again

@timo-bes
Copy link
Member

timo-bes commented Nov 4, 2011

i added a file to integration/processed by mistake. should it be in svn-ignore?

@timo-bes
Copy link
Member

timo-bes commented Nov 4, 2011

(In [5407]) refs #1454 removing mistakenly added processed integration tests

@timo-bes
Copy link
Member

timo-bes commented Nov 4, 2011

(In [5408]) refs #1820 metrics picker, refs #1454 using Api.get for cross-plugin evolution graphs

@timo-bes
Copy link
Member

timo-bes commented Nov 4, 2011

(In [5409]) refs #1454 report meta data update

@timo-bes
Copy link
Member

timo-bes commented Nov 4, 2011

The visitors overview now works without the ugly cross-plugin API calls. It uses the brand new API.get which combines the *.get methods.

Also, there is a metrics picker in the visitors overview evolution graph that makes all metrics from VisitsSummary.get and Actions.get selectable.

@timo-bes
Copy link
Member

timo-bes commented Nov 4, 2011

Replying to EZdesign:

i added a file to integration/processed by mistake. should it be in svn-ignore?

I think it is in svn-ignore, isn't it? It just wasn't ignored by my git-svn setup.

@timo-bes
Copy link
Member

timo-bes commented Nov 4, 2011

(In [5410]) refs #1454 export and dashboard widget fix

@timo-bes
Copy link
Member

timo-bes commented Nov 8, 2011

(In [5414]) refs #1454 API.get has no plugin prefix for metrics anymore

@mattab
Copy link
Member Author

mattab commented Nov 9, 2011

(In [5416]) Fixes #1454

  • By default, show all columns merged
  • thanks timo for last patches now looks consistent

@timo-bes
Copy link
Member

timo-bes commented Nov 9, 2011

(In [5418]) refs #1454 now that API.get shows all metrics by default, we can export the full report when the export icon below the graph is clicked

@timo-bes
Copy link
Member

timo-bes commented Nov 9, 2011

(In [5419]) refs #2714 additional column translations and number format fix for csv exports, refs #1454 metadata for API.get report (needed for csv column translations)

@timo-bes
Copy link
Member

timo-bes commented Nov 9, 2011

(In [5420]) refs #2714, #1454 test fix for previous commit

@julienmoumne
Copy link
Member

To fix a bug I'm having in the PDFReports plugin, I'd like to understand the purpose of the new API.get mentioned in comment:39.

Could you elaborate on its rationale?

I'm no expert in archiving and data aggregation so don't hesitate to put it in layman's terms.

@timo-bes
Copy link
Member

@julien: API.get combines the get methods of other plugins (e.g. Actions.get / VisitsSummary.get). You can get the data from the *.get reports in a single report. This makes comparing metrics from different .get methods possible, because the comparison takes place on one report only. The method introduces no additional data, it's just a proxy for the other methods. In PDFReports, you probably don't need to worry about it.

@julienmoumne
Copy link
Member

Thanks for the clarification.

PDFReports plugin creation and edit forms use the output of 'API.getReportMetadata' to display the list of available reports.

The new API is returned in 'API.getReportMetadata'. It is therefore selectable when creating/editing mail reports.

Is this expected ?

@timo-bes
Copy link
Member

Since the report doesn't contain new data it probably souldn't be selectable in the PDFReports plugin. If you want, take a look at the API output and see for yourself...

@mattab
Copy link
Member Author

mattab commented Nov 10, 2011

julien, please add an exception to exclude this report from the PDF/Email list. thx!

@julienmoumne
Copy link
Member

(In [5436]) refs #2706, #1454 API category reports excluded from PDFReports Plugin forms

@mattab
Copy link
Member Author

mattab commented Dec 16, 2012

Milestone 1.8.x Piwik 1.8.x deleted

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Development

No branches or pull requests

4 participants