Replace Sentry metrics with Prometheus metrics (#918)

Co-authored-by: Arpad Borsos <[email protected]>

Author: lvthanh03

graphql_api/tests/test_views.py

@@ -1,5 +1,5 @@
 import json
-from unittest.mock import Mock, call, patch
+from unittest.mock import Mock, patch
 
 from ariadne import ObjectType, make_executable_schema
 from ariadne.validation import cost_directive
@@ -201,11 +201,12 @@ async def test_query_metrics_extension_set_type_and_name_timeout(
         assert extension.operation_type == "unknown_type"
         assert extension.operation_name == "unknown_name"
 
-    @patch("sentry_sdk.metrics.incr")
+    @patch("graphql_api.views.GQL_REQUEST_MADE_COUNTER.labels")
+    @patch("graphql_api.views.GQL_ERROR_TYPE_COUNTER.labels")
     @patch("graphql_api.views.AsyncGraphqlView._check_ratelimit")
     @override_settings(DEBUG=False, GRAPHQL_RATE_LIMIT_RPM=1000)
     async def test_when_rate_limit_reached(
-        self, mocked_check_ratelimit, mocked_sentry_incr
+        self, mocked_check_ratelimit, mocked_error_counter, mocked_request_counter
     ):
         schema = generate_cost_test_schema()
         mocked_check_ratelimit.return_value = True
@@ -217,11 +218,10 @@ async def test_when_rate_limit_reached(
             == "It looks like you've hit the rate limit of 1000 req/min. Try again later."
         )
 
-        expected_calls = [
-            call("graphql.info.request_made", tags={"path": "/graphql/gh"}),
-            call("graphql.error.rate_limit", tags={"path": "/graphql/gh"}),
-        ]
-        mocked_sentry_incr.assert_has_calls(expected_calls)
+        mocked_error_counter.assert_called_with(
+            error_type="rate_limit", path="/graphql/gh"
+        )
+        mocked_request_counter.assert_called_with(path="/graphql/gh")
 
     @override_settings(
         DEBUG=False, GRAPHQL_RATE_LIMIT_RPM=0, GRAPHQL_RATE_LIMIT_ENABLED=False

graphql_api/views.py

@@ -19,8 +19,7 @@
 )
 from graphql import DocumentNode
 from sentry_sdk import capture_exception
-from sentry_sdk import metrics as sentry_metrics
-from shared.metrics import Counter, Histogram
+from shared.metrics import Counter, Histogram, inc_counter
 
 from codecov.commands.exceptions import BaseException
 from codecov.commands.executor import get_executor_from_request
@@ -51,6 +50,17 @@
     buckets=[0.05, 0.1, 0.25, 0.5, 0.75, 1, 2, 5, 10, 30],
 )
 
+GQL_REQUEST_MADE_COUNTER = Counter(
+    "api_gql_requests_made",
+    "Total API GQL requests made",
+    ["path"],
+)
+
+GQL_ERROR_TYPE_COUNTER = Counter(
+    "api_gql_errors",
+    "Number of times API GQL endpoint failed with an exception by type",
+    ["error_type", "path"],
+)
 
 # covers named and 3 unnamed operations (see graphql_api/types/query/query.py)
 GQL_TYPE_AND_NAME_PATTERN = r"^(query|mutation|subscription)(?:\(\$input:|) (\w+)(?:\(| \(|{| {|!)|^(?:{) (me|owner|config)(?:\(| |{)"
@@ -109,9 +119,13 @@ def request_started(self, context):
         """
         self.set_type_and_name(query=context["clean_query"])
         self.start_timestamp = time.perf_counter()
-        GQL_HIT_COUNTER.labels(
-            operation_type=self.operation_type, operation_name=self.operation_name
-        ).inc()
+        inc_counter(
+            GQL_HIT_COUNTER,
+            labels=dict(
+                operation_type=self.operation_type,
+                operation_name=self.operation_name,
+            ),
+        )
 
     def request_finished(self, context):
         """
@@ -226,10 +240,12 @@ async def post(self, request, *args, **kwargs):
             "user": request.user,
         }
         log.info("GraphQL Request", extra=log_data)
-        sentry_metrics.incr("graphql.info.request_made", tags={"path": req_path})
-
+        inc_counter(GQL_REQUEST_MADE_COUNTER, labels=dict(path=req_path))
         if self._check_ratelimit(request=request):
