diff --git a/codecov/settings_base.py b/codecov/settings_base.py index 359ca78ea6..b0dce53faa 100644 --- a/codecov/settings_base.py +++ b/codecov/settings_base.py @@ -356,6 +356,9 @@ "setup", "upload_throttling_enabled", default=True ) +HIDE_ALL_CODECOV_TOKENS = get_config("setup", "hide_all_codecov_tokens", default=False) + + SENTRY_JWT_SHARED_SECRET = get_config( "sentry", "jwt_shared_secret", default=None ) or get_config("setup", "sentry", "jwt_shared_secret", default=None) diff --git a/graphql_api/tests/test_owner.py b/graphql_api/tests/test_owner.py index 8797d5d0f9..8428755154 100644 --- a/graphql_api/tests/test_owner.py +++ b/graphql_api/tests/test_owner.py @@ -27,6 +27,7 @@ UnauthorizedGuestAccess, ) from codecov_auth.models import GithubAppInstallation, OwnerProfile +from graphql_api.types.repository.repository import TOKEN_UNAVAILABLE from plan.constants import PlanName, TrialStatus from reports.tests.factories import CommitReportFactory, UploadFactory @@ -426,6 +427,28 @@ def test_get_org_upload_token(self, mocker): data = self.gql_request(query, owner=self.owner) assert data["owner"]["orgUploadToken"] == "upload_token" + @override_settings(HIDE_ALL_CODECOV_TOKENS=True) + def test_get_org_upload_token_hide_tokens_setting_owner_not_admin(self): + random_owner = OwnerFactory() + query = """{ + owner(username: "%s") { + orgUploadToken + } + } + """ % (self.owner.username) + random_owner.organizations = [self.owner.ownerid] + random_owner.save() + data = self.gql_request(query, owner=random_owner) + assert data["owner"]["orgUploadToken"] == TOKEN_UNAVAILABLE + + @patch("codecov_auth.commands.owner.owner.OwnerCommands.get_org_upload_token") + @override_settings(HIDE_ALL_CODECOV_TOKENS=True) + def test_get_org_upload_token_hide_tokens_setting_owner_is_admin(self, mocker): + mocker.return_value = "upload_token" + query = query_repositories % (self.owner.username, "", "") + data = self.gql_request(query, owner=self.owner) + assert data["owner"]["orgUploadToken"] == "upload_token" + # Applies for old users that didn't get their owner profiles created w/ their owner def test_when_owner_profile_doesnt_exist(self): owner = OwnerFactory(username="no-profile-user") diff --git a/graphql_api/tests/test_repository.py b/graphql_api/tests/test_repository.py index a7088bf8a6..260203d0a5 100644 --- a/graphql_api/tests/test_repository.py +++ b/graphql_api/tests/test_repository.py @@ -11,6 +11,7 @@ RepositoryTokenFactory, ) +from graphql_api.types.repository.repository import TOKEN_UNAVAILABLE from services.profiling import CriticalFile from .helper import GraphQLTestHelper @@ -73,6 +74,11 @@ """ +def mock_get_config_global_upload_tokens(*args): + if args == ("setup", "hide_all_codecov_tokens"): + return True + + class TestFetchRepository(GraphQLTestHelper, TransactionTestCase): def fetch_repository(self, name, fields=None): data = self.gql_request( @@ -683,6 +689,68 @@ def test_fetch_is_github_rate_limited_not_on_gh_service(self): assert data["me"]["owner"]["repository"]["isGithubRateLimited"] == False + @override_settings(HIDE_ALL_CODECOV_TOKENS=True) + def test_repo_upload_token_not_available_config_setting_owner_not_admin(self): + owner = OwnerFactory(service="gitlab") + + repo = RepositoryFactory( + author=owner, + author__service="gitlab", + service_id=12345, + active=True, + ) + new_owner = OwnerFactory(service="gitlab", organizations=[owner.ownerid]) + new_owner.permission = [repo.repoid] + new_owner.save() + owner.admins = [] + + query = """ + query { + owner(username: "%s") { + repository(name: "%s") { + ... on Repository { + uploadToken + } + } + } + } + """ % ( + owner.username, + repo.name, + ) + + data = self.gql_request( + query, + owner=new_owner, + variables={"name": repo.name}, + provider="gitlab", + ) + + assert data["owner"]["repository"]["uploadToken"] == TOKEN_UNAVAILABLE + + @override_settings(HIDE_ALL_CODECOV_TOKENS=True) + def test_repo_upload_token_not_available_config_setting_owner_is_admin(self): + owner = OwnerFactory(service="gitlab") + repo = RepositoryFactory( + author=owner, + author__service="gitlab", + service_id=12345, + active=True, + ) + owner.admins = [owner.ownerid] + + data = self.gql_request( + query_repository + % """ + uploadToken + """, + owner=owner, + variables={"name": repo.name}, + provider="gitlab", + ) + + assert data["me"]["owner"]["repository"]["uploadToken"] != TOKEN_UNAVAILABLE + @patch("shared.rate_limits.determine_entity_redis_key") @patch("shared.rate_limits.determine_if_entity_is_rate_limited") @patch("logging.Logger.warning") diff --git a/graphql_api/tests/test_views.py b/graphql_api/tests/test_views.py index 9d6b68cc90..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 @@ -201,11 +201,12 @@ async def test_query_metrics_extension_set_type_and_name_timeout( assert extension.operation_type == "unknown_type" assert extension.operation_name == "unknown_name" - @patch("sentry_sdk.metrics.incr") + @patch("graphql_api.views.GQL_REQUEST_MADE_COUNTER.labels") + @patch("graphql_api.views.GQL_ERROR_TYPE_COUNTER.labels") @patch("graphql_api.views.AsyncGraphqlView._check_ratelimit") @override_settings(DEBUG=False, GRAPHQL_RATE_LIMIT_RPM=1000) async def test_when_rate_limit_reached( - self, mocked_check_ratelimit, mocked_sentry_incr + self, mocked_check_ratelimit, mocked_error_counter, mocked_request_counter ): schema = generate_cost_test_schema() mocked_check_ratelimit.return_value = True @@ -217,11 +218,10 @@ async def test_when_rate_limit_reached( == "It looks like you've hit the rate limit of 1000 req/min. Try again later." ) - expected_calls = [ - call("graphql.info.request_made", tags={"path": "/graphql/gh"}), - call("graphql.error.rate_limit", tags={"path": "/graphql/gh"}), - ] - mocked_sentry_incr.assert_has_calls(expected_calls) + mocked_error_counter.assert_called_with( + error_type="rate_limit", path="/graphql/gh" + ) + mocked_request_counter.assert_called_with(path="/graphql/gh") @override_settings( DEBUG=False, GRAPHQL_RATE_LIMIT_RPM=0, GRAPHQL_RATE_LIMIT_ENABLED=False diff --git a/graphql_api/types/owner/owner.py b/graphql_api/types/owner/owner.py index 77717c0f5b..8d18dabafb 100644 --- a/graphql_api/types/owner/owner.py +++ b/graphql_api/types/owner/owner.py @@ -6,6 +6,7 @@ import stripe import yaml from ariadne import ObjectType +from django.conf import settings from graphql import GraphQLResolveInfo import services.activation as activation @@ -37,6 +38,7 @@ ) from graphql_api.types.enums import OrderingDirection, RepositoryOrdering from graphql_api.types.errors.errors import NotFoundError, OwnerNotActivatedError +from graphql_api.types.repository.repository import TOKEN_UNAVAILABLE from plan.constants import FREE_PLAN_REPRESENTATIONS, PlanData, PlanName from plan.service import PlanService from services.billing import BillingService @@ -205,7 +207,16 @@ def resolve_hash_ownerid(owner: Owner, info: GraphQLResolveInfo) -> str: def resolve_org_upload_token( owner: Owner, info: GraphQLResolveInfo, **kwargs: Any ) -> str: + should_hide_tokens = settings.HIDE_ALL_CODECOV_TOKENS + current_owner = info.context["request"].current_owner command = info.context["executor"].get_command("owner") + if not current_owner: + is_owner_admin = False + else: + is_owner_admin = current_owner.is_admin(owner) + if should_hide_tokens and not is_owner_admin: + return TOKEN_UNAVAILABLE + return command.get_org_upload_token(owner) diff --git a/graphql_api/types/repository/repository.py b/graphql_api/types/repository/repository.py index 81946f8c6e..e53992727f 100644 --- a/graphql_api/types/repository/repository.py +++ b/graphql_api/types/repository/repository.py @@ -5,6 +5,7 @@ import shared.rate_limits as rate_limits import yaml from ariadne import ObjectType, UnionType +from django.conf import settings from graphql.type.definition import GraphQLResolveInfo from codecov.db import sync_to_async @@ -24,6 +25,8 @@ from services.profiling import CriticalFile, ProfilingSummary from services.redis_configuration import get_redis_connection +TOKEN_UNAVAILABLE = "Token Unavailable. Please contact your admin." + log = logging.getLogger(__name__) repository_bindable = ObjectType("Repository") @@ -68,6 +71,16 @@ def resolve_commit(repository: Repository, info: GraphQLResolveInfo, id): @repository_bindable.field("uploadToken") def resolve_upload_token(repository: Repository, info: GraphQLResolveInfo): + should_hide_tokens = settings.HIDE_ALL_CODECOV_TOKENS + + current_owner = info.context["request"].current_owner + if not current_owner: + is_current_user_admin = False + else: + is_current_user_admin = current_owner.is_admin(repository.author) + + if should_hide_tokens and not is_current_user_admin: + return TOKEN_UNAVAILABLE command = info.context["executor"].get_command("repository") return command.get_upload_token(repository) diff --git a/graphql_api/views.py b/graphql_api/views.py index b5b16a8ed3..1d096b6a00 100644 --- a/graphql_api/views.py +++ b/graphql_api/views.py @@ -19,8 +19,7 @@ ) from graphql import DocumentNode from sentry_sdk import capture_exception -from sentry_sdk import metrics as sentry_metrics -from shared.metrics import Counter, Histogram +from shared.metrics import Counter, Histogram, inc_counter from codecov.commands.exceptions import BaseException from codecov.commands.executor import get_executor_from_request @@ -51,6 +50,17 @@ buckets=[0.05, 0.1, 0.25, 0.5, 0.75, 1, 2, 5, 10, 30], ) +GQL_REQUEST_MADE_COUNTER = Counter( + "api_gql_requests_made", + "Total API GQL requests made", + ["path"], +) + +GQL_ERROR_TYPE_COUNTER = Counter( + "api_gql_errors", + "Number of times API GQL endpoint failed with an exception by type", + ["error_type", "path"], +) # covers named and 3 unnamed operations (see graphql_api/types/query/query.py) GQL_TYPE_AND_NAME_PATTERN = r"^(query|mutation|subscription)(?:\(\$input:|) (\w+)(?:\(| \(|{| {|!)|^(?:{) (me|owner|config)(?:\(| |{)" @@ -109,9 +119,13 @@ def request_started(self, context): """ self.set_type_and_name(query=context["clean_query"]) self.start_timestamp = time.perf_counter() - GQL_HIT_COUNTER.labels( - operation_type=self.operation_type, operation_name=self.operation_name - ).inc() + inc_counter( + GQL_HIT_COUNTER, + labels=dict( + operation_type=self.operation_type, + operation_name=self.operation_name, + ), + ) def request_finished(self, context): """ @@ -226,10 +240,12 @@ async def post(self, request, *args, **kwargs): "user": request.user, } log.info("GraphQL Request", extra=log_data) - sentry_metrics.incr("graphql.info.request_made", tags={"path": req_path}) - + inc_counter(GQL_REQUEST_MADE_COUNTER, labels=dict(path=req_path)) if self._check_ratelimit(request=request): - sentry_metrics.incr("graphql.error.rate_limit", tags={"path": req_path}) + inc_counter( + GQL_ERROR_TYPE_COUNTER, + labels=dict(error_type="rate_limit", path=req_path), + ) return JsonResponse( data={ "status": 429, @@ -250,7 +266,10 @@ async def post(self, request, *args, **kwargs): data = json.loads(content) if "errors" in data: - sentry_metrics.incr("graphql.error.all", tags={"path": req_path}) + inc_counter( + GQL_ERROR_TYPE_COUNTER, + labels=dict(error_type="all", path=req_path), + ) try: if data["errors"][0]["extensions"]["cost"]: costs = data["errors"][0]["extensions"]["cost"] @@ -262,9 +281,12 @@ async def post(self, request, *args, **kwargs): request_body=req_body, ), ) - sentry_metrics.incr( - "graphql.error.query_cost_exceeded", - tags={"path": req_path}, + inc_counter( + GQL_ERROR_TYPE_COUNTER, + labels=dict( + error_type="query_cost_exceeded", + path=req_path, + ), ) return HttpResponseBadRequest( JsonResponse("Your query is too costly.") diff --git a/requirements.txt b/requirements.txt index 08cfd2a4f5..b1270023c7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -177,6 +177,7 @@ freezegun==1.1.0 # via -r requirements.in google-api-core[grpc]==2.11.1 # via + # google-api-core # google-cloud-core # google-cloud-pubsub # google-cloud-storage @@ -408,7 +409,9 @@ requests==2.32.3 # shared # stripe rfc3986[idna2008]==1.4.0 - # via httpx + # via + # httpx + # rfc3986 rsa==4.7.2 # via google-auth s3transfer==0.5.0 diff --git a/upload/helpers.py b/upload/helpers.py index 77194a8595..cd472358d2 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_labels( 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, + include_empty_labels: bool = True, ): metrics_tags = dict( agent=get_agent_from_headers(request.headers), @@ -806,13 +807,23 @@ def generate_upload_sentry_metrics_tags( endpoint=endpoint, is_using_shelter="yes" if is_shelter_request else "no", ) + + repo_visibility = None if repository: - metrics_tags["repo_visibility"] = ( - "private" if repository.private is True else "public" - ) - if position: - metrics_tags["position"] = position - if upload_version: - metrics_tags["upload_version"] = upload_version + repo_visibility = "private" if repository.private else "public" + + optional_fields = { + "repo_visibility": repo_visibility, + "position": position, + "upload_version": upload_version, + } + + metrics_tags.update( + { + field: value + for field, value in optional_fields.items() + if value or include_empty_labels + } + ) return metrics_tags 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", + ], +) 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/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/tests/views/test_reports.py b/upload/tests/views/test_reports.py index 213b2be2f0..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_sentry_metrics = mocker.patch("upload.views.reports.sentry_metrics.incr") + mock_prometheus_metrics = mocker.patch("upload.metrics.API_UPLOAD_COUNTER.labels") repository = RepositoryFactory( name="the_repo", author__username="codecov", author__service="github" ) @@ -65,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_prometheus_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/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/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/tests/views/test_uploads.py b/upload/tests/views/test_uploads.py index 9f00230e01..ff618866bb 100644 --- a/upload/tests/views/test_uploads.py +++ b/upload/tests/views/test_uploads.py @@ -699,8 +699,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" @@ -736,9 +735,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", @@ -746,19 +744,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/bundle_analysis.py b/upload/views/bundle_analysis.py index 4b225b1bc0..b3f000ee8f 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, @@ -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_sentry_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,14 +83,18 @@ def get_exception_handler(self) -> Callable: return repo_auth_custom_exception_handler def post(self, request: HttpRequest) -> Response: - labels = generate_upload_sentry_metrics_tags( + labels = generate_upload_prometheus_metrics_labels( action="bundle_analysis", endpoint="bundle_analysis", request=self.request, is_shelter_request=self.is_shelter_request(), position="start", + include_empty_labels=False, + ) + inc_counter( + BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, + labels=labels, ) - BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels(**labels).inc() serializer = UploadSerializer(data=request.data) if not serializer.is_valid(): @@ -167,14 +174,18 @@ def post(self, request: HttpRequest) -> Response: task_arguments=task_arguments, ), ) - labels = generate_upload_sentry_metrics_tags( + labels = generate_upload_prometheus_metrics_labels( action="bundle_analysis", endpoint="bundle_analysis", request=self.request, is_shelter_request=self.is_shelter_request(), position="end", + include_empty_labels=False, + ) + inc_counter( + BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, + labels=labels, ) - BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER.labels(**labels).inc() dispatch_upload_task( task_arguments, diff --git a/upload/views/commits.py b/upload/views/commits.py index cac17e0c33..2e77916001 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 inc_counter from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -14,7 +14,8 @@ 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_labels +from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import CommitSerializer from upload.views.base import GetterMixin from upload.views.uploads import CanDoCoverageUploadsPermission @@ -53,9 +54,9 @@ 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( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_commit", request=self.request, @@ -72,9 +73,9 @@ def perform_create(self, serializer): extra=dict(repo=repository.name, commit=commit.commitid), ) - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + 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 aecef561e7..6d7f8d5ba3 100644 --- a/upload/views/empty_upload.py +++ b/upload/views/empty_upload.py @@ -8,7 +8,7 @@ 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.metrics import inc_counter from shared.torngit.exceptions import TorngitClientError, TorngitClientGeneralError from codecov_auth.authentication.repo_auth import ( @@ -23,9 +23,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_labels, 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,9 +83,9 @@ 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( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="empty_upload", request=self.request, @@ -149,9 +150,9 @@ def post(self, request, *args, **kwargs): ) ) ] - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + 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 81feb64c55..1de6656d72 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 sentry_sdk import metrics as sentry_metrics +from shared.metrics import inc_counter from codecov.db import sync_to_async from codecov_auth.commands.owner import OwnerCommands @@ -31,13 +31,14 @@ determine_upload_commit_to_use, determine_upload_pr_to_use, dispatch_upload_task, - generate_upload_sentry_metrics_tags, + generate_upload_prometheus_metrics_labels, 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 @@ -74,9 +75,9 @@ 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( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="legacy_upload", request=self.request, @@ -156,9 +157,9 @@ def post(self, request, *args, **kwargs): ), ) - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="legacy_upload", request=self.request, @@ -169,18 +170,6 @@ def post(self, request, *args, **kwargs): ), ) - 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(), - ), - ) - # Validate the upload to make sure the org has enough repo credits and is allowed to upload for this commit redis = get_redis_connection() validate_upload(upload_params, repository, redis) diff --git a/upload/views/reports.py b/upload/views/reports.py index a0020cbf9a..77ea7b8cf0 100644 --- a/upload/views/reports.py +++ b/upload/views/reports.py @@ -3,7 +3,7 @@ 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 shared.metrics import inc_counter from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -16,7 +16,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_labels +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,9 +41,9 @@ 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( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_report", request=self.request, @@ -68,9 +69,9 @@ def perform_create(self, serializer): repository.repoid, commit.commitid, instance.code ) - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_report", request=self.request, @@ -105,9 +106,9 @@ 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( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_report_results", request=self.request, @@ -131,9 +132,9 @@ def perform_create(self, serializer): repoid=repository.repoid, report_code=report.code, ) - sentry_metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + 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 b3e2609ebd..fbe9f40da2 100644 --- a/upload/views/test_results.py +++ b/upload/views/test_results.py @@ -7,7 +7,7 @@ from rest_framework.permissions import BasePermission from rest_framework.response import Response from rest_framework.views import APIView -from sentry_sdk import metrics +from shared.metrics import inc_counter from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -23,7 +23,11 @@ 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_labels, +) +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,9 +70,9 @@ def get_exception_handler(self): return repo_auth_custom_exception_handler def post(self, request): - metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="test_results", endpoint="test_results", request=request, @@ -107,9 +111,9 @@ def post(self, request): if update_fields: repo.save(update_fields=update_fields) - metrics.incr( - "upload", - tags=generate_upload_sentry_metrics_tags( + inc_counter( + API_UPLOAD_COUNTER, + 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 2fe637ef83..cbc6cc04dc 100644 --- a/upload/views/upload_completion.py +++ b/upload/views/upload_completion.py @@ -3,7 +3,7 @@ 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 shared.metrics import inc_counter from codecov_auth.authentication.repo_auth import ( GitHubOIDCTokenAuthentication, @@ -14,7 +14,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_labels +from upload.metrics import API_UPLOAD_COUNTER from upload.views.base import GetterMixin from upload.views.uploads import CanDoCoverageUploadsPermission @@ -34,9 +35,9 @@ 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( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="upload_complete", request=self.request, @@ -78,9 +79,9 @@ 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( + inc_counter( + API_UPLOAD_COUNTER, + 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 f6bdb60ede..4157b57d82 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 sentry_sdk import metrics as sentry_metrics +from shared.metrics import inc_counter from shared.upload.utils import UploaderType, insert_coverage_measurement from codecov_auth.authentication.repo_auth import ( @@ -28,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_labels, 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 @@ -67,9 +68,9 @@ 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( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_upload", request=self.request, @@ -82,18 +83,6 @@ def perform_create(self, serializer: UploadSerializer): 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 @@ -138,9 +127,9 @@ 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( + inc_counter( + API_UPLOAD_COUNTER, + labels=generate_upload_prometheus_metrics_labels( action="coverage", endpoint="create_upload", request=self.request, 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..251c1e1b3c 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, inc_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,10 @@ 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}) + inc_counter( + API_VALIDATE_V2_COUNTER, + labels=dict(source=source), + ) if not self.request.body: return Response(