-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(escalating-v2): Emit escalating metrics #52774
Changes from all commits
4d9ab22
40329fe
58ae535
5999cf8
c0ef368
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,8 @@ | |
from sentry.issues.grouptype import GroupCategory | ||
from sentry.issues.issue_occurrence import IssueOccurrence | ||
from sentry.killswitches import killswitch_matches_context | ||
from sentry.sentry_metrics.kafka import KafkaMetricsBackend | ||
from sentry.sentry_metrics.use_case_id_registry import UseCaseID | ||
from sentry.signals import event_processed, issue_unignored, transaction_processed | ||
from sentry.tasks.base import instrumented_task | ||
from sentry.utils import metrics | ||
|
@@ -115,6 +117,23 @@ def _capture_event_stats(event: Event) -> None: | |
metrics.timing("events.size.data", event.size, tags=tags) | ||
|
||
|
||
def _update_escalating_metrics(event: Event) -> None: | ||
""" | ||
Update metrics for escalating issues when an event is processed. | ||
""" | ||
metrics_backend = KafkaMetricsBackend() | ||
metrics_backend.counter( | ||
UseCaseID.ESCALATING_ISSUES, | ||
org_id=event.project.organization_id, | ||
project_id=event.project.id, | ||
metric_name="event_ingested", | ||
value=1, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ayirr7 this should get aggregated as part of the counter right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, that's correct |
||
tags={"group": str(event.group_id)}, | ||
unit=None, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's the mri_unit in this template ":/@". I've mostly seen units of time (millisecond, second) here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ayirr7 what should this be set to for our needs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The unit for the metric you are emitting (whatever you are measuring). If there is no unit then |
||
) | ||
metrics_backend.close() | ||
|
||
|
||
def _capture_group_stats(job: PostProcessJob) -> None: | ||
event = job["event"] | ||
if not job["group_state"]["is_new"] or not should_write_event_stats(event): | ||
|
@@ -571,6 +590,7 @@ def get_event_raise_exception() -> Event: | |
update_event_groups(event, group_states) | ||
bind_organization_context(event.project.organization) | ||
_capture_event_stats(event) | ||
_update_escalating_metrics(event) | ||
|
||
group_events: Mapping[int, GroupEvent] = { | ||
ge.group_id: ge for ge in list(event.build_group_events()) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ayirr7 I think we should hide which backend gets used from the caller.
it should be something like
This also makes it easier to only have one connection (pool) per process
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sentry has the service delegator / LazyServiceWrapper abstraction which is commonly used for this