Skip to content

Commit 118ba69

Browse files
authored
Reapply "feat(integrations): Add integration_id tag to SLO lifecycle metrics" (#108270)
## Summary - Reverts the revert (378f650) to re-apply the original PR #108137 - The Sentry issue (SENTRY-5JQ3) that triggered the revert was a pre-existing error (SENTRY-5HAA, 19k occurrences, archived) — transient 500s from the integration proxy on GitHub's `/rate_limit` endpoint - The new SLO wrapper on `get_rate_limit` simply created a new issue group for the same underlying error that was already being tracked under `get_blame_for_files` ## Original PR #108137 ## Test plan - [x] Verified SENTRY-5JQ3 is the same root cause as pre-existing SENTRY-5HAA - [ ] Monitor after merge for any unexpected issues
1 parent d3d7955 commit 118ba69

File tree

13 files changed

+89
-2
lines changed

13 files changed

+89
-2
lines changed

src/sentry/integrations/github/client.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525
FileBlameInfo,
2626
SourceLineInfo,
2727
)
28+
from sentry.integrations.source_code_management.metrics import (
29+
SCMIntegrationInteractionEvent,
30+
SCMIntegrationInteractionType,
31+
)
2832
from sentry.integrations.source_code_management.repo_trees import RepoTreesClient
2933
from sentry.integrations.source_code_management.repository import RepositoryClient
3034
from sentry.integrations.source_code_management.status_check import StatusCheckClient
@@ -410,7 +414,12 @@ def get_rate_limit(self, specific_resource: str = "core") -> GithubRateLimitInfo
410414
"""This gives information of the current rate limit"""
411415
# There's more but this is good enough
412416
assert specific_resource in ("core", "search", "graphql")
413-
return GithubRateLimitInfo(self.get("/rate_limit")["resources"][specific_resource])
417+
with SCMIntegrationInteractionEvent(
418+
interaction_type=SCMIntegrationInteractionType.GET_RATE_LIMIT,
419+
provider_key=self.integration_name,
420+
integration_id=self.integration.id,
421+
).capture():
422+
return GithubRateLimitInfo(self.get("/rate_limit")["resources"][specific_resource])
414423

415424
# This method is used by RepoTreesIntegration
416425
def get_remaining_api_requests(self) -> int:

src/sentry/integrations/github/integration.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ class GitHubIntegration(
237237
):
238238
integration_name = IntegrationProviderSlug.GITHUB
239239

240+
@property
241+
def integration_id(self) -> int:
242+
return self.model.id
243+
240244
# IssueSyncIntegration configuration keys
241245
comment_key = "sync_comments"
242246
outbound_status_key = "sync_status_forward"

