Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(escalating-v2): Emit escalating metrics #54240

Merged
merged 14 commits into from
Aug 14, 2023
Merged

feat(escalating-v2): Emit escalating metrics #54240

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7e4fdad
Emit escalating metrics
snigdhas Aug 4, 2023
c7b9052
Switch to the generic_metrics_backend wrapper
snigdhas Aug 4, 2023
83ded5c
Merge branch 'master' into snigdha/emit-metrics
snigdhas Aug 7, 2023
08f6125
use default project id
ayirr7 Aug 8, 2023
92267bc
Add a feature flag
snigdhas Aug 9, 2023
0013277
fix test_notifications
ayirr7 Aug 10, 2023
b7a7bf9
Add a passthrough for metrics BE requests
snigdhas Aug 10, 2023
92e47b0
TESTING DO NOT MERGE - enable feature flag to run CI
snigdhas Aug 10, 2023
9b4d4d3
Merge branch 'master' into snigdha/emit-metrics
snigdhas Aug 10, 2023
8667a08
try adding responses
ayirr7 Aug 11, 2023
d16a6fa
add feature (for now)
ayirr7 Aug 11, 2023
d71691a
Revert to a passthrough for the metrics request
snigdhas Aug 11, 2023
144e44c
Revert feature flag to False
snigdhas Aug 11, 2023
9aaffb5
Revert changes to get_attachment
snigdhas Aug 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/sentry/conf/server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1378,6 +1378,8 @@ def SOCIAL_AUTH_DEFAULT_USERNAME() -> str:
"organizations:escalating-issues-msteams": False,
# Enable archive/escalating issue workflow features in v2
"organizations:escalating-issues-v2": False,
# Enable emiting escalating data to the metrics backend
"organizations:escalating-metrics-backend": True,
# Allows an org to have a larger set of project ownership rules per project
"organizations:higher-ownership-limit": False,
# Enable Monitors (Crons) view
Expand Down
1 change: 1 addition & 0 deletions src/sentry/features/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@
default_manager.add("organizations:escalating-issues", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:escalating-issues-msteams", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
default_manager.add("organizations:escalating-issues-v2", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:escalating-metrics-backend", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:integrations-gh-invite", OrganizationFeature, FeatureHandlerStrategy.REMOTE)
default_manager.add("organizations:event-attachments", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
default_manager.add("organizations:global-views", OrganizationFeature, FeatureHandlerStrategy.INTERNAL)
Expand Down
19 changes: 19 additions & 0 deletions src/sentry/tasks/post_process.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.client import generic_metrics_backend
from sentry.sentry_metrics.use_case_id_registry import UseCaseID
from sentry.signals import event_processed, issue_unignored, transaction_processed
from sentry.silo import SiloMode
from sentry.tasks.base import instrumented_task
Expand Down Expand Up @@ -116,6 +118,21 @@ 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.
"""
generic_metrics_backend.counter(
UseCaseID.ESCALATING_ISSUES,
org_id=event.project.organization_id,
project_id=event.project.id,
metric_name="event_ingested",
value=1,
tags={"group": str(event.group_id)},
unit=None,
)


def _capture_group_stats(job: PostProcessJob) -> None:
event = job["event"]
if not job["group_state"]["is_new"] or not should_write_event_stats(event):
Expand Down Expand Up @@ -573,6 +590,8 @@ def get_event_raise_exception() -> Event:
update_event_groups(event, group_states)
bind_organization_context(event.project.organization)
_capture_event_stats(event)
if features.has("organizations:escalating-metrics-backend", event.project.organization):
_update_escalating_metrics(event)

group_events: Mapping[int, GroupEvent] = {
ge.group_id: ge for ge in list(event.build_group_events())
Expand Down
8 changes: 8 additions & 0 deletions tests/relay_integration/lang/javascript/test_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import pytest
import responses
from django.conf import settings
from django.core.files.base import ContentFile
from django.utils import timezone

Expand Down Expand Up @@ -1308,6 +1309,9 @@ def test_no_fetch_from_http(self):
body=load_fixture("node_app.min.js.map"),
content_type="application/javascript; charset=utf-8",
)
responses.add_passthru(
settings.SENTRY_SNUBA + "/tests/entities/generic_metrics_counters/insert",
)

data = {
"timestamp": self.min_ago,
Expand Down Expand Up @@ -1383,6 +1387,10 @@ def test_html_file_with_query_param_ending_with_js_extension(self):
"<!doctype html><html><head></head><body><script>/*legit case*/</script></body></html>"
),
)
responses.add_passthru(
settings.SENTRY_SNUBA + "/tests/entities/generic_metrics_counters/insert",
)

data = {
"timestamp": self.min_ago,
"message": "hello",
Expand Down
1 change: 1 addition & 0 deletions tests/sentry/api/serializers/test_organization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def test_simple(self):
"performance-issues-search",
"transaction-name-normalize",
"transaction-name-mark-scrubbed-as-sanitized",
"escalating-metrics-backend",
}

@mock.patch("sentry.features.batch_has")
Expand Down
22 changes: 13 additions & 9 deletions tests/sentry/notifications/test_notifications.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from urllib.parse import parse_qs

import responses
from django.conf import settings
from django.core import mail
from django.core.mail.message import EmailMultiAlternatives
from django.utils import timezone
Expand Down Expand Up @@ -42,9 +43,9 @@ def make_event(**kwargs):
return result


def get_attachment():
def get_attachment(response_num):
assert len(responses.calls) >= 1
data = parse_qs(responses.calls[0].request.body)
data = parse_qs(responses.calls[response_num].request.body)
assert "text" in data
assert "attachments" in data
attachments = json.loads(data["attachments"][0])
Expand Down Expand Up @@ -106,6 +107,9 @@ def setUp(self):
status=200,
content_type="application/json",
)
responses.add_passthru(
settings.SENTRY_SNUBA + "/tests/entities/generic_metrics_counters/insert",
)
self.name = self.user.get_display_name()
self.short_id = self.group.qualified_short_id

Expand All @@ -131,7 +135,7 @@ def test_sends_note_notification(self):
assert isinstance(msg.alternatives[0][0], str)
assert "blah blah</p></div>" in msg.alternatives[0][0]

attachment, text = get_attachment()
attachment, text = get_attachment(0)
# check the Slack version
assert text == f"New comment by {self.name}"
assert attachment["title"] == f"{self.group.title}"
Expand Down Expand Up @@ -171,7 +175,7 @@ def test_sends_unassignment_notification(self):
assert isinstance(msg.alternatives[0][0], str)
assert f"{self.user.username}</strong> unassigned" in msg.alternatives[0][0]

attachment, text = get_attachment()
attachment, text = get_attachment(0)

assert text == f"Issue unassigned by {self.name}"
assert attachment["title"] == self.group.title
Expand Down Expand Up @@ -201,7 +205,7 @@ def test_sends_resolution_notification(self, record_analytics):
assert isinstance(msg.alternatives[0][0], str)
assert f"{self.short_id}</a> as resolved</p>" in msg.alternatives[0][0]

attachment, text = get_attachment()
attachment, text = get_attachment(0)

assert (
text
Expand Down Expand Up @@ -258,7 +262,7 @@ def test_sends_deployment_notification(self, record_analytics):
in msg.alternatives[0][0]
)

attachment, text = get_attachment()
attachment, text = get_attachment(0)

assert (
text
Expand Down Expand Up @@ -325,7 +329,7 @@ def test_sends_regression_notification(self, record_analytics):
assert isinstance(msg.alternatives[0][0], str)
assert f"{group.qualified_short_id}</a> as a regression</p>" in msg.alternatives[0][0]

attachment, text = get_attachment()
attachment, text = get_attachment(0)

assert text == "Issue marked as regression"
assert (
Expand Down Expand Up @@ -379,7 +383,7 @@ def test_sends_resolved_in_release_notification(self, record_analytics):
f'text-decoration: none">{self.short_id}</a> as resolved in' in msg.alternatives[0][0]
)

attachment, text = get_attachment()
attachment, text = get_attachment(0)
assert text == f"Issue marked as resolved in {parsed_version} by {self.name}"
assert attachment["title"] == self.group.title
assert (
Expand Down Expand Up @@ -457,7 +461,7 @@ def test_sends_issue_notification(self, record_analytics):
assert isinstance(msg.alternatives[0][0], str)
assert "Hello world</pre>" in msg.alternatives[0][0]

attachment, text = get_attachment()
attachment, text = get_attachment(1)
Copy link
Member

@ayirr7 ayirr7 Aug 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know if the response_num needs to be passed in explicitly like this still? This was a different attempt to fix the tests. From my understanding, the passthru should be sufficient for fixing the tests and we no longer need this other change.

i.e. Do all tests still pass if we don't have response_num passed in like this?


assert attachment["title"] == "Hello world"
assert (
Expand Down
6 changes: 2 additions & 4 deletions tests/sentry/sentry_metrics/test_snuba.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
class MetricsInterfaceTestCase(BaseMetricsLayerTestCase, TestCase, GenericMetricsTestMixIn):
def setUp(self):
super().setUp()
self.test_project = self.create_project()


class SnubaMetricsInterfaceTest(MetricsInterfaceTestCase):
Expand All @@ -33,7 +32,7 @@ def test_count_query(self):
generic_metrics_backend.distribution(
self.use_case_id,
self.organization.id,
self.test_project.id,
self.project.id,
self.metric_name,
[100, 200, 300],
{},
Expand All @@ -49,7 +48,6 @@ def test_count_query(self):
metric_mri=self.get_mri(self.metric_name, "d", self.use_case_id, self.unit),
),
],
project_ids=[self.test_project.id],
groupby=[],
orderby=[],
limit=Limit(limit=1),
Expand All @@ -58,7 +56,7 @@ def test_count_query(self):
)

data = get_series(
[self.test_project],
[self.project],
metrics_query=metrics_query,
include_meta=True,
use_case_id=self.use_case_id,
Expand Down
Loading