Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

push-bouncer: Exclude LoggingCountStats with partial data. 

LoggingCountStats with a daily duration and that are directly stored
on the RealmCount table (not via aggregation in process_count_stat),
can be in a state, after the hourly cron job to update analytics
counts, where the logged value will be live-updated later, because
the end time for the stat is still in the future.

As these logging counts are designed to be used on the self-hosted
installation for either debugging or rate limiting, sending these
partial/incomplete counts to the bouncer has low value.

(cherry picked from commit 40daf10a74)
This commit is contained in:
Lauryn Menard 2024-02-26 21:14:52 +01:00 committed by Tim Abbott
parent b4f95a54e1
commit 85bd8dd33d
3 changed files with 67 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
analytics/lib/counts.py
View File

@ -797,6 +797,7 @@ def get_count_stats(realm: Optional[Realm] = None) -> Dict[str, CountStat]:
# In RealmCount, 'active_users_audit:is_bot:day' should be the partial
# sum sequence of 'active_users_log:is_bot:day', for any realm that
# started after the latter stat was introduced.
# Included in LOGGING_COUNT_STAT_PROPERTIES_NOT_SENT_TO_BOUNCER.
LoggingCountStat("active_users_log:is_bot:day", RealmCount, CountStat.DAY),
# Another sanity check on 'active_users_audit:is_bot:day'. Is only an
# approximation, e.g. if a user is deactivated between the end of the
@ -843,13 +844,15 @@ def get_count_stats(realm: Optional[Realm] = None) -> Dict[str, CountStat]:
"minutes_active::day", DataCollector(UserCount, do_pull_minutes_active), CountStat.DAY
),
# Tracks the number of push notifications requested by the server.
# Included in LOGGING_COUNT_STAT_PROPERTIES_NOT_SENT_TO_BOUNCER.
LoggingCountStat(
"mobile_pushes_sent::day",
RealmCount,
CountStat.DAY,
),
# Rate limiting stats
# Used to limit the number of invitation emails sent by a realm
# Used to limit the number of invitation emails sent by a realm.
# Included in LOGGING_COUNT_STAT_PROPERTIES_NOT_SENT_TO_BOUNCER.
LoggingCountStat("invites_sent::day", RealmCount, CountStat.DAY),
# Dependent stats
# Must come after their dependencies.
@ -890,6 +893,19 @@ BOUNCER_ONLY_REMOTE_COUNT_STAT_PROPERTIES = [
"mobile_pushes_forwarded::day",
]
# LoggingCountStats with a daily duration and that are directly stored on
# the RealmCount table (instead of via aggregation in process_count_stat),
# can be in a state, after the hourly cron job to update analytics counts,
# where the logged value will be live-updated later (as the end time for
# the stat is still in the future). As these logging counts are designed
# to be used on the self-hosted installation for either debugging or rate
# limiting, sending these incomplete counts to the bouncer has low value.
LOGGING_COUNT_STAT_PROPERTIES_NOT_SENT_TO_BOUNCER = [
"invites_sent::day",
"mobile_pushes_sent::day",
"active_users_log:is_bot:day",
]
# To avoid refactoring for now COUNT_STATS can be used as before
COUNT_STATS = get_count_stats()

5
zerver/lib/remote_server.py
View File

