Skip to content

Commit 845014a

Browse files
authored
fix(integrations): add interaction events for /repos and /installation/repositories (#108722)
<!-- Describe your PR here. --> We were previously hiding many nested API calls that happen as part of the `derive_codemappings` call which is making our tracking numbers be off. This adds new interaction types for `GET_REPO_TREE` and `GET_REPOSITORIES` which each make individual calls to the Github API. I took a look at adding this tracking for other integration providers, but the way this code is set up with mix-ins makes that a bit of a pain. We can revisit this later if needed.
1 parent 5f1eaa9 commit 845014a

File tree

3 files changed

+77
-28
lines changed

3 files changed

+77
-28
lines changed

src/sentry/integrations/github/client.py

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -433,23 +433,28 @@ def get_remaining_api_requests(self) -> int:
433433
# This method is used by RepoTreesIntegration
434434
# https://docs.github.com/en/rest/git/trees#get-a-tree
435435
def get_tree(self, repo_full_name: str, tree_sha: str) -> list[dict[str, Any]]:
436-
# We do not cache this call since it is a rather large object
437-
contents: dict[str, Any] = self.get(
438-
f"/repos/{repo_full_name}/git/trees/{tree_sha}",
439-
# Will cause all objects or subtrees referenced by the tree specified in :tree_sha
440-
params={"recursive": 1},
441-
)
442-
# If truncated is true in the response then the number of items in the tree array exceeded our maximum limit.
443-
# If you need to fetch more items, use the non-recursive method of fetching trees, and fetch one sub-tree at a time.
444-
# Note: The limit for the tree array is 100,000 entries with a maximum size of 7 MB when using the recursive parameter.
445-
# XXX: We will need to improve this by iterating through trees without using the recursive parameter
446-
if contents.get("truncated"):
447-
# e.g. getsentry/DataForThePeople
448-
logger.warning(
449-
"The tree for %s has been truncated. Use different a approach for retrieving contents of tree.",
450-
repo_full_name,
436+
with SCMIntegrationInteractionEvent(
437+
interaction_type=SCMIntegrationInteractionType.GET_REPO_TREE,
438+
provider_key=self.integration_name,
439+
integration_id=self.integration.id,
440+
).capture():
441+
# We do not cache this call since it is a rather large object
442+
contents: dict[str, Any] = self.get(
443+
f"/repos/{repo_full_name}/git/trees/{tree_sha}",
444+
# Will cause all objects or subtrees referenced by the tree specified in :tree_sha
445+
params={"recursive": 1},
451446
)
452-
return contents["tree"]
447+
# If truncated is true in the response then the number of items in the tree array exceeded our maximum limit.
448+
# If you need to fetch more items, use the non-recursive method of fetching trees, and fetch one sub-tree at a time.
449+
# Note: The limit for the tree array is 100,000 entries with a maximum size of 7 MB when using the recursive parameter.
450+
# XXX: We will need to improve this by iterating through trees without using the recursive parameter
451+
if contents.get("truncated"):
452+
# e.g. getsentry/DataForThePeople
453+
logger.warning(
454+
"The tree for %s has been truncated. Use different a approach for retrieving contents of tree.",
455+
repo_full_name,
456+
)
457+
return contents["tree"]
453458

454459
# Used by RepoTreesIntegration
455460
def should_count_api_error(self, error: ApiError, extra: dict[str, str]) -> bool:
@@ -496,11 +501,16 @@ def get_repos(self, page_number_limit: int | None = None) -> list[dict[str, Any]
496501
It uses page_size from the base class to specify how many items per page.
497502
The upper bound of requests is controlled with self.page_number_limit to prevent infinite requests.
498503
"""
499-
return self._get_with_pagination(
500-
"/installation/repositories",
501-
response_key="repositories",
502-
page_number_limit=page_number_limit,
503-
)
504+
with SCMIntegrationInteractionEvent(
505+
interaction_type=SCMIntegrationInteractionType.GET_REPOSITORIES,
506+
provider_key=self.integration_name,
507+
integration_id=self.integration.id,
508+
).capture():
509+
return self._get_with_pagination(
510+
"/installation/repositories",
511+
response_key="repositories",
512+
page_number_limit=page_number_limit,
513+
)
504514

505515
def search_repositories(self, query: bytes) -> Mapping[str, Sequence[Any]]:
506516
"""

src/sentry/integrations/source_code_management/metrics.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ class SCMIntegrationInteractionType(StrEnum):
5555
# Rate Limiting
5656
GET_RATE_LIMIT = "get_rate_limit"
5757

58+
# Repo Trees
59+
GET_REPOSITORIES = "get_repositories"
60+
GET_REPO_TREE = "get_repo_tree"
61+
5862

5963
@dataclass
6064
class SCMIntegrationInteractionEvent(IntegrationEventLifecycleMetric):

tests/sentry/integrations/github/tasks/test_link_all_repos.py

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,12 @@ def test_link_all_repos(self, mock_record: MagicMock, _: MagicMock) -> None:
8181
assert repos[0].name == "getsentry/sentry"
8282
assert repos[1].name == "getsentry/snuba"
8383

84-
assert_slo_metric(mock_record, EventLifecycleOutcome.SUCCESS)
84+
assert len(mock_record.mock_calls) == 4
85+
start1, start2, end2, end1 = mock_record.mock_calls
86+
assert start1.args[0] == EventLifecycleOutcome.STARTED
87+
assert start2.args[0] == EventLifecycleOutcome.STARTED
88+
assert end2.args[0] == EventLifecycleOutcome.SUCCESS
89+
assert end1.args[0] == EventLifecycleOutcome.SUCCESS
8590

8691
@responses.activate
8792
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@@ -121,7 +126,12 @@ def test_link_all_repos_api_response_keyerror(
121126

122127
assert repos[0].name == "getsentry/snuba"
123128

124-
assert_slo_metric(mock_record, EventLifecycleOutcome.HALTED)
129+
assert len(mock_record.mock_calls) == 4
130+
start1, start2, end2, end1 = mock_record.mock_calls
131+
assert start1.args[0] == EventLifecycleOutcome.STARTED
132+
assert start2.args[0] == EventLifecycleOutcome.STARTED
133+
assert end2.args[0] == EventLifecycleOutcome.SUCCESS
134+
assert end1.args[0] == EventLifecycleOutcome.HALTED
125135
assert_halt_metric(
126136
mock_record, LinkAllReposHaltReason.REPOSITORY_NOT_CREATED.value
127137
) # should be halt because it didn't complete successfully
@@ -155,7 +165,12 @@ def test_link_all_repos_api_response_keyerror_single_repo(
155165
repos = Repository.objects.all()
156166
assert len(repos) == 0
157167

158-
assert_slo_metric(mock_record, EventLifecycleOutcome.HALTED)
168+
assert len(mock_record.mock_calls) == 4
169+
start1, start2, end2, end1 = mock_record.mock_calls
170+
assert start1.args[0] == EventLifecycleOutcome.STARTED
171+
assert start2.args[0] == EventLifecycleOutcome.STARTED
172+
assert end2.args[0] == EventLifecycleOutcome.SUCCESS
173+
assert end1.args[0] == EventLifecycleOutcome.HALTED
159174
assert_halt_metric(mock_record, LinkAllReposHaltReason.REPOSITORY_NOT_CREATED.value)
160175

161176
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@@ -198,7 +213,12 @@ def test_link_all_repos_api_error(self, mock_record: MagicMock, _: MagicMock) ->
198213
organization_id=self.organization.id,
199214
)
200215

201-
assert_slo_metric(mock_record, EventLifecycleOutcome.FAILURE)
216+
assert len(mock_record.mock_calls) == 4
217+
start1, start2, end2, end1 = mock_record.mock_calls
218+
assert start1.args[0] == EventLifecycleOutcome.STARTED
219+
assert start2.args[0] == EventLifecycleOutcome.STARTED
220+
assert end2.args[0] == EventLifecycleOutcome.FAILURE
221+
assert end1.args[0] == EventLifecycleOutcome.FAILURE
202222

203223
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
204224
@responses.activate
@@ -221,7 +241,12 @@ def test_link_all_repos_api_error_rate_limited(
221241
organization_id=self.organization.id,
222242
)
223243

224-
assert_slo_metric(mock_record, EventLifecycleOutcome.HALTED)
244+
assert len(mock_record.mock_calls) == 4
245+
start1, start2, end2, end1 = mock_record.mock_calls
246+
assert start1.args[0] == EventLifecycleOutcome.STARTED
247+
assert start2.args[0] == EventLifecycleOutcome.STARTED
248+
assert end2.args[0] == EventLifecycleOutcome.FAILURE
249+
assert end1.args[0] == EventLifecycleOutcome.HALTED
225250
assert_halt_metric(mock_record, LinkAllReposHaltReason.RATE_LIMITED.value)
226251

227252
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@@ -240,7 +265,12 @@ def test_link_all_repos_repo_creation_error(
240265
organization_id=self.organization.id,
241266
)
242267

243-
assert_slo_metric(mock_record, EventLifecycleOutcome.HALTED)
268+
assert len(mock_record.mock_calls) == 4
269+
start1, start2, end2, end1 = mock_record.mock_calls
270+
assert start1.args[0] == EventLifecycleOutcome.STARTED
271+
assert start2.args[0] == EventLifecycleOutcome.STARTED
272+
assert end2.args[0] == EventLifecycleOutcome.SUCCESS
273+
assert end1.args[0] == EventLifecycleOutcome.HALTED
244274
assert_halt_metric(mock_record, LinkAllReposHaltReason.REPOSITORY_NOT_CREATED.value)
245275

246276
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@@ -262,4 +292,9 @@ def test_link_all_repos_repo_creation_exception(
262292
organization_id=self.organization.id,
263293
)
264294

265-
assert_slo_metric(mock_record, EventLifecycleOutcome.FAILURE)
295+
assert len(mock_record.mock_calls) == 4
296+
start1, start2, end2, end1 = mock_record.mock_calls
297+
assert start1.args[0] == EventLifecycleOutcome.STARTED
298+
assert start2.args[0] == EventLifecycleOutcome.STARTED
299+
assert end2.args[0] == EventLifecycleOutcome.SUCCESS
300+
assert end1.args[0] == EventLifecycleOutcome.FAILURE

0 commit comments

Comments
 (0)