From cbcc01bd28ab7137577c2243aae7362196d58b40 Mon Sep 17 00:00:00 2001 From: JerrySentry Date: Fri, 18 Oct 2024 06:10:06 +0900 Subject: [PATCH] Revert "bundle analysis: sentry->prometheus metrics for upload view (#899)" This reverts commit 389581190172323a43e2c280c6132e98aa726d8e. --- upload/tests/views/test_bundle_analysis.py | 134 ++------------------- upload/views/bundle_analysis.py | 36 ++---- 2 files changed, 24 insertions(+), 146 deletions(-) diff --git a/upload/tests/views/test_bundle_analysis.py b/upload/tests/views/test_bundle_analysis.py index 5f5ee9108c..80457e6bed 100644 --- a/upload/tests/views/test_bundle_analysis.py +++ b/upload/tests/views/test_bundle_analysis.py @@ -18,8 +18,8 @@ @pytest.mark.django_db(databases={"default", "timeseries"}) def test_upload_bundle_analysis_success(db, client, mocker, mock_redis): upload = mocker.patch.object(TaskService, "upload") - mock_metrics = mocker.patch( - "upload.views.bundle_analysis.BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels" + mock_sentry_metrics = mocker.patch( + "upload.views.bundle_analysis.sentry_metrics.incr" ) create_presigned_put = mocker.patch( "services.archive.StorageService.create_presigned_put", @@ -89,8 +89,9 @@ def test_upload_bundle_analysis_success(db, client, mocker, mock_redis): report_code=None, report_type="bundle_analysis", ) - mock_metrics.assert_called_with( - **{ + mock_sentry_metrics.assert_called_with( + "upload", + tags={ "agent": "cli", "version": "0.4.7", "action": "bundle_analysis", @@ -106,8 +107,8 @@ def test_upload_bundle_analysis_success(db, client, mocker, mock_redis): @override_settings(SHELTER_SHARED_SECRET="shelter-shared-secret") def test_upload_bundle_analysis_success_shelter(db, client, mocker, mock_redis): upload = mocker.patch.object(TaskService, "upload") - mock_metrics = mocker.patch( - "upload.views.bundle_analysis.BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels" + mock_sentry_metrics = mocker.patch( + "upload.views.bundle_analysis.sentry_metrics.incr" ) create_presigned_put = mocker.patch( "services.archive.StorageService.create_presigned_put", @@ -179,8 +180,9 @@ def test_upload_bundle_analysis_success_shelter(db, client, mocker, mock_redis): report_code=None, report_type="bundle_analysis", ) - mock_metrics.assert_called_with( - **{ + mock_sentry_metrics.assert_called_with( + "upload", + tags={ "agent": "cli", "version": "0.4.7", "action": "bundle_analysis", @@ -199,9 +201,6 @@ def test_upload_bundle_analysis_org_token(db, client, mocker, mock_redis): "services.archive.StorageService.create_presigned_put", return_value="test-presigned-put", ) - mock_metrics = mocker.patch( - "upload.views.bundle_analysis.BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels" - ) repository = RepositoryFactory.create() org_token = OrganizationLevelTokenFactory.create(owner=repository.author) @@ -218,17 +217,6 @@ def test_upload_bundle_analysis_org_token(db, client, mocker, mock_redis): format="json", ) assert res.status_code == 201 - mock_metrics.assert_called_with( - **{ - "agent": "unknown-user-agent", - "version": "unknown-user-agent", - "action": "bundle_analysis", - "endpoint": "bundle_analysis", - "repo_visibility": "private", - "is_using_shelter": "no", - "position": "end", - }, - ) @pytest.mark.django_db(databases={"default", "timeseries"}) @@ -238,9 +226,6 @@ def test_upload_bundle_analysis_existing_commit(db, client, mocker, mock_redis): "services.archive.StorageService.create_presigned_put", return_value="test-presigned-put", ) - mock_metrics = mocker.patch( - "upload.views.bundle_analysis.BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels" - ) repository = RepositoryFactory.create() commit = CommitFactory.create(repository=repository) @@ -265,17 +250,6 @@ def test_upload_bundle_analysis_existing_commit(db, client, mocker, mock_redis): report_code=None, report_type="bundle_analysis", ) - mock_metrics.assert_called_with( - **{ - "agent": "unknown-user-agent", - "version": "unknown-user-agent", - "action": "bundle_analysis", - "endpoint": "bundle_analysis", - "repo_visibility": "private", - "is_using_shelter": "no", - "position": "end", - }, - ) def test_upload_bundle_analysis_missing_args(db, client, mocker, mock_redis): @@ -284,9 +258,6 @@ def test_upload_bundle_analysis_missing_args(db, client, mocker, mock_redis): "services.archive.StorageService.create_presigned_put", return_value="test-presigned-put", ) - mock_metrics = mocker.patch( - "upload.views.bundle_analysis.BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels" - ) repository = RepositoryFactory.create() commit = CommitFactory.create(repository=repository) @@ -315,16 +286,6 @@ def test_upload_bundle_analysis_missing_args(db, client, mocker, mock_redis): assert res.status_code == 400 assert res.json() == {"commit": ["This field is required."]} assert not upload.called - mock_metrics.assert_called_with( - **{ - "agent": "unknown-user-agent", - "version": "unknown-user-agent", - "action": "bundle_analysis", - "endpoint": "bundle_analysis", - "is_using_shelter": "no", - "position": "start", - }, - ) def test_upload_bundle_analysis_invalid_token(db, client, mocker, mock_redis): @@ -363,9 +324,6 @@ def test_upload_bundle_analysis_github_oidc_auth( "services.archive.StorageService.create_presigned_put", return_value="test-presigned-put", ) - mock_metrics = mocker.patch( - "upload.views.bundle_analysis.BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels" - ) repository = RepositoryFactory() mock_jwt_decode.return_value = { "repository": f"url/{repository.name}", @@ -386,17 +344,6 @@ def test_upload_bundle_analysis_github_oidc_auth( format="json", ) assert res.status_code == 201 - mock_metrics.assert_called_with( - **{ - "agent": "unknown-user-agent", - "version": "unknown-user-agent", - "action": "bundle_analysis", - "endpoint": "bundle_analysis", - "repo_visibility": "private", - "is_using_shelter": "no", - "position": "end", - }, - ) @pytest.mark.django_db(databases={"default", "timeseries"}) @@ -404,13 +351,11 @@ def test_upload_bundle_analysis_measurement_datasets_created( db, client, mocker, mock_redis ): mocker.patch.object(TaskService, "upload") + mocker.patch("upload.views.bundle_analysis.sentry_metrics.incr") mocker.patch( "services.archive.StorageService.create_presigned_put", return_value="test-presigned-put", ) - mock_metrics = mocker.patch( - "upload.views.bundle_analysis.BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels" - ) repository = RepositoryFactory.create() commit_sha = "6fd5b89357fc8cdf34d6197549ac7c6d7e5977ef" @@ -447,18 +392,6 @@ def test_upload_bundle_analysis_measurement_datasets_created( repository_id=repository.pk, ).exists() - mock_metrics.assert_called_with( - **{ - "agent": "cli", - "version": "0.4.7", - "action": "bundle_analysis", - "endpoint": "bundle_analysis", - "repo_visibility": "private", - "is_using_shelter": "no", - "position": "end", - }, - ) - @override_settings(TIMESERIES_ENABLED=False) @pytest.mark.django_db(databases={"default", "timeseries"}) @@ -466,13 +399,11 @@ def test_upload_bundle_analysis_measurement_timeseries_disabled( db, client, mocker, mock_redis ): mocker.patch.object(TaskService, "upload") + mocker.patch("upload.views.bundle_analysis.sentry_metrics.incr") mocker.patch( "services.archive.StorageService.create_presigned_put", return_value="test-presigned-put", ) - mock_metrics = mocker.patch( - "upload.views.bundle_analysis.BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels" - ) repository = RepositoryFactory.create() commit_sha = "6fd5b89357fc8cdf34d6197549ac7c6d7e5977ef" @@ -509,18 +440,6 @@ def test_upload_bundle_analysis_measurement_timeseries_disabled( repository_id=repository.pk, ).exists() - mock_metrics.assert_called_with( - **{ - "agent": "cli", - "version": "0.4.7", - "action": "bundle_analysis", - "endpoint": "bundle_analysis", - "repo_visibility": "private", - "is_using_shelter": "no", - "position": "end", - }, - ) - @pytest.mark.django_db(databases={"default", "timeseries"}) def test_upload_bundle_analysis_no_repo(db, client, mocker, mock_redis): @@ -530,9 +449,6 @@ def test_upload_bundle_analysis_no_repo(db, client, mocker, mock_redis): "services.archive.StorageService.create_presigned_put", return_value="test-presigned-put", ) - mock_metrics = mocker.patch( - "upload.views.bundle_analysis.BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels" - ) repository = RepositoryFactory.create() org_token = OrganizationLevelTokenFactory.create(owner=repository.author) @@ -552,24 +468,10 @@ def test_upload_bundle_analysis_no_repo(db, client, mocker, mock_redis): assert res.json() == {"detail": "Repository not found."} assert not upload.called - mock_metrics.assert_called_with( - **{ - "agent": "unknown-user-agent", - "version": "unknown-user-agent", - "action": "bundle_analysis", - "endpoint": "bundle_analysis", - "is_using_shelter": "no", - "position": "start", - }, - ) - @pytest.mark.django_db(databases={"default", "timeseries"}) def test_upload_bundle_analysis_tokenless_success(db, client, mocker, mock_redis): upload = mocker.patch.object(TaskService, "upload") - mock_metrics = mocker.patch( - "upload.views.bundle_analysis.BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels" - ) create_presigned_put = mocker.patch( "services.archive.StorageService.create_presigned_put", @@ -606,18 +508,6 @@ def test_upload_bundle_analysis_tokenless_success(db, client, mocker, mock_redis assert upload.called create_presigned_put.assert_called_once_with("bundle-analysis", ANY, 30) - mock_metrics.assert_called_with( - **{ - "agent": "cli", - "version": "0.4.7", - "action": "bundle_analysis", - "endpoint": "bundle_analysis", - "repo_visibility": "public", - "is_using_shelter": "no", - "position": "end", - }, - ) - @pytest.mark.django_db(databases={"default", "timeseries"}) def test_upload_bundle_analysis_tokenless_no_repo(db, client, mocker, mock_redis): diff --git a/upload/views/bundle_analysis.py b/upload/views/bundle_analysis.py index f8b4af3989..e54ee5b04d 100644 --- a/upload/views/bundle_analysis.py +++ b/upload/views/bundle_analysis.py @@ -9,8 +9,8 @@ from rest_framework.permissions import BasePermission from rest_framework.response import Response from rest_framework.views import APIView +from sentry_sdk import metrics as sentry_metrics from shared.bundle_analysis.storage import StoragePaths, get_bucket_name -from shared.metrics import Counter from codecov_auth.authentication.repo_auth import ( BundleAnalysisTokenlessAuthentication, @@ -33,21 +33,6 @@ log = logging.getLogger(__name__) -BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER = Counter( - "bundle_analysis_upload_views_runs", - "Number of times a raw report processor was run and with what result", - [ - "agent", - "version", - "action", - "endpoint", - "is_using_shelter", - "repo_visibility", - "position", - ], -) - - class UploadBundleAnalysisPermission(BasePermission): def has_permission(self, request: HttpRequest, view: Any) -> bool: return request.auth is not None and "upload" in request.auth.get_scopes() @@ -81,15 +66,16 @@ def get_exception_handler(self) -> Callable: return repo_auth_custom_exception_handler def post(self, request: HttpRequest) -> Response: - BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels( - **generate_upload_sentry_metrics_tags( + sentry_metrics.incr( + "upload", + tags=generate_upload_sentry_metrics_tags( action="bundle_analysis", endpoint="bundle_analysis", request=self.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) @@ -168,16 +154,18 @@ def post(self, request: HttpRequest) -> Response: task_arguments=task_arguments, ), ) - BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels( - **generate_upload_sentry_metrics_tags( + + sentry_metrics.incr( + "upload", + tags=generate_upload_sentry_metrics_tags( action="bundle_analysis", endpoint="bundle_analysis", request=self.request, repository=repo, is_shelter_request=self.is_shelter_request(), position="end", - ) - ).inc() + ), + ) dispatch_upload_task( task_arguments,