Skip to content

Commit 0315383

Browse files
authored
Merge pull request #11823 from CenterForOpenScience/feature/ENG-11735-record-download-events
[ENG-11735] | record download events
2 parents 9cda5cc + 65c87bb commit 0315383

5 files changed

Lines changed: 481 additions & 5 deletions

File tree

addons/base/views.py

Lines changed: 75 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,11 @@
5252
DraftRegistration,
5353
Guid,
5454
FileVersionUserMetadata,
55-
FileVersion, NotificationTypeEnum
55+
FileVersion, NotificationTypeEnum,
56+
DownloadEvent,
5657
)
5758
from osf.utils import permissions
59+
from osf.utils.download_telemetry import never_breaks_downloads, record_download
5860
from osf.external.gravy_valet import request_helpers
5961
from website.profile.utils import get_profile_image_url
6062
from website.project import decorators
@@ -181,6 +183,71 @@ def _download_is_from_mfr(waterbutler_data):
181183
)
182184

183185

186+
def _download_request_is_from_mfr(query_params):
187+
"""Same question as :func:`_download_is_from_mfr`, asked of a browser request.
188+
189+
Here the render mode is a query param on the request itself rather than something
190+
WaterButler reported to us.
191+
"""
192+
return bool(
193+
request.headers.get('X-Cos-Mfr-Render-Request', None) or
194+
query_params.get('mode') == 'render'
195+
)
196+
197+
198+
@never_breaks_downloads
199+
def _record_file_download(target, file_node, query_params, auth, version=None):
200+
"""Record a single-file download from the redirect view.
201+
202+
Only identifiers are handed over — the size, region and materialized path are looked
203+
up in the celery task so the download itself doesn't pay for them.
204+
"""
205+
if _download_request_is_from_mfr(query_params):
206+
return
207+
208+
record_download(
209+
download_type=DownloadEvent.FILE,
210+
resource_guid=getattr(target, '_id', '') or '',
211+
file_id=getattr(file_node, '_id', None),
212+
version_identifier=getattr(version, 'identifier', None),
213+
user_guid=getattr(getattr(auth, 'user', None), '_id', None),
214+
ip=request.remote_addr,
215+
source_area=query_params.get('source', ''),
216+
tz=query_params.get('tz', ''),
217+
)
218+
219+
220+
@never_breaks_downloads
221+
def _record_zip_download(payload):
222+
"""Record a folder or project zip from the WaterButler callback.
223+
224+
Zips are requested straight from WaterButler, so this callback is the only point at
225+
which we hear about them. The user's IP and the ``source``/``tz`` link tags are
226+
forwarded to us in ``action_meta``.
227+
"""
228+
metadata = payload.get('metadata') or {}
229+
action_meta = payload.get('action_meta') or {}
230+
231+
if action_meta.get('is_mfr_render'):
232+
return
233+
234+
materialized = metadata.get('materialized') or metadata.get('path') or ''
235+
# The provider root is the whole project; anything below it is one folder.
236+
is_whole_project = not materialized.strip('/')
237+
238+
record_download(
239+
download_type=DownloadEvent.PROJECT if is_whole_project else DownloadEvent.FOLDER_ZIP,
240+
resource_guid=metadata.get('nid') or '',
241+
path=materialized,
242+
size_bytes=action_meta.get('bytes_downloaded'),
243+
zip_completed=action_meta.get('completed'),
244+
user_guid=(payload.get('auth') or {}).get('id'),
245+
ip=action_meta.get('ip'),
246+
source_area=action_meta.get('source', ''),
247+
tz=action_meta.get('tz', ''),
248+
)
249+
250+
184251
def make_auth(user):
185252
if user is not None:
186253
return {
@@ -483,11 +550,11 @@ def create_waterbutler_log(payload, **kwargs):
483550
with transaction.atomic():
484551
try:
485552
auth = payload['auth']
486-
# Don't log download actions
553+
# Downloads produce no NodeLog, but zips are recorded for telemetry here —
554+
# they never pass through the redirect view where single files are caught.
487555
if payload['action'] in DOWNLOAD_ACTIONS:
488-
guid_id = payload['metadata'].get('nid')
489-
490-
node, _ = Guid.load_referent(guid_id)
556+
if payload['action'] == 'download_zip':
557+
_record_zip_download(payload)
491558
return {'status': 'success'}
492559

493560
user = OSFUser.load(auth['id'])
@@ -983,6 +1050,7 @@ def addon_view_or_download_file(auth, path, provider, **kwargs):
9831050
}))
9841051

9851052
if action == 'download':
1053+
_record_file_download(target, file_node, extras, auth, version=version)
9861054
format = extras.get('format')
9871055
_, extension = os.path.splitext(file_node.name)
9881056
# avoid rendering files with the same format type.
@@ -1045,6 +1113,8 @@ def persistent_file_download(auth, **kwargs):
10451113

10461114
query_params = request.args.to_dict()
10471115

