From c8685db054a54943b3bc553195400b34851c26f0 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Tue, 22 Oct 2024 15:49:49 +0200 Subject: [PATCH 01/27] Update `shared` and remove statsd metrics usage `shared.metrics` does not re-export `statsd` anymore. --- labelanalysis/serializers.py | 8 ------ requirements.in | 2 +- requirements.txt | 8 ++++-- staticanalysis/serializers.py | 10 ------- upload/tests/test_upload.py | 12 -------- upload/tests/views/test_uploads.py | 44 +++++------------------------- upload/views/legacy.py | 13 +-------- upload/views/uploads.py | 7 ----- webhook_handlers/views/github.py | 32 ---------------------- 9 files changed, 14 insertions(+), 122 deletions(-) diff --git a/labelanalysis/serializers.py b/labelanalysis/serializers.py index ebad60b1eb..a75237a1c5 100644 --- a/labelanalysis/serializers.py +++ b/labelanalysis/serializers.py @@ -1,5 +1,4 @@ from rest_framework import exceptions, serializers -from shared.metrics import metrics from core.models import Commit from labelanalysis.models import ( @@ -23,25 +22,21 @@ def to_internal_value(self, commit_sha): commitid=commit_sha, ).first() if commit is None: - metrics.incr("label_analysis_request.errors.commit_not_found") raise exceptions.NotFound(f"Commit {commit_sha[:7]} not found.") if commit.staticanalysissuite_set.exists(): return commit if not self.accepts_fallback: - metrics.incr("label_analysis_request.errors.static_analysis_not_found") raise serializers.ValidationError("No static analysis found") attempted_commits = [] for _ in range(10): attempted_commits.append(commit.commitid) commit = commit.parent_commit if commit is None: - metrics.incr("label_analysis_request.errors.static_analysis_not_found") raise serializers.ValidationError( f"No possible commits have static analysis sent. Attempted commits: {','.join(attempted_commits)}" ) if commit.staticanalysissuite_set.exists(): return commit - metrics.incr("label_analysis_request.errors.static_analysis_not_found") raise serializers.ValidationError( f"No possible commits have static analysis sent. Attempted too many commits: {','.join(attempted_commits)}" ) @@ -71,13 +66,10 @@ class LabelAnalysisRequestSerializer(serializers.ModelSerializer): errors = ProcessingErrorList(required=False) def validate(self, data): - metrics.incr("label_analysis_request.count") if data["base_commit"] == data["head_commit"]: - metrics.incr("label_analysis_request.errors.base_head_equal") raise serializers.ValidationError( {"base_commit": "Base and head must be different commits"} ) - metrics.incr("label_analysis_request.valid.count") return data class Meta: diff --git a/requirements.in b/requirements.in index e3838d9068..2491f0aa53 100644 --- a/requirements.in +++ b/requirements.in @@ -20,7 +20,7 @@ factory-boy fakeredis freezegun https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem -https://github.com/codecov/shared/archive/f0e213c4399563990d43fb424f4be36faa5ce5eb.tar.gz#egg=shared +https://github.com/codecov/shared/archive/4e2792779e3efc65f876ab8ac38ffd244189e34d.tar.gz#egg=shared google-cloud-pubsub gunicorn>=22.0.0 https://github.com/photocrowd/django-cursor-pagination/archive/f560902696b0c8509e4d95c10ba0d62700181d84.tar.gz diff --git a/requirements.txt b/requirements.txt index 87f5295048..8d2897a9aa 100644 --- a/requirements.txt +++ b/requirements.txt @@ -205,6 +205,8 @@ googleapis-common-protos[grpc]==1.59.1 # grpcio-status graphql-core==3.2.3 # via ariadne +greenlet==3.1.1 + # via sqlalchemy grpc-google-iam-v1==0.12.6 # via google-cloud-pubsub grpcio==1.62.1 @@ -299,6 +301,8 @@ opentelemetry-util-http==0.45b0 # opentelemetry-instrumentation-wsgi opentracing==2.4.0 # via -r requirements.in +orjson==3.10.9 + # via shared packaging==24.1 # via # gunicorn @@ -417,7 +421,7 @@ sentry-sdk[celery]==2.13.0 # shared setproctitle==1.1.10 # via -r requirements.in -shared @ https://github.com/codecov/shared/archive/f0e213c4399563990d43fb424f4be36faa5ce5eb.tar.gz +shared @ https://github.com/codecov/shared/archive/4e2792779e3efc65f876ab8ac38ffd244189e34d.tar.gz # via -r requirements.in simplejson==3.17.2 # via -r requirements.in @@ -447,8 +451,6 @@ sqlparse==0.5.0 # django starlette==0.36.2 # via ariadne -statsd==3.3.0 - # via shared stripe==9.6.0 # via -r requirements.in text-unidecode==1.3 diff --git a/staticanalysis/serializers.py b/staticanalysis/serializers.py index 64a3479743..bb387d6670 100644 --- a/staticanalysis/serializers.py +++ b/staticanalysis/serializers.py @@ -2,7 +2,6 @@ import math from rest_framework import exceptions, serializers -from shared.metrics import metrics from core.models import Commit from services.archive import ArchiveService, MinioEndpoints @@ -154,13 +153,4 @@ def create(self, validated_data): created_ids=[f.id for f in created_filepaths], repoid=repository.repoid ), ) - metrics.gauge( - "static_analysis.suite.files_created", - len(file_metadata_array) - len(existing_values_mapping), - ) - metrics.gauge("static_analysis.suite.total_files", len(file_metadata_array)) - metrics.gauge( - "static_analysis.suite.existing_files", len(existing_values_mapping) - ) - metrics.incr("static_analysis.suite.count") return obj diff --git a/upload/tests/test_upload.py b/upload/tests/test_upload.py index f970c3763d..f290fb0a32 100644 --- a/upload/tests/test_upload.py +++ b/upload/tests/test_upload.py @@ -997,7 +997,6 @@ def test_invalid_request_params_invalid_package(self): assert response.status_code == status.HTTP_400_BAD_REQUEST assert metrics.data["uploads.rejected"] == 1 - @patch("shared.metrics.metrics.incr") @patch("upload.views.legacy.get_redis_connection") @patch("upload.views.legacy.uuid4") @patch("upload.views.legacy.dispatch_upload_task") @@ -1009,7 +1008,6 @@ def test_successful_upload_v2( mock_dispatch_upload, mock_uuid4, mock_get_redis, - mock_metrics, ): class MockRepoProviderAdapter: async def get_commit(self, commit, token): @@ -1036,7 +1034,6 @@ async def get_commit(self, commit, token): ) assert response.status_code == 200 - mock_metrics.assert_called_once_with("uploads.accepted", 1) headers = response.headers @@ -1074,7 +1071,6 @@ async def get_commit(self, commit, token): == "https://app.codecov.io/github/codecovtest/upload-test-repo/commit/b521e55aef79b101f48e2544837ca99a7fa3bf6b" ) - @patch("shared.metrics.metrics.incr") @patch("upload.views.legacy.get_redis_connection") @patch("upload.views.legacy.uuid4") @patch("upload.views.legacy.dispatch_upload_task") @@ -1086,7 +1082,6 @@ def test_successful_upload_v2_slash( mock_dispatch_upload, mock_uuid4, mock_get_redis, - mock_metrics, ): class MockRepoProviderAdapter: async def get_commit(self, commit, token): @@ -1113,7 +1108,6 @@ async def get_commit(self, commit, token): ) assert response.status_code == 200 - mock_metrics.assert_called_once_with("uploads.accepted", 1) headers = response.headers @@ -1151,7 +1145,6 @@ async def get_commit(self, commit, token): == "https://app.codecov.io/github/codecovtest/upload-test-repo/commit/b521e55aef79b101f48e2544837ca99a7fa3bf6b" ) - @patch("shared.metrics.metrics.incr") @patch("upload.views.legacy.get_redis_connection") @patch("upload.views.legacy.uuid4") @patch("upload.views.legacy.determine_repo_for_upload") @@ -1163,7 +1156,6 @@ def test_repo_validation_error_v2( mock_determine_repo_for_upload, mock_uuid4, mock_get_redis, - mock_metrics, ): class MockRepoProviderAdapter: async def get_commit(self, commit, token): @@ -1193,7 +1185,6 @@ async def get_commit(self, commit, token): ) assert response.status_code == 400 - mock_metrics.assert_called_once_with("uploads.rejected", 1) headers = response.headers @@ -1206,7 +1197,6 @@ async def get_commit(self, commit, token): assert response.content == b"Could not determine repo and owner" - @patch("shared.metrics.metrics.incr") @patch("upload.views.legacy.get_redis_connection") @patch("upload.views.legacy.uuid4") @patch("upload.views.legacy.determine_repo_for_upload") @@ -1218,7 +1208,6 @@ def test_too_many_repos_found_v2( mock_determine_repo_for_upload, mock_uuid4, mock_get_redis, - mock_metrics, ): class MockRepoProviderAdapter: async def get_commit(self, commit, token): @@ -1248,7 +1237,6 @@ async def get_commit(self, commit, token): ) assert response.status_code == 400 - mock_metrics.assert_called_once_with("uploads.rejected", 1) headers = response.headers diff --git a/upload/tests/views/test_uploads.py b/upload/tests/views/test_uploads.py index 5eced188a1..9f00230e01 100644 --- a/upload/tests/views/test_uploads.py +++ b/upload/tests/views/test_uploads.py @@ -1,4 +1,4 @@ -from unittest.mock import MagicMock, call, patch +from unittest.mock import MagicMock, patch import pytest from django.conf import settings @@ -106,24 +106,20 @@ def test_get_repo(db): assert recovered_repo == repository -@patch("shared.metrics.metrics.incr") -def test_get_repo_with_invalid_service(mock_metrics, db): +def test_get_repo_with_invalid_service(db): upload_views = UploadViews() upload_views.kwargs = dict(repo="repo", service="wrong service") with pytest.raises(ValidationError) as exp: upload_views.get_repo() assert exp.match("Service not found: wrong service") - mock_metrics.assert_called_once_with("uploads.rejected", 1) -@patch("shared.metrics.metrics.incr") -def test_get_repo_not_found(mock_metrics, db): +def test_get_repo_not_found(db): upload_views = UploadViews() upload_views.kwargs = dict(repo="repo", service="github") with pytest.raises(ValidationError) as exp: upload_views.get_repo() assert exp.match("Repository not found") - mock_metrics.assert_called_once_with("uploads.rejected", 1) def test_get_commit(db): @@ -137,8 +133,7 @@ def test_get_commit(db): assert recovered_commit == commit -@patch("shared.metrics.metrics.incr") -def test_get_commit_error(mock_metrics, db): +def test_get_commit_error(db): repository = RepositoryFactory(name="the_repo", author__username="codecov") repository.save() upload_views = UploadViews() @@ -146,7 +141,6 @@ def test_get_commit_error(mock_metrics, db): with pytest.raises(ValidationError) as exp: upload_views.get_commit(repository) assert exp.match("Commit SHA not found") - mock_metrics.assert_called_once_with("uploads.rejected", 1) def test_get_report(db): @@ -179,8 +173,7 @@ def test_get_default_report(db): assert recovered_report == report -@patch("shared.metrics.metrics.incr") -def test_get_report_error(mock_metrics, db): +def test_get_report_error(db): repository = RepositoryFactory(name="the_repo", author__username="codecov") commit = CommitFactory(repository=repository) repository.save() @@ -191,12 +184,10 @@ def test_get_report_error(mock_metrics, db): ) with pytest.raises(ValidationError) as exp: upload_views.get_report(commit) - mock_metrics.assert_called_once_with("uploads.rejected", 1) assert exp.match("Report not found") -@patch("shared.metrics.metrics.incr") -def test_uploads_post(mock_metrics, db, mocker, mock_redis): +def test_uploads_post(db, mocker, mock_redis): # TODO remove the mock object and test the flow with the permissions mocker.patch.object( CanDoCoverageUploadsPermission, "has_permission", return_value=True @@ -277,9 +268,6 @@ def test_uploads_post(mock_metrics, db, mocker, mock_redis): report_session_id=upload.id, flag_id=flag2.id ).exists() assert [flag for flag in upload.flags.all()] == [flag1, flag2] - mock_metrics.assert_has_calls( - [call("upload.cli.version"), call("uploads.accepted", 1)] - ) archive_service = ArchiveService(repository) assert upload.storage_path == MinioEndpoints.raw_with_upload_id.get_path( @@ -294,15 +282,12 @@ def test_uploads_post(mock_metrics, db, mocker, mock_redis): upload_task_mock.assert_called() -@patch("shared.metrics.metrics.incr") @pytest.mark.parametrize("private", [False, True]) @pytest.mark.parametrize("branch", ["branch", "fork:branch", "someone/fork:branch"]) @pytest.mark.parametrize( "branch_sent", [None, "branch", "fork:branch", "someone/fork:branch"] ) -def test_uploads_post_tokenless( - mock_metrics, db, mocker, mock_redis, private, branch, branch_sent -): +def test_uploads_post_tokenless(db, mocker, mock_redis, private, branch, branch_sent): presigned_put_mock = mocker.patch( "services.archive.StorageService.create_presigned_put", return_value="presigned put", @@ -392,9 +377,6 @@ def test_uploads_post_tokenless( report_session_id=upload.id, flag_id=flag2.id ).exists() assert [flag for flag in upload.flags.all()] == [flag1, flag2] - mock_metrics.assert_has_calls( - [call("upload.cli.version"), call("uploads.accepted", 1)] - ) archive_service = ArchiveService(repository) assert upload.storage_path == MinioEndpoints.raw_with_upload_id.get_path( @@ -431,7 +413,6 @@ def test_uploads_post_tokenless( assert response.json().get("detail") == "Not valid tokenless upload" -@patch("shared.metrics.metrics.incr") @pytest.mark.parametrize("private", [False, True]) @pytest.mark.parametrize("branch", ["branch", "fork:branch", "someone/fork:branch"]) @pytest.mark.parametrize( @@ -439,7 +420,6 @@ def test_uploads_post_tokenless( ) @pytest.mark.parametrize("upload_token_required_for_public_repos", [True, False]) def test_uploads_post_token_required_auth_check( - mock_metrics, db, mocker, mock_redis, @@ -545,9 +525,6 @@ def test_uploads_post_token_required_auth_check( report_session_id=upload.id, flag_id=flag2.id ).exists() assert [flag for flag in upload.flags.all()] == [flag1, flag2] - mock_metrics.assert_has_calls( - [call("upload.cli.version"), call("uploads.accepted", 1)] - ) archive_service = ArchiveService(repository) assert upload.storage_path == MinioEndpoints.raw_with_upload_id.get_path( @@ -587,9 +564,7 @@ def test_uploads_post_token_required_auth_check( @patch("upload.views.uploads.AnalyticsService") @patch("upload.helpers.jwt.decode") @patch("upload.helpers.PyJWKClient") -@patch("shared.metrics.metrics.incr") def test_uploads_post_github_oidc_auth( - mock_metrics, mock_jwks_client, mock_jwt_decode, analytics_service_mock, @@ -679,9 +654,6 @@ def test_uploads_post_github_oidc_auth( report_session_id=upload.id, flag_id=flag2.id ).exists() assert [flag for flag in upload.flags.all()] == [flag1, flag2] - mock_metrics.assert_has_calls( - [call("upload.cli.version"), call("uploads.accepted", 1)] - ) archive_service = ArchiveService(repository) assert upload.storage_path == MinioEndpoints.raw_with_upload_id.get_path( @@ -887,10 +859,8 @@ def mock_config(self, mocker): @patch("upload.views.uploads.AnalyticsService") @patch("upload.helpers.jwt.decode") @patch("upload.helpers.PyJWKClient") - @patch("shared.metrics.metrics.incr") def test_uploads_post_github_enterprise_oidc_auth_jwks_url( self, - mock_metrics, mock_jwks_client, mock_jwt_decode, analytics_service_mock, diff --git a/upload/views/legacy.py b/upload/views/legacy.py index 3922fd4c74..81feb64c55 100644 --- a/upload/views/legacy.py +++ b/upload/views/legacy.py @@ -17,7 +17,6 @@ from rest_framework.permissions import AllowAny from rest_framework.views import APIView from sentry_sdk import metrics as sentry_metrics -from shared.metrics import metrics from codecov.db import sync_to_async from codecov_auth.commands.owner import OwnerCommands @@ -116,12 +115,7 @@ def post(self, request, *args, **kwargs): if package is not None: package_format = r"((codecov-cli/)|((.+-)?uploader-))(\d+.\d+.\d+)" match = re.fullmatch(package_format, package) - if match: - if match.group(2): # Matches codecov-cli/ - metrics.incr(f"upload.cli.{match.group(5)}") - else: # Matches (.+-)?uploader- - metrics.incr(f"upload.uploader.{match.group(5)}") - else: + if not match: log.warning( "Package query parameter failed to match CLI or uploader format", extra=dict(package=package), @@ -136,7 +130,6 @@ def post(self, request, *args, **kwargs): ) response.status_code = status.HTTP_400_BAD_REQUEST response.content = "Invalid request parameters" - metrics.incr("uploads.rejected", 1) return response # Try to determine the repository associated with the upload based on the params provided @@ -146,12 +139,10 @@ def post(self, request, *args, **kwargs): except ValidationError: response.status_code = status.HTTP_400_BAD_REQUEST response.content = "Could not determine repo and owner" - metrics.incr("uploads.rejected", 1) return response except MultipleObjectsReturned: response.status_code = status.HTTP_400_BAD_REQUEST response.content = "Found too many repos" - metrics.incr("uploads.rejected", 1) return response log.info( @@ -309,7 +300,6 @@ def post(self, request, *args, **kwargs): upload_params=upload_params, ), ) - metrics.incr("uploads.rejected", 1) return HttpResponseServerError("Unknown error, please try again later") log.info( "Returning presign put", @@ -378,7 +368,6 @@ def post(self, request, *args, **kwargs): response["Content-Type"] = "application/json" response.status_code = status.HTTP_200_OK - metrics.incr("uploads.accepted", 1) return response diff --git a/upload/views/uploads.py b/upload/views/uploads.py index ec80a40957..f6bdb60ede 100644 --- a/upload/views/uploads.py +++ b/upload/views/uploads.py @@ -6,7 +6,6 @@ from rest_framework.generics import ListCreateAPIView from rest_framework.permissions import BasePermission from sentry_sdk import metrics as sentry_metrics -from shared.metrics import metrics from shared.upload.utils import UploaderType, insert_coverage_measurement from codecov_auth.authentication.repo_auth import ( @@ -108,8 +107,6 @@ def perform_create(self, serializer: UploadSerializer): cli_version=version, ), ) - if version: - metrics.incr("upload.cli." + f"{version}") archive_service = ArchiveService(repository) # Create upload record instance: ReportSession = serializer.save( @@ -152,7 +149,6 @@ def perform_create(self, serializer: UploadSerializer): position="end", ), ) - metrics.incr("uploads.accepted", 1) self.activate_repo(repository) self.send_analytics_data(commit, instance, version) return instance @@ -254,7 +250,6 @@ def get_repo(self) -> Repository: repo = super().get_repo() return repo except ValidationError as exception: - metrics.incr("uploads.rejected", 1) raise exception def get_commit(self, repo: Repository) -> Commit: @@ -262,7 +257,6 @@ def get_commit(self, repo: Repository) -> Commit: commit = super().get_commit(repo) return commit except ValidationError as excpetion: - metrics.incr("uploads.rejected", 1) raise excpetion def get_report(self, commit: Commit) -> CommitReport: @@ -270,5 +264,4 @@ def get_report(self, commit: Commit) -> CommitReport: report = super().get_report(commit) return report except ValidationError as exception: - metrics.incr("uploads.rejected", 1) raise exception diff --git a/webhook_handlers/views/github.py b/webhook_handlers/views/github.py index ad20f2041e..953faf5b7c 100644 --- a/webhook_handlers/views/github.py +++ b/webhook_handlers/views/github.py @@ -12,7 +12,6 @@ from rest_framework.permissions import AllowAny from rest_framework.response import Response from rest_framework.views import APIView -from shared.metrics import metrics from codecov_auth.models import ( GITHUB_APP_INSTALLATION_DEFAULT_NAME, @@ -37,22 +36,6 @@ regexp_ci_skip = re.compile(r"\[(ci|skip| |-){3,}\]").search -def _incr(name: str): - """ - Increment a statsd counter. The passed-in counter will be prefixed with - "webhooks.github." - """ - metrics.incr("webhooks.github." + name) - - -def _incr_event(name: str): - """ - Increment a statsd counter. The passed in counter will be prefixed with - "webhooks.github.received." - """ - _incr("received." + name) - - class GithubWebhookHandler(APIView): """ GitHub Webhook Handler. Method names correspond to events as defined in @@ -95,11 +78,9 @@ def validate_signature(self, request): or len(computed_sig) != len(expected_sig) or not constant_time_compare(computed_sig, expected_sig) ): - _incr("invalid_signature") raise PermissionDenied() def unhandled_webhook_event(self, request, *args, **kwargs): - _incr_event("unhandled") return Response(data=WebhookHandlerErrorMessages.UNSUPPORTED_EVENT) def _get_repo(self, request): @@ -163,12 +144,10 @@ def _get_repo(self, request): raise NotFound("Repository does not exist") def ping(self, request, *args, **kwargs): - _incr_event(GitHubWebhookEvents.PING) return Response(data="pong") def repository(self, request, *args, **kwargs): action, repo = self.request.data.get("action"), self._get_repo(request) - _incr_event(GitHubWebhookEvents.REPOSITORY + "." + action) if action == "publicized": repo.private, repo.activated = False, False repo.save() @@ -202,7 +181,6 @@ def repository(self, request, *args, **kwargs): def delete(self, request, *args, **kwargs): ref_type = request.data.get("ref_type", "") - _incr_event(GitHubWebhookEvents.DELETE + "." + ref_type) repo = self._get_repo(request) if ref_type != "branch": log.info( @@ -221,7 +199,6 @@ def delete(self, request, *args, **kwargs): return Response() def public(self, request, *args, **kwargs): - _incr_event(GitHubWebhookEvents.PUBLIC) repo = self._get_repo(request) repo.private, repo.activated = False, False repo.save() @@ -233,7 +210,6 @@ def public(self, request, *args, **kwargs): def push(self, request, *args, **kwargs): ref_type = "branch" if request.data.get("ref", "")[5:10] == "heads" else "tag" - _incr_event(GitHubWebhookEvents.PUSH + "." + ref_type) repo = self._get_repo(request) if ref_type != "branch": log.debug( @@ -331,7 +307,6 @@ def push(self, request, *args, **kwargs): return Response() def status(self, request, *args, **kwargs): - _incr_event(GitHubWebhookEvents.STATUS) repo = self._get_repo(request) commitid = request.data.get("sha") @@ -377,7 +352,6 @@ def status(self, request, *args, **kwargs): return Response() def pull_request(self, request, *args, **kwargs): - _incr_event(GitHubWebhookEvents.PULL_REQUEST) repo = self._get_repo(request) if not repo.active: @@ -580,13 +554,11 @@ def _handle_installation_events( return Response(data="Integration webhook received") def installation(self, request, *args, **kwargs): - _incr_event(GitHubWebhookEvents.INSTALLATION) return self._handle_installation_events( request, *args, **kwargs, event=GitHubWebhookEvents.INSTALLATION ) def installation_repositories(self, request, *args, **kwargs): - _incr_event(GitHubWebhookEvents.INSTALLATION_REPOSITORIES) self._handle_installation_repository_events(request, *args, **kwargs) # This is kept to preserve the logic for deprecated usage of owner.installation_id # It can be removed once the move to codecov_auth.GithubAppInstallation is complete @@ -599,7 +571,6 @@ def installation_repositories(self, request, *args, **kwargs): def organization(self, request, *args, **kwargs): action = request.data.get("action") - _incr_event(GitHubWebhookEvents.ORGANIZATION + "." + action) if action == "member_removed": log.info( f"Removing user with service-id {request.data['membership']['user']['id']} " @@ -689,12 +660,10 @@ def _handle_marketplace_events(self, request, *args, **kwargs): return Response() def marketplace_purchase(self, request, *args, **kwargs): - _incr_event(GitHubWebhookEvents.MARKETPLACE_PURCHASE) return self._handle_marketplace_events(request, *args, **kwargs) def member(self, request, *args, **kwargs): action = request.data["action"] - _incr_event(GitHubWebhookEvents.MEMBER + "." + action) if action == "removed": repo = self._get_repo(request) log.info( @@ -747,7 +716,6 @@ def post(self, request, *args, **kwargs): self.validate_signature(request) - _incr_event("total") handler = getattr(self, self.event, self.unhandled_webhook_event) return handler(request, *args, **kwargs) From 52dd85cdf60f66f9349791f850f2be81431249da Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Tue, 22 Oct 2024 16:56:32 +0200 Subject: [PATCH 02/27] remove usage of mock_metrics --- upload/tests/test_upload.py | 9 -- webhook_handlers/tests/test_github.py | 114 +------------------------- 2 files changed, 1 insertion(+), 122 deletions(-) diff --git a/upload/tests/test_upload.py b/upload/tests/test_upload.py index f290fb0a32..56209c855f 100644 --- a/upload/tests/test_upload.py +++ b/upload/tests/test_upload.py @@ -22,7 +22,6 @@ TorngitObjectNotFoundError, TorngitRateLimitError, ) -from shared.utils.test_utils import mock_metrics as utils_mock_metrics from simplejson import JSONDecodeError from codecov_auth.models import Owner @@ -969,33 +968,25 @@ def test_options_headers(self): ) def test_invalid_request_params(self): - metrics = utils_mock_metrics(self.mocker) query_params = {"pr": 9838, "flags": "flags!!!", "package": "codecov-cli/0.0.0"} response = self._post(kwargs={"version": "v5"}, query=query_params) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert metrics.data["upload.cli.0.0.0"] == 1 - assert metrics.data["uploads.rejected"] == 1 def test_invalid_request_params_uploader_package(self): - metrics = utils_mock_metrics(self.mocker) query_params = {"pr": 9838, "flags": "flags!!!", "package": "uploader-0.0.0"} response = self._post(kwargs={"version": "v5"}, query=query_params) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert metrics.data["upload.uploader.0.0.0"] == 1 - assert metrics.data["uploads.rejected"] == 1 def test_invalid_request_params_invalid_package(self): - metrics = utils_mock_metrics(self.mocker) query_params = {"pr": 9838, "flags": "flags!!!", "package": ""} response = self._post(kwargs={"version": "v5"}, query=query_params) assert response.status_code == status.HTTP_400_BAD_REQUEST - assert metrics.data["uploads.rejected"] == 1 @patch("upload.views.legacy.get_redis_connection") @patch("upload.views.legacy.uuid4") diff --git a/webhook_handlers/tests/test_github.py b/webhook_handlers/tests/test_github.py index 2dfb6ae34c..b754e37753 100644 --- a/webhook_handlers/tests/test_github.py +++ b/webhook_handlers/tests/test_github.py @@ -7,7 +7,6 @@ import pytest from freezegun import freeze_time from rest_framework import status -from rest_framework.response import Response from rest_framework.reverse import reverse from rest_framework.test import APITestCase from shared.django_apps.core.tests.factories import ( @@ -17,7 +16,7 @@ PullFactory, RepositoryFactory, ) -from shared.utils.test_utils import mock_config_helper, mock_metrics +from shared.utils.test_utils import mock_config_helper from codecov_auth.models import ( GITHUB_APP_INSTALLATION_DEFAULT_NAME, @@ -86,113 +85,6 @@ def setUp(self): active=True, ) - @patch( - "webhook_handlers.views.github.GithubWebhookHandler._handle_installation_events", - lambda self, request, *args, **kwargs: Response(), - ) - @patch( - "webhook_handlers.views.github.GithubWebhookHandler._handle_marketplace_events", - lambda self, request, *args, **kwargs: Response(), - ) - @patch( - "webhook_handlers.views.github.GithubWebhookHandler._handle_installation_repository_events", - lambda self, request, *args, **kwargs: Response(), - ) - @patch( - "webhook_handlers.views.github.GithubWebhookHandler._handle_installation_events", - lambda self, request, *args, **kwargs: Response(), - ) - def test_webhook_counters(self): - metrics = mock_metrics(self.mocker) - # Simple events - for event in [ - "unhandled", - GitHubWebhookEvents.PING, - GitHubWebhookEvents.PUBLIC, - GitHubWebhookEvents.STATUS, - GitHubWebhookEvents.PULL_REQUEST, - GitHubWebhookEvents.INSTALLATION, - GitHubWebhookEvents.INSTALLATION_REPOSITORIES, - GitHubWebhookEvents.MARKETPLACE_PURCHASE, - ]: - with self.subTest("with event " + event): - _ = self._post_event_data( - event=event, - data={}, - ) - assert metrics.data["webhooks.github.received." + event] == 1 - - # Repository event + actions - for action in ["publicized", "privatized", "deleted"]: - with self.subTest("repository " + action): - _ = self._post_event_data( - event=GitHubWebhookEvents.REPOSITORY, - data={ - "action": action, - "repository": { - "id": self.repo.service_id, - "owner": {"id": -1}, - }, - }, - ) - assert ( - metrics.data["webhooks.github.received.repository." + action] == 1 - ) - - # Delete event + ref_types - for ref_type in ["branch", "other"]: - with self.subTest("delete " + ref_type): - branch = BranchFactory(repository=self.repo) - _ = self._post_event_data( - event=GitHubWebhookEvents.DELETE, - data={ - "ref": "refs/heads/" + branch.name, - "ref_type": ref_type, - "repository": {"id": self.repo.service_id}, - }, - ) - assert metrics.data["webhooks.github.received.delete." + ref_type] == 1 - - # Push event + ref_types - for ref_type, uri in [("branch", "refs/heads/"), ("tag", "refs/tags/")]: - with self.subTest("push " + ref_type): - _ = self._post_event_data( - event=GitHubWebhookEvents.PUSH, - data={ - "ref": uri + "unmerged", - "repository": {"id": self.repo.service_id}, - "commits": [], - }, - ) - assert metrics.data["webhooks.github.received.push." + ref_type] == 1 - - # Organization event + actions - for action in ["member_removed", "other"]: - with self.subTest("organization " + action): - _ = self._post_event_data( - event=GitHubWebhookEvents.ORGANIZATION, - data={ - "action": action, - "membership": {"user": {"id": -1}}, - "organization": {"id": -1}, - }, - ) - assert ( - metrics.data["webhooks.github.received.organization." + action] == 1 - ) - - # Member event + actions - for action in ["removed", "other"]: - with self.subTest("member " + action): - _ = self._post_event_data( - event=GitHubWebhookEvents.MEMBER, - data={ - "action": action, - "member": {"id": -1}, - "repository": {"id": self.repo.service_id}, - }, - ) - def test_get_repo_paths_dont_crash(self): with self.subTest("with ownerid success"): self._post_event_data( @@ -1275,7 +1167,6 @@ def test_marketplace_purchase_but_user_has_stripe_subscription( ) def test_signature_validation(self): - metrics = mock_metrics(self.mocker) response = self.client.post( reverse("github-webhook"), **{ @@ -1288,7 +1179,6 @@ def test_signature_validation(self): ) assert response.status_code == status.HTTP_403_FORBIDDEN - assert metrics.data["webhooks.github.invalid_signature"] == 1 response = self.client.post( reverse("github-webhook"), @@ -1307,7 +1197,6 @@ def test_signature_validation(self): ) assert response.status_code == status.HTTP_200_OK - assert metrics.data["webhooks.github.received.total"] == 1 response = self.client.post( reverse("github-webhook"), @@ -1326,7 +1215,6 @@ def test_signature_validation(self): ) assert response.status_code == status.HTTP_200_OK - assert metrics.data["webhooks.github.received.total"] == 2 @patch("webhook_handlers.views.github.get_config") def test_signature_validation_with_string_key(self, get_config_mock): From 2909aa6b80d6df89565003ed89412c93a67d28b1 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:30:08 -0400 Subject: [PATCH 03/27] feat: add Prometheus metrics for uploads --- upload/metrics.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 upload/metrics.py diff --git a/upload/metrics.py b/upload/metrics.py new file mode 100644 index 0000000000..378459e54a --- /dev/null +++ b/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", + ], +) From 91a8578410d47c79e752e279263683bfc7037bbf Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:30:49 -0400 Subject: [PATCH 04/27] Replace GQL Sentry Metrics with Prometheus metrics --- graphql_api/tests/test_views.py | 12 +++++------- graphql_api/views.py | 24 +++++++++++++++--------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/graphql_api/tests/test_views.py b/graphql_api/tests/test_views.py index 9d6b68cc90..6d9661c324 100644 --- a/graphql_api/tests/test_views.py +++ b/graphql_api/tests/test_views.py @@ -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,8 @@ 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 diff --git a/graphql_api/views.py b/graphql_api/views.py index bb2afd631d..da9ceda4f0 100644 --- a/graphql_api/views.py +++ b/graphql_api/views.py @@ -19,7 +19,6 @@ ) 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 codecov.commands.exceptions import BaseException @@ -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)(?:\(| |{)" @@ -226,10 +236,9 @@ 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}) - + GQL_REQUEST_MADE_COUNTER.labels(path=req_path).inc() if self._check_ratelimit(request=request): - sentry_metrics.incr("graphql.error.rate_limit", tags={"path": req_path}) + GQL_ERROR_TYPE_COUNTER.labels(error_type="rate_limit", path=req_path).inc() return JsonResponse( data={ "status": 429, @@ -245,7 +254,7 @@ 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}) + GQL_ERROR_TYPE_COUNTER.labels(error_type="all", path=req_path).inc() try: if data["errors"][0]["extensions"]["cost"]: costs = data["errors"][0]["extensions"]["cost"] @@ -257,10 +266,7 @@ async def post(self, request, *args, **kwargs): request_body=req_body, ), ) - sentry_metrics.incr( - "graphql.error.query_cost_exceeded", - tags={"path": req_path}, - ) + GQL_ERROR_TYPE_COUNTER.labels(error_type="query_cost_exceeded", path=req_path).inc() return HttpResponseBadRequest( JsonResponse("Your query is too costly.") ) From ac58dbf4a5b8aa9a4657205d6f1156fbf5bc16bf Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:31:35 -0400 Subject: [PATCH 05/27] validate: Replace Sentry metrics with Prometheus metrics --- validate/tests/test_validate_v2.py | 14 +++++++------- validate/views.py | 9 +++++++-- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/validate/tests/test_validate_v2.py b/validate/tests/test_validate_v2.py index d0cd010d84..d1dbd22fa9 100644 --- a/validate/tests/test_validate_v2.py +++ b/validate/tests/test_validate_v2.py @@ -4,7 +4,7 @@ from rest_framework.reverse import reverse -@patch("validate.views.sentry_metrics.incr") +@patch("validate.views.API_VALIDATE_V2_COUNTER.labels") class TestValidateYamlV2Handler(TestCase): def _post(self, data, query_source=""): client = Client() @@ -20,7 +20,7 @@ def test_no_data(self, mock_metrics): res = self._post("") assert res.status_code == 400 assert res.json() == {"valid": False, "message": "YAML is empty"} - mock_metrics.assert_called_once_with("validate_v2", tags={"source": "unknown"}) + mock_metrics.assert_called_once_with(**{"source": "unknown"}) def test_list_type(self, mock_metrics): res = self._post("- testing: 123") @@ -29,7 +29,7 @@ def test_list_type(self, mock_metrics): "valid": False, "message": "YAML must be a dictionary type", } - mock_metrics.assert_called_once_with("validate_v2", tags={"source": "unknown"}) + mock_metrics.assert_called_once_with(**{"source": "unknown"}) def test_parse_error(self, mock_metrics): res = self._post("foo: - 123") @@ -43,7 +43,7 @@ def test_parse_error(self, mock_metrics): "problem": "sequence entries are not allowed here", }, } - mock_metrics.assert_called_once_with("validate_v2", tags={"source": "unknown"}) + mock_metrics.assert_called_once_with(**{"source": "unknown"}) def test_parse_invalid(self, mock_metrics): res = self._post("comment: 123") @@ -53,7 +53,7 @@ def test_parse_invalid(self, mock_metrics): "message": "YAML does not match the accepted schema", "validation_error": {"comment": ["must be of ['dict', 'boolean'] type"]}, } - mock_metrics.assert_called_once_with("validate_v2", tags={"source": "unknown"}) + mock_metrics.assert_called_once_with(**{"source": "unknown"}) def test_parse_valid(self, mock_metrics): res = self._post("comment: true") @@ -65,9 +65,9 @@ def test_parse_valid(self, mock_metrics): "comment": True, }, } - mock_metrics.assert_called_once_with("validate_v2", tags={"source": "unknown"}) + mock_metrics.assert_called_once_with(**{"source": "unknown"}) def test_query_source_metric(self, mock_metrics): self._post("comment: true", query_source="vscode") mock_metrics.assert_called() - mock_metrics.assert_called_with("validate_v2", tags={"source": "vscode"}) + mock_metrics.assert_called_with(**{"source": "vscode"}) diff --git a/validate/views.py b/validate/views.py index e0d885e0bc..7940067009 100644 --- a/validate/views.py +++ b/validate/views.py @@ -7,13 +7,18 @@ from rest_framework.permissions import AllowAny from rest_framework.response import Response from rest_framework.views import APIView -from sentry_sdk import metrics as sentry_metrics +from shared.metrics import Counter from shared.validation.exceptions import InvalidYamlException from shared.yaml.validation import validate_yaml from yaml import YAMLError, safe_load log = logging.getLogger(__name__) +API_VALIDATE_V2_COUNTER = Counter( + "api_validate_v2", + "Number of times the validate v2 endpoint has been hit", +) + class V1ValidateYamlHandler(APIView): permission_classes = [AllowAny] @@ -79,7 +84,7 @@ class V2ValidateYamlHandler(V1ValidateYamlHandler): def post(self, request, *args, **kwargs): source = self.request.query_params.get("source", "unknown") - sentry_metrics.incr("validate_v2", tags={"source": source}) + API_VALIDATE_V2_COUNTER.labels(source=source).inc() if not self.request.body: return Response( From 6d7249701701ba24ac2fd4531685427f441045a2 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:32:29 -0400 Subject: [PATCH 06/27] upload: Modify generate metrics labels helper function --- upload/helpers.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/upload/helpers.py b/upload/helpers.py index 77194a8595..194600ef47 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -790,7 +790,7 @@ def get_version_from_headers(headers): return "unknown-user-agent" -def generate_upload_sentry_metrics_tags( +def generate_upload_prometheus_metrics_tags( action, request, is_shelter_request, @@ -798,6 +798,7 @@ def generate_upload_sentry_metrics_tags( repository: Optional[Repository] = None, position: Optional[str] = None, upload_version: Optional[str] = None, + fill_labels: bool = True, ): metrics_tags = dict( agent=get_agent_from_headers(request.headers), @@ -806,13 +807,19 @@ 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 + } + + for field, value in optional_fields.items(): + if value or fill_labels: + metrics_tags[field] = value return metrics_tags From 5af73696ed7e7a8335799c00011b7ab15a8675b0 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:34:03 -0400 Subject: [PATCH 07/27] commits: replace Sentry metrics with Prometheus metrics --- upload/tests/views/test_commits.py | 8 ++++---- upload/views/commits.py | 20 +++++++++----------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/upload/tests/views/test_commits.py b/upload/tests/views/test_commits.py index e54a8e2327..8cdf7d117d 100644 --- a/upload/tests/views/test_commits.py +++ b/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,9 +374,8 @@ 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", @@ -384,5 +383,6 @@ def test_commit_github_oidc_auth(mock_jwks_client, mock_jwt_decode, db, mocker): "repo_visibility": "public", "is_using_shelter": "no", "position": "end", + "upload_version": None, }, ) diff --git a/upload/views/commits.py b/upload/views/commits.py index cac17e0c33..4a6563ac09 100644 --- a/upload/views/commits.py +++ b/upload/views/commits.py @@ -2,7 +2,7 @@ from rest_framework.exceptions import NotAuthenticated from rest_framework.generics import ListCreateAPIView -from sentry_sdk import metrics as sentry_metrics +from shared.metrics import Counter from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -14,14 +14,14 @@ repo_auth_custom_exception_handler, ) from core.models import Commit -from upload.helpers import generate_upload_sentry_metrics_tags +from upload.helpers import generate_upload_prometheus_metrics_tags +from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import CommitSerializer from upload.views.base import GetterMixin from upload.views.uploads import CanDoCoverageUploadsPermission log = logging.getLogger(__name__) - class CommitViews(ListCreateAPIView, GetterMixin): serializer_class = CommitSerializer permission_classes = [CanDoCoverageUploadsPermission] @@ -53,16 +53,15 @@ def create(self, request, *args, **kwargs): return super().create(request, *args, **kwargs) def perform_create(self, serializer): - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_commit", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ) + ).inc() repository = self.get_repo() commit = serializer.save(repository=repository) @@ -72,9 +71,8 @@ def perform_create(self, serializer): extra=dict(repo=repository.name, commit=commit.commitid), ) - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_commit", request=self.request, @@ -82,6 +80,6 @@ def perform_create(self, serializer): is_shelter_request=self.is_shelter_request(), position="end", ), - ) + ).inc() return commit From af3f0f05edcc6ae0e3be1ea197dc4ab7e06319d3 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:37:30 -0400 Subject: [PATCH 08/27] Add fill_labels=False to avoid redundant labels --- upload/views/bundle_analysis.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/upload/views/bundle_analysis.py b/upload/views/bundle_analysis.py index f9a37e2cfe..4b721e83cd 100644 --- a/upload/views/bundle_analysis.py +++ b/upload/views/bundle_analysis.py @@ -26,7 +26,7 @@ from services.archive import ArchiveService from services.redis_configuration import get_redis_connection from timeseries.models import Dataset, MeasurementName -from upload.helpers import dispatch_upload_task, generate_upload_sentry_metrics_tags +from upload.helpers import dispatch_upload_task, generate_upload_prometheus_metrics_tags from upload.views.base import ShelterMixin from upload.views.helpers import get_repository_from_string @@ -81,12 +81,13 @@ def get_exception_handler(self) -> Callable: def post(self, request: HttpRequest) -> Response: try: - labels = generate_upload_sentry_metrics_tags( + labels = generate_upload_prometheus_metrics_tags( action="bundle_analysis", endpoint="bundle_analysis", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", + fill_labels=False, ) BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels(**labels).inc() except Exception: @@ -175,12 +176,13 @@ def post(self, request: HttpRequest) -> Response: ), ) try: - labels = generate_upload_sentry_metrics_tags( + labels = generate_upload_prometheus_metrics_tags( action="bundle_analysis", endpoint="bundle_analysis", request=self.request, is_shelter_request=self.is_shelter_request(), position="end", + fill_labels=False, ) BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels(**labels).inc() except Exception: From 4c79e540f0eebb7274476d4c6aa2d2b91822364d Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:37:56 -0400 Subject: [PATCH 09/27] empty_upload: Replace Sentry metrics with Prometheus metrics --- upload/tests/views/test_empty_upload.py | 8 ++++---- upload/views/empty_upload.py | 19 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/upload/tests/views/test_empty_upload.py b/upload/tests/views/test_empty_upload.py index b1fd74fa39..7ad51f3a46 100644 --- a/upload/tests/views/test_empty_upload.py +++ b/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,9 +93,8 @@ 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", @@ -103,6 +102,7 @@ def test_empty_upload_with_yaml_ignored_files( "repo_visibility": "private", "is_using_shelter": "no", "position": "end", + "upload_version": None, }, ) diff --git a/upload/views/empty_upload.py b/upload/views/empty_upload.py index aecef561e7..82dc826acb 100644 --- a/upload/views/empty_upload.py +++ b/upload/views/empty_upload.py @@ -8,7 +8,6 @@ from rest_framework.exceptions import NotFound from rest_framework.generics import CreateAPIView from rest_framework.response import Response -from sentry_sdk import metrics as sentry_metrics from shared.torngit.exceptions import TorngitClientError, TorngitClientGeneralError from codecov_auth.authentication.repo_auth import ( @@ -23,9 +22,10 @@ from services.task import TaskService from services.yaml import final_commit_yaml from upload.helpers import ( - generate_upload_sentry_metrics_tags, + generate_upload_prometheus_metrics_tags, try_to_get_best_possible_bot_token, ) +from upload.metrics import API_UPLOAD_COUNTER from upload.views.base import GetterMixin from upload.views.uploads import CanDoCoverageUploadsPermission @@ -82,16 +82,15 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def post(self, request, *args, **kwargs): - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="empty_upload", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ) + ).inc() serializer = EmptyUploadSerializer(data=request.data) if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) @@ -149,9 +148,9 @@ def post(self, request, *args, **kwargs): ) ) ] - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( + tags=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="empty_upload", request=self.request, @@ -159,7 +158,7 @@ def post(self, request, *args, **kwargs): is_shelter_request=self.is_shelter_request(), position="end", ), - ) + ).inc() if set(changed_files) == set(ignored_changed_files): TaskService().notify( repoid=repo.repoid, commitid=commit.commitid, empty_upload="pass" From cfbe611e57a1dc914cde1f0474a9607494dc904e Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:39:21 -0400 Subject: [PATCH 10/27] legacy upload: replace Sentry metrics with Prometheus metrics --- upload/views/legacy.py | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/upload/views/legacy.py b/upload/views/legacy.py index 3922fd4c74..c9f57c7bf3 100644 --- a/upload/views/legacy.py +++ b/upload/views/legacy.py @@ -16,8 +16,7 @@ from rest_framework.exceptions import ValidationError from rest_framework.permissions import AllowAny from rest_framework.views import APIView -from sentry_sdk import metrics as sentry_metrics -from shared.metrics import metrics +from shared.metrics import metrics, Counter from codecov.db import sync_to_async from codecov_auth.commands.owner import OwnerCommands @@ -32,19 +31,25 @@ determine_upload_commit_to_use, determine_upload_pr_to_use, dispatch_upload_task, - generate_upload_sentry_metrics_tags, + generate_upload_prometheus_metrics_tags, insert_commit, parse_headers, parse_params, store_report_in_redis, validate_upload, ) +from upload.metrics import API_UPLOAD_COUNTER from upload.views.base import ShelterMixin from utils.config import get_config from utils.services import get_long_service_name log = logging.getLogger(__name__) +API_UPLOAD_COUNTER = Counter( + "api_upload", + "Number of times API upload endpoint request starts", + ["version"], +) class PlainTextRenderer(renderers.BaseRenderer): media_type = "text/plain" @@ -75,9 +80,8 @@ def options(self, request, *args, **kwargs): def post(self, request, *args, **kwargs): # Extract the version version = self.kwargs["version"] - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="legacy_upload", request=self.request, @@ -85,7 +89,7 @@ def post(self, request, *args, **kwargs): position="start", upload_version=version, ), - ) + ).inc() log.info( f"Received upload request {version}", @@ -165,9 +169,8 @@ def post(self, request, *args, **kwargs): ), ) - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="legacy_upload", request=self.request, @@ -176,19 +179,7 @@ def post(self, request, *args, **kwargs): position="end", upload_version=version, ), - ) - - sentry_metrics.set( - "upload_set", - repository.author.ownerid, - tags=generate_upload_sentry_metrics_tags( - action="coverage", - endpoint="legacy_upload", - request=self.request, - repository=repository, - is_shelter_request=self.is_shelter_request(), - ), - ) + ).inc() # Validate the upload to make sure the org has enough repo credits and is allowed to upload for this commit redis = get_redis_connection() From 9fe6d4bf439bf4d757b4274887d093d5a0aaa885 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:40:15 -0400 Subject: [PATCH 11/27] create_upload: replace Sentry metrics with Prometheus metrics --- upload/tests/views/test_uploads.py | 22 ++++------------------ upload/views/uploads.py | 30 ++++++++---------------------- 2 files changed, 12 insertions(+), 40 deletions(-) diff --git a/upload/tests/views/test_uploads.py b/upload/tests/views/test_uploads.py index 5eced188a1..332b89d829 100644 --- a/upload/tests/views/test_uploads.py +++ b/upload/tests/views/test_uploads.py @@ -727,8 +727,7 @@ def test_uploads_post_shelter(db, mocker, mock_redis): mocker.patch( "upload.views.uploads.UploadViews.trigger_upload_task", return_value=True ) - mock_sentry_metrics = mocker.patch("upload.views.uploads.sentry_metrics.incr") - mock_sentry_metrics_set = mocker.patch("upload.views.uploads.sentry_metrics.set") + mock_prometheus_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels") repository = RepositoryFactory( name="the_repo", author__username="codecov", author__service="github" @@ -764,9 +763,8 @@ def test_uploads_post_shelter(db, mocker, mock_redis): }, ) - mock_sentry_metrics.assert_called_with( - "upload", - tags={ + mock_prometheus_metrics.assert_called_with( + **{ "agent": "cli", "version": "0.4.7", "action": "coverage", @@ -774,19 +772,7 @@ def test_uploads_post_shelter(db, mocker, mock_redis): "repo_visibility": "private", "is_using_shelter": "yes", "position": "end", - }, - ) - - mock_sentry_metrics_set.assert_called_with( - "upload_set", - owner.ownerid, - tags={ - "agent": "cli", - "version": "0.4.7", - "action": "coverage", - "endpoint": "create_upload", - "repo_visibility": "private", - "is_using_shelter": "yes", + "upload_version": None, }, ) diff --git a/upload/views/uploads.py b/upload/views/uploads.py index ec80a40957..970f47e4d6 100644 --- a/upload/views/uploads.py +++ b/upload/views/uploads.py @@ -5,7 +5,6 @@ from rest_framework.exceptions import ValidationError from rest_framework.generics import ListCreateAPIView from rest_framework.permissions import BasePermission -from sentry_sdk import metrics as sentry_metrics from shared.metrics import metrics from shared.upload.utils import UploaderType, insert_coverage_measurement @@ -29,9 +28,10 @@ from services.redis_configuration import get_redis_connection from upload.helpers import ( dispatch_upload_task, - generate_upload_sentry_metrics_tags, + generate_upload_prometheus_metrics_tags, validate_activated_repo, ) +from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import UploadSerializer from upload.throttles import UploadsPerCommitThrottle, UploadsPerWindowThrottle from upload.views.base import GetterMixin @@ -68,33 +68,20 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def perform_create(self, serializer: UploadSerializer): - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_upload", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ) + ).incr() repository: Repository = self.get_repo() validate_activated_repo(repository) commit: Commit = self.get_commit(repository) report: CommitReport = self.get_report(commit) - sentry_metrics.set( - "upload_set", - repository.author.ownerid, - tags=generate_upload_sentry_metrics_tags( - action="coverage", - endpoint="create_upload", - request=self.request, - repository=repository, - is_shelter_request=self.is_shelter_request(), - ), - ) - version = ( serializer.validated_data["version"] if "version" in serializer.validated_data @@ -141,9 +128,8 @@ def perform_create(self, serializer: UploadSerializer): instance.storage_path = path instance.save() self.trigger_upload_task(repository, commit.commitid, instance, report) - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_upload", request=self.request, @@ -151,7 +137,7 @@ def perform_create(self, serializer: UploadSerializer): is_shelter_request=self.is_shelter_request(), position="end", ), - ) + ).incr() metrics.incr("uploads.accepted", 1) self.activate_repo(repository) self.send_analytics_data(commit, instance, version) From 66f02a990883b6fc0992063bfd138802415f6743 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:41:09 -0400 Subject: [PATCH 12/27] upload_complete: replace Sentry metrics with Prometheus metrics --- upload/tests/views/test_upload_completion.py | 10 ++++------ upload/views/upload_completion.py | 18 ++++++++---------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/upload/tests/views/test_upload_completion.py b/upload/tests/views/test_upload_completion.py index 2d636af599..9c6b5f44e0 100644 --- a/upload/tests/views/test_upload_completion.py +++ b/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,9 +86,8 @@ 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", @@ -98,6 +95,7 @@ def test_upload_completion_view_processed_uploads(mocked_manual_trigger, db, moc "repo_visibility": "private", "is_using_shelter": "no", "position": "end", + "upload_version": None, }, ) diff --git a/upload/views/upload_completion.py b/upload/views/upload_completion.py index 2fe637ef83..9c2b41e85e 100644 --- a/upload/views/upload_completion.py +++ b/upload/views/upload_completion.py @@ -3,7 +3,6 @@ from rest_framework import status from rest_framework.generics import CreateAPIView from rest_framework.response import Response -from sentry_sdk import metrics as sentry_metrics from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -14,7 +13,8 @@ ) from reports.models import ReportSession from services.task import TaskService -from upload.helpers import generate_upload_sentry_metrics_tags +from upload.helpers import generate_upload_prometheus_metrics_tags +from upload.metrics import API_UPLOAD_COUNTER from upload.views.base import GetterMixin from upload.views.uploads import CanDoCoverageUploadsPermission @@ -34,16 +34,15 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def post(self, request, *args, **kwargs): - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="upload_complete", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ) + ).inc() repo = self.get_repo() commit = self.get_commit(repo) uploads_queryset = ReportSession.objects.filter( @@ -78,9 +77,8 @@ def post(self, request, *args, **kwargs): errored_uploads += 1 TaskService().manual_upload_completion_trigger(repo.repoid, commit.commitid) - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="upload_complete", request=self.request, @@ -88,7 +86,7 @@ def post(self, request, *args, **kwargs): is_shelter_request=self.is_shelter_request(), position="end", ), - ) + ).inc() return Response( data={ "uploads_total": uploads_count, From 3f86b598d92d95172978ca36d1ef1595933b5d0d Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:41:44 -0400 Subject: [PATCH 13/27] test_results: replace Sentry metrics with Prometheus metrics --- upload/tests/views/test_test_results.py | 8 ++++---- upload/views/test_results.py | 18 ++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/upload/tests/views/test_test_results.py b/upload/tests/views/test_test_results.py index ab2b132a4b..ddec708d33 100644 --- a/upload/tests/views/test_test_results.py +++ b/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,9 +100,8 @@ 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", @@ -110,6 +109,7 @@ def test_upload_test_results(db, client, mocker, mock_redis): "repo_visibility": "private", "is_using_shelter": "no", "position": "end", + "upload_version": None, }, ) diff --git a/upload/views/test_results.py b/upload/views/test_results.py index b3e2609ebd..38bd2b6077 100644 --- a/upload/views/test_results.py +++ b/upload/views/test_results.py @@ -7,7 +7,6 @@ from rest_framework.permissions import BasePermission from rest_framework.response import Response from rest_framework.views import APIView -from sentry_sdk import metrics from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -23,7 +22,8 @@ from reports.models import CommitReport from services.archive import ArchiveService, MinioEndpoints from services.redis_configuration import get_redis_connection -from upload.helpers import dispatch_upload_task, generate_upload_sentry_metrics_tags +from upload.helpers import dispatch_upload_task, generate_upload_prometheus_metrics_tags +from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import FlagListField from upload.views.base import ShelterMixin from upload.views.helpers import get_repository_from_string @@ -66,16 +66,15 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def post(self, request): - metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="test_results", endpoint="test_results", request=request, is_shelter_request=self.is_shelter_request(), position="start", ), - ) + ).inc() serializer = UploadSerializer(data=request.data) if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) @@ -107,9 +106,8 @@ def post(self, request): if update_fields: repo.save(update_fields=update_fields) - metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="test_results", endpoint="test_results", request=request, @@ -117,7 +115,7 @@ def post(self, request): is_shelter_request=self.is_shelter_request(), position="end", ), - ) + ).inc() commit, _ = Commit.objects.get_or_create( commitid=data["commit"], From e9ef0b415bf4670e42a897050d1b82c9eb0997b0 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:42:03 -0400 Subject: [PATCH 14/27] reports: replace Sentry metrics with Prometheus metrics --- upload/tests/views/test_reports.py | 16 +++++++-------- upload/views/reports.py | 32 +++++++++++++----------------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/upload/tests/views/test_reports.py b/upload/tests/views/test_reports.py index 213b2be2f0..3c4462a1ca 100644 --- a/upload/tests/views/test_reports.py +++ b/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_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels") repository = RepositoryFactory( name="the_repo", author__username="codecov", author__service="github" ) @@ -65,9 +65,8 @@ 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_metrics.assert_called_with( + **{ "agent": "cli", "version": "0.4.7", "action": "coverage", @@ -75,6 +74,7 @@ def test_reports_post(client, db, mocker): "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,9 +383,8 @@ 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", @@ -393,6 +392,7 @@ def test_reports_results_post_successful_github_oidc_auth( "repo_visibility": "private", "is_using_shelter": "no", "position": "end", + "upload_version": None, }, ) diff --git a/upload/views/reports.py b/upload/views/reports.py index a0020cbf9a..e9cc9a3eb3 100644 --- a/upload/views/reports.py +++ b/upload/views/reports.py @@ -3,7 +3,6 @@ from django.http import HttpRequest, HttpResponseNotAllowed from rest_framework.exceptions import ValidationError from rest_framework.generics import CreateAPIView, ListCreateAPIView, RetrieveAPIView -from sentry_sdk import metrics as sentry_metrics from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -16,7 +15,8 @@ ) from reports.models import CommitReport, ReportResults from services.task import TaskService -from upload.helpers import generate_upload_sentry_metrics_tags +from upload.helpers import generate_upload_prometheus_metrics_tags +from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import CommitReportSerializer, ReportResultsSerializer from upload.views.base import GetterMixin from upload.views.uploads import CanDoCoverageUploadsPermission @@ -40,16 +40,15 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def perform_create(self, serializer): - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_report", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ) + ).inc() repository = self.get_repo() commit = self.get_commit(repository) log.info( @@ -68,9 +67,8 @@ def perform_create(self, serializer): repository.repoid, commit.commitid, instance.code ) - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_report", request=self.request, @@ -78,7 +76,7 @@ def perform_create(self, serializer): is_shelter_request=self.is_shelter_request(), position="end", ), - ) + ).inc() return instance def list(self, request: HttpRequest, service: str, repo: str, commit_sha: str): @@ -105,16 +103,15 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def perform_create(self, serializer): - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_report_results", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ) + ).inc() repository = self.get_repo() commit = self.get_commit(repository) report = self.get_report(commit) @@ -131,9 +128,8 @@ def perform_create(self, serializer): repoid=repository.repoid, report_code=report.code, ) - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + API_UPLOAD_COUNTER.labels( + **generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_report_results", request=self.request, @@ -141,7 +137,7 @@ def perform_create(self, serializer): is_shelter_request=self.is_shelter_request(), position="end", ), - ) + ).incr() return instance def get_object(self): From 9ff1ba87c68749988a0ad35b88d07648495b6468 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 16:50:16 -0400 Subject: [PATCH 15/27] Fix incr() and inc() typos --- upload/views/uploads.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/upload/views/uploads.py b/upload/views/uploads.py index 970f47e4d6..ff5fab1687 100644 --- a/upload/views/uploads.py +++ b/upload/views/uploads.py @@ -76,7 +76,7 @@ def perform_create(self, serializer: UploadSerializer): is_shelter_request=self.is_shelter_request(), position="start", ), - ).incr() + ).inc() repository: Repository = self.get_repo() validate_activated_repo(repository) commit: Commit = self.get_commit(repository) @@ -137,7 +137,7 @@ def perform_create(self, serializer: UploadSerializer): is_shelter_request=self.is_shelter_request(), position="end", ), - ).incr() + ).inc() metrics.incr("uploads.accepted", 1) self.activate_repo(repository) self.send_analytics_data(commit, instance, version) From 2dfa87bd0b61e86e6b734c8f93fef70f76e5ab9b Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 17:08:44 -0400 Subject: [PATCH 16/27] fix: Lints --- graphql_api/tests/test_views.py | 4 +++- graphql_api/views.py | 4 +++- upload/helpers.py | 4 ++-- upload/views/commits.py | 1 + upload/views/empty_upload.py | 1 - upload/views/legacy.py | 1 + 6 files changed, 10 insertions(+), 5 deletions(-) diff --git a/graphql_api/tests/test_views.py b/graphql_api/tests/test_views.py index 6d9661c324..e5805ca1e2 100644 --- a/graphql_api/tests/test_views.py +++ b/graphql_api/tests/test_views.py @@ -218,7 +218,9 @@ async def test_when_rate_limit_reached( == "It looks like you've hit the rate limit of 1000 req/min. Try again later." ) - mocked_error_counter.assert_called_with(error_type="rate_limit", path="/graphql/gh") + mocked_error_counter.assert_called_with( + error_type="rate_limit", path="/graphql/gh" + ) mocked_request_counter.assert_called_with(path="/graphql/gh") @override_settings( diff --git a/graphql_api/views.py b/graphql_api/views.py index da9ceda4f0..9cfa8594bd 100644 --- a/graphql_api/views.py +++ b/graphql_api/views.py @@ -266,7 +266,9 @@ async def post(self, request, *args, **kwargs): request_body=req_body, ), ) - GQL_ERROR_TYPE_COUNTER.labels(error_type="query_cost_exceeded", path=req_path).inc() + GQL_ERROR_TYPE_COUNTER.labels( + error_type="query_cost_exceeded", path=req_path + ).inc() return HttpResponseBadRequest( JsonResponse("Your query is too costly.") ) diff --git a/upload/helpers.py b/upload/helpers.py index 194600ef47..6bd559ae18 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -807,7 +807,7 @@ def generate_upload_prometheus_metrics_tags( endpoint=endpoint, is_using_shelter="yes" if is_shelter_request else "no", ) - + repo_visibility = None if repository: repo_visibility = "private" if repository.private else "public" @@ -815,7 +815,7 @@ def generate_upload_prometheus_metrics_tags( optional_fields = { "repo_visibility": repo_visibility, "position": position, - "upload_version": upload_version + "upload_version": upload_version, } for field, value in optional_fields.items(): diff --git a/upload/views/commits.py b/upload/views/commits.py index 4a6563ac09..d7dd358d0d 100644 --- a/upload/views/commits.py +++ b/upload/views/commits.py @@ -22,6 +22,7 @@ log = logging.getLogger(__name__) + class CommitViews(ListCreateAPIView, GetterMixin): serializer_class = CommitSerializer permission_classes = [CanDoCoverageUploadsPermission] diff --git a/upload/views/empty_upload.py b/upload/views/empty_upload.py index 82dc826acb..c51557fdc6 100644 --- a/upload/views/empty_upload.py +++ b/upload/views/empty_upload.py @@ -150,7 +150,6 @@ def post(self, request, *args, **kwargs): ] API_UPLOAD_COUNTER.labels( **generate_upload_prometheus_metrics_tags( - tags=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="empty_upload", request=self.request, diff --git a/upload/views/legacy.py b/upload/views/legacy.py index c9f57c7bf3..edaef286f0 100644 --- a/upload/views/legacy.py +++ b/upload/views/legacy.py @@ -51,6 +51,7 @@ ["version"], ) + class PlainTextRenderer(renderers.BaseRenderer): media_type = "text/plain" format = "txt" From 2e7153109120a5a3c1a39d2e66ee088172a1421a Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 17:13:41 -0400 Subject: [PATCH 17/27] More syntax and lint fixes --- graphql_api/tests/test_views.py | 2 +- upload/helpers.py | 4 +--- upload/views/commits.py | 1 - upload/views/legacy.py | 8 +------- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/graphql_api/tests/test_views.py b/graphql_api/tests/test_views.py index e5805ca1e2..118769ad94 100644 --- a/graphql_api/tests/test_views.py +++ b/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 diff --git a/upload/helpers.py b/upload/helpers.py index 6bd559ae18..9260b290ec 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -818,8 +818,6 @@ def generate_upload_prometheus_metrics_tags( "upload_version": upload_version, } - for field, value in optional_fields.items(): - if value or fill_labels: - metrics_tags[field] = value + metrics_tags.update({field: value for field, value in optional_fields.items() if value or fill_labels}) return metrics_tags diff --git a/upload/views/commits.py b/upload/views/commits.py index d7dd358d0d..6ed91ac660 100644 --- a/upload/views/commits.py +++ b/upload/views/commits.py @@ -2,7 +2,6 @@ from rest_framework.exceptions import NotAuthenticated from rest_framework.generics import ListCreateAPIView -from shared.metrics import Counter from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, diff --git a/upload/views/legacy.py b/upload/views/legacy.py index edaef286f0..e43beb853f 100644 --- a/upload/views/legacy.py +++ b/upload/views/legacy.py @@ -16,7 +16,7 @@ from rest_framework.exceptions import ValidationError from rest_framework.permissions import AllowAny from rest_framework.views import APIView -from shared.metrics import metrics, Counter +from shared.metrics import metrics from codecov.db import sync_to_async from codecov_auth.commands.owner import OwnerCommands @@ -45,12 +45,6 @@ log = logging.getLogger(__name__) -API_UPLOAD_COUNTER = Counter( - "api_upload", - "Number of times API upload endpoint request starts", - ["version"], -) - class PlainTextRenderer(renderers.BaseRenderer): media_type = "text/plain" From eaf9a4166293e8db9d312315ff32e8b0a5b89e79 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 17:17:22 -0400 Subject: [PATCH 18/27] helpers: prometheus metrics labels function linting fix --- upload/helpers.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/upload/helpers.py b/upload/helpers.py index 9260b290ec..f5c360fa20 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -818,6 +818,12 @@ def generate_upload_prometheus_metrics_tags( "upload_version": upload_version, } - metrics_tags.update({field: value for field, value in optional_fields.items() if value or fill_labels}) + metrics_tags.update( + { + field: value + for field, value in optional_fields.items() + if value or fill_labels + } + ) return metrics_tags From 4d2c20e455a664a78604485d35405c8cd2184eb5 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Tue, 22 Oct 2024 17:26:05 -0400 Subject: [PATCH 19/27] reports: Make mock metrics name consistent --- upload/tests/views/test_reports.py | 4 ++-- upload/views/reports.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/upload/tests/views/test_reports.py b/upload/tests/views/test_reports.py index 3c4462a1ca..f6a61b75a0 100644 --- a/upload/tests/views/test_reports.py +++ b/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_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels") + mock_prometheus_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels") repository = RepositoryFactory( name="the_repo", author__username="codecov", author__service="github" ) @@ -65,7 +65,7 @@ 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_metrics.assert_called_with( + mock_prometheus_metrics.assert_called_with( **{ "agent": "cli", "version": "0.4.7", diff --git a/upload/views/reports.py b/upload/views/reports.py index e9cc9a3eb3..2895ace725 100644 --- a/upload/views/reports.py +++ b/upload/views/reports.py @@ -137,7 +137,7 @@ def perform_create(self, serializer): is_shelter_request=self.is_shelter_request(), position="end", ), - ).incr() + ).inc() return instance def get_object(self): From 61865f0f54bf903daed6cbb0251ea31ca4adba4d Mon Sep 17 00:00:00 2001 From: Tony Le Date: Wed, 23 Oct 2024 12:02:51 -0400 Subject: [PATCH 20/27] Use inc_counter in shared.metrics --- graphql_api/views.py | 34 ++++++++++++++++++++++--------- upload/views/bundle_analysis.py | 12 ++++++++--- upload/views/commits.py | 15 ++++++++------ upload/views/empty_upload.py | 15 ++++++++------ upload/views/legacy.py | 16 ++++++++------- upload/views/reports.py | 29 +++++++++++++++----------- upload/views/test_results.py | 15 ++++++++------ upload/views/upload_completion.py | 15 ++++++++------ upload/views/uploads.py | 16 ++++++++------- validate/views.py | 7 +++++-- 10 files changed, 109 insertions(+), 65 deletions(-) diff --git a/graphql_api/views.py b/graphql_api/views.py index 9cfa8594bd..6763a29c7e 100644 --- a/graphql_api/views.py +++ b/graphql_api/views.py @@ -19,7 +19,7 @@ ) from graphql import DocumentNode from sentry_sdk import capture_exception -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 @@ -119,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): """ @@ -236,9 +240,12 @@ async def post(self, request, *args, **kwargs): "user": request.user, } log.info("GraphQL Request", extra=log_data) - GQL_REQUEST_MADE_COUNTER.labels(path=req_path).inc() + inc_counter(GQL_REQUEST_MADE_COUNTER, labels=dict(path=req_path)) if self._check_ratelimit(request=request): - GQL_ERROR_TYPE_COUNTER.labels(error_type="rate_limit", path=req_path).inc() + inc_counter( + GQL_ERROR_TYPE_COUNTER, + labels=dict(error_type="rate_limit", path=req_path), + ) return JsonResponse( data={ "status": 429, @@ -254,7 +261,10 @@ async def post(self, request, *args, **kwargs): data = json.loads(content) if "errors" in data: - GQL_ERROR_TYPE_COUNTER.labels(error_type="all", path=req_path).inc() + 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"] @@ -266,9 +276,13 @@ async def post(self, request, *args, **kwargs): request_body=req_body, ), ) - GQL_ERROR_TYPE_COUNTER.labels( - error_type="query_cost_exceeded", path=req_path - ).inc() + inc_counter( + GQL_ERROR_TYPE_COUNTER, + labels=dict( + error_type="query_cost_exceeded", + path=req_path, + ), + ) return HttpResponseBadRequest( JsonResponse("Your query is too costly.") ) diff --git a/upload/views/bundle_analysis.py b/upload/views/bundle_analysis.py index 52a08800b2..ed9cd8486f 100644 --- a/upload/views/bundle_analysis.py +++ b/upload/views/bundle_analysis.py @@ -10,7 +10,7 @@ from rest_framework.response import Response from rest_framework.views import APIView from shared.bundle_analysis.storage import StoragePaths, get_bucket_name -from shared.metrics import Counter +from shared.metrics import Counter, inc_counter from codecov_auth.authentication.repo_auth import ( BundleAnalysisTokenlessAuthentication, @@ -89,7 +89,10 @@ def post(self, request: HttpRequest) -> Response: position="start", fill_labels=False, ) - BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels(**labels).inc() + inc_counter( + BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, + labels=labels, + ) except Exception: log.warn( "Failed to BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER", @@ -184,7 +187,10 @@ def post(self, request: HttpRequest) -> Response: position="end", fill_labels=False, ) - BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels(**labels).inc() + inc_counter( + BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, + labels=labels, + ) except Exception: log.warn( "Failed to BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER", diff --git a/upload/views/commits.py b/upload/views/commits.py index 6ed91ac660..a75f94f864 100644 --- a/upload/views/commits.py +++ b/upload/views/commits.py @@ -2,6 +2,7 @@ from rest_framework.exceptions import NotAuthenticated from rest_framework.generics import ListCreateAPIView +from shared.metrics import inc_counter from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -53,15 +54,16 @@ def create(self, request, *args, **kwargs): return super().create(request, *args, **kwargs) def perform_create(self, serializer): - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_commit", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ).inc() + ) repository = self.get_repo() commit = serializer.save(repository=repository) @@ -71,8 +73,9 @@ def perform_create(self, serializer): extra=dict(repo=repository.name, commit=commit.commitid), ) - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_commit", request=self.request, @@ -80,6 +83,6 @@ def perform_create(self, serializer): is_shelter_request=self.is_shelter_request(), position="end", ), - ).inc() + ) return commit diff --git a/upload/views/empty_upload.py b/upload/views/empty_upload.py index c51557fdc6..7bdcbc233c 100644 --- a/upload/views/empty_upload.py +++ b/upload/views/empty_upload.py @@ -8,6 +8,7 @@ from rest_framework.exceptions import NotFound from rest_framework.generics import CreateAPIView from rest_framework.response import Response +from shared.metrics import inc_counter from shared.torngit.exceptions import TorngitClientError, TorngitClientGeneralError from codecov_auth.authentication.repo_auth import ( @@ -82,15 +83,16 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def post(self, request, *args, **kwargs): - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="empty_upload", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ).inc() + ) serializer = EmptyUploadSerializer(data=request.data) if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) @@ -148,8 +150,9 @@ def post(self, request, *args, **kwargs): ) ) ] - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="empty_upload", request=self.request, @@ -157,7 +160,7 @@ def post(self, request, *args, **kwargs): is_shelter_request=self.is_shelter_request(), position="end", ), - ).inc() + ) if set(changed_files) == set(ignored_changed_files): TaskService().notify( repoid=repo.repoid, commitid=commit.commitid, empty_upload="pass" diff --git a/upload/views/legacy.py b/upload/views/legacy.py index e43beb853f..748573d1c9 100644 --- a/upload/views/legacy.py +++ b/upload/views/legacy.py @@ -16,7 +16,7 @@ from rest_framework.exceptions import ValidationError from rest_framework.permissions import AllowAny from rest_framework.views import APIView -from shared.metrics import metrics +from shared.metrics import inc_counter, metrics from codecov.db import sync_to_async from codecov_auth.commands.owner import OwnerCommands @@ -75,8 +75,9 @@ def options(self, request, *args, **kwargs): def post(self, request, *args, **kwargs): # Extract the version version = self.kwargs["version"] - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="legacy_upload", request=self.request, @@ -84,7 +85,7 @@ def post(self, request, *args, **kwargs): position="start", upload_version=version, ), - ).inc() + ) log.info( f"Received upload request {version}", @@ -164,8 +165,9 @@ def post(self, request, *args, **kwargs): ), ) - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="legacy_upload", request=self.request, @@ -174,7 +176,7 @@ def post(self, request, *args, **kwargs): position="end", upload_version=version, ), - ).inc() + ) # Validate the upload to make sure the org has enough repo credits and is allowed to upload for this commit redis = get_redis_connection() diff --git a/upload/views/reports.py b/upload/views/reports.py index 2895ace725..28791736eb 100644 --- a/upload/views/reports.py +++ b/upload/views/reports.py @@ -3,6 +3,7 @@ from django.http import HttpRequest, HttpResponseNotAllowed from rest_framework.exceptions import ValidationError from rest_framework.generics import CreateAPIView, ListCreateAPIView, RetrieveAPIView +from shared.metrics import inc_counter from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -40,15 +41,16 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def perform_create(self, serializer): - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_report", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ).inc() + ) repository = self.get_repo() commit = self.get_commit(repository) log.info( @@ -67,8 +69,9 @@ def perform_create(self, serializer): repository.repoid, commit.commitid, instance.code ) - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_report", request=self.request, @@ -76,7 +79,7 @@ def perform_create(self, serializer): is_shelter_request=self.is_shelter_request(), position="end", ), - ).inc() + ) return instance def list(self, request: HttpRequest, service: str, repo: str, commit_sha: str): @@ -103,15 +106,16 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def perform_create(self, serializer): - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_report_results", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ).inc() + ) repository = self.get_repo() commit = self.get_commit(repository) report = self.get_report(commit) @@ -128,8 +132,9 @@ def perform_create(self, serializer): repoid=repository.repoid, report_code=report.code, ) - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_report_results", request=self.request, @@ -137,7 +142,7 @@ def perform_create(self, serializer): is_shelter_request=self.is_shelter_request(), position="end", ), - ).inc() + ) return instance def get_object(self): diff --git a/upload/views/test_results.py b/upload/views/test_results.py index 38bd2b6077..0200c608fa 100644 --- a/upload/views/test_results.py +++ b/upload/views/test_results.py @@ -7,6 +7,7 @@ from rest_framework.permissions import BasePermission from rest_framework.response import Response from rest_framework.views import APIView +from shared.metrics import inc_counter from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -66,15 +67,16 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def post(self, request): - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="test_results", endpoint="test_results", request=request, is_shelter_request=self.is_shelter_request(), position="start", ), - ).inc() + ) serializer = UploadSerializer(data=request.data) if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) @@ -106,8 +108,9 @@ def post(self, request): if update_fields: repo.save(update_fields=update_fields) - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="test_results", endpoint="test_results", request=request, @@ -115,7 +118,7 @@ def post(self, request): is_shelter_request=self.is_shelter_request(), position="end", ), - ).inc() + ) commit, _ = Commit.objects.get_or_create( commitid=data["commit"], diff --git a/upload/views/upload_completion.py b/upload/views/upload_completion.py index 9c2b41e85e..43454a35f1 100644 --- a/upload/views/upload_completion.py +++ b/upload/views/upload_completion.py @@ -3,6 +3,7 @@ from rest_framework import status from rest_framework.generics import CreateAPIView from rest_framework.response import Response +from shared.metrics import inc_counter from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -34,15 +35,16 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def post(self, request, *args, **kwargs): - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="upload_complete", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ).inc() + ) repo = self.get_repo() commit = self.get_commit(repo) uploads_queryset = ReportSession.objects.filter( @@ -77,8 +79,9 @@ def post(self, request, *args, **kwargs): errored_uploads += 1 TaskService().manual_upload_completion_trigger(repo.repoid, commit.commitid) - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="upload_complete", request=self.request, @@ -86,7 +89,7 @@ def post(self, request, *args, **kwargs): is_shelter_request=self.is_shelter_request(), position="end", ), - ).inc() + ) return Response( data={ "uploads_total": uploads_count, diff --git a/upload/views/uploads.py b/upload/views/uploads.py index ff5fab1687..4d3bc6dfcb 100644 --- a/upload/views/uploads.py +++ b/upload/views/uploads.py @@ -5,7 +5,7 @@ from rest_framework.exceptions import ValidationError from rest_framework.generics import ListCreateAPIView from rest_framework.permissions import BasePermission -from shared.metrics import metrics +from shared.metrics import inc_counter, metrics from shared.upload.utils import UploaderType, insert_coverage_measurement from codecov_auth.authentication.repo_auth import ( @@ -68,15 +68,16 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def perform_create(self, serializer: UploadSerializer): - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_upload", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", ), - ).inc() + ) repository: Repository = self.get_repo() validate_activated_repo(repository) commit: Commit = self.get_commit(repository) @@ -128,8 +129,9 @@ def perform_create(self, serializer: UploadSerializer): instance.storage_path = path instance.save() self.trigger_upload_task(repository, commit.commitid, instance, report) - API_UPLOAD_COUNTER.labels( - **generate_upload_prometheus_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_tags( action="coverage", endpoint="create_upload", request=self.request, @@ -137,7 +139,7 @@ def perform_create(self, serializer: UploadSerializer): is_shelter_request=self.is_shelter_request(), position="end", ), - ).inc() + ) metrics.incr("uploads.accepted", 1) self.activate_repo(repository) self.send_analytics_data(commit, instance, version) diff --git a/validate/views.py b/validate/views.py index 7940067009..251c1e1b3c 100644 --- a/validate/views.py +++ b/validate/views.py @@ -7,7 +7,7 @@ from rest_framework.permissions import AllowAny from rest_framework.response import Response from rest_framework.views import APIView -from shared.metrics import Counter +from shared.metrics import Counter, inc_counter from shared.validation.exceptions import InvalidYamlException from shared.yaml.validation import validate_yaml from yaml import YAMLError, safe_load @@ -84,7 +84,10 @@ class V2ValidateYamlHandler(V1ValidateYamlHandler): def post(self, request, *args, **kwargs): source = self.request.query_params.get("source", "unknown") - API_VALIDATE_V2_COUNTER.labels(source=source).inc() + inc_counter( + API_VALIDATE_V2_COUNTER, + labels=dict(source=source), + ) if not self.request.body: return Response( From 27f74ad69d8e5e8010ed98b557bd661e344389b2 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Wed, 23 Oct 2024 12:04:35 -0400 Subject: [PATCH 21/27] bundle_analysis: remove try block --- upload/views/bundle_analysis.py | 62 +++++++++++++-------------------- 1 file changed, 24 insertions(+), 38 deletions(-) diff --git a/upload/views/bundle_analysis.py b/upload/views/bundle_analysis.py index ed9cd8486f..e4ccbe2758 100644 --- a/upload/views/bundle_analysis.py +++ b/upload/views/bundle_analysis.py @@ -80,25 +80,18 @@ def get_exception_handler(self) -> Callable: return repo_auth_custom_exception_handler def post(self, request: HttpRequest) -> Response: - try: - labels = generate_upload_prometheus_metrics_tags( - action="bundle_analysis", - endpoint="bundle_analysis", - request=self.request, - is_shelter_request=self.is_shelter_request(), - position="start", - fill_labels=False, - ) - inc_counter( - BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, - labels=labels, - ) - except Exception: - log.warn( - "Failed to BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER", - exc_info=True, - extra=labels, - ) + labels = generate_upload_prometheus_metrics_tags( + action="bundle_analysis", + endpoint="bundle_analysis", + request=self.request, + is_shelter_request=self.is_shelter_request(), + position="start", + fill_labels=False, + ) + inc_counter( + BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, + labels=labels, + ) serializer = UploadSerializer(data=request.data) if not serializer.is_valid(): @@ -178,25 +171,18 @@ def post(self, request: HttpRequest) -> Response: task_arguments=task_arguments, ), ) - try: - labels = generate_upload_prometheus_metrics_tags( - action="bundle_analysis", - endpoint="bundle_analysis", - request=self.request, - is_shelter_request=self.is_shelter_request(), - position="end", - fill_labels=False, - ) - inc_counter( - BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, - labels=labels, - ) - except Exception: - log.warn( - "Failed to BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER", - exc_info=True, - extra=labels, - ) + labels = generate_upload_prometheus_metrics_tags( + action="bundle_analysis", + endpoint="bundle_analysis", + request=self.request, + is_shelter_request=self.is_shelter_request(), + position="end", + fill_labels=False, + ) + inc_counter( + BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, + labels=labels, + ) dispatch_upload_task( task_arguments, From 13086fdc12f6083e310cc1d7464a5b9070b7da1c Mon Sep 17 00:00:00 2001 From: Tony Le Date: Wed, 23 Oct 2024 12:09:22 -0400 Subject: [PATCH 22/27] Function name clarity ("labels" instead of "tags") --- upload/helpers.py | 2 +- upload/views/bundle_analysis.py | 6 +++--- upload/views/commits.py | 6 +++--- upload/views/empty_upload.py | 6 +++--- upload/views/legacy.py | 6 +++--- upload/views/reports.py | 10 +++++----- upload/views/test_results.py | 6 +++--- upload/views/upload_completion.py | 6 +++--- upload/views/uploads.py | 6 +++--- 9 files changed, 27 insertions(+), 27 deletions(-) diff --git a/upload/helpers.py b/upload/helpers.py index f5c360fa20..7c7680165b 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -790,7 +790,7 @@ def get_version_from_headers(headers): return "unknown-user-agent" -def generate_upload_prometheus_metrics_tags( +def generate_upload_prometheus_metrics_labels( action, request, is_shelter_request, diff --git a/upload/views/bundle_analysis.py b/upload/views/bundle_analysis.py index e4ccbe2758..c99bcce342 100644 --- a/upload/views/bundle_analysis.py +++ b/upload/views/bundle_analysis.py @@ -26,7 +26,7 @@ from services.archive import ArchiveService from services.redis_configuration import get_redis_connection from timeseries.models import Dataset, MeasurementName -from upload.helpers import dispatch_upload_task, generate_upload_prometheus_metrics_tags +from upload.helpers import dispatch_upload_task, generate_upload_prometheus_metrics_labels from upload.views.base import ShelterMixin from upload.views.helpers import get_repository_from_string @@ -80,7 +80,7 @@ def get_exception_handler(self) -> Callable: return repo_auth_custom_exception_handler def post(self, request: HttpRequest) -> Response: - labels = generate_upload_prometheus_metrics_tags( + labels = generate_upload_prometheus_metrics_labels( action="bundle_analysis", endpoint="bundle_analysis", request=self.request, @@ -171,7 +171,7 @@ def post(self, request: HttpRequest) -> Response: task_arguments=task_arguments, ), ) - labels = generate_upload_prometheus_metrics_tags( + labels = generate_upload_prometheus_metrics_labels( action="bundle_analysis", endpoint="bundle_analysis", request=self.request, diff --git a/upload/views/commits.py b/upload/views/commits.py index a75f94f864..2e77916001 100644 --- a/upload/views/commits.py +++ b/upload/views/commits.py @@ -14,7 +14,7 @@ repo_auth_custom_exception_handler, ) from core.models import Commit -from upload.helpers import generate_upload_prometheus_metrics_tags +from upload.helpers import generate_upload_prometheus_metrics_labels from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import CommitSerializer from upload.views.base import GetterMixin @@ -56,7 +56,7 @@ def create(self, request, *args, **kwargs): def perform_create(self, serializer): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_commit", request=self.request, @@ -75,7 +75,7 @@ def perform_create(self, serializer): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_commit", request=self.request, diff --git a/upload/views/empty_upload.py b/upload/views/empty_upload.py index 7bdcbc233c..6d7f8d5ba3 100644 --- a/upload/views/empty_upload.py +++ b/upload/views/empty_upload.py @@ -23,7 +23,7 @@ from services.task import TaskService from services.yaml import final_commit_yaml from upload.helpers import ( - generate_upload_prometheus_metrics_tags, + generate_upload_prometheus_metrics_labels, try_to_get_best_possible_bot_token, ) from upload.metrics import API_UPLOAD_COUNTER @@ -85,7 +85,7 @@ def get_exception_handler(self): def post(self, request, *args, **kwargs): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="empty_upload", request=self.request, @@ -152,7 +152,7 @@ def post(self, request, *args, **kwargs): ] inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="empty_upload", request=self.request, diff --git a/upload/views/legacy.py b/upload/views/legacy.py index 748573d1c9..1f1b2b07f1 100644 --- a/upload/views/legacy.py +++ b/upload/views/legacy.py @@ -31,7 +31,7 @@ determine_upload_commit_to_use, determine_upload_pr_to_use, dispatch_upload_task, - generate_upload_prometheus_metrics_tags, + generate_upload_prometheus_metrics_labels, insert_commit, parse_headers, parse_params, @@ -77,7 +77,7 @@ def post(self, request, *args, **kwargs): version = self.kwargs["version"] inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="legacy_upload", request=self.request, @@ -167,7 +167,7 @@ def post(self, request, *args, **kwargs): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="legacy_upload", request=self.request, diff --git a/upload/views/reports.py b/upload/views/reports.py index 28791736eb..77ea7b8cf0 100644 --- a/upload/views/reports.py +++ b/upload/views/reports.py @@ -16,7 +16,7 @@ ) from reports.models import CommitReport, ReportResults from services.task import TaskService -from upload.helpers import generate_upload_prometheus_metrics_tags +from upload.helpers import generate_upload_prometheus_metrics_labels from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import CommitReportSerializer, ReportResultsSerializer from upload.views.base import GetterMixin @@ -43,7 +43,7 @@ def get_exception_handler(self): def perform_create(self, serializer): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_report", request=self.request, @@ -71,7 +71,7 @@ def perform_create(self, serializer): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_report", request=self.request, @@ -108,7 +108,7 @@ def get_exception_handler(self): def perform_create(self, serializer): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_report_results", request=self.request, @@ -134,7 +134,7 @@ def perform_create(self, serializer): ) inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_report_results", request=self.request, diff --git a/upload/views/test_results.py b/upload/views/test_results.py index 0200c608fa..6dfc3a5208 100644 --- a/upload/views/test_results.py +++ b/upload/views/test_results.py @@ -23,7 +23,7 @@ from reports.models import CommitReport from services.archive import ArchiveService, MinioEndpoints from services.redis_configuration import get_redis_connection -from upload.helpers import dispatch_upload_task, generate_upload_prometheus_metrics_tags +from upload.helpers import dispatch_upload_task, generate_upload_prometheus_metrics_labels from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import FlagListField from upload.views.base import ShelterMixin @@ -69,7 +69,7 @@ def get_exception_handler(self): def post(self, request): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="test_results", endpoint="test_results", request=request, @@ -110,7 +110,7 @@ def post(self, request): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="test_results", endpoint="test_results", request=request, diff --git a/upload/views/upload_completion.py b/upload/views/upload_completion.py index 43454a35f1..cbc6cc04dc 100644 --- a/upload/views/upload_completion.py +++ b/upload/views/upload_completion.py @@ -14,7 +14,7 @@ ) from reports.models import ReportSession from services.task import TaskService -from upload.helpers import generate_upload_prometheus_metrics_tags +from upload.helpers import generate_upload_prometheus_metrics_labels from upload.metrics import API_UPLOAD_COUNTER from upload.views.base import GetterMixin from upload.views.uploads import CanDoCoverageUploadsPermission @@ -37,7 +37,7 @@ def get_exception_handler(self): def post(self, request, *args, **kwargs): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="upload_complete", request=self.request, @@ -81,7 +81,7 @@ def post(self, request, *args, **kwargs): TaskService().manual_upload_completion_trigger(repo.repoid, commit.commitid) inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="upload_complete", request=self.request, diff --git a/upload/views/uploads.py b/upload/views/uploads.py index 4d3bc6dfcb..eef628bffe 100644 --- a/upload/views/uploads.py +++ b/upload/views/uploads.py @@ -28,7 +28,7 @@ from services.redis_configuration import get_redis_connection from upload.helpers import ( dispatch_upload_task, - generate_upload_prometheus_metrics_tags, + generate_upload_prometheus_metrics_labels, validate_activated_repo, ) from upload.metrics import API_UPLOAD_COUNTER @@ -70,7 +70,7 @@ def get_exception_handler(self): def perform_create(self, serializer: UploadSerializer): inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_upload", request=self.request, @@ -131,7 +131,7 @@ def perform_create(self, serializer: UploadSerializer): self.trigger_upload_task(repository, commit.commitid, instance, report) inc_counter( API_UPLOAD_COUNTER, - labels=generate_upload_prometheus_metrics_tags( + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_upload", request=self.request, From 7077928331f4334afad970fd4a18685e448579d8 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Wed, 23 Oct 2024 12:12:04 -0400 Subject: [PATCH 23/27] fix: Lints --- upload/views/bundle_analysis.py | 5 ++++- upload/views/test_results.py | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/upload/views/bundle_analysis.py b/upload/views/bundle_analysis.py index c99bcce342..9ebdb3ba9c 100644 --- a/upload/views/bundle_analysis.py +++ b/upload/views/bundle_analysis.py @@ -26,7 +26,10 @@ from services.archive import ArchiveService from services.redis_configuration import get_redis_connection from timeseries.models import Dataset, MeasurementName -from upload.helpers import dispatch_upload_task, generate_upload_prometheus_metrics_labels +from upload.helpers import ( + dispatch_upload_task, + generate_upload_prometheus_metrics_labels, +) from upload.views.base import ShelterMixin from upload.views.helpers import get_repository_from_string diff --git a/upload/views/test_results.py b/upload/views/test_results.py index 6dfc3a5208..fbe9f40da2 100644 --- a/upload/views/test_results.py +++ b/upload/views/test_results.py @@ -23,7 +23,10 @@ from reports.models import CommitReport from services.archive import ArchiveService, MinioEndpoints from services.redis_configuration import get_redis_connection -from upload.helpers import dispatch_upload_task, generate_upload_prometheus_metrics_labels +from upload.helpers import ( + dispatch_upload_task, + generate_upload_prometheus_metrics_labels, +) from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import FlagListField from upload.views.base import ShelterMixin From 08b925622d27c7b53a706649e4effc0b2a2804a0 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Wed, 23 Oct 2024 12:23:57 -0400 Subject: [PATCH 24/27] Bump shared version --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 87f5295048..2d87fb37ac 100644 --- a/requirements.txt +++ b/requirements.txt @@ -417,7 +417,7 @@ sentry-sdk[celery]==2.13.0 # shared setproctitle==1.1.10 # via -r requirements.in -shared @ https://github.com/codecov/shared/archive/f0e213c4399563990d43fb424f4be36faa5ce5eb.tar.gz +shared @ https://github.com/codecov/shared/archive/cde98920fbec05ad7f71f934fed35a5eebab4dd3.tar.gz # via -r requirements.in simplejson==3.17.2 # via -r requirements.in From 3ad8aff18739e3a5161f2562e7ffd99f8b76a3ea Mon Sep 17 00:00:00 2001 From: Tony Le Date: Wed, 23 Oct 2024 15:42:48 -0400 Subject: [PATCH 25/27] Remove self.test_instances[0].test.failure_rate --- api/public/v2/tests/test_test_results_view.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/api/public/v2/tests/test_test_results_view.py b/api/public/v2/tests/test_test_results_view.py index ab83175496..797ef081b2 100644 --- a/api/public/v2/tests/test_test_results_view.py +++ b/api/public/v2/tests/test_test_results_view.py @@ -53,7 +53,6 @@ def test_list(self): "outcome": self.test_instances[0].outcome, "branch": self.test_instances[0].branch, "repoid": self.test_instances[0].repoid, - "failure_rate": self.test_instances[0].test.failure_rate, "commits_where_fail": self.test_instances[ 0 ].test.commits_where_fail, @@ -103,7 +102,6 @@ def test_list_filters(self): "outcome": self.test_instances[0].outcome, "branch": self.test_instances[0].branch, "repoid": self.test_instances[0].repoid, - "failure_rate": self.test_instances[0].test.failure_rate, "commits_where_fail": self.test_instances[ 0 ].test.commits_where_fail, @@ -137,7 +135,6 @@ def test_retrieve(self, get_repo_permissions): "outcome": self.test_instances[0].outcome, "branch": self.test_instances[0].branch, "repoid": self.test_instances[0].repoid, - "failure_rate": self.test_instances[0].test.failure_rate, "commits_where_fail": self.test_instances[0].test.commits_where_fail, } @@ -240,6 +237,5 @@ def test_result_with_valid_super_token( "outcome": self.test_instances[0].outcome, "branch": self.test_instances[0].branch, "repoid": self.test_instances[0].repoid, - "failure_rate": self.test_instances[0].test.failure_rate, "commits_where_fail": self.test_instances[0].test.commits_where_fail, } From bb7757acda7c06ec61fb29f6189c9b690bcc48d4 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Wed, 23 Oct 2024 17:02:00 -0400 Subject: [PATCH 26/27] Remove commits_where_fail --- api/public/v2/tests/test_test_results_view.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/api/public/v2/tests/test_test_results_view.py b/api/public/v2/tests/test_test_results_view.py index 797ef081b2..aec5bcc6a7 100644 --- a/api/public/v2/tests/test_test_results_view.py +++ b/api/public/v2/tests/test_test_results_view.py @@ -53,9 +53,6 @@ def test_list(self): "outcome": self.test_instances[0].outcome, "branch": self.test_instances[0].branch, "repoid": self.test_instances[0].repoid, - "commits_where_fail": self.test_instances[ - 0 - ].test.commits_where_fail, }, { "id": self.test_instances[1].id, @@ -67,10 +64,6 @@ def test_list(self): "outcome": self.test_instances[1].outcome, "branch": self.test_instances[1].branch, "repoid": self.test_instances[1].repoid, - "failure_rate": self.test_instances[1].test.failure_rate, - "commits_where_fail": self.test_instances[ - 1 - ].test.commits_where_fail, }, ], "total_pages": 1, @@ -102,9 +95,6 @@ def test_list_filters(self): "outcome": self.test_instances[0].outcome, "branch": self.test_instances[0].branch, "repoid": self.test_instances[0].repoid, - "commits_where_fail": self.test_instances[ - 0 - ].test.commits_where_fail, }, ], "total_pages": 1, @@ -135,7 +125,6 @@ def test_retrieve(self, get_repo_permissions): "outcome": self.test_instances[0].outcome, "branch": self.test_instances[0].branch, "repoid": self.test_instances[0].repoid, - "commits_where_fail": self.test_instances[0].test.commits_where_fail, } @patch("api.shared.permissions.RepositoryArtifactPermissions.has_permission") @@ -237,5 +226,4 @@ def test_result_with_valid_super_token( "outcome": self.test_instances[0].outcome, "branch": self.test_instances[0].branch, "repoid": self.test_instances[0].repoid, - "commits_where_fail": self.test_instances[0].test.commits_where_fail, } From 919ee718154abdf75dc2987fd7d6d0ff9fc7f502 Mon Sep 17 00:00:00 2001 From: Tony Le Date: Mon, 28 Oct 2024 13:48:15 -0400 Subject: [PATCH 27/27] Use include_empty_labels for extra clarity --- upload/helpers.py | 4 ++-- upload/views/bundle_analysis.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/upload/helpers.py b/upload/helpers.py index 7c7680165b..cd472358d2 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -798,7 +798,7 @@ def generate_upload_prometheus_metrics_labels( repository: Optional[Repository] = None, position: Optional[str] = None, upload_version: Optional[str] = None, - fill_labels: bool = True, + include_empty_labels: bool = True, ): metrics_tags = dict( agent=get_agent_from_headers(request.headers), @@ -822,7 +822,7 @@ def generate_upload_prometheus_metrics_labels( { field: value for field, value in optional_fields.items() - if value or fill_labels + if value or include_empty_labels } ) diff --git a/upload/views/bundle_analysis.py b/upload/views/bundle_analysis.py index 9ebdb3ba9c..b3f000ee8f 100644 --- a/upload/views/bundle_analysis.py +++ b/upload/views/bundle_analysis.py @@ -89,7 +89,7 @@ def post(self, request: HttpRequest) -> Response: request=self.request, is_shelter_request=self.is_shelter_request(), position="start", - fill_labels=False, + include_empty_labels=False, ) inc_counter( BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, @@ -180,7 +180,7 @@ def post(self, request: HttpRequest) -> Response: request=self.request, is_shelter_request=self.is_shelter_request(), position="end", - fill_labels=False, + include_empty_labels=False, ) inc_counter( BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER,