Skip to content

Commit b2d06fc

Browse files
authored
fix(spans-migration): commit changes once everything passes (#104580)
made a bunch of changes to the alerts migration scripts to make sure it works the way we want to 1. Ensure that on translation and rollback that all the changes in the db get made only after all actions are successful 2. For all `count(...)` functions with parameters, we're going to default them to the regular count so they get translated properly 3. Some functions get translated with the `equation|` prefix because we needed that in dashboards and explore but it's not needed for alerts so I've stripped out the equation prefix for these functions 4. Added a BUNCH of logs so that we know where things are failing if they fail during the migration
1 parent 58fe719 commit b2d06fc

File tree

2 files changed

+285
-64
lines changed

2 files changed

+285
-64
lines changed

src/sentry/explore/translation/alerts_translation.py

Lines changed: 89 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import logging
22

33
import sentry_sdk
4-
from django.db import router, transaction
4+
from django.db import router
55

66
from sentry import features
7+
from sentry.discover.arithmetic import is_equation, strip_equation
78
from sentry.discover.translation.mep_to_eap import QueryParts, translate_mep_to_eap
89
from sentry.incidents.models.alert_rule import AlertRuleDetectionType
910
from sentry.incidents.subscription_processor import MetricIssueDetectorConfig
@@ -20,7 +21,7 @@
2021
SnubaQuery,
2122
SnubaQueryEventType,
2223
)
23-
from sentry.snuba.tasks import update_subscription_in_snuba
24+
from sentry.snuba.subscriptions import bulk_update_snuba_subscriptions
2425
from sentry.utils.db import atomic_transaction
2526
from sentry.workflow_engine.models.data_condition import DataCondition
2627
from sentry.workflow_engine.models.data_source import DataSource
@@ -35,9 +36,14 @@
3536
"count_unique",
3637
]
3738

39+
COUNT_AGGREGATE_PREFIX = "count("
40+
3841

3942
def snapshot_snuba_query(snuba_query: SnubaQuery):
40-
if snuba_query.dataset in [Dataset.PerformanceMetrics.value, Dataset.Transactions.value]:
43+
if not snuba_query.query_snapshot and snuba_query.dataset in [
44+
Dataset.PerformanceMetrics.value,
45+
Dataset.Transactions.value,
46+
]:
4147
query_snapshot = {
4248
"type": snuba_query.type,
4349
"dataset": snuba_query.dataset,
@@ -89,6 +95,7 @@ def translate_detector_and_update_subscription_in_snuba(snuba_query: SnubaQuery)
8995

9096
snapshot = snuba_query.query_snapshot
9197
if not snapshot:
98+
logger.info("No snapshot created for snuba query %s", snuba_query.id)
9299
return
93100

94101
if snapshot.get("user_updated"):
@@ -99,23 +106,31 @@ def translate_detector_and_update_subscription_in_snuba(snuba_query: SnubaQuery)
99106

100107
old_query_type, old_dataset, old_query, old_aggregate = _get_old_query_info(snuba_query)
101108

109+
snapshot_aggregate = snapshot["aggregate"]
110+
if snapshot["aggregate"].startswith(COUNT_AGGREGATE_PREFIX):
111+
snapshot_aggregate = "count()"
112+
102113
eap_query_parts, dropped_fields = translate_mep_to_eap(
103114
QueryParts(
104-
selected_columns=[snapshot["aggregate"]],
115+
selected_columns=[snapshot_aggregate],
105116
query=snapshot["query"],
106117
equations=None,
107118
orderby=None,
108119
)
109120
)
110121

111122
if dropped_fields["selected_columns"]:
123+
logger.info("Unsupported column dropped for snuba query %s", snuba_query.id)
112124
with sentry_sdk.isolation_scope() as scope:
113125
scope.set_tag("dropped_fields", dropped_fields["selected_columns"])
114126
scope.set_tag("snuba_query", snuba_query.id)
115127
sentry_sdk.capture_message("Unsported column")
116128
return
117129

118130
translated_aggregate = eap_query_parts["selected_columns"][0]
131+
# some functions like apdex() are translated to equations, but alerts doesn't need that so we can just strip the equation prefix
132+
if is_equation(translated_aggregate):
133+
translated_aggregate = strip_equation(translated_aggregate)
119134
translated_query = eap_query_parts["query"]
120135

121136
snuba_query.aggregate = translated_aggregate
@@ -135,6 +150,8 @@ def translate_detector_and_update_subscription_in_snuba(snuba_query: SnubaQuery)
135150
using=(
136151
router.db_for_write(SnubaQuery),
137152
router.db_for_write(SnubaQueryEventType),
153+
router.db_for_write(QuerySubscription),
154+
router.db_for_read(DataCondition),
138155
)
139156
):
140157
snuba_query.save()
@@ -143,38 +160,42 @@ def translate_detector_and_update_subscription_in_snuba(snuba_query: SnubaQuery)
143160
snuba_query=snuba_query, type=SnubaQueryEventType.EventType.TRACE_ITEM_SPAN.value
144161
)
145162