@ -9,6 +9,7 @@ from django.db.models import QuerySet
from django.utils.translation import gettext as _
from pydantic import UUID4, BaseModel, ConfigDict, Field, Json, field_validator
from analytics.lib.counts import LOGGING_COUNT_STAT_PROPERTIES_NOT_SENT_TO_BOUNCER
from analytics.models import InstallationCount, RealmCount
from version import API_FEATURE_LEVEL, ZULIP_VERSION
from zerver.actions.realm_settings import (
@ -364,8 +365,10 @@ def send_server_data_to_push_bouncer(consider_usage_statistics: bool = True) ->
# uploading such statistics enabled.
installation_count_query = InstallationCount.objects.filter(
id__gt=last_acked_installation_count_id
).exclude(property__in=LOGGING_COUNT_STAT_PROPERTIES_NOT_SENT_TO_BOUNCER)
realm_count_query = RealmCount.objects.filter(id__gt=last_acked_realm_count_id).exclude(
property__in=LOGGING_COUNT_STAT_PROPERTIES_NOT_SENT_TO_BOUNCER
)
realm_count_query = RealmCount.objects.filter(id__gt=last_acked_realm_count_id)
else:
installation_count_query = InstallationCount.objects.none()
realm_count_query = RealmCount.objects.none()

65
zerver/tests/test_push_notifications.py
View File

@ -23,7 +23,7 @@ from requests.models import PreparedRequest
from typing_extensions import override
from analytics.lib.counts import CountStat, LoggingCountStat
from analytics.models import InstallationCount, RealmCount
from analytics.models import InstallationCount, RealmCount, UserCount
from corporate.models import CustomerPlan
from version import ZULIP_VERSION
from zerver.actions.message_delete import do_delete_messages
@ -1478,17 +1478,35 @@ class AnalyticsBouncerTest(BouncerTestCase):
)
# Create some rows we'll send to remote server
realm_stat = LoggingCountStat("invites_sent::day", RealmCount, CountStat.DAY)
RealmCount.objects.create(
realm=user.realm, property=realm_stat.property, end_time=end_time, value=5
# LoggingCountStat that should be included;
# i.e. not in LOGGING_COUNT_STAT_PROPERTIES_NOT_SENT_TO_BOUNCER
messages_read_logging_stat = LoggingCountStat(
"messages_read::hour", UserCount, CountStat.HOUR
)
InstallationCount.objects.create(
property=realm_stat.property,
RealmCount.objects.create(
realm=user.realm,
property=messages_read_logging_stat.property,
end_time=end_time,
value=5,
)
InstallationCount.objects.create(
property=messages_read_logging_stat.property,
end_time=end_time,
value=5,
)
# LoggingCountStat that should not be included;
# i.e. in LOGGING_COUNT_STAT_PROPERTIES_NOT_SENT_TO_BOUNCER
invites_sent_logging_stat = LoggingCountStat("invites_sent::day", RealmCount, CountStat.DAY)
RealmCount.objects.create(
realm=user.realm,
property=invites_sent_logging_stat.property,
end_time=end_time,
value=5,
)
InstallationCount.objects.create(
property=invites_sent_logging_stat.property,
end_time=end_time,
value=5,
# We set a subgroup here to work around:
# https://github.com/zulip/zulip/issues/12362
subgroup="test_subgroup",
)
# Event type in SYNCED_BILLING_EVENTS -- should be included
RealmAuditLog.objects.create(
@ -1510,8 +1528,8 @@ class AnalyticsBouncerTest(BouncerTestCase):
event_time=end_time,
extra_data=orjson.dumps({"foo": "bar"}).decode(),
)
self.assertEqual(RealmCount.objects.count(), 1)
self.assertEqual(InstallationCount.objects.count(), 1)
self.assertEqual(RealmCount.objects.count(), 2)
self.assertEqual(InstallationCount.objects.count(), 2)
self.assertEqual(RealmAuditLog.objects.filter(id__gt=audit_log_max_id).count(), 2)
with self.settings(SUBMIT_USAGE_STATISTICS=False):
@ -1686,13 +1704,13 @@ class AnalyticsBouncerTest(BouncerTestCase):
# Test only having new RealmCount rows
RealmCount.objects.create(
realm=user.realm,
property=realm_stat.property,
property=messages_read_logging_stat.property,
end_time=end_time + timedelta(days=1),
value=6,
)
RealmCount.objects.create(
realm=user.realm,
property=realm_stat.property,
property=messages_read_logging_stat.property,
end_time=end_time + timedelta(days=2),
value=9,
)
@ -1701,7 +1719,9 @@ class AnalyticsBouncerTest(BouncerTestCase):
# Test only having new InstallationCount rows
InstallationCount.objects.create(
property=realm_stat.property, end_time=end_time + timedelta(days=1), value=6
property=messages_read_logging_stat.property,
end_time=end_time + timedelta(days=1),
value=6,
)
send_server_data_to_push_bouncer()
check_counts(8, 8, 3, 2, 7)
@ -1820,12 +1840,17 @@ class AnalyticsBouncerTest(BouncerTestCase):
end_time = self.TIME_ZERO
# Create some rows we'll send to remote server
realm_stat = LoggingCountStat("invites_sent::day", RealmCount, CountStat.DAY)
messages_read_logging_stat = LoggingCountStat(
"messages_read::hour", UserCount, CountStat.HOUR
)
realm_count = RealmCount.objects.create(
realm=user.realm, property=realm_stat.property, end_time=end_time, value=5
realm=user.realm,
property=messages_read_logging_stat.property,
end_time=end_time,
value=5,
)
installation_count = InstallationCount.objects.create(
property=realm_stat.property,
property=messages_read_logging_stat.property,
end_time=end_time,
value=5,
)
@ -1917,12 +1942,14 @@ class AnalyticsBouncerTest(BouncerTestCase):
# set as it should.
RealmCount.objects.create(
realm=user.realm,
property=realm_stat.property,
property=messages_read_logging_stat.property,
end_time=end_time + timedelta(days=1),
value=6,
)
InstallationCount.objects.create(
property=realm_stat.property, end_time=end_time + timedelta(days=1), value=6
property=messages_read_logging_stat.property,
end_time=end_time + timedelta(days=1),
value=6,
)
RealmAuditLog.objects.create(
realm=user.realm,