Skip to content

Commit 645478e

Browse files
yashikakhuranaYashika KhuranaYashika Khurana
authored
feat(nimbus): Add targetingSql field to v8 API (#16354)
Because - `bigquery-etl` needs the translated SQL to run sizing queries against `nimbus_targeting_context` for the draft experiment to estimate the population This commit - Adds `targetingSql` field to `/api/v8/experiments/` API response - For Draft experiments: returns the translated BigQuery SQL WHERE clause and any warnings for untranslatable attributes - For Live/Complete experiments: returns null — they don't need sizing - bigquery-etl reads this field directly, no JEXL parser needed on its side Fixes #16352 --------- Co-authored-by: Yashika Khurana <yashikakhurana@Yashikas-MBP.home.local> Co-authored-by: Yashika Khurana <yashikakhurana@Yashikas-MacBook-Pro.local>
1 parent 1147554 commit 645478e

6 files changed

Lines changed: 93 additions & 1 deletion

File tree

docs/experimenter/openapi-schema.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1660,6 +1660,10 @@
16601660
"requiresRestart": {
16611661
"type": "string",
16621662
"readOnly": true
1663+
},
1664+
"targetingSql": {
1665+
"type": "string",
1666+
"readOnly": true
16631667
}
16641668
},
16651669
"required": [

docs/experimenter/swagger-ui.html

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1672,6 +1672,10 @@
16721672
"requiresRestart": {
16731673
"type": "string",
16741674
"readOnly": true
1675+
},
1676+
"targetingSql": {
1677+
"type": "string",
1678+
"readOnly": true
16751679
}
16761680
},
16771681
"required": [

experimenter/experimenter/experiments/api/v8/serializers.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from django.conf import settings
66
from rest_framework import serializers
77

8+
from experimenter.experiments.jexl_to_sql import jexl_to_sql
89
from experimenter.experiments.models import (
910
NimbusBranch,
1011
NimbusBucketRange,
@@ -120,6 +121,7 @@ class NimbusExperimentSerializer(serializers.ModelSerializer):
120121
firefoxLabsDescriptionLinks = serializers.SerializerMethodField()
121122
firefoxLabsGroup = serializers.ReadOnlyField(source="firefox_labs_group")
122123
requiresRestart = serializers.ReadOnlyField(source="requires_restart")
124+
targetingSql = serializers.SerializerMethodField()
123125

124126
class Meta:
125127
model = NimbusExperiment
@@ -160,8 +162,20 @@ class Meta:
160162
"firefoxLabsDescriptionLinks",
161163
"firefoxLabsGroup",
162164
"requiresRestart",
165+
"targetingSql",
163166
)
164167

168+
def get_targetingSql(self, obj):
169+
if obj.status != NimbusExperiment.Status.DRAFT:
170+
return None
171+
result = jexl_to_sql(obj.targeting)
172+
if result.sql is None and not result.warnings:
173+
return None
174+
return {
175+
"sql": result.sql,
176+
"warnings": result.warnings,
177+
}
178+
165179
def _holdback_enrollment_end(self, obj):
166180
if obj.is_holdback and not obj.end_date and not obj.actual_enrollment_end_date:
167181
return datetime.date.today() - datetime.timedelta(

experimenter/experimenter/experiments/jexl_to_sql.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,11 @@ def _array_to_sql(node: ArrayLiteral, warnings: list[str]) -> Optional[str]:
293293
def _transform_to_sql(node: Transform, warnings: list[str]) -> Optional[str]:
294294
subject_path = _identifier_path(node.subject)
295295

296+
# If the subject itself is untranslatable, warn about it rather than the transform
297+
if subject_path and _is_untranslatable(subject_path):
298+
_add_warning(warnings, subject_path)
299+
return None
300+
296301
if node.name == "date":
297302
# profileAgeCreated is stored as epoch ms — return column directly for arithmetic
298303
if subject_path == "profileAgeCreated":

experimenter/experimenter/experiments/tests/api/v8/test_serializers.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88

99
from experimenter.base.tests.factories import LocaleFactory
1010
from experimenter.experiments.api.v8.serializers import NimbusExperimentSerializer
11+
from experimenter.experiments.jexl_to_sql import jexl_to_sql
1112
from experimenter.experiments.models import NimbusBranchFeatureValue, NimbusExperiment
1213
from experimenter.experiments.tests.factories import (
1314
TEST_LOCALIZATIONS,
1415
NimbusBranchFactory,
1516
NimbusExperimentFactory,
1617
NimbusFeatureConfigFactory,
1718
)
19+
from experimenter.targeting.constants import WINDOWS_ONLY
1820

1921

2022
class TestNimbusExperimentSerializer(TestCase):
@@ -496,4 +498,67 @@ def _experiment_data_without_branches_and_featureIds(
496498
"firefoxLabsDescriptionLinks": None,
497499
"firefoxLabsGroup": None,
498500
"requiresRestart": False,
501+
"targetingSql": None, # None for non-Draft experiments
499502
}
503+
504+
def test_targeting_sql_populated_for_draft(self):
505+
_os = "metrics.object.nimbus_targeting_context_os"
506+
experiment = NimbusExperimentFactory.create_with_lifecycle(
507+
NimbusExperimentFactory.Lifecycles.CREATED,
508+
targeting_config_slug=WINDOWS_ONLY.slug,
509+
channels=[],
510+
firefox_min_version=NimbusExperiment.Version.NO_VERSION,
511+
firefox_max_version=NimbusExperiment.Version.NO_VERSION,
512+
)
513+
experiment.locales.clear()
514+
experiment.countries.clear()
515+
experiment.languages.clear()
516+
517+
expected_sql = (
518+
f"(NOT CAST(JSON_VALUE({_os}, '$.isMac') AS BOOL)"
519+
f" AND NOT CAST(JSON_VALUE({_os}, '$.isLinux') AS BOOL))"
520+
)
521+
522+
jexl_result = jexl_to_sql(experiment.targeting)
523+
self.assertEqual(jexl_result.sql, expected_sql)
524+
self.assertEqual(jexl_result.warnings, [])
525+
526+
# Validate the serializer returns the same SQL in the correct shape
527+
serializer = NimbusExperimentSerializer(experiment)
528+
self.assertEqual(
529+
serializer.data["targetingSql"],
530+
{"sql": expected_sql, "warnings": []},
531+
)
532+
533+
def test_targeting_sql_with_warnings_for_draft(self):
534+
# relay_user uses attachedFxAOAuthClients which is KNOWN_UNTRANSLATABLE
535+
# → sql is None, warning is attachedFxAOAuthClients
536+
experiment = NimbusExperimentFactory.create_with_lifecycle(
537+
NimbusExperimentFactory.Lifecycles.CREATED,
538+
targeting_config_slug="relay_user",
539+
channels=[],
540+
firefox_min_version=NimbusExperiment.Version.NO_VERSION,
541+
firefox_max_version=NimbusExperiment.Version.NO_VERSION,
542+
)
543+
experiment.locales.clear()
544+
experiment.countries.clear()
545+
experiment.languages.clear()
546+
547+
# relay_user uses attachedFxAOAuthClients (KNOWN_UNTRANSLATABLE) → sql=None
548+
jexl_result = jexl_to_sql(experiment.targeting)
549+
self.assertIsNone(jexl_result.sql)
550+
self.assertEqual(jexl_result.warnings, ["attachedFxAOAuthClients"])
551+
552+
# Serializer should produce the same result as jexl_to_sql
553+
serializer = NimbusExperimentSerializer(experiment)
554+
self.assertEqual(
555+
serializer.data["targetingSql"],
556+
{"sql": jexl_result.sql, "warnings": jexl_result.warnings},
557+
)
558+
559+
def test_targeting_sql_none_for_live_experiment(self):
560+
experiment = NimbusExperimentFactory.create_with_lifecycle(
561+
NimbusExperimentFactory.Lifecycles.LIVE_ENROLLING,
562+
)
563+
serializer = NimbusExperimentSerializer(experiment)
564+
self.assertIsNone(serializer.data["targetingSql"])

experimenter/experimenter/experiments/tests/test_jexl_to_sql.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ def test_untranslatable_returns_none_and_warning(self, _name, jexl, expected_war
181181
(
182182
"length_untranslatable_subject",
183183
"attachedFxAOAuthClients|length >= 1",
184-
"|length",
184+
"attachedFxAOAuthClients",
185185
),
186186
("preference_value_variable", "someVar|preferenceValue", "|preferenceValue"),
187187
(

0 commit comments

Comments
 (0)