146-
query_subscriptions = list(snuba_query.subscriptions.all())
147-
for subscription in query_subscriptions:
148-
with transaction.atomic(router.db_for_write(QuerySubscription)):
149-
subscription.update(status=QuerySubscription.Status.UPDATING.value)
150-
151-
transaction.on_commit(
152-
lambda: update_subscription_in_snuba(
153-
query_subscription_id=subscription.id,
154-
old_query_type=old_query_type.value,
155-
old_dataset=old_dataset.value,
156-
old_aggregate=old_aggregate,
157-
old_query=old_query,
158-
),
159-
using=router.db_for_write(QuerySubscription),
163+
query_subscriptions = list(snuba_query.subscriptions.all())
164+
try:
165+
bulk_update_snuba_subscriptions(
166+
query_subscriptions, old_query_type, old_dataset, old_aggregate, old_query
160167
)
161-
162-
for detector in detectors:
163-
detector_cfg: MetricIssueDetectorConfig = detector.config
164-
if detector_cfg["detection_type"] == AlertRuleDetectionType.DYNAMIC.value:
165-
data_condition = DataCondition.objects.get(
166-
condition_group=detector.workflow_condition_group
168+
except Exception as e:
169+
logger.info(
170+
"Query not migrated: error updating subscriptions in snuba",
171+
extra={"snuba_query_id": snuba_query.id, "error": e},
167172
)
168-
handle_send_historical_data_to_seer(
169-
detector,
170-
data_source,
171-
data_condition,
172-
snuba_query,
173-
detector.project,
174-
SeerMethod.UPDATE,
175-
event_types=[SnubaQueryEventType.EventType.TRACE_ITEM_SPAN],
173+
raise
174+
175+
try:
176+
for detector in detectors:
177+
detector_cfg: MetricIssueDetectorConfig = detector.config
178+
if detector_cfg["detection_type"] == AlertRuleDetectionType.DYNAMIC.value:
179+
data_condition = DataCondition.objects.get(
180+
condition_group=detector.workflow_condition_group
181+
)
182+
handle_send_historical_data_to_seer(
183+
detector,
184+
data_source,
185+
data_condition,
186+
snuba_query,
187+
detector.project,
188+
SeerMethod.UPDATE,
189+
event_types=[SnubaQueryEventType.EventType.TRACE_ITEM_SPAN],
190+
)
191+
except Exception as e:
192+
logger.info(
193+
"Query not migrated: error sending historical data to seer",
194+
extra={"snuba_query_id": snuba_query.id, "error": e},
176195
)
196+
raise
177197

198+
logger.info("Query successfully migrated to EAP", extra={"snuba_query_id": snuba_query.id})
178199
return
179200

180201

@@ -220,6 +241,8 @@ def rollback_detector_query_and_update_subscription_in_snuba(snuba_query: SnubaQ
220241
using=(
221242
router.db_for_write(SnubaQuery),
222243
router.db_for_write(SnubaQueryEventType),
244+
router.db_for_write(QuerySubscription),
245+
router.db_for_read(DataCondition),
223246
)
224247
):
225248
snuba_query.update(
@@ -234,39 +257,42 @@ def rollback_detector_query_and_update_subscription_in_snuba(snuba_query: SnubaQ
234257
snuba_query=snuba_query, type=SnubaQueryEventType.EventType.TRANSACTION.value
235258
)
236259

237-
query_subscriptions = list(snuba_query.subscriptions.all())
238-
for subscription in query_subscriptions:
239-
with transaction.atomic(router.db_for_write(QuerySubscription)):
240-
subscription.update(status=QuerySubscription.Status.UPDATING.value)
241-
242-
transaction.on_commit(
243-
lambda: update_subscription_in_snuba(
244-
query_subscription_id=subscription.id,
245-
old_query_type=old_query_type.value,
246-
old_dataset=old_dataset.value,
247-
old_aggregate=old_aggregate,
248-
old_query=old_query,
249-
),
250-
using=router.db_for_write(QuerySubscription),
260+
query_subscriptions = list(snuba_query.subscriptions.all())
261+
try:
262+
bulk_update_snuba_subscriptions(
263+
query_subscriptions, old_query_type, old_dataset, old_aggregate, old_query
251264
)
252-
253-
for detector in detectors:
254-
detector_cfg: MetricIssueDetectorConfig = detector.config
255-
if detector_cfg["detection_type"] == AlertRuleDetectionType.DYNAMIC.value:
256-
data_condition = DataCondition.objects.get(
257-
condition_group=detector.workflow_condition_group
265+
except Exception as e:
266+
logger.info(
267+
"Query not rolled back: error updating subscriptions in snuba",
268+
extra={"snuba_query_id": snuba_query.id, "error": e},
258269
)
259-
handle_send_historical_data_to_seer(
260-
detector,
261-
data_source,
262-
data_condition,
263-
snuba_query,
264-
detector.project,
265-
SeerMethod.UPDATE,
266-
event_types=[SnubaQueryEventType.EventType.TRANSACTION],
270+
raise
271+
272+
try:
273+
for detector in detectors:
274+
detector_cfg: MetricIssueDetectorConfig = detector.config
275+
if detector_cfg["detection_type"] == AlertRuleDetectionType.DYNAMIC.value:
276+
data_condition = DataCondition.objects.get(
277+
condition_group=detector.workflow_condition_group
278+
)
279+
handle_send_historical_data_to_seer(
280+
detector,
281+
data_source,
282+
data_condition,
283+
snuba_query,
284+
detector.project,
285+
SeerMethod.UPDATE,
286+
event_types=[SnubaQueryEventType.EventType.TRANSACTION],
287+
)
288+
except Exception as e:
289+
logger.info(
290+
"Query not rolled back: error sending historical data to seer",
291+
extra={"snuba_query_id": snuba_query.id, "error": e},
267292
)
293+
raise
268294

269-
logger.info(
270-
"Query successfully rolled back to legacy", extra={"snuba_query_id": snuba_query.id}
271-
)
295+
logger.info(
296+
"Query successfully rolled back to legacy", extra={"snuba_query_id": snuba_query.id}
297+
)
272298
return

0 commit comments

Comments
 (0)