1116+
_record_file_download(file.target, file, query_params, auth)
1117+
10481118
return make_response(
10491119
'', http_status.HTTP_302_FOUND, {
10501120
'Location': file.generate_waterbutler_url(**query_params),

osf/migrations/0045_downloadevent.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
'bgeiger@cos.io',
1313
'osmand@cos.io',
1414
'ramya@cos.io',
15+
'eric@cos.io',
1516
]
1617

1718

osf/utils/download_telemetry.py

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
import functools
2+
import logging
3+
4+
from addons.osfstorage.settings import DEFAULT_REGION_NAME
5+
from framework.celery_tasks import app
6+
from framework.postcommit_tasks.handlers import enqueue_postcommit_task
7+
8+
logger = logging.getLogger(__name__)
9+
10+
# Set on every user until they pick something in their profile, so it says nothing
11+
# about where they actually are.
12+
UNSET_USER_TIMEZONE = 'Etc/UTC'
13+
14+
# Identifiers worth having in the log line to track a failure back to one download.
15+
# Deliberately excludes the IP.
16+
LOGGED_CONTEXT_KEYS = ('download_type', 'resource_guid', 'file_id', 'user_guid')
17+
18+
19+
def never_breaks_downloads(fn):
20+
"""Swallow and log anything this raises.
21+
22+
Wraps the whole capture, not just the write — gathering the values is as capable of
23+
raising as storing them is, and neither is a reason for a download to fail.
24+
"""
25+
@functools.wraps(fn)
26+
def wrapped(*args, **kwargs):
27+
try:
28+
return fn(*args, **kwargs)
29+
except Exception as exc:
30+
# exc_info carries the traceback; the rest names the failure and which
31+
# download it was, so a report is actionable without reproducing it.
32+
logger.exception(
33+
'Failed to record a download event in %s: %s: %s [%s]',
34+
fn.__name__,
35+
type(exc).__name__,
36+
exc,
37+
', '.join(
38+
f'{key}={kwargs[key]!r}'
39+
for key in LOGGED_CONTEXT_KEYS
40+
if kwargs.get(key)
41+
) or 'no context',
42+
)
43+
return wrapped
44+
45+
46+
@never_breaks_downloads
47+
def record_download(**kwargs):
48+
"""Enqueue a :class:`DownloadEvent` write."""
49+
enqueue_postcommit_task(write_download_event, (), kwargs, celery=True)
50+
51+
52+
@app.task(max_retries=5, default_retry_delay=60)
53+
def write_download_event(
54+
download_type,
55+
resource_guid='',
56+
path='',
57+
file_id=None,
58+
version_identifier=None,
59+
size_bytes=None,
60+
storage_region_id=None,
61+
zip_completed=None,
62+
user_guid=None,
63+
ip=None,
64+
source_area='',
65+
tz='',
66+
):
67+
"""Resolve the expensive bits and write one row.
68+
69+
Callers hand over identifiers rather than loaded objects so that the download request
70+
itself does no extra queries — everything that needs a lookup is resolved here.
71+
"""
72+
from osf.models import BaseFileNode, DownloadEvent, OSFUser
73+
74+
user = OSFUser.load(user_guid) if user_guid else None
75+
file_node = BaseFileNode.load(file_id) if file_id else None
76+
file_version = _load_file_version(file_node, version_identifier)
77+
78+
if file_version is not None:
79+
if size_bytes is None:
80+
size_bytes = file_version.size
81+
if storage_region_id is None:
82+
storage_region_id = file_version.region_id
83+
84+
storage_region = _region_name(storage_region_id) or _resource_region_name(resource_guid)
85+
86+
if not path and file_node is not None:
87+
path = getattr(file_node, 'materialized_path', '') or ''
88+
89+
DownloadEvent.objects.create(
90+
download_type=download_type,
91+
resource_guid=_truncate(resource_guid, 255),
92+
path=path or '',
93+
size_bytes=size_bytes if size_bytes is not None and size_bytes >= 0 else None,
94+
zip_completed=zip_completed,
95+
storage_region=_truncate(storage_region, 64),
96+
user_region=_truncate(derive_user_region(tz, user, storage_region), 64),
97+
ip=ip or None,
98+
source_area=_truncate(source_area, 128),
99+
user=user,
100+
)
101+
102+
103+
def derive_user_region(tz, user, storage_region):
104+
"""Best available guess at where the user is, most to least trustworthy.
105+
106+
The live browser timezone is the only real signal; the rest are fallbacks so the
107+
dashboard isn't mostly blank. An empty string means we genuinely don't know, which
108+
is more useful than a wrong guess.
109+
"""
110+
if tz:
111+
return tz
112+
113+
profile_timezone = getattr(user, 'timezone', '')
114+
if profile_timezone and profile_timezone != UNSET_USER_TIMEZONE:
115+
return profile_timezone
116+
117+
# Everything defaults to the US region, so it only tells us something when it's been
118+
# deliberately changed.
119+
if storage_region and storage_region != DEFAULT_REGION_NAME:
120+
return storage_region
121+
122+
return ''
123+
124+
125+
def _load_file_version(file_node, version_identifier):
126+
"""The version that was served, for its size and region."""
127+
if file_node is None:
128+
return None
129+
130+
from osf.models import FileVersion
131+
132+
versions = FileVersion.objects.filter(basefilenode=file_node)
133+
if version_identifier:
134+
return versions.filter(identifier=version_identifier).first()
135+
return versions.order_by('-created').first()
136+
137+
138+
def _region_name(region_id):
139+
if not region_id:
140+
return ''
141+
142+
from addons.osfstorage.models import Region
143+
144+
region = Region.objects.filter(id=region_id).first()
145+
return region.name if region else ''
146+
147+
148+
def _resource_region_name(resource_guid):
149+
"""Where a zip was served from — zips have no single file version to read it off."""
150+
if not resource_guid:
151+
return ''
152+
153+
from osf.models import Guid
154+
155+
resource, _ = Guid.load_referent(resource_guid)
156+
region = getattr(resource, 'osfstorage_region', None)
157+
return getattr(region, 'name', '') or ''
158+
159+
160+
def _truncate(value, max_length):
161+
"""Keep user-controllable values inside their column.
162+
163+
``source`` and ``tz`` arrive off the query string, so they're whatever the caller
164+
put there.
165+
"""
166+
return (value or '')[:max_length]

0 commit comments

Comments
 (0)