997465 - All the stats on AMO are empty

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Statistics, defect, P1)

Description

[Andy McKay]

https://addons.mozilla.org/en-US/statistics/reviews_created/?last=30

Where did they go?

Comment 1

[Kris Maglione [:kmag]]

These stats pages are kind of critically important. This needs to be fixed ASAP.

Comment 3

[David Larlet [:davidbgk]]

I don't know how that bug can be related to/fixed by that one: https://bugzilla.mozilla.org/show_bug.cgi?id=1014752

Comment 4

[Christopher Van Wiemeersch [:cvan]]

https://addons.mozilla.org/en-US/statistics/addons_updated-day-20130624-20140624.csv contains real data
https://addons.mozilla.org/en-US/statistics/addons_updated-day-20130624-20140624.json appears completely blank

Comment 5

[David Larlet [:davidbgk]]

The second link doesn't look to be blank (anymore?), can it be related to a cache issue?

Comment 6

[Yohan Boniface [:ybon]]

I don't get it all clear.
Here is a summary of what I understand atm.

The short version is that we send counts as strings in our json views, but the JavaScript expects integers.

In a bit more details:

1. JavaScript side, z.StatsManager.getField only wants integers as values, and this is since a while (https://github.com/mozilla/olympia/blame/master/media/js/impala/stats/manager.js#L528)

2. Views.py side, the function _site_query calls a SQL that makes a SUM(count), which ends in a decimal.Decimal value (see https://code.djangoproject.com/ticket/17796), and this also seems to be the case since a long time (https://github.com/mozilla/olympia/blame/master/apps/stats/tests/test_views.py#L773)

3. When we dump our stats to json, we use the DjangoJSONSerializer, which maps a decimal.Decimal to a string (since a long time too https://github.com/django/django/blame/master/django/core/serializers/json.py#L106)

4. The only related thing that changed recently (and the timing corresponds) I can see, is during the Django 1.6 upgrade, when we switched from json to simplejson (https://github.com/mozilla/olympia/commit/5ab4b4a16d96007e9f859c6746495f880aa6aa88#diff-feec04c1d8b07338fc9ba9236dd7f048R714). But I don't connect all the hints in a clear explanation, so it may not be this at all.

In two words, by default, json doesn't serialize decimal.Decimal, while simplejson does serialize it to an integer. But we are using DjangoJSONSerializer, that in any case take control of the process. And it's a bit hard to reproduce the exact configuration of olympia two months ago: many db changes, libs upgrade, Django 1.4=>1.6, etc., and I'm not sure it worth it spending more time on it at this point.

So again, I don't connect all the elements, so I'm missing something.

Now, casting counts to int, instead of decimal.Decimal, does fix the problem in local.

So at this point, I'm tempted to push that.

Thoughts?

Comment 7

[Yohan Boniface [:ybon]]

PR: https://github.com/mozilla/olympia/pull/135

Comment 8

[Mathieu Agopian [:magopian]]

fixed in https://github.com/mozilla/olympia/commit/f1427505f989516a8ff5a41253af7e25e0ec41ac

Comment 9

[Victor Carciu]

Verified as fixed with https://addons.allizom.org/en-US/statistics/reviews_created/?last=365

Using last 30 days will not work because of https://bugzilla.mozilla.org/show_bug.cgi?id=1036947