src/sentry/integrations/github_enterprise/integration.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,10 @@ class GitHubEnterpriseIntegration(
177177
def integration_name(self) -> str:
178178
return IntegrationProviderSlug.GITHUB_ENTERPRISE.value
179179

180+
@property
181+
def integration_id(self) -> int:
182+
return self.model.id
183+
180184
def get_client(self):
181185
if not self.org_integration:
182186
raise IntegrationError("Organization Integration does not exist")

src/sentry/integrations/gitlab/integration.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ class GitlabIntegration(
123123
def integration_name(self) -> str:
124124
return IntegrationProviderSlug.GITLAB
125125

126+
@property
127+
def integration_id(self) -> int:
128+
return self.model.id
129+
126130
def get_client(self) -> GitLabApiClient:
127131
try:
128132
# eagerly populate this just for the error message

src/sentry/integrations/perforce/integration.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ class PerforceIntegration(RepositoryIntegration, CommitContextIntegration):
172172

173173
integration_name = "perforce"
174174

175+
@property
176+
def integration_id(self) -> int:
177+
return self.model.id
178+
175179
def __init__(
176180
self,
177181
model: Integration,

src/sentry/integrations/project_management/metrics.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,6 @@ def get_integration_domain(self) -> IntegrationDomain:
4545

4646
def get_interaction_type(self) -> str:
4747
return str(self.action_type)
48+
49+
def get_integration_id(self) -> int | None:
50+
return self.integration.id

src/sentry/integrations/source_code_management/commit_context.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ class CommitContextIntegration(ABC):
130130
def integration_name(self) -> str:
131131
raise NotImplementedError
132132

133+
@property
134+
@abstractmethod
135+
def integration_id(self) -> int:
136+
raise NotImplementedError
137+
133138
@abstractmethod
134139
def get_client(self) -> CommitContextClient:
135140
raise NotImplementedError
@@ -145,6 +150,7 @@ def get_blame_for_files(
145150
with CommitContextIntegrationInteractionEvent(
146151
interaction_type=SCMIntegrationInteractionType.GET_BLAME_FOR_FILES,
147152
provider_key=self.integration_name,
153+
integration_id=self.integration_id,
148154
).capture() as lifecycle:
149155
try:
150156
client = self.get_client()
@@ -224,6 +230,7 @@ def queue_pr_comment_task_if_needed(
224230
with CommitContextIntegrationInteractionEvent(
225231
interaction_type=SCMIntegrationInteractionType.QUEUE_COMMENT_TASK,
226232
provider_key=self.integration_name,
233+
integration_id=self.integration_id,
227234
organization_id=project.organization_id,
228235
project=project,
229236
commit=commit,
@@ -334,6 +341,7 @@ def create_or_update_comment(
334341
with CommitContextIntegrationInteractionEvent(
335342
interaction_type=interaction_type,
336343
provider_key=self.integration_name,
344+
integration_id=self.integration_id,
337345
repository=repo,
338346
pull_request_id=pr.id,
339347
).capture():

src/sentry/integrations/source_code_management/metrics.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ class SCMIntegrationInteractionType(StrEnum):
5252
# Status Checks
5353
CREATE_STATUS_CHECK = "create_status_check"
5454

55+
# Rate Limiting
56+
GET_RATE_LIMIT = "get_rate_limit"
57+
5558

5659
@dataclass
5760
class SCMIntegrationInteractionEvent(IntegrationEventLifecycleMetric):
@@ -73,6 +76,9 @@ def get_integration_name(self) -> str:
7376
def get_interaction_type(self) -> str:
7477
return str(self.interaction_type)
7578

79+
def get_integration_id(self) -> int | None:
80+
return self.integration_id
81+
7682
def get_extras(self) -> Mapping[str, Any]:
7783
return {
7884
"organization_id": (self.organization_id if self.organization_id else None),

src/sentry/integrations/utils/metrics.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import sentry_sdk
1111

12+
from sentry import options
1213
from sentry.exceptions import RestrictedIPAddress
1314
from sentry.integrations.base import IntegrationDomain
1415
from sentry.integrations.types import EventLifecycleOutcome
@@ -86,12 +87,23 @@ def get_metric_key(self, outcome: EventLifecycleOutcome) -> str:
8687
tokens = ("integrations", self.get_metrics_domain(), str(outcome))
8788
return ".".join(tokens)
8889

90+
def get_integration_id(self) -> int | None:
91+
"""Return the integration ID if available. Override in subclasses."""
92+
return None
93+
8994
def get_metric_tags(self) -> Mapping[str, str]:
90-
return {
95+
tags: dict[str, str] = {
9196
"integration_domain": str(self.get_integration_domain()),
9297
"integration_name": self.get_integration_name(),
9398
"interaction_type": self.get_interaction_type(),
9499
}
100+
# TODO(telkins): Remove killswitch once we no longer need integration_id on SLO metrics
101+
integration_id = self.get_integration_id()
102+
if integration_id is not None and options.get(
103+
"integrations.slo.integration-id-tag-enabled"
104+
):
105+
tags["integration_id"] = str(integration_id)
106+
return tags
95107

96108
def capture(
97109
self, assume_success: bool = True, sample_log_rate: float = 1.0

src/sentry/metrics/middleware.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@
2121
"batching_consumer.batch.size",
2222
"batching_consumer.batch.flush",
2323
"batching_consumer.batch.flush.normalized",
24+
# TODO(telkins): Remove once we figure out interaction_type org breakdown
25+
"integrations.slo.started",
26+
"integrations.slo.success",
27+
"integrations.slo.halted",
28+
"integrations.slo.failure",
2429
]
2530
)
2631

0 commit comments

Comments
 (0)