From 53dcf84faf6bf53e801155166ca48310cf6c20e0 Mon Sep 17 00:00:00 2001 From: Yashika Khurana Date: Wed, 15 Jan 2025 10:37:11 -0800 Subject: [PATCH] feat(cirrus): V2 api (#12008) Because - We want to return features under the key `Features` - We don't send back enrollment responses to the calling application and sometimes its hard to find if they are in the experiment or not This commit - Returns feature and enrollments as part of the new `v2 api` Fixes #12000 #12001 --- cirrus/README.md | 56 ++- cirrus/server/cirrus/docs/apidoc.html | 2 +- cirrus/server/cirrus/docs/openapi.json | 2 +- cirrus/server/cirrus/feature_manifest.py | 4 +- cirrus/server/cirrus/main.py | 106 ++++-- cirrus/server/tests/test_main.py | 456 ++++++++++++++++++++++- cirrus/server/tests/test_telemetry.py | 218 +++++++++-- 7 files changed, 762 insertions(+), 82 deletions(-) diff --git a/cirrus/README.md b/cirrus/README.md index b49c83bdc6..c6b3aed7da 100644 --- a/cirrus/README.md +++ b/cirrus/README.md @@ -106,9 +106,7 @@ The following are the available commands for working with Cirrus: [Cirrus Api Doc](/cirrus/server/cirrus/docs/apidoc.html) for the Cirrus API -## Endpoint - -`POST /v1/features/` +## Endpoint: `POST /v1/features/` - When making a POST request, please make sure to set headers content type as JSON ```javascript @@ -116,6 +114,15 @@ The following are the available commands for working with Cirrus: "Content-Type": "application/json", } ``` +# Endpoint: `POST /v2/features/` + +The v2 endpoint extends the functionality of v1 by also returning enrollments data alongside features. + +```javascript + headers: { + "Content-Type": "application/json", + } + ``` ## Input @@ -204,7 +211,7 @@ curl -X POST "http://localhost:8001/v1/features/?nimbus_preview=true" -H 'Conten } }' ``` -## Output +### Output The output will be a JSON object with the following properties: @@ -229,7 +236,48 @@ Example output: } ``` +```shell +curl -X POST "http://localhost:8001/v2/features/?nimbus_preview=true" -H 'Content-Type: application/json' -d '{ + "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", + "context": { + "language": "en", + "region": "US" + } +}' +``` +### Output + +The output will be a JSON object with the following properties: + +- `features` (object): An object that contains the set of features. Each feature is represented as a sub-object with its own set of variables. +- `Enrollments` (array): An array of objects representing the client's enrollment into experiments. Each enrollment object contains details about the experiment, such as the experiment ID, branch, and type. + +Example output: + +```json +{ + "Features": { + "Feature1": {"Variable1.1": "valueA", "Variable1.2": "valueB"}, + "Feature2": {"Variable2.1": "valueC", "Variable2.2": "valueD"} + }, + "Enrollments": [ + { + "nimbus_user_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", + "app_id": "test_app_id", + "experiment": "experiment-slug", + "branch": "control", + "experiment_type": "rollout", + "is_preview": false + } + ] +} + +``` + + ## Notes - This API only accepts POST requests. - All parameters should be supplied in the body as JSON. +- `v2 Endpoint`: Returns both features and enrollments. Use this if you need detailed enrollment data. +- Query Parameter: Use nimbus_preview=true to compute enrollments based on preview experiments. diff --git a/cirrus/server/cirrus/docs/apidoc.html b/cirrus/server/cirrus/docs/apidoc.html index 1107dba806..34772c8fc3 100644 --- a/cirrus/server/cirrus/docs/apidoc.html +++ b/cirrus/server/cirrus/docs/apidoc.html @@ -19,7 +19,7 @@ diff --git a/cirrus/server/cirrus/docs/openapi.json b/cirrus/server/cirrus/docs/openapi.json index 05bbf7f63b..1d8af12375 100644 --- a/cirrus/server/cirrus/docs/openapi.json +++ b/cirrus/server/cirrus/docs/openapi.json @@ -1 +1 @@ -{"openapi": "3.1.0", "info": {"title": "FastAPI", "version": "0.1.0"}, "paths": {"/": {"get": {"summary": "Read Root", "operationId": "read_root__get", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}}}}, "/v1/features/": {"post": {"summary": "Compute Features", "operationId": "compute_features_v1_features__post", "parameters": [{"name": "nimbus_preview", "in": "query", "required": false, "schema": {"type": "boolean", "default": false, "title": "Nimbus Preview"}}], "requestBody": {"required": true, "content": {"application/json": {"schema": {"$ref": "#/components/schemas/FeatureRequest"}}}}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/__lbheartbeat__": {"get": {"summary": "Health Check Lbheartbeat", "operationId": "health_check_lbheartbeat___lbheartbeat___get", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}}}}, "/__heartbeat__": {"get": {"summary": "Health Check Heartbeat", "operationId": "health_check_heartbeat___heartbeat___get", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}}}}}, "components": {"schemas": {"FeatureRequest": {"properties": {"client_id": {"type": "string", "title": "Client Id"}, "context": {"type": "object", "title": "Context"}}, "type": "object", "required": ["client_id", "context"], "title": "FeatureRequest"}, "HTTPValidationError": {"properties": {"detail": {"items": {"$ref": "#/components/schemas/ValidationError"}, "type": "array", "title": "Detail"}}, "type": "object", "title": "HTTPValidationError"}, "ValidationError": {"properties": {"loc": {"items": {"anyOf": [{"type": "string"}, {"type": "integer"}]}, "type": "array", "title": "Location"}, "msg": {"type": "string", "title": "Message"}, "type": {"type": "string", "title": "Error Type"}}, "type": "object", "required": ["loc", "msg", "type"], "title": "ValidationError"}}}} \ No newline at end of file +{"openapi": "3.1.0", "info": {"title": "FastAPI", "version": "0.1.0"}, "paths": {"/": {"get": {"summary": "Read Root", "operationId": "read_root__get", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}}}}, "/v1/features/": {"post": {"summary": "Compute Features V1", "operationId": "compute_features_v1_v1_features__post", "parameters": [{"name": "nimbus_preview", "in": "query", "required": false, "schema": {"type": "boolean", "default": false, "title": "Nimbus Preview"}}], "requestBody": {"required": true, "content": {"application/json": {"schema": {"$ref": "#/components/schemas/FeatureRequest"}}}}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/v2/features/": {"post": {"summary": "Compute Features Enrollments V2", "operationId": "compute_features_enrollments_v2_v2_features__post", "parameters": [{"name": "nimbus_preview", "in": "query", "required": false, "schema": {"type": "boolean", "default": false, "title": "Nimbus Preview"}}], "requestBody": {"required": true, "content": {"application/json": {"schema": {"$ref": "#/components/schemas/FeatureRequest"}}}}, "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}, "422": {"description": "Validation Error", "content": {"application/json": {"schema": {"$ref": "#/components/schemas/HTTPValidationError"}}}}}}}, "/__lbheartbeat__": {"get": {"summary": "Health Check Lbheartbeat", "operationId": "health_check_lbheartbeat___lbheartbeat___get", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}}}}, "/__heartbeat__": {"get": {"summary": "Health Check Heartbeat", "operationId": "health_check_heartbeat___heartbeat___get", "responses": {"200": {"description": "Successful Response", "content": {"application/json": {"schema": {}}}}}}}}, "components": {"schemas": {"FeatureRequest": {"properties": {"client_id": {"type": "string", "title": "Client Id"}, "context": {"type": "object", "title": "Context"}}, "type": "object", "required": ["client_id", "context"], "title": "FeatureRequest"}, "HTTPValidationError": {"properties": {"detail": {"items": {"$ref": "#/components/schemas/ValidationError"}, "type": "array", "title": "Detail"}}, "type": "object", "title": "HTTPValidationError"}, "ValidationError": {"properties": {"loc": {"items": {"anyOf": [{"type": "string"}, {"type": "integer"}]}, "type": "array", "title": "Location"}, "msg": {"type": "string", "title": "Message"}, "type": {"type": "string", "title": "Error Type"}}, "type": "object", "required": ["loc", "msg", "type"], "title": "ValidationError"}}}} \ No newline at end of file diff --git a/cirrus/server/cirrus/feature_manifest.py b/cirrus/server/cirrus/feature_manifest.py index c9c0581523..3043105340 100644 --- a/cirrus/server/cirrus/feature_manifest.py +++ b/cirrus/server/cirrus/feature_manifest.py @@ -22,9 +22,7 @@ def compute_feature_configurations( "enrolledFeatureConfigMap" # slug, featureid, value, ].items() } - merged_res: MergedJsonWithErrors = self.fml_client.merge( # type: ignore - feature_configs - ) + merged_res: MergedJsonWithErrors = self.fml_client.merge(feature_configs) self.merge_errors = merged_res.errors if self.merge_errors: diff --git a/cirrus/server/cirrus/main.py b/cirrus/server/cirrus/main.py index c258ee8505..fff2c238a9 100644 --- a/cirrus/server/cirrus/main.py +++ b/cirrus/server/cirrus/main.py @@ -2,7 +2,7 @@ import sys from contextlib import asynccontextmanager from pathlib import Path -from typing import Any, List, NamedTuple +from typing import Any, List, NamedTuple, TypedDict import sentry_sdk from apscheduler.schedulers.asyncio import AsyncIOScheduler # type: ignore @@ -161,53 +161,59 @@ def initialize_glean(): class EnrollmentMetricData(NamedTuple): + nimbus_user_id: str + app_id: str experiment_slug: str branch_slug: str experiment_type: str + is_preview: bool + + +class ComputeFeaturesEnrollmentResult(TypedDict): + features: dict[str, dict[str, Any]] + enrollments: list[EnrollmentMetricData] def collate_enrollment_metric_data( - enrolled_partial_configuration: dict[str, Any], nimbus_preview_flag: bool + enrolled_partial_configuration: dict[str, Any], + client_id: str, + nimbus_preview_flag: bool, ) -> list[EnrollmentMetricData]: events: list[dict[str, Any]] = enrolled_partial_configuration.get("events", []) + remote_settings = ( + app.state.remote_setting_preview + if nimbus_preview_flag + else app.state.remote_setting_live + ) data: list[EnrollmentMetricData] = [] for event in events: if event.get("change") == "Enrollment": experiment_slug = event.get("experiment_slug", "") branch_slug = event.get("branch_slug", "") - experiment_type = None - remote_settings = app.state.remote_setting_live - if nimbus_preview_flag: - remote_settings = app.state.remote_setting_preview experiment_type = remote_settings.get_recipe_type(experiment_slug) data.append( EnrollmentMetricData( + nimbus_user_id=client_id, + app_id=app_id, experiment_slug=experiment_slug, branch_slug=branch_slug, experiment_type=experiment_type, + is_preview=nimbus_preview_flag, ) ) return data -async def record_metrics( - enrolled_partial_configuration: dict[str, Any], - client_id: str, - nimbus_preview_flag: bool, -): - metrics = collate_enrollment_metric_data( - enrolled_partial_configuration=enrolled_partial_configuration, - nimbus_preview_flag=nimbus_preview_flag, - ) - for experiment_slug, branch_slug, experiment_type in metrics: +async def record_metrics(enrollment_data: list[EnrollmentMetricData]): + for enrollment in enrollment_data: app.state.metrics.cirrus_events.enrollment.record( app.state.metrics.cirrus_events.EnrollmentExtra( - user_id=client_id, - app_id=app_id, - experiment=experiment_slug, - branch=branch_slug, - experiment_type=experiment_type, - is_preview=nimbus_preview_flag, + user_id=enrollment.nimbus_user_id, + app_id=enrollment.app_id, + experiment=enrollment.experiment_slug, + branch=enrollment.branch_slug, + experiment_type=enrollment.experiment_type, + is_preview=enrollment.is_preview, ) ) app.state.pings.enrollment.submit() @@ -221,11 +227,10 @@ def read_root(): return {"Hello": "World"} -@app.post("/v1/features/", status_code=status.HTTP_200_OK) -async def compute_features( +async def compute_features_enrollments( request_data: FeatureRequest, nimbus_preview: bool = Query(default=False, alias="nimbus_preview"), -): +) -> ComputeFeaturesEnrollmentResult: if not request_data.client_id: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, @@ -246,9 +251,8 @@ async def compute_features( "clientId": request_data.client_id, "requestContext": request_data.context, } - sdk = app.state.sdk_live - if nimbus_preview: - sdk = app.state.sdk_preview + + sdk = app.state.sdk_preview if nimbus_preview else app.state.sdk_live enrolled_partial_configuration: dict[str, Any] = sdk.compute_enrollments( targeting_context ) @@ -257,13 +261,51 @@ async def compute_features( app.state.fml.compute_feature_configurations(enrolled_partial_configuration) ) - await record_metrics( - enrolled_partial_configuration=enrolled_partial_configuration, + # Enrollments data + enrollment_data = collate_enrollment_metric_data( + enrolled_partial_configuration, client_id=request_data.client_id, - nimbus_preview_flag=nimbus_preview or False, + nimbus_preview_flag=nimbus_preview, ) - return client_feature_configuration + # Record metrics + await record_metrics(enrollment_data) + + return { + "features": client_feature_configuration, + "enrollments": enrollment_data, + } + + +@app.post("/v1/features/", status_code=status.HTTP_200_OK) +async def compute_features_v1( + request_data: FeatureRequest, + nimbus_preview: bool = Query(default=False, alias="nimbus_preview"), +): + result = await compute_features_enrollments(request_data, nimbus_preview) + return result["features"] + + +@app.post("/v2/features/", status_code=status.HTTP_200_OK) +async def compute_features_enrollments_v2( + request_data: FeatureRequest, + nimbus_preview: bool = Query(default=False, alias="nimbus_preview"), +): + result = await compute_features_enrollments(request_data, nimbus_preview) + return { + "Features": result["features"], + "Enrollments": [ + { + "nimbus_user_id": enrollment.nimbus_user_id, + "app_id": enrollment.app_id, + "experiment": enrollment.experiment_slug, + "branch": enrollment.branch_slug, + "experiment_type": enrollment.experiment_type, + "is_preview": enrollment.is_preview, + } + for enrollment in result["enrollments"] + ], + } async def fetch_schedule_recipes() -> None: diff --git a/cirrus/server/tests/test_main.py b/cirrus/server/tests/test_main.py index e260e1f947..66217ce7c6 100644 --- a/cirrus/server/tests/test_main.py +++ b/cirrus/server/tests/test_main.py @@ -9,6 +9,7 @@ from fml_sdk import FmlError from cirrus.main import ( + EnrollmentMetricData, create_fml, create_scheduler, create_sdk, @@ -49,7 +50,7 @@ def test_read_root(client): assert response.json() == {"Hello": "World"} -def test_get_features_with_required_field(client): +def test_get_features_v1_with_required_field(client): request_data = { "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", "context": { @@ -65,6 +66,26 @@ def test_get_features_with_required_field(client): } +def test_get_features_v2_with_required_field(client): + request_data = { + "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + }, + } + + response = client.post("/v2/features/", json=request_data) + assert response.status_code == 200 + assert response.json() == { + "Features": { + "example-feature": {"enabled": False, "something": "wicked"}, + # return default features + }, + "Enrollments": [], + } + + @pytest.mark.parametrize( "request_data, expected_status, expected_message", [ @@ -138,7 +159,7 @@ def test_get_features_with_required_field(client): ), ], ) -def test_get_features_missing_required_field( +def test_get_features_v1_missing_required_field( client, request_data, expected_status, expected_message ): response = client.post("/v1/features/", json=request_data) @@ -146,6 +167,87 @@ def test_get_features_missing_required_field( assert response.json()["detail"] == expected_message +@pytest.mark.parametrize( + "request_data, expected_status, expected_message", + [ + ( + { + "client_id": "", + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + }, + }, + status.HTTP_400_BAD_REQUEST, + "Client ID value is missing or empty", + ), + ( + { + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + } + }, + status.HTTP_422_UNPROCESSABLE_ENTITY, + [ + { + "type": "missing", + "loc": ["body", "client_id"], + "msg": "Field required", + "input": { + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + } + }, + } + ], + ), + ( + {"client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", "context": {}}, + status.HTTP_400_BAD_REQUEST, + "Context value is missing or empty", + ), + ( + {"client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449"}, + status.HTTP_422_UNPROCESSABLE_ENTITY, + [ + { + "type": "missing", + "loc": ["body", "context"], + "msg": "Field required", + "input": {"client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449"}, + } + ], + ), + ( + {}, + status.HTTP_422_UNPROCESSABLE_ENTITY, + [ + { + "type": "missing", + "loc": ["body", "client_id"], + "msg": "Field required", + "input": {}, + }, + { + "type": "missing", + "loc": ["body", "context"], + "msg": "Field required", + "input": {}, + }, + ], + ), + ], +) +def test_get_features_v2_missing_required_field( + client, request_data, expected_status, expected_message +): + response = client.post("/v2/features/", json=request_data) + assert response.status_code == expected_status + assert response.json()["detail"] == expected_message + + @pytest.mark.asyncio @patch("cirrus.main.app.state.remote_setting_live") @patch("cirrus.main.app.state.remote_setting_preview") @@ -325,7 +427,7 @@ def test_heartbeat_endpoint(client): assert response.json() == {"status": "ok"} -def test_get_features_with_nimbus_preview(client): +def test_get_features_v1_with_nimbus_preview_flag(client): request_data = { "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", "context": { @@ -341,6 +443,26 @@ def test_get_features_with_nimbus_preview(client): } +def test_get_features_v2_with_nimbus_preview(client): + request_data = { + "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + }, + } + + response = client.post("/v2/features/?nimbus_preview=true", json=request_data) + assert response.status_code == 200 + assert response.json() == { + "Features": { + "example-feature": {"enabled": False, "something": "wicked"}, + # return default features + }, + "Enrollments": [], + } + + @pytest.mark.parametrize( "request_data, expected_status, expected_message", [ @@ -414,7 +536,7 @@ def test_get_features_with_nimbus_preview(client): ), ], ) -def test_get_features_missing_required_field_nimbus_preview( +def test_get_features_v1_missing_required_field_nimbus_preview( client, request_data, expected_status, expected_message ): response = client.post("/v1/features/?nimbus_preview=true", json=request_data) @@ -422,9 +544,88 @@ def test_get_features_missing_required_field_nimbus_preview( assert response.json()["detail"] == expected_message -def test_get_features_with_and_without_nimbus_preview( - client, +@pytest.mark.parametrize( + "request_data, expected_status, expected_message", + [ + ( + { + "client_id": "", + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + }, + }, + status.HTTP_400_BAD_REQUEST, + "Client ID value is missing or empty", + ), + ( + { + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + } + }, + status.HTTP_422_UNPROCESSABLE_ENTITY, + [ + { + "type": "missing", + "loc": ["body", "client_id"], + "msg": "Field required", + "input": { + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + } + }, + } + ], + ), + ( + {"client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", "context": {}}, + status.HTTP_400_BAD_REQUEST, + "Context value is missing or empty", + ), + ( + {"client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449"}, + status.HTTP_422_UNPROCESSABLE_ENTITY, + [ + { + "type": "missing", + "loc": ["body", "context"], + "msg": "Field required", + "input": {"client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449"}, + } + ], + ), + ( + {}, + status.HTTP_422_UNPROCESSABLE_ENTITY, + [ + { + "type": "missing", + "loc": ["body", "client_id"], + "msg": "Field required", + "input": {}, + }, + { + "type": "missing", + "loc": ["body", "context"], + "msg": "Field required", + "input": {}, + }, + ], + ), + ], +) +def test_get_features_v2_missing_required_field_nimbus_preview( + client, request_data, expected_status, expected_message ): + response = client.post("/v2/features/?nimbus_preview=true", json=request_data) + assert response.status_code == expected_status + assert response.json()["detail"] == expected_message + + +def test_get_features_v1_without_nimbus_preview(client): request_data = { "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", "context": { @@ -435,9 +636,7 @@ def test_get_features_with_and_without_nimbus_preview( with patch( "cirrus.main.app.state.sdk_live.compute_enrollments" - ) as mock_sdk_live_compute_enrollments, patch( - "cirrus.main.app.state.sdk_preview.compute_enrollments" - ) as mock_sdk_preview_compute_enrollments: + ) as mock_sdk_live_compute_enrollments: mock_sdk_live_compute_enrollments.return_value = { "enrolledFeatureConfigMap": { @@ -473,6 +672,28 @@ def test_get_features_with_and_without_nimbus_preview( } ], } + + # Without nimbus_preview + response = client.post("/v1/features/", json=request_data) + assert response.status_code == 200 + assert response.json() == { + "example-feature": {"enabled": False, "something": "wicked"} + } + + +def test_get_features_v1_with_nimbus_preview(client): + request_data = { + "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + }, + } + + with patch( + "cirrus.main.app.state.sdk_preview.compute_enrollments" + ) as mock_sdk_preview_compute_enrollments: + mock_sdk_preview_compute_enrollments.return_value = { "enrolledFeatureConfigMap": { "example-feature": { @@ -508,22 +729,207 @@ def test_get_features_with_and_without_nimbus_preview( ], } - # Without nimbus_preview - response = client.post("/v1/features/", json=request_data) + # With nimbus_preview + response = client.post("/v1/features/?nimbus_preview=true", json=request_data) assert response.status_code == 200 assert response.json() == { - "example-feature": {"enabled": False, "something": "wicked"} + "example-feature": {"enabled": True, "something": "preview"} } - # With nimbus_preview - response = client.post("/v1/features/?nimbus_preview=true", json=request_data) + +def test_get_features_v2_enrollments_without_nimbus_preview(client): + request_data = { + "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + }, + } + + with patch( + "cirrus.main.app.state.sdk_live.compute_enrollments" + ) as mock_sdk_live_compute_enrollments, patch( + "cirrus.main.collate_enrollment_metric_data" + ) as mock_collate_enrollment_metric_data, patch( + "cirrus.main.app.state.fml.compute_feature_configurations" + ) as mock_compute_feature_configurations: + + # Mock live compute_enrollments response + mock_sdk_live_compute_enrollments.return_value = { + "enrolledFeatureConfigMap": { + "example-feature": { + "feature": { + "featureId": "example-feature", + "value": {"enabled": False, "something": "wicked"}, + }, + "branch": "treatment", + "featureId": "example-feature", + "slug": "experiment_slug_1", + } + }, + "enrollments": [ + { + "slug": "experiment_slug_1", + "status": { + "Enrolled": { + "branch": "treatment", + "enrollment_id": "enrollment_id_1", + "reason": "Qualified", + } + }, + } + ], + "events": [ + { + "branch_slug": "treatment", + "change": "Enrollment", + "enrollment_id": "enrollment_id_1", + "experiment_slug": "experiment_slug_1", + "reason": None, + } + ], + } + + # Mock collate_enrollment_metric_data and compute_feature_configurations + mock_collate_enrollment_metric_data.side_effect = ( + lambda enrolled_partial_configuration, client_id, nimbus_preview_flag: [ + EnrollmentMetricData( + nimbus_user_id=client_id, + app_id="test_app_id", + experiment_slug=event["experiment_slug"], + branch_slug=event["branch_slug"], + experiment_type="rollout", + is_preview=nimbus_preview_flag, + ) + for event in enrolled_partial_configuration["events"] + ] + ) + mock_compute_feature_configurations.side_effect = ( + lambda enrolled_partial_configuration: { + feature_id: feature_data["feature"]["value"] + for feature_id, feature_data in enrolled_partial_configuration[ + "enrolledFeatureConfigMap" + ].items() + } + ) + + # Test for live SDK (no nimbus_preview) + response = client.post("/v2/features/", json=request_data) assert response.status_code == 200 assert response.json() == { - "example-feature": {"enabled": True, "something": "preview"} + "Features": { + "example-feature": {"enabled": False, "something": "wicked"}, + }, + "Enrollments": [ + { + "nimbus_user_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", + "app_id": "test_app_id", + "experiment": "experiment_slug_1", + "branch": "treatment", + "experiment_type": "rollout", + "is_preview": False, + } + ], } -def test_get_features_preview_url_not_provided(client): +def test_get_features_v2_enrollments_with_nimbus_preview(client): + request_data = { + "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + }, + } + + with patch( + "cirrus.main.app.state.sdk_preview.compute_enrollments" + ) as mock_sdk_preview_compute_enrollments, patch( + "cirrus.main.collate_enrollment_metric_data" + ) as mock_collate_enrollment_metric_data, patch( + "cirrus.main.app.state.fml.compute_feature_configurations" + ) as mock_compute_feature_configurations: + + # Mock preview compute_enrollments response + mock_sdk_preview_compute_enrollments.return_value = { + "enrolledFeatureConfigMap": { + "example-feature": { + "feature": { + "featureId": "example-feature", + "value": {"enabled": True, "something": "preview"}, + }, + "branch": "treatment", + "featureId": "example-feature", + "slug": "experiment_slug_2", + } + }, + "enrollments": [ + { + "slug": "experiment_slug_2", + "status": { + "Enrolled": { + "branch": "treatment", + "enrollment_id": "enrollment_id_2", + "reason": "Qualified", + } + }, + } + ], + "events": [ + { + "branch_slug": "treatment", + "change": "Enrollment", + "enrollment_id": "enrollment_id_2", + "experiment_slug": "experiment_slug_2", + "reason": None, + } + ], + } + + # Mock collate_enrollment_metric_data and compute_feature_configurations + mock_collate_enrollment_metric_data.side_effect = ( + lambda enrolled_partial_configuration, client_id, nimbus_preview_flag: [ + EnrollmentMetricData( + nimbus_user_id=client_id, + app_id="test_app_id", + experiment_slug=event["experiment_slug"], + branch_slug=event["branch_slug"], + experiment_type="experiment", + is_preview=nimbus_preview_flag, + ) + for event in enrolled_partial_configuration["events"] + ] + ) + mock_compute_feature_configurations.side_effect = ( + lambda enrolled_partial_configuration: { + feature_id: feature_data["feature"]["value"] + for feature_id, feature_data in enrolled_partial_configuration[ + "enrolledFeatureConfigMap" + ].items() + } + ) + + # Test for preview SDK (nimbus_preview=true) + response = client.post("/v2/features/?nimbus_preview=true", json=request_data) + assert response.status_code == 200 + assert response.json() == { + "Features": { + "example-feature": {"enabled": True, "something": "preview"}, + }, + "Enrollments": [ + { + "nimbus_user_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", + "app_id": "test_app_id", + "experiment": "experiment_slug_2", + "branch": "treatment", + "experiment_type": "experiment", + "is_preview": True, + } + ], + } + + +def test_get_features_v1_preview_url_not_provided(client): request_data = { "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", "context": { @@ -535,7 +941,23 @@ def test_get_features_preview_url_not_provided(client): # Assuming the remote_setting_preview_url is not set in the settings with patch("cirrus.main.remote_setting_preview_url", ""): response = client.post("/v1/features/?nimbus_preview=true", json=request_data) - assert response.status_code == 400 + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json() == {"detail": "This Cirrus doesn’t support preview mode"} + + +def test_get_features_v2_preview_url_not_provided(client): + request_data = { + "client_id": "4a1d71ab-29a2-4c5f-9e1d-9d9df2e6e449", + "context": { + "key1": "value1", + "key2": {"key2.1": "value2", "key2.2": "value3"}, + }, + } + + # Assuming the remote_setting_preview_url is not set in the settings + with patch("cirrus.main.remote_setting_preview_url", ""): + response = client.post("/v2/features/?nimbus_preview=true", json=request_data) + assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json() == {"detail": "This Cirrus doesn’t support preview mode"} diff --git a/cirrus/server/tests/test_telemetry.py b/cirrus/server/tests/test_telemetry.py index bf34f51162..265eeabdb1 100644 --- a/cirrus/server/tests/test_telemetry.py +++ b/cirrus/server/tests/test_telemetry.py @@ -5,7 +5,9 @@ from cirrus.experiment_recipes import RecipeType from cirrus.main import ( + EnrollmentMetricData, app, + collate_enrollment_metric_data, initialize_glean, record_metrics, send_instance_name_metric, @@ -44,44 +46,149 @@ def before_enrollment_ping(data): async def test_enrollment_metrics_recorded_with_record_metrics(mocker, recipes): app.state.remote_setting_live.update_recipes(recipes) ping_spy = mocker.spy(app.state.pings.enrollment, "submit") + + # Create the enrollment data + enrollment_data = [ + EnrollmentMetricData( + nimbus_user_id="test_client_id", + app_id="test_app_id", + experiment_slug="cirrus-test-1", + branch_slug="control", + experiment_type=RecipeType.ROLLOUT.value, + is_preview=False, + ), + EnrollmentMetricData( + nimbus_user_id="test_client_id", + app_id="test_app_id", + experiment_slug="cirrus-test-2", + branch_slug="control", + experiment_type=RecipeType.EXPERIMENT.value, + is_preview=False, + ), + ] + app.state.pings.enrollment.test_before_next_submit(before_enrollment_ping) + + await record_metrics(enrollment_data) + + assert ping_spy.call_count == 1 + assert app.state.metrics.cirrus_events.enrollment.test_get_value() is None + + +def test_collate_enrollment_metric_data(mocker): + mock_remote_settings = mocker.patch("cirrus.main.app.state.remote_setting_live") + mock_remote_settings.get_recipe_type.return_value = "rollout" + enrolled_partial_configuration = { - "events": [ + "enrolledFeatureConfigMap": { + "example-feature": { + "branch": None, + "feature": { + "featureId": "example-feature", + "value": {"enabled": False, "something": "You are enrolled"}, + }, + "featureId": "example-feature", + "slug": "experiment-slug", + } + }, + "enrollments": [ { - "branch_slug": "control", - "change": "Enrollment", - "enrollment_id": "enrollment_id", - "experiment_slug": "cirrus-test-1", - "reason": None, - }, + "slug": "experiment-slug", + "status": {"Enrolled": {"branch": "control", "reason": "Qualified"}}, + } + ], + "events": [ { "branch_slug": "control", "change": "Enrollment", - "enrollment_id": "enrollment_id", - "experiment_slug": "cirrus-test-2", + "experiment_slug": "experiment-slug", "reason": None, - }, - { - "branch_slug": "", - "change": "EnrollFailed", - "enrollment_id": "blah", - "experiment_slug": "fake-experiment", - "reason": "not_selected", - }, + } ], } + result = collate_enrollment_metric_data( + enrolled_partial_configuration, "test-client-id", nimbus_preview_flag=False + ) + expected_result = [ + EnrollmentMetricData( + nimbus_user_id="test-client-id", + app_id="test_app_id", + experiment_slug="experiment-slug", + branch_slug="control", + experiment_type="rollout", + is_preview=False, + ) + ] + + assert result == expected_result - app.state.pings.enrollment.test_before_next_submit(before_enrollment_ping) - await record_metrics( - enrolled_partial_configuration, "test_client_id", nimbus_preview_flag=False +@pytest.mark.asyncio +async def test_enrollment_metrics_recorded_with_compute_features_v1( + client, mocker, recipes +): + _, app.state.metrics = initialize_glean() + context = json.dumps( + { + "app_id": "org.mozilla.test", + "app_name": "test_app", + "channel": "release", + } + ) + sdk = SDK( + context=context, + coenrolling_feature_ids=[], + metrics_handler=CirrusMetricsHandler(app.state.metrics, app.state.pings), ) + request_data = { + "client_id": "test_client_id", + "context": {"user_id": "test-client-id"}, + } + + app.state.remote_setting_live.update_recipes(recipes) + sdk.set_experiments(json.dumps(recipes)) + + def validate_before_submit(data): + snapshot = app.state.metrics.cirrus_events.enrollment.test_get_value() + assert snapshot is not None + assert len(snapshot) == 2 + assert snapshot[0].extra["is_preview"] == "false" + assert snapshot[1].extra["is_preview"] == "false" + + app.state.pings.enrollment.test_before_next_submit(validate_before_submit) + + mocker.patch.object(app.state, "sdk_live", sdk) + ping_spy = mocker.spy(app.state.pings.enrollment, "submit") + + response = client.post("/v1/features/", json=request_data) + assert response.status_code == 200 + assert ping_spy.call_count == 1 + assert app.state.metrics.cirrus_events.enrollment.test_get_value() is None + + ping_spy.reset_mock() + + app.state.remote_setting_preview.update_recipes(recipes) + mocker.patch.object(app.state, "sdk_preview", sdk) + + def validate_before_submit_preview(data): + snapshot = app.state.metrics.cirrus_events.enrollment.test_get_value() + assert snapshot is not None + assert len(snapshot) == 2 + assert snapshot[0].extra["is_preview"] == "true" + assert snapshot[1].extra["is_preview"] == "true" + + app.state.pings.enrollment.test_before_next_submit(validate_before_submit_preview) + + response = client.post("/v1/features/?nimbus_preview=true", json=request_data) + assert response.status_code == 200 assert ping_spy.call_count == 1 assert app.state.metrics.cirrus_events.enrollment.test_get_value() is None @pytest.mark.asyncio -async def test_enrollment_metrics_recorded_with_compute_features(client, mocker, recipes): +async def test_enrollment_metrics_recorded_with_compute_features_v2( + client, mocker, recipes +): _, app.state.metrics = initialize_glean() context = json.dumps( { @@ -116,7 +223,7 @@ def validate_before_submit(data): mocker.patch.object(app.state, "sdk_live", sdk) ping_spy = mocker.spy(app.state.pings.enrollment, "submit") - response = client.post("/v1/features/", json=request_data) + response = client.post("/v2/features/", json=request_data) assert response.status_code == 200 assert ping_spy.call_count == 1 assert app.state.metrics.cirrus_events.enrollment.test_get_value() is None @@ -135,14 +242,14 @@ def validate_before_submit_preview(data): app.state.pings.enrollment.test_before_next_submit(validate_before_submit_preview) - response = client.post("/v1/features/?nimbus_preview=true", json=request_data) + response = client.post("/v2/features/?nimbus_preview=true", json=request_data) assert response.status_code == 200 assert ping_spy.call_count == 1 assert app.state.metrics.cirrus_events.enrollment.test_get_value() is None @pytest.mark.asyncio -async def test_enrollment_status_metrics_recorded_with_metrics_handler( +async def test_enrollment_status_metrics_recorded_with_metrics_handler_v1( client, mocker, recipes ): _, app.state.metrics = initialize_glean() @@ -204,6 +311,69 @@ def test_ping(data): assert app.state.metrics.cirrus_events.enrollment_status.test_get_value() is None +@pytest.mark.asyncio +async def test_enrollment_status_metrics_recorded_with_metrics_handler_v2( + client, mocker, recipes +): + _, app.state.metrics = initialize_glean() + context = json.dumps( + { + "app_id": "org.mozilla.test", + "app_name": "test_app", + "channel": "release", + } + ) + sdk = SDK( + context=context, + coenrolling_feature_ids=[], + metrics_handler=CirrusMetricsHandler(app.state.metrics, app.state.pings), + ) + + request_data = { + "client_id": "test_client_id", + "context": {"user_id": "test-client-id"}, + } + + app.state.remote_setting_live.update_recipes(recipes) + sdk.set_experiments(json.dumps(recipes)) + + def test_ping(data): + assert ( + app.state.metrics.cirrus_events.enrollment_status.test_get_num_recorded_errors( + ErrorType.INVALID_OVERFLOW + ) + == 0 + ) + snapshot = app.state.metrics.cirrus_events.enrollment_status.test_get_value() + assert len(snapshot) == 5 + assert snapshot[0].extra["status"] == "Enrolled" + assert snapshot[1].extra["status"] == "Enrolled" + assert snapshot[2].extra["status"] == "NotEnrolled" + assert snapshot[2].extra["reason"] == "NotSelected" + assert snapshot[3].extra["status"] == "NotEnrolled" + assert snapshot[3].extra["reason"] == "NotTargeted" + assert snapshot[4].extra["status"] == "Error" + + app.state.pings.enrollment_status.test_before_next_submit(test_ping) + + mocker.patch.object(app.state, "sdk_live", sdk) + ping_spy = mocker.spy(app.state.pings.enrollment_status, "submit") + + response = client.post("/v2/features/", json=request_data) + assert response.status_code == 200 + assert ping_spy.call_count == 1 + assert app.state.metrics.cirrus_events.enrollment_status.test_get_value() is None + + ping_spy.reset_mock() + app.state.remote_setting_preview.update_recipes(recipes) + mocker.patch.object(app.state, "sdk_preview", sdk) + + response = client.post("/v2/features/?nimbus_preview=true", json=request_data) + assert response.status_code == 200 + assert ping_spy.call_count == 1 + assert app.state.metrics.cirrus_events.enrollment_status.test_get_value() is None + + def test_instance_name_metric(mocker): app.state.pings, _ = initialize_glean() ping_spy = mocker.spy(app.state.pings.startup, "submit")