Bundle Analysis: Add metric label for upload result (#1255)

Author: JerrySentry

upload/tests/views/test_bundle_analysis.py

@@ -104,6 +104,7 @@ def test_upload_bundle_analysis_success(db, client, mocker, mock_redis):
             "endpoint": "bundle_analysis",
             "is_using_shelter": "no",
             "position": "end",
+            "result": "success",
         },
     )
 
@@ -207,6 +208,7 @@ def test_upload_bundle_analysis_success_shelter(db, client, mocker, mock_redis):
             "endpoint": "bundle_analysis",
             "is_using_shelter": "no",
             "position": "end",
+            "result": "success",
         },
     )
 
@@ -245,6 +247,7 @@ def test_upload_bundle_analysis_org_token(db, client, mocker, mock_redis):
             "endpoint": "bundle_analysis",
             "is_using_shelter": "no",
             "position": "end",
+            "result": "success",
         },
     )
 
@@ -292,6 +295,7 @@ def test_upload_bundle_analysis_existing_commit(db, client, mocker, mock_redis):
             "endpoint": "bundle_analysis",
             "is_using_shelter": "no",
             "position": "end",
+            "result": "success",
         },
     )
 
@@ -340,7 +344,8 @@ def test_upload_bundle_analysis_missing_args(db, client, mocker, mock_redis):
             "action": "bundle_analysis",
             "endpoint": "bundle_analysis",
             "is_using_shelter": "no",
-            "position": "start",
+            "position": "end",
+            "result": "bad_request",
         },
     )
 
@@ -412,6 +417,7 @@ def test_upload_bundle_analysis_github_oidc_auth(
             "endpoint": "bundle_analysis",
             "is_using_shelter": "no",
             "position": "end",
+            "result": "success",
         },
     )
 
@@ -472,6 +478,7 @@ def test_upload_bundle_analysis_measurement_datasets_created(
             "endpoint": "bundle_analysis",
             "is_using_shelter": "no",
             "position": "end",
+            "result": "success",
         },
     )
 
@@ -533,6 +540,7 @@ def test_upload_bundle_analysis_measurement_timeseries_disabled(
             "endpoint": "bundle_analysis",
             "is_using_shelter": "no",
             "position": "end",
+            "result": "success",
         },
     )
 
@@ -574,7 +582,8 @@ def test_upload_bundle_analysis_no_repo(db, client, mocker, mock_redis):
             "action": "bundle_analysis",
             "endpoint": "bundle_analysis",
             "is_using_shelter": "no",
-            "position": "start",
+            "position": "end",
+            "result": "error",
         },
     )
 
@@ -629,6 +638,7 @@ def test_upload_bundle_analysis_tokenless_success(db, client, mocker, mock_redis
             "endpoint": "bundle_analysis",
             "is_using_shelter": "no",
             "position": "end",
+            "result": "success",
         },
     )
 
@@ -810,3 +820,34 @@ def test_upload_bundle_analysis_tokenless_mismatched_branch(
     assert res.status_code == 401
     assert res.json() == {"detail": "Not valid tokenless upload"}
     assert not upload.called
+
+
+def test_upload_bundle_analysis_view_exception_handling(db, client, mocker, mock_redis):
+    try:
+        with patch(
+            "upload.views.bundle_analysis.BundleAnalysisView._handle_upload",
+            side_effect=Exception("Test Exception"),
+        ):
+            client = APIClient()
+            repository = RepositoryFactory.create()
+            client.credentials(HTTP_AUTHORIZATION=f"token {repository.upload_token}")
+
+            mock_inc_counter = mocker.patch("upload.views.bundle_analysis.inc_counter")
+
+            client.post(
+                reverse("upload-bundle-analysis"),
+                {
+                    "commit": "6fd5b89357fc8cdf34d6197549ac7c6d7e5977ef",
+                    "slug": f"{repository.author.username}::::{repository.name}",
+                },
+                format="json",
+            )
+    except Exception as e:
+        # Check that the Test Exception was raised and the inc_counter went up
+        assert str(e) == "Test Exception"
+        mock_inc_counter.assert_any_call(
+            ANY,
+            labels=mocker.ANY,
+        )
+        labels = mock_inc_counter.call_args[1]["labels"]
+        assert labels["result"] == "error"

upload/views/bundle_analysis.py

@@ -1,6 +1,6 @@
 import logging
 import uuid
-from typing import Any, Callable
+from typing import Any, Callable, Tuple
 
 from django.conf import settings
 from django.http import HttpRequest
@@ -48,6 +48,7 @@
         "endpoint",
         "is_using_shelter",
         "position",
+        "result",
     ],
 )
 
@@ -85,23 +86,13 @@ class BundleAnalysisView(APIView, ShelterMixin):
     def get_exception_handler(self) -> Callable:
         return repo_auth_custom_exception_handler
 
-    def post(self, request: HttpRequest) -> Response:
-        labels = generate_upload_prometheus_metrics_labels(
-            action="bundle_analysis",
-            endpoint="bundle_analysis",
-            request=self.request,
-            is_shelter_request=self.is_shelter_request(),
-            position="start",
-            include_empty_labels=False,
-        )
-        inc_counter(
-            BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER,
-            labels=labels,
-        )
-
+    def _handle_upload(self, request: HttpRequest) -> Tuple[str, Response]:
         serializer = UploadSerializer(data=request.data)
         if not serializer.is_valid():
-            return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+            return (
+                "bad_request",
+                Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST),
+            )
         data = serializer.validated_data
 
         if isinstance(request.user, Owner):
@@ -191,18 +182,6 @@ def post(self, request: HttpRequest) -> Response:
                 task_arguments=task_arguments,
             ),
         )
-        labels = generate_upload_prometheus_metrics_labels(
-            action="bundle_analysis",
-            endpoint="bundle_analysis",
-            request=self.request,
-            is_shelter_request=self.is_shelter_request(),
-            position="end",
-            include_empty_labels=False,
-        )
-        inc_counter(
-            BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER,
-            labels=labels,
-        )
 
         dispatch_upload_task(
             task_arguments,
@@ -235,4 +214,40 @@ def post(self, request: HttpRequest) -> Response:
                         ),
                     )
 
-        return Response({"url": url}, status=201)
+        return ("success", Response({"url": url}, status=201))
+
+    def post(self, request: HttpRequest) -> Response:
+        labels = generate_upload_prometheus_metrics_labels(
+            action="bundle_analysis",
+            endpoint="bundle_analysis",
+            request=self.request,
+            is_shelter_request=self.is_shelter_request(),
+            position="start",
+            include_empty_labels=False,
+        )
+        labels["result"] = "pending"
+        inc_counter(
+            BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER,
+            labels=labels,
+        )
+
+        try:
+            upload_result, response = self._handle_upload(request)
+            return response
+        except Exception as e:
+            log.error(
+                "Error handling bundle analysis upload",
+                extra=dict(
+                    error=e,
+                ),
+                exc_info=True,
+            )
+            upload_result = "error"
+            raise
+        finally:
+            labels["position"] = "end"
+            labels["result"] = upload_result
+            inc_counter(
+                BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER,
+                labels=labels,
+            )