From a1b1ffe1e47bec3f1acf8a49827ccf2d7e657f5c Mon Sep 17 00:00:00 2001 From: Rishi Gupta Date: Tue, 7 Feb 2017 23:04:10 -0800 Subject: [PATCH] analytics: Base default views end_time on FillState, not current time. --- .../commands/populate_analytics_db.py | 12 +++++++++-- analytics/models.py | 9 ++++++++ analytics/tests/test_views.py | 16 +++++++++++++- analytics/views.py | 21 +++++++++++++------ 4 files changed, 49 insertions(+), 9 deletions(-) diff --git a/analytics/management/commands/populate_analytics_db.py b/analytics/management/commands/populate_analytics_db.py index 64811eb1db1de..3bcaafd1babcb 100644 --- a/analytics/management/commands/populate_analytics_db.py +++ b/analytics/management/commands/populate_analytics_db.py @@ -5,11 +5,11 @@ from django.core.management.base import BaseCommand from django.utils import timezone -from analytics.models import BaseCount, InstallationCount, RealmCount, \ - UserCount, StreamCount from analytics.lib.counts import COUNT_STATS, CountStat, do_drop_all_analytics_tables from analytics.lib.fixtures import generate_time_series_data from analytics.lib.time_utils import time_range +from analytics.models import BaseCount, InstallationCount, RealmCount, \ + UserCount, StreamCount, FillState from zerver.lib.timestamp import floor_to_day from zerver.models import Realm, UserProfile, Stream, Message, Client @@ -76,6 +76,8 @@ def insert_fixture_data(stat, fixture_data, table): 'true': self.generate_fixture_data(stat, .01, 0, 1, 0, 1) } # type: Dict[Optional[str], List[int]] insert_fixture_data(stat, realm_data, RealmCount) + FillState.objects.create(property=stat.property, end_time=last_end_time, + state=FillState.DONE) stat = COUNT_STATS['messages_sent:is_bot:hour'] user_data = {'false': self.generate_fixture_data(stat, 2, 1, 1.5, .6, 8, holiday_rate=.1)} @@ -83,6 +85,8 @@ def insert_fixture_data(stat, fixture_data, table): realm_data = {'false': self.generate_fixture_data(stat, 35, 15, 6, .6, 4), 'true': self.generate_fixture_data(stat, 15, 15, 3, .4, 2)} insert_fixture_data(stat, realm_data, RealmCount) + FillState.objects.create(property=stat.property, end_time=last_end_time, + state=FillState.DONE) stat = COUNT_STATS['messages_sent:message_type:day'] user_data = { @@ -94,6 +98,8 @@ def insert_fixture_data(stat, fixture_data, table): 'private_stream': self.generate_fixture_data(stat, 7, 7, 5, .6, 4), 'private_message': self.generate_fixture_data(stat, 13, 5, 5, .6, 4)} insert_fixture_data(stat, realm_data, RealmCount) + FillState.objects.create(property=stat.property, end_time=last_end_time, + state=FillState.DONE) website_ = Client.objects.create(name='website_') API_ = Client.objects.create(name='API_') @@ -119,5 +125,7 @@ def insert_fixture_data(stat, fixture_data, table): barnowl_.id: self.generate_fixture_data(stat, 1, 1, 3, .6, 3), plan9_.id: self.generate_fixture_data(stat, 0, 0, 0, 0, 0, 0)} insert_fixture_data(stat, realm_data, RealmCount) + FillState.objects.create(property=stat.property, end_time=last_end_time, + state=FillState.DONE) # TODO: messages_sent_to_stream:is_bot diff --git a/analytics/models.py b/analytics/models.py index 92257b8f5daf2..dc6d0c66e01c7 100644 --- a/analytics/models.py +++ b/analytics/models.py @@ -31,6 +31,15 @@ def installation_epoch(): earliest_realm_creation = Realm.objects.aggregate(models.Min('date_created'))['date_created__min'] return floor_to_day(datetime_to_UTC(earliest_realm_creation)) +def last_successful_fill(property): + # type: (str) -> Optional[datetime.datetime] + fillstate = FillState.objects.filter(property=property).first() + if fillstate is None: + return None + if fillstate.state == FillState.DONE: + return fillstate.end_time + return fillstate.end_time - datetime.timedelta(hours=1) + # would only ever make entries here by hand class Anomaly(ModelReprMixin, models.Model): info = models.CharField(max_length=1000) # type: Text diff --git a/analytics/tests/test_views.py b/analytics/tests/test_views.py index a85ee45c39359..80bff5828336a 100644 --- a/analytics/tests/test_views.py +++ b/analytics/tests/test_views.py @@ -1,8 +1,9 @@ -from django.utils.timezone import get_fixed_timezone +from django.utils.timezone import get_fixed_timezone, utc from zerver.lib.test_classes import ZulipTestCase from analytics.lib.counts import CountStat from analytics.lib.time_utils import time_range +from analytics.models import FillState, last_successful_fill from analytics.views import rewrite_client_arrays from datetime import datetime, timedelta @@ -60,3 +61,16 @@ def test_map_arrays(self): 'SomethingRandom': [4, 5, 6], 'GitHub webhook': [7, 7, 9], 'Android app': [64, 63, 65]}) + +class TestGetChartData(ZulipTestCase): + def test_last_successful_fill(self): + # type: () -> None + self.assertIsNone(last_successful_fill('non-existant')) + a_time = datetime(2016, 3, 14, 19).replace(tzinfo=utc) + one_hour_before = datetime(2016, 3, 14, 18).replace(tzinfo=utc) + fillstate = FillState.objects.create(property='property', end_time=a_time, + state=FillState.DONE) + self.assertEqual(last_successful_fill('property'), a_time) + fillstate.state = FillState.STARTED + fillstate.save() + self.assertEqual(last_successful_fill('property'), one_hour_before) diff --git a/analytics/views.py b/analytics/views.py index ce7f490601a6a..dd755847c0676 100644 --- a/analytics/views.py +++ b/analytics/views.py @@ -13,7 +13,7 @@ from analytics.lib.counts import CountStat, process_count_stat, COUNT_STATS from analytics.lib.time_utils import time_range from analytics.models import BaseCount, InstallationCount, RealmCount, \ - UserCount, StreamCount + UserCount, StreamCount, last_successful_fill from zerver.decorator import has_request_variables, REQ, zulip_internal, \ zulip_login_required, to_non_negative_int, to_utc_datetime @@ -28,6 +28,7 @@ from datetime import datetime, timedelta import itertools import json +import logging import pytz import re import time @@ -75,15 +76,23 @@ def get_chart_data(request, user_profile, chart_name=REQ(), else: raise JsonableError(_("Unknown chart name: %s") % (chart_name,)) + # Most likely someone using our API endpoint. The /stats page does not + # pass a start or end in its requests. + if start is not None and end is not None and start > end: + raise JsonableError(_("Start time is later than end time. Start: %(start)s, End: %(end)s") % + {'start': start, 'end': end}) + realm = user_profile.realm - # These are implicitly relying on realm.date_created and timezone.now being in UTC. if start is None: start = realm.date_created if end is None: - end = timezone.now() - if start > end: - raise JsonableError(_("Start time is later than end time. Start: %(start)s, End: %(end)s") % - {'start': start, 'end': end}) + end = last_successful_fill(stat.property) + if end is None or start > end: + logging.warning("User from realm %s attempted to access /stats, but the computed " + "start time, %s (creation date of realm) is later than the computed " + "end time, %s (last successful analytics update). Is the " + "update_analytics_counts cron job running?" % (realm.string_id, start, end)) + raise JsonableError(_("No analytics data available. Please contact your server administrator.")) end_times = time_range(start, end, stat.frequency, min_length) data = {'end_times': end_times, 'frequency': stat.frequency, 'interval': stat.interval}