-            sentry_metrics.incr("graphql.error.rate_limit", tags={"path": req_path})
+            inc_counter(
+                GQL_ERROR_TYPE_COUNTER,
+                labels=dict(error_type="rate_limit", path=req_path),
+            )
             return JsonResponse(
                 data={
                     "status": 429,
@@ -245,7 +261,10 @@ async def post(self, request, *args, **kwargs):
             data = json.loads(content)
 
             if "errors" in data:
-                sentry_metrics.incr("graphql.error.all", tags={"path": req_path})
+                inc_counter(
+                    GQL_ERROR_TYPE_COUNTER,
+                    labels=dict(error_type="all", path=req_path),
+                )
                 try:
                     if data["errors"][0]["extensions"]["cost"]:
                         costs = data["errors"][0]["extensions"]["cost"]
@@ -257,9 +276,12 @@ async def post(self, request, *args, **kwargs):
                                 request_body=req_body,
                             ),
                         )
-                        sentry_metrics.incr(
-                            "graphql.error.query_cost_exceeded",
-                            tags={"path": req_path},
+                        inc_counter(
+                            GQL_ERROR_TYPE_COUNTER,
+                            labels=dict(
+                                error_type="query_cost_exceeded",
+                                path=req_path,
+                            ),
                         )
                         return HttpResponseBadRequest(
                             JsonResponse("Your query is too costly.")

upload/helpers.py

@@ -790,14 +790,15 @@ def get_version_from_headers(headers):
         return "unknown-user-agent"
 
 
-def generate_upload_sentry_metrics_tags(
+def generate_upload_prometheus_metrics_labels(
     action,
     request,
     is_shelter_request,
     endpoint: Optional[str] = None,
     repository: Optional[Repository] = None,
     position: Optional[str] = None,
     upload_version: Optional[str] = None,
+    include_empty_labels: bool = True,
 ):
     metrics_tags = dict(
         agent=get_agent_from_headers(request.headers),
@@ -806,13 +807,23 @@ def generate_upload_sentry_metrics_tags(
         endpoint=endpoint,
         is_using_shelter="yes" if is_shelter_request else "no",
     )
+
+    repo_visibility = None
     if repository:
-        metrics_tags["repo_visibility"] = (
-            "private" if repository.private is True else "public"
-        )
-    if position:
-        metrics_tags["position"] = position
-    if upload_version:
-        metrics_tags["upload_version"] = upload_version
+        repo_visibility = "private" if repository.private else "public"
+
+    optional_fields = {
+        "repo_visibility": repo_visibility,
+        "position": position,
+        "upload_version": upload_version,
+    }
+
+    metrics_tags.update(
+        {
+            field: value
+            for field, value in optional_fields.items()
+            if value or include_empty_labels
+        }
+    )
 
     return metrics_tags

upload/metrics.py

@@ -0,0 +1,16 @@
+from shared.metrics import Counter
+
+API_UPLOAD_COUNTER = Counter(
+    "api_upload",
+    "Total API upload endpoint requests",
+    [
+        "agent",
+        "version",
+        "action",
+        "endpoint",
+        "is_using_shelter",
+        "repo_visibility",
+        "position",
+        "upload_version",
+    ],
+)

upload/tests/views/test_commits.py

@@ -328,7 +328,7 @@ def test_commit_github_oidc_auth(mock_jwks_client, mock_jwt_decode, db, mocker):
         private=False, author__username="codecov", name="the_repo"
     )
     mocked_call = mocker.patch.object(TaskService, "update_commit")
-    mock_sentry_metrics = mocker.patch("upload.views.commits.sentry_metrics.incr")
+    mock_prometheus_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels")
     mock_jwt_decode.return_value = {
         "repository": f"url/{repository.name}",
         "repository_owner": repository.author.username,
@@ -374,15 +374,15 @@ def test_commit_github_oidc_auth(mock_jwks_client, mock_jwt_decode, db, mocker):
     }
     assert expected_response == response_json
     mocked_call.assert_called_with(commitid="commit_sha", repoid=repository.repoid)
-    mock_sentry_metrics.assert_called_with(
-        "upload",
-        tags={
+    mock_prometheus_metrics.assert_called_with(
+        **{
             "agent": "cli",
             "version": "0.4.7",
             "action": "coverage",
             "endpoint": "create_commit",
             "repo_visibility": "public",
             "is_using_shelter": "no",
             "position": "end",
+            "upload_version": None,
         },
     )

upload/tests/views/test_empty_upload.py

@@ -51,7 +51,7 @@ def test_empty_upload_with_yaml_ignored_files(
     mocker.patch.object(
         CanDoCoverageUploadsPermission, "has_permission", return_value=True
     )
-    mock_sentry_metrics = mocker.patch("upload.views.empty_upload.sentry_metrics.incr")
+    mock_prometheus_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels")
     mock_final_yaml.return_value = UserYaml(
         {
             "ignore": [
@@ -93,16 +93,16 @@ def test_empty_upload_with_yaml_ignored_files(
     notify_mock.assert_called_once_with(
         repoid=repository.repoid, commitid=commit.commitid, empty_upload="pass"
     )
-    mock_sentry_metrics.assert_called_with(
-        "upload",
-        tags={
+    mock_prometheus_metrics.assert_called_with(
+        **{
             "agent": "cli",
             "version": "0.4.7",
             "action": "coverage",
             "endpoint": "empty_upload",
             "repo_visibility": "private",
             "is_using_shelter": "no",
             "position": "end",
+            "upload_version": None,
         },
     )
 

upload/tests/views/test_reports.py

@@ -41,7 +41,7 @@ def test_reports_get_not_allowed(client, mocker, db):
 
 def test_reports_post(client, db, mocker):
     mocked_call = mocker.patch.object(TaskService, "preprocess_upload")
-    mock_sentry_metrics = mocker.patch("upload.views.reports.sentry_metrics.incr")
+    mock_prometheus_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels")
     repository = RepositoryFactory(
         name="the_repo", author__username="codecov", author__service="github"
     )
@@ -65,16 +65,16 @@ def test_reports_post(client, db, mocker):
         commit_id=commit.id, code="code1", report_type=CommitReport.ReportType.COVERAGE
     ).exists()
     mocked_call.assert_called_with(repository.repoid, commit.commitid, "code1")
-    mock_sentry_metrics.assert_called_with(
-        "upload",
-        tags={
+    mock_prometheus_metrics.assert_called_with(
+        **{
             "agent": "cli",
             "version": "0.4.7",
             "action": "coverage",
             "endpoint": "create_report",
             "repo_visibility": "private",
             "is_using_shelter": "no",
             "position": "end",
+            "upload_version": None,
         },
     )
 
@@ -343,7 +343,7 @@ def test_reports_results_post_successful_github_oidc_auth(
     mock_jwks_client, mock_jwt_decode, client, db, mocker
 ):
     mocked_task = mocker.patch("services.task.TaskService.create_report_results")
-    mock_sentry_metrics = mocker.patch("upload.views.reports.sentry_metrics.incr")
+    mock_prometheus_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels")
     mocker.patch.object(
         CanDoCoverageUploadsPermission, "has_permission", return_value=True
     )
@@ -383,16 +383,16 @@ def test_reports_results_post_successful_github_oidc_auth(
         report_id=commit_report.id,
     ).exists()
     mocked_task.assert_called_once()
-    mock_sentry_metrics.assert_called_with(
-        "upload",
-        tags={
+    mock_prometheus_metrics.assert_called_with(
+        **{
             "agent": "cli",
             "version": "0.4.7",
             "action": "coverage",
             "endpoint": "create_report_results",
             "repo_visibility": "private",
             "is_using_shelter": "no",
             "position": "end",
+            "upload_version": None,
         },
     )
 

upload/tests/views/test_test_results.py

@@ -20,7 +20,7 @@
 
 def test_upload_test_results(db, client, mocker, mock_redis):
     upload = mocker.patch.object(TaskService, "upload")
-    mock_sentry_metrics = mocker.patch("upload.views.test_results.metrics.incr")
+    mock_prometheus_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels")
     create_presigned_put = mocker.patch(
         "services.archive.StorageService.create_presigned_put",
         return_value="test-presigned-put",
@@ -100,16 +100,16 @@ def test_upload_test_results(db, client, mocker, mock_redis):
         report_code=None,
         report_type="test_results",
     )
-    mock_sentry_metrics.assert_called_with(
-        "upload",
-        tags={
+    mock_prometheus_metrics.assert_called_with(
+        **{
             "agent": "cli",
             "version": "0.4.7",
             "action": "test_results",
             "endpoint": "test_results",
             "repo_visibility": "private",
             "is_using_shelter": "no",
             "position": "end",
+            "upload_version": None,
         },
     )
 

upload/tests/views/test_upload_completion.py

@@ -51,9 +51,7 @@ def test_upload_completion_view_processed_uploads(mocked_manual_trigger, db, moc
     mocker.patch.object(
         CanDoCoverageUploadsPermission, "has_permission", return_value=True
     )
-    mock_sentry_metrics = mocker.patch(
-        "upload.views.upload_completion.sentry_metrics.incr"
-    )
+    mock_prometheus_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels")
     repository = RepositoryFactory(
         name="the_repo", author__username="codecov", author__service="github"
     )
@@ -88,16 +86,16 @@ def test_upload_completion_view_processed_uploads(mocked_manual_trigger, db, moc
         "uploads_error": 0,
     }
     mocked_manual_trigger.assert_called_once_with(repository.repoid, commit.commitid)
-    mock_sentry_metrics.assert_called_with(
-        "upload",
-        tags={
+    mock_prometheus_metrics.assert_called_with(
+        **{
             "agent": "cli",
             "version": "0.4.7",
             "action": "coverage",
             "endpoint": "upload_complete",
             "repo_visibility": "private",
             "is_using_shelter": "no",
             "position": "end",
+            "upload_version": None,
         },
     )