Analytics: add status and regiter analytics from client only (#12346)

We are moving the pageview for 404 pages to the client side, similar to
what we
are doing for 200 pages.

This should reduce the load that bots produce when scanning our docs and
hitting
a bunch of 404 pages.

The new approach allows us to enable analytics 200/404 for all the
projects
without a feature flag.

Closes #12307

Author: humitos

readthedocs/analytics/proxied_api.py

@@ -6,18 +6,20 @@
 import structlog
 from django.http import JsonResponse
 from django.shortcuts import get_object_or_404
-from rest_framework import status
+from rest_framework import status as status_codes
 from rest_framework.response import Response
 from rest_framework.views import APIView
 
 from readthedocs.analytics.models import PageView
 from readthedocs.api.v2.permissions import IsAuthorizedToViewVersion
 from readthedocs.core.mixins import CDNCacheControlMixin
+from readthedocs.core.unresolver import InvalidPathForVersionedProjectError
 from readthedocs.core.unresolver import UnresolverError
-from readthedocs.core.unresolver import unresolve
+from readthedocs.core.unresolver import unresolver
 from readthedocs.core.utils.extend import SettingsOverrideObject
 from readthedocs.core.utils.requests import is_suspicious_request
 from readthedocs.projects.models import Project
+from readthedocs.proxito.views.hosting import IsAuthorizedToViewProject
 
 
 log = structlog.get_logger(__name__)  # noqa
@@ -38,7 +40,7 @@ class BaseAnalyticsView(CDNCacheControlMixin, APIView):
     # so we capture all views/interactions.
     cache_response = False
     http_method_names = ["get"]
-    permission_classes = [IsAuthorizedToViewVersion]
+    permission_classes = [IsAuthorizedToViewProject | IsAuthorizedToViewVersion]
 
     @lru_cache(maxsize=1)
     def _get_project(self):
@@ -50,31 +52,38 @@ def _get_project(self):
     def _get_version(self):
         version_slug = self.request.GET.get("version")
         project = self._get_project()
-        version = get_object_or_404(
-            project.versions.all(),
-            slug=version_slug,
-        )
+        # Do not call `get_object_or_404` because there may be some invalid URLs without versions.
+        # We do want to track those 404 pages as well. In that case, the `filename` attribute is the `path`.
+        version = project.versions.filter(slug=version_slug).first()
         return version
 
     def get(self, request, *args, **kwargs):
         # TODO: Use absolute_uri only, we don't need project and version.
         project = self._get_project()
         version = self._get_version()
         absolute_uri = self.request.GET.get("absolute_uri")
+        status = self.request.GET.get("status", "200")
         if not absolute_uri:
             return JsonResponse(
                 {"error": "'absolute_uri' GET attribute is required"},
-                status=status.HTTP_400_BAD_REQUEST,
+                status=status_codes.HTTP_400_BAD_REQUEST,
+            )
+
+        if status not in ("200", "404"):
+            return JsonResponse(
+                {"error": "'status' GET attribute should be 200 or 404"},
+                status=status_codes.HTTP_400_BAD_REQUEST,
             )
 
         self.increase_page_view_count(
             project=project,
             version=version,
             absolute_uri=absolute_uri,
+            status=status,
         )
-        return Response(status=status.HTTP_204_NO_CONTENT)
+        return Response(status=status_codes.HTTP_204_NO_CONTENT)
 
-    def increase_page_view_count(self, project, version, absolute_uri):
+    def increase_page_view_count(self, project, version, absolute_uri, status):
         """Increase the page view count for the given project."""
         if is_suspicious_request(self.request):
             log.info(
@@ -83,28 +92,49 @@ def increase_page_view_count(self, project, version, absolute_uri):
             )
             return
 
+        # Don't track 200 if the version doesn't exist
+        if status == "200" and not version:
+            return
+
         # Don't allow tracking page views from external domains.
         if self.request.unresolved_domain.is_from_external_domain:
             return
 
+        # Don't track external versions.
+        if version and version.is_external:
+            return
+
+        absolute_uri_parsed = urlparse(absolute_uri)
         try:
-            unresolved = unresolve(absolute_uri)
+            unresolved = unresolver.unresolve_url(absolute_uri)
+            filename = unresolved.filename
+            absolute_uri_project = unresolved.project
+        except InvalidPathForVersionedProjectError as exc:
+            # If the version is missing, we still want to log this request.
+            #
+            # If we don't have a version, the filename is the path,
+            # otherwise it would be empty.
+            filename = exc.path
+            absolute_uri_project = exc.project
         except UnresolverError:
             # If we were unable to resolve the URL, it
             # isn't pointing to a valid RTD project.
             return
 
-        # Don't track external versions.
-        if version.is_external or not unresolved.filename:
+        if absolute_uri_project.slug != project.slug:
+            log.warning(
+                "Skipping page view count since projects don't match",
+                project_slug=project.slug,
+                uri_project_slug=absolute_uri_project.slug,
+            )
             return
 
-        path = urlparse(absolute_uri).path
         PageView.objects.register_page_view(
             project=project,
             version=version,
-            filename=unresolved.filename,
-            path=path,
-            status=200,
+            filename=filename,
+            path=absolute_uri_parsed.path,
+            status=status,
         )
 
 

readthedocs/analytics/tests.py

@@ -135,6 +135,41 @@ def test_invalid_uri(self):
         self.client.get(url, headers={"host": self.host})
         assert PageView.objects.all().count() == 0
 
+    def test_uri_for_another_project(self):
+        other_project = get(
+            Project,
+            slug="other",
+        )
+        other_project.versions.all().update(privacy_level=PUBLIC)
+
+        # Host and ``absolute_uri`` are from different projects
+        assert PageView.objects.all().count() == 0
+        url = (
+            reverse("analytics_api")
+            + f"?project={self.project.slug}&version=latest"
+            f"&absolute_uri=https://other.readthedocs.io/en/latest/"
+        )
+        self.client.get(url, headers={"host": self.host})
+        assert PageView.objects.all().count() == 0
+
+        # Host and ``absolute_uri`` are from different projects with no ``?version`` attribute
+        url = (
+            reverse("analytics_api")
+            + f"?project={self.project.slug}"
+            f"&absolute_uri=https://other.readthedocs.io/en/latest/"
+        )
+        self.client.get(url, headers={"host": self.host})
+        assert PageView.objects.all().count() == 0
+
+        # Host and ``absolute_uri`` are from the same project
+        url = (
+            reverse("analytics_api")
+            + f"?project=other&version=latest"
+            f"&absolute_uri=https://other.readthedocs.io/en/latest/"
+        )
+        self.client.get(url, headers={"host": "other.readthedocs.io"})
+        assert PageView.objects.all().count() == 1
+
     def test_cache_headers(self):
         resp = self.client.get(self.url, headers={"host": self.host})
         self.assertEqual(resp.status_code, 204)
@@ -207,3 +242,33 @@ def test_dont_track_external_domains(self):
         r = self.client.get(self.url, headers={"host": host})
         self.assertEqual(r.status_code, 204)
         self.assertEqual(PageView.objects.all().count(), 0)
+
+    def test_notfound_404_pages(self):
+        self.assertEqual(PageView.objects.all().count(), 0)
+        url = self.url + "&status=404"
+        resp = self.client.get(url, headers={"host": self.host})
+        self.assertEqual(resp.status_code, 204)
+        self.assertEqual(PageView.objects.all().count(), 1)
+        self.assertEqual(PageView.objects.filter(status=404).count(), 1)
+
+    def test_notfound_404_page_without_version(self):
+        self.assertEqual(PageView.objects.all().count(), 0)
+        absolute_uri = (
+            f"https://{self.project.slug}.readthedocs.io/index.html"
+        )
+        url = (
+            reverse("analytics_api")
+            + f"?project={self.project.slug}&version=null"
+            f"&absolute_uri={absolute_uri}"
+            "&status=404"
+        )
+
+        resp = self.client.get(url, headers={"host": self.host})
+        pageview = PageView.objects.all().first()
+
+        self.assertEqual(resp.status_code, 204)
+        self.assertEqual(PageView.objects.all().count(), 1)
+        self.assertIsNone(pageview.version)
+        self.assertEqual(pageview.project.slug, self.project.slug)
+        self.assertEqual(pageview.path, "/index.html")
+        self.assertEqual(pageview.status, 404)

readthedocs/projects/models.py

@@ -1950,7 +1950,6 @@ def add_features(sender, **kwargs):
 
     # Feature constants - this is not a exhaustive list of features, features
     # may be added by other packages
-    RECORD_404_PAGE_VIEWS = "record_404_page_views"
     DISABLE_PAGEVIEWS = "disable_pageviews"
     RESOLVE_PROJECT_FROM_HEADER = "resolve_project_from_header"
     USE_PROXIED_APIS_WITH_PREFIX = "use_proxied_apis_with_prefix"
@@ -1975,10 +1974,6 @@ def add_features(sender, **kwargs):
     BUILD_NO_ACKS_LATE = "build_no_acks_late"
 
     FEATURES = (
-        (
-            RECORD_404_PAGE_VIEWS,
-            _("Proxito: Record 404s page views."),
-        ),
         (
             DISABLE_PAGEVIEWS,
             _("Proxito: Disable all page views"),

readthedocs/projects/views/private.py

@@ -71,7 +71,6 @@
 from readthedocs.projects.models import Domain
 from readthedocs.projects.models import EmailHook
 from readthedocs.projects.models import EnvironmentVariable
-from readthedocs.projects.models import Feature
 from readthedocs.projects.models import Project
 from readthedocs.projects.models import ProjectRelationship
 from readthedocs.projects.models import WebHook
@@ -1229,15 +1228,12 @@ def get_context_data(self, **kwargs):
 
         # Count of views for top pages over the month
         top_pages_200 = PageView.top_viewed_pages(project, limit=25)
-        track_404 = project.has_feature(Feature.RECORD_404_PAGE_VIEWS)
-        top_pages_404 = []
-        if track_404:
-            top_pages_404 = PageView.top_viewed_pages(
-                project,
-                limit=25,
-                status=404,
-                per_version=True,
-            )
+        top_pages_404 = PageView.top_viewed_pages(
+            project,
+            limit=25,
+            status=404,
+            per_version=True,
+        )
 
         # Aggregate pageviews grouped by day
         page_data = PageView.page_views_by_date(
@@ -1249,7 +1245,6 @@ def get_context_data(self, **kwargs):
                 "top_pages_200": top_pages_200,
                 "page_data": page_data,
                 "top_pages_404": top_pages_404,
-                "track_404": track_404,
             